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

Issue #65 ValidateTwoFactorPin always returns false, if the secretKey parameter is base32 encoded string - fixed #66

Merged
merged 4 commits into from
Jul 9, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion Google.Authenticator.Tests/AuthCodeTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Shouldly;
using System.Text;
using Shouldly;
using Xunit;

namespace Google.Authenticator.Tests
Expand All @@ -21,5 +22,22 @@ public void BasicAuthCodeTest()

actual.ShouldBe(expected);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

💯

[Fact]
public void Base32AuthCodeTest()
{
var secretKey = Base32Encoding.ToString(Encoding.UTF8.GetBytes("PJWUMZKAUUFQKJBAMD6VGJ6RULFVW4ZH"));
var expected = "551508";

var tfa = new TwoFactorAuthenticator();

var currentTime = 1416643820;

// I actually think you are supposed to divide the time by 30 seconds?
// Maybe need an overload that takes a DateTime?
var actual = tfa.GeneratePINAtInterval(secretKey, currentTime, 6, true);

actual.ShouldBe(expected);
}
}
}
39 changes: 26 additions & 13 deletions Google.Authenticator/Base32Encoding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ public class Base32Encoding
public static byte[] ToBytes(string input)
{
if (string.IsNullOrEmpty(input))
{
throw new ArgumentNullException(nameof(input));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here :)

Copy link
Author

Choose a reason for hiding this comment

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

Done :)

}

input = input.TrimEnd('='); //remove padding characters
var byteCount = input.Length * 5 / 8; //this must be TRUNCATED
Expand All @@ -31,15 +33,15 @@ public static byte[] ToBytes(string input)
if (bitsRemaining > 5)
{
mask = cValue << (bitsRemaining - 5);
curByte = (byte)(curByte | mask);
curByte = (byte) (curByte | mask);
bitsRemaining -= 5;
}
else
{
mask = cValue >> (5 - bitsRemaining);
curByte = (byte)(curByte | mask);
curByte = (byte) (curByte | mask);
returnArray[arrayIndex++] = curByte;
curByte = (byte)(cValue << (3 + bitsRemaining));
curByte = (byte) (cValue << (3 + bitsRemaining));
bitsRemaining += 3;
}
}
Expand All @@ -59,28 +61,30 @@ public static byte[] ToBytes(string input)
public static string ToString(byte[] input)
{
if (input == null || input.Length == 0)
{
throw new ArgumentNullException(nameof(input));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

Copy link
Author

Choose a reason for hiding this comment

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

Done

}

var charCount = (int)Math.Ceiling(input.Length / 5d) * 8;
var charCount = (int) Math.Ceiling(input.Length / 5d) * 8;
var returnArray = new char[charCount];

byte nextChar = 0, bitsRemaining = 5;
var arrayIndex = 0;

foreach (var b in input)
{
nextChar = (byte)(nextChar | (b >> (8 - bitsRemaining)));
nextChar = (byte) (nextChar | (b >> (8 - bitsRemaining)));
returnArray[arrayIndex++] = ValueToChar(nextChar);

if (bitsRemaining < 4)
{
nextChar = (byte)((b >> (3 - bitsRemaining)) & 31);
nextChar = (byte) ((b >> (3 - bitsRemaining)) & 31);
returnArray[arrayIndex++] = ValueToChar(nextChar);
bitsRemaining += 5;
}

bitsRemaining -= 3;
nextChar = (byte)((b << bitsRemaining) & 31);
nextChar = (byte) ((b << bitsRemaining) & 31);
}

//if we didn't end with a full char
Expand All @@ -95,33 +99,42 @@ public static string ToString(byte[] input)

private static int CharToValue(char c)
{
var value = (int)c;
var value = (int) c;

//65-90 == uppercase letters
if (value < 91 && value > 64)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

Copy link
Author

Choose a reason for hiding this comment

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

Done

{
return value - 65;

}

//50-55 == numbers 2-7
if (value < 56 && value > 49)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

{
return value - 24;

}

//97-122 == lowercase letters
if (value < 123 && value > 96)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

{
return value - 97;
}

throw new ArgumentException("Character is not a Base32 character.", nameof(c));
}

private static char ValueToChar(byte b)
{
if (b < 26)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

return (char)(b + 65);
{
return (char) (b + 65);
}

if (b < 32)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

return (char)(b + 24);
{
return (char) (b + 24);
}

throw new ArgumentException("Byte is not a value Base32 value.", nameof(b));
}

}
}
63 changes: 39 additions & 24 deletions Google.Authenticator/TwoFactorAuthenticator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ namespace Google.Authenticator
/// </summary>
public class TwoFactorAuthenticator
{
private static readonly DateTime _epoch =
private static readonly DateTime _epoch =
new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);

private TimeSpan DefaultClockDriftTolerance { get; set; }

public TwoFactorAuthenticator() => DefaultClockDriftTolerance = TimeSpan.FromMinutes(5);
Expand All @@ -35,16 +36,16 @@ public class TwoFactorAuthenticator
/// should be 10 or less)</param>
/// <returns>SetupCode object</returns>
public SetupCode GenerateSetupCode(
string issuer,
string accountTitleNoSpaces,
string issuer,
string accountTitleNoSpaces,
string accountSecretKey,
bool secretIsBase32,
bool secretIsBase32,
int qrPixelsPerModule = 3)
{
var key = secretIsBase32
? Base32Encoding.ToBytes(accountSecretKey)
: Encoding.UTF8.GetBytes(accountSecretKey);

return GenerateSetupCode(issuer, accountTitleNoSpaces, key, qrPixelsPerModule);
}

