Skip to content

Commit

Permalink
Issue BrandonPotter#65 ValidateTwoFactorPin always returns false, if …
Browse files Browse the repository at this point in the history 
…the secretKey parameter is base32 encoded string - fixes for review
  • Loading branch information
@kateryna-novak
kateryna-novak committed Jul 8, 2021
1 parent ff44a79 commit 3cdfd22
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 38 deletions.
20 changes: 19 additions & 1 deletion Google.Authenticator.Tests/AuthCodeTest.cs
View file
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);
}

[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
View file
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));
}

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));
}

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)
{
return value - 65;

}

//50-55 == numbers 2-7
if (value < 56 && value > 49)
{
return value - 24;

}

//97-122 == lowercase letters
if (value < 123 && value > 96)
{
return value - 97;
}

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

private static char ValueToChar(byte b)
{
if (b < 26)
return (char)(b + 65);
{
return (char) (b + 65);
}

if (b < 32)
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
View file
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))
{
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)
{
result.AppendFormat("%{0:X2}", (int) symbol);
}
else
{
result.Append(symbol);
}
}

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

0 comments on commit 3cdfd22

Please sign in to comment.