Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak base32 implementation #131

Merged
merged 5 commits into from
Apr 25, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
109 changes: 109 additions & 0 deletions KeeTrayTOTP.Tests/Base32Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
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();
}

[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("!======!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this should then only return !! as invalidChars, shouldn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would depend on the outcome above. I think it could say just '!', or '!!', but in this example the = is not used as padding and is invalid as well...

}

/// <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}";
}
}
}
}
2 changes: 1 addition & 1 deletion KeeTrayTOTP.Tests/TrayTOTP_ColumnproviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
81 changes: 66 additions & 15 deletions KeeTrayTOTP/Libraries/Base32.cs
Original file line number Diff line number Diff line change
@@ -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>
Expand All @@ -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;
Expand All @@ -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];
Expand All @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -136,5 +164,28 @@ public static byte[] Decode(string base32)

return outputBuffer;
}

public static bool IsBase32(this string input)
{
return GetUnpaddedBase32(input).All(EncodingChars.Contains);
}

public static bool IsBase32(this string input, out string invalidCharacters)
{
invalidCharacters = null;
foreach (var c in GetUnpaddedBase32(input))
{
if (!EncodingChars.Contains(c))
dannoe marked this conversation as resolved.
Show resolved Hide resolved
{
invalidCharacters += c.ToString();
}
}

return invalidCharacters == null;
}
private static string GetUnpaddedBase32(string input)
{
return input.ToUpperInvariant().TrimEnd(PaddingCharacter);
}
}
}
3 changes: 2 additions & 1 deletion KeeTrayTOTP/SetupTOTP.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using KeePass.UI;
using KeePassLib;
using KeePassLib.Security;
using KeeTrayTOTP.Libraries;

namespace KeeTrayTOTP
{
Expand Down Expand Up @@ -102,7 +103,7 @@ 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 (!TextBoxSeedSetup.Text.ExtWithoutSpaces().IsBase32(out invalidBase32Chars)) // TODO: Add support to other known formats
{
ErrorProviderSetup.SetError(TextBoxSeedSetup, Localization.Strings.SetupInvalidCharacter + "(" + invalidBase32Chars + ")!");
}
Expand Down
33 changes: 1 addition & 32 deletions KeeTrayTOTP/TrayTOTP_Extensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using KeePassLib;
using KeeTrayTOTP.Libraries;
using System;

namespace KeeTrayTOTP
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 5 additions & 6 deletions KeeTrayTOTP/TrayTOTP_Plugin.cs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ private void OnEntryMenuShowQRClick(object sender, EventArgs e)
}

var rawSeed = this.SeedGet(entry).ReadString();
var cleanSeed = Regex.Replace(rawSeed, @"\s+", "");
var cleanSeed = Regex.Replace(rawSeed, @"\s+", "").TrimEnd('=');
var issuer = entry.Strings.Get("Title").ReadString();
var username = entry.Strings.Get("UserName").ReadString();
UIUtil.ShowDialogAndDestroy(new ShowQR(cleanSeed, issuer, username));
Expand Down Expand Up @@ -800,19 +800,18 @@ internal bool SeedCheck(PwEntry pe)
/// <returns>Validity of the Seed's characters for Base32 format.</returns>
internal bool SeedValidate(PwEntry passwordEntry)
{
string invalidCharacters;
return SeedGet(passwordEntry).ReadString().ExtWithoutSpaces().ExtIsBase32(out invalidCharacters);
return SeedGet(passwordEntry).ReadString().ExtWithoutSpaces().IsBase32();
}

/// <summary>
/// Validates the entry's Seed making sure it's a valid Base32 string. Invalid characters are available as out string.
/// </summary>
/// <param name="passwordEntry">Password Entry.</param>
/// <param name="invalidChars">Password Entry.</param>
/// <param name="invalidCharacters">Password Entry.</param>
/// <returns>Validity of the Seed's characters.</returns>
internal bool SeedValidate(PwEntry passwordEntry, out string invalidChars)
internal bool SeedValidate(PwEntry passwordEntry, out string invalidCharacters)
{
return SeedGet(passwordEntry).ReadString().ExtWithoutSpaces().ExtIsBase32(out invalidChars);
return SeedGet(passwordEntry).ReadString().ExtWithoutSpaces().IsBase32(out invalidCharacters);
}

/// <summary>
Expand Down