Expand All @@ -67,21 +68,24 @@ public SetupCode GenerateSetupCode(
bool generateQrCode = true)
{
if (string.IsNullOrWhiteSpace(accountTitleNoSpaces))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

{
throw new NullReferenceException("Account Title is null");

}

accountTitleNoSpaces = RemoveWhitespace(Uri.EscapeUriString(accountTitleNoSpaces));
var encodedSecretKey = Base32Encoding.ToString(accountSecretKey);

var provisionUrl = string.IsNullOrWhiteSpace(issuer)
? $"otpauth://totp/{accountTitleNoSpaces}?secret={encodedSecretKey.Trim('=')}"
// https://github.com/google/google-authenticator/wiki/Conflicting-Accounts
// Added additional prefix to account otpauth://totp/Company:[email protected]
// for backwards compatibility
: $"otpauth://totp/{UrlEncode(issuer)}:{accountTitleNoSpaces}?secret={encodedSecretKey.Trim('=')}&issuer={UrlEncode(issuer)}";

if (!generateQrCode)
return new SetupCode(accountTitleNoSpaces, encodedSecretKey.Trim('='), "");

return new SetupCode(
accountTitleNoSpaces,
encodedSecretKey.Trim('='),
GenerateQrCodeUrl(qrPixelsPerModule, provisionUrl));
generateQrCode ? GenerateQrCodeUrl(qrPixelsPerModule, provisionUrl) : "");
}

private static string GenerateQrCodeUrl(int qrPixelsPerModule, string provisionUrl)
Expand All @@ -96,7 +100,6 @@ private static string GenerateQrCodeUrl(int qrPixelsPerModule, string provisionU
using (var ms = new MemoryStream())
{
qrCodeImage.Save(ms, System.Drawing.Imaging.ImageFormat.Png);

qrCodeUrl = $"data:image/png;base64,{Convert.ToBase64String(ms.ToArray())}";
}
}
Expand Down Expand Up @@ -126,7 +129,7 @@ private static string GenerateQrCodeUrl(int qrPixelsPerModule, string provisionU
return qrCodeUrl;
}

private static string RemoveWhitespace(string str) =>
private static string RemoveWhitespace(string str) =>
new string(str.Where(c => !char.IsWhiteSpace(c)).ToArray());

private string UrlEncode(string value)
Expand All @@ -137,29 +140,37 @@ private string UrlEncode(string value)
foreach (var symbol in value)
{
if (validChars.IndexOf(symbol) == -1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

{
result.AppendFormat("%{0:X2}", (int) symbol);
}
else
{
result.Append(symbol);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove {} here

}
}

return result.Replace(" ", "%20").ToString();
}

public string GeneratePINAtInterval(
string accountSecretKey,
string accountSecretKey,
long counter,
int digits = 6,
int digits = 6,
bool secretIsBase32 = false)
=> GenerateHashedCode(accountSecretKey, counter, secretIsBase32, digits);
{
return GenerateHashedCode(accountSecretKey, counter, secretIsBase32, digits);
}

private string GenerateHashedCode(string secret,
long iterationNumber,
bool secretIsBase32,
int digits = 6)
=> GenerateHashedCode(
secretIsBase32 ? Base32Encoding.ToBytes(secret):Encoding.UTF8.GetBytes(secret),
{
return GenerateHashedCode(
secretIsBase32 ? Base32Encoding.ToBytes(secret) : Encoding.UTF8.GetBytes(secret),
iterationNumber,
digits);
}

private string GenerateHashedCode(byte[] key, long iterationNumber, int digits = 6)
{
Expand All @@ -169,9 +180,7 @@ private string GenerateHashedCode(byte[] key, long iterationNumber, int digits =
Array.Reverse(counter);

var hmac = new HMACSHA1(key);

var hash = hmac.ComputeHash(counter);

var offset = hash[hash.Length - 1] & 0xf;

// Convert the 4 bytes into an integer, ignoring the sign.
Expand All @@ -191,17 +200,23 @@ private long GetCurrentCounter(DateTime now, DateTime epoch, int timeStep) =>
(long) (now - epoch).TotalSeconds / timeStep;

public bool ValidateTwoFactorPIN(
string accountSecretKey,
string accountSecretKey,
string twoFactorCodeFromClient,
bool secretIsBase32 = false) =>
ValidateTwoFactorPIN(accountSecretKey, twoFactorCodeFromClient, DefaultClockDriftTolerance, secretIsBase32);
bool secretIsBase32 = false)
{
return ValidateTwoFactorPIN(accountSecretKey, twoFactorCodeFromClient, DefaultClockDriftTolerance,
secretIsBase32);
}

public bool ValidateTwoFactorPIN(
string accountSecretKey,
string twoFactorCodeFromClient,
TimeSpan timeTolerance,
bool secretIsBase32 = false) =>
GetCurrentPINs(accountSecretKey, timeTolerance, secretIsBase32).Any(c => c == twoFactorCodeFromClient);
bool secretIsBase32 = false)
{
return GetCurrentPINs(accountSecretKey, timeTolerance, secretIsBase32)
.Any(c => c == twoFactorCodeFromClient);
}

public string GetCurrentPIN(string accountSecretKey, bool secretIsBase32 = false) =>
GeneratePINAtInterval(accountSecretKey, GetCurrentCounter(), secretIsBase32: secretIsBase32);
Expand Down