Tweak base32 implementation (#131)

KeeTrayTOTP.Tests/Base32Tests.cs

@@ -0,0 +1,121 @@
+using FluentAssertions;
+using KeeTrayTOTP.Libraries;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace KeeTrayTOTP.Tests
+{
+    [TestClass]
+    public class Base32Tests
+    {
+        private const string InvalidSeed = "MZXW6=YTBOI!===!";
+
+        [DataTestMethod]
+        [DynamicData(nameof(GetBase32Data), DynamicDataSourceType.Method)]
+        public void Encode_ShouldWorkOnRfc4648TestCases(Base32TestCase testCase)
+        {
+            var actual = Base32.Encode(Encoding.UTF8.GetBytes(testCase.Decoded));
+
+            actual.Should().Be(testCase.Encoded);
+        }
+
+        [DataTestMethod]
+        [DynamicData(nameof(GetBase32Data), DynamicDataSourceType.Method)]
+        public void Decode_ShouldWorkOnRfc4648TestCases(Base32TestCase testCase)
+        {
+            var actual = Base32.Decode(testCase.Encoded);
+
+            var actualString = Encoding.UTF8.GetString(actual);
+
+            actualString.Should().Be(testCase.Decoded);
+        }
+
+        [DataTestMethod]
+        [DynamicData(nameof(GetBase32Data), DynamicDataSourceType.Method)]
+        public void IsBase32Out_ShouldReturnTrueFor(Base32TestCase testCase)
+        {
+            var actual = testCase.Encoded.IsBase32(out string invalidChars);
+
+            actual.Should().BeTrue();
+            invalidChars.Should().BeNullOrEmpty();
+        }
+
+        [DataTestMethod]
+        [DynamicData(nameof(GetBase32Data), DynamicDataSourceType.Method)]
+        public void IsBase32_ShouldReturnTrueFor(Base32TestCase testCase)
+        {
+            var actual = testCase.Encoded.IsBase32();
+
+            actual.Should().BeTrue();
+        }
+
+        [DataTestMethod]
+        [DynamicData(nameof(GetBase32Data), DynamicDataSourceType.Method)]
+        public void HasInvalidPadding_ShouldReturnFalseFor(Base32TestCase testCase)
+        {
+            Base32.HasInvalidPadding(testCase.Encoded).Should().BeFalse();
+        }
+
+        [TestMethod]
+        public void Decode_ShouldThrowWithInvalidSeed()
+        {
+            Action act = () => Base32.Decode(InvalidSeed);
+
+            act.Should().Throw<ArgumentException>().WithMessage("Base32 contains illegal characters.");
+        }
+
+        [TestMethod]
+        public void ExtIsBase32_ShouldReturnFalseWithInvalidChars()
+        {
+            var actual = InvalidSeed.IsBase32(out string invalidChars);
+            actual.Should().BeFalse();
+            invalidChars.Should().Be("=!");
+        }
+
+        [TestMethod]
+        public void HasInvalidPadding_ShouldReturnTrueWhenNotPadding()
+        {
+            Base32.HasInvalidPadding(InvalidSeed).Should().BeTrue();
+        }
+
+        /// <summary>
+        /// Testcases were taken from <see cref="https://tools.ietf.org/html/rfc4648"/>
+        /// </summary>
+        public static IEnumerable<object[]> GetBase32Data()
+        {
+            yield return new object[] { new Base32TestCase("", "") };
+            yield return new object[] { new Base32TestCase("f", "MY======") };
+            yield return new object[] { new Base32TestCase("fo", "MZXQ====") };
+            yield return new object[] { new Base32TestCase("foo", "MZXW6===") };
+            yield return new object[] { new Base32TestCase("foob", "MZXW6YQ=") };
+            yield return new object[] { new Base32TestCase("fooba", "MZXW6YTB") };
+            yield return new object[] { new Base32TestCase("foobar", "MZXW6YTBOI======") };
+        }
+
+        public sealed class Base32TestCase
+        {
+            public Base32TestCase(string decoded, string encoded)
+            {
+                this.Decoded = decoded;
+                this.Encoded = encoded;
+            }
+
+            /// <summary>
+            /// Input for Encoding, expected Output for Decoding
+            /// </summary>
+            public string Decoded { get; }
+
+            /// <summary>
+            /// String in Base32 encoding
+            /// </summary>
+            public string Encoded { get; }
+
+            public override string ToString()
+            {
+                return $"Decoded = {Decoded}, Encoded = {Encoded}";
+            }
+        }
+    }
+}

KeeTrayTOTP.Tests/TrayTOTP_ColumnproviderTests.cs

@@ -16,7 +16,7 @@ public class TrayTOTP_ColumnproviderTests : IDisposable
         private readonly KeeTrayTOTPExt _plugin;
         private readonly IPluginHost _pluginHost;
 
-        const string InvalidSeed = "C5CYMIHWQUUZMKUGZHGEOSJSQDE4L===";
+        const string InvalidSeed = "C5CYMIHWQUUZMKUGZHGEOSJSQDE4L===!";
         const string ValidSeed = "JBSWY3DPEHPK3PXP";
         const string ValidSettings = "30;6";
 

KeeTrayTOTP/Libraries/Base32.cs

@@ -1,21 +1,45 @@
-using System;
+// Taken from https://raw.githubusercontent.com/telehash/telehash.net/master/Telehash.Net/Base32.cs
+//
+// Copyright(c) 2015 Thomas Muldowney
+// 
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+// 
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
 
 namespace KeeTrayTOTP.Libraries
 {
     /// <summary>
-    /// Utility to deal with Base32 encoding and decoding.
+    /// Utility to deal with Base32 encoding and decoding
     /// </summary>
     /// <remarks>
     /// http://tools.ietf.org/html/rfc4648
     /// </remarks>
     public static class Base32
     {
         /// <summary>
-        /// The number of bits in a base32 encoded character.
+        /// The number of bits in a base32 encoded character
         /// </summary>
         private const int EncodedBitCount = 5;
         /// <summary>
-        /// The number of bits in a byte.
+        /// The number of bits in a byte
         /// </summary>
         private const int ByteBitCount = 8;
         /// <summary>
@@ -25,26 +49,30 @@ public static class Base32
         /// </summary>
         private const string EncodingChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567";
         /// <summary>
-        /// Takes a block of data and converts it to a base 32 encoded string.
+        /// The rfc defines '=' as the padding character
         /// </summary>
-        /// <param name="data">Input data.</param>
-        /// <returns>base 32 string.</returns>
+        private const char PaddingCharacter = '=';
+        /// <summary>
+        /// Takes a block of data and converts it to a base 32 encoded string
+        /// </summary>
+        /// <param name="data">Input data</param>
+        /// <returns>base 32 string</returns>
         public static string Encode(byte[] data)
         {
             if (data == null)
             {
-                throw new ArgumentNullException();
+                throw new ArgumentNullException("data");
             }
 
             if (data.Length == 0)
             {
-                throw new ArgumentNullException();
+                return string.Empty;
             }
 
             // The output character count is calculated in 40 bit blocks.  That is because the least
             // common blocks size for both binary (8 bit) and base 32 (5 bit) is 40.  Padding must be used
             // to fill in the difference.
-            int outputCharacterCount = (int)Math.Ceiling(data.Length / (decimal)EncodedBitCount) * ByteBitCount;
+            var outputCharacterCount = (int)decimal.Ceiling(data.Length / (decimal)EncodedBitCount) * ByteBitCount;
             char[] outputBuffer = new char[outputCharacterCount];
 
             byte workingValue = 0;
@@ -67,7 +95,7 @@ public static string Encode(byte[] data)
                 workingValue = (byte)((workingByte << remainingBits) & 31);
             }
 
-            // If we didn't finish, write the last current working char.
+            // If we didn't finish, write the last current working char
             if (currentPosition != outputCharacterCount)
             {
                 outputBuffer[currentPosition++] = EncodingChars[workingValue];
@@ -77,8 +105,7 @@ public static string Encode(byte[] data)
             // Since the outputCharacterCount does account for the paddingCharacters, fill it out.
             while (currentPosition < outputCharacterCount)
             {
-                // The RFC defined paddinc char is '='.
-                outputBuffer[currentPosition++] = '=';
+                outputBuffer[currentPosition++] = PaddingCharacter;
             }
 
             return new string(outputBuffer);
@@ -93,10 +120,11 @@ public static byte[] Decode(string base32)
         {
             if (string.IsNullOrEmpty(base32))
             {
-                throw new ArgumentNullException();
+                return new byte[0];
             }
 
-            var unpaddedBase32 = base32.ToUpperInvariant().TrimEnd('=');
+            var unpaddedBase32 = GetUnpaddedBase32(base32);
+
             foreach (var c in unpaddedBase32)
             {
                 if (EncodingChars.IndexOf(c) < 0)
@@ -136,5 +164,36 @@ public static byte[] Decode(string base32)
 
             return outputBuffer;
         }
+
+        public static bool HasInvalidPadding(string input)
+        {
+            return GetUnpaddedBase32(input).Contains(PaddingCharacter);
+        }
+
+        public static bool IsBase32(this string input)
+        {
+            return GetUnpaddedBase32(input).All(EncodingChars.Contains);
+        }
+
+        public static bool IsBase32(this string input, out string invalidCharacters)
+        {
+            var hashSet = new HashSet<char>();
+            foreach (var c in GetUnpaddedBase32(input))
+            {
+                if (!EncodingChars.Contains(c))
+                {
+                    hashSet.Add(c);
+                }
+            }
+
+            invalidCharacters = new string(hashSet.ToArray());
+
+            return hashSet.Count == 0;
+        }
+
+        private static string GetUnpaddedBase32(string input)
+        {
+            return input.ToUpperInvariant().TrimEnd(PaddingCharacter);
+        }
     }
 }

KeeTrayTOTP/Localization/Strings.resx

@@ -210,6 +210,9 @@
   <data name="SetupInvalidCharacter" xml:space="preserve">
     <value>Invalid character</value>
   </data>
+  <data name="SetupInvalidPadding" xml:space="preserve">
+    <value>Padding (=) can only appear at the end!</value>
+  </data>
   <data name="SetupInvalidUrl" xml:space="preserve">
     <value>Invalid URL!</value>
   </data>

KeeTrayTOTP/SetupTOTP.cs

@@ -3,6 +3,7 @@
 using KeePass.UI;
 using KeePassLib;
 using KeePassLib.Security;
+using KeeTrayTOTP.Libraries;
 
 namespace KeeTrayTOTP
 {
@@ -102,9 +103,13 @@ private void ButtonFinish_Click(object sender, EventArgs e)
             {
                 ErrorProviderSetup.SetError(TextBoxSeedSetup, Localization.Strings.SetupSeedCantBeEmpty);
             }
-            else if (!TextBoxSeedSetup.Text.ExtWithoutSpaces().ExtIsBase32(out invalidBase32Chars)) // TODO: Add support to other known formats
+            else if (Base32.HasInvalidPadding(TextBoxSeedSetup.Text.ExtWithoutSpaces()))
             {
-                ErrorProviderSetup.SetError(TextBoxSeedSetup, Localization.Strings.SetupInvalidCharacter + "(" + invalidBase32Chars + ")!");
+                ErrorProviderSetup.SetError(TextBoxSeedSetup, Localization.Strings.SetupInvalidPadding);
+            }
+            else if (!TextBoxSeedSetup.Text.ExtWithoutSpaces().IsBase32(out invalidBase32Chars))
+            {
+                ErrorProviderSetup.SetError(TextBoxSeedSetup, Localization.Strings.SetupInvalidCharacter + "(" + invalidBase32Chars + ")");
             }
             else
             {

KeeTrayTOTP/TrayTOTP_Extensions.cs

@@ -1,4 +1,5 @@
 using KeePassLib;
+using KeeTrayTOTP.Libraries;
 using System;
 
 namespace KeeTrayTOTP
@@ -113,38 +114,6 @@ internal static string ExtSplit(this string extension, int index, char seperator
             return string.Empty;
         }
 
-        /// <summary>
-        /// Makes sure the string provided as a Seed is Base32. Invalid characters are available as out string.
-        /// </summary>
-        /// <param name="extension">Current string.</param>
-        /// <param name="invalidChars">Invalid characters.</param>
-        /// <returns>Validity of the string's characters for Base32 format.</returns>
-        internal static bool ExtIsBase32(this string extension, out string invalidChars)
-        {
-            invalidChars = null;
-            try
-            {
-                foreach (var currentChar in extension)
-                {
-                    var currentCharValue = char.GetNumericValue(currentChar);
-                    if (char.IsLetter(currentChar))
-                    {
-                        continue;
-                    }
-                    if (char.IsDigit(currentChar) && (currentCharValue > 1) && (currentCharValue < 8))
-                    {
-                        continue;
-                    }
-                    invalidChars = (invalidChars + currentCharValue.ToString().ExtWithSpaceBefore()).Trim();
-                }
-            }
-            catch (Exception)
-            {
-                invalidChars = Localization.Strings.Error;
-            }
-            return invalidChars == null;
-        }
-
         internal static bool IsExpired(this PwEntry passwordEntry)
         {
             if (!passwordEntry.Expires)