From 6f71d431c921e8b9aab0bbe89b3293671b81563e Mon Sep 17 00:00:00 2001 From: Guillaume Lacasa Date: Mon, 17 Mar 2014 18:01:40 +0100 Subject: [PATCH] The UsedCodeManager needs to have an user object, in order to avoid a user to block a temp code for every other users --- .../Controllers/HomeController.cs | 2 +- .../MockUsedCodesManager.cs | 6 +-- .../TimeAuthenticatorTests.cs | 4 +- .../UsedCodesManagerTests.cs | 6 +-- TwoStepsAuthenticator/IUsedCodesManager.cs | 8 +-- TwoStepsAuthenticator/TimeAuthenticator.cs | 52 ++++++++++++++----- TwoStepsAuthenticator/UsedCodesManager.cs | 41 ++++++++------- 7 files changed, 76 insertions(+), 43 deletions(-) diff --git a/TwoStepsAuthenticator.TestWebsite/Controllers/HomeController.cs b/TwoStepsAuthenticator.TestWebsite/Controllers/HomeController.cs index ca6666c..18b0c29 100644 --- a/TwoStepsAuthenticator.TestWebsite/Controllers/HomeController.cs +++ b/TwoStepsAuthenticator.TestWebsite/Controllers/HomeController.cs @@ -45,7 +45,7 @@ public ActionResult DoubleAuth(string code) { WebsiteUser user = (WebsiteUser)Session["AuthenticatedUser"]; var auth = new TwoStepsAuthenticator.TimeAuthenticator(usedCodeManager: usedCodesManager); - if (auth.CheckCode(user.DoubleAuthKey, code)) + if (auth.CheckCode(user.DoubleAuthKey, code, user)) { FormsAuthentication.SetAuthCookie(user.Login, true); return RedirectToAction("Welcome"); diff --git a/TwoStepsAuthenticator.UnitTests/MockUsedCodesManager.cs b/TwoStepsAuthenticator.UnitTests/MockUsedCodesManager.cs index cb0065f..3ed8ba8 100644 --- a/TwoStepsAuthenticator.UnitTests/MockUsedCodesManager.cs +++ b/TwoStepsAuthenticator.UnitTests/MockUsedCodesManager.cs @@ -1,15 +1,15 @@ namespace TwoStepsAuthenticator.UnitTests { internal class MockUsedCodesManager : IUsedCodesManager { - public ulong? LastChallenge { get; private set; } + public long? LastChallenge { get; private set; } public string LastCode { get; private set; } - public void AddCode(ulong challenge, string code) { + public void AddCode(long challenge, string code, object user) { this.LastChallenge = challenge; this.LastCode = code; } - public bool IsCodeUsed(ulong challenge, string code) { + public bool IsCodeUsed(long challenge, string code, object user) { return false; } } diff --git a/TwoStepsAuthenticator.UnitTests/TimeAuthenticatorTests.cs b/TwoStepsAuthenticator.UnitTests/TimeAuthenticatorTests.cs index 99d1957..8aacfe1 100644 --- a/TwoStepsAuthenticator.UnitTests/TimeAuthenticatorTests.cs +++ b/TwoStepsAuthenticator.UnitTests/TimeAuthenticatorTests.cs @@ -46,7 +46,7 @@ public void Uses_usedCodesManager() public void Prevent_code_reuse() { var date = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc); var usedCodesManager = new UsedCodesManager(); - var authenticator = new TimeAuthenticator(() => date, usedCodeManager: usedCodesManager); + var authenticator = new TimeAuthenticator(usedCodesManager, () => date); var secret = Authenticator.GenerateKey(); var code = authenticator.GetCode(secret); @@ -76,7 +76,7 @@ public void VerifyUsedTime() DateTime usedTime; - Assert.True(authenticator.CheckCode("H22Q7WAMQYFZOJ2Q", "696227", out usedTime)); + Assert.True(authenticator.CheckCode("H22Q7WAMQYFZOJ2Q", "696227", null, out usedTime)); // 17:23:50 - 30s Assert.AreEqual(usedTime.Hour, 17); diff --git a/TwoStepsAuthenticator.UnitTests/UsedCodesManagerTests.cs b/TwoStepsAuthenticator.UnitTests/UsedCodesManagerTests.cs index d72474c..ffe40ae 100644 --- a/TwoStepsAuthenticator.UnitTests/UsedCodesManagerTests.cs +++ b/TwoStepsAuthenticator.UnitTests/UsedCodesManagerTests.cs @@ -14,9 +14,9 @@ public class UsedCodesManagerTests { public void Can_add_codes() { var manager = new UsedCodesManager(); - Assert.IsFalse(manager.IsCodeUsed(42L, "def")); - manager.AddCode(42L, "def"); - Assert.IsTrue(manager.IsCodeUsed(42L, "def")); + Assert.IsFalse(manager.IsCodeUsed(42L, "def","u")); + manager.AddCode(42L, "def", "u"); + Assert.IsTrue(manager.IsCodeUsed(42L, "def", "u")); } } diff --git a/TwoStepsAuthenticator/IUsedCodesManager.cs b/TwoStepsAuthenticator/IUsedCodesManager.cs index 8ff1fe3..9455d88 100644 --- a/TwoStepsAuthenticator/IUsedCodesManager.cs +++ b/TwoStepsAuthenticator/IUsedCodesManager.cs @@ -12,14 +12,16 @@ public interface IUsedCodesManager { /// /// Used Challenge /// Used Code - void AddCode(long timestamp, string code); + /// The user + void AddCode(long timestamp, string code, object user); /// /// Checks if code was previously used. /// /// Used Challenge /// Used Code - /// - bool IsCodeUsed(long timestamp, string code); + /// The user + /// True if the user as already used the code + bool IsCodeUsed(long timestamp, string code, object user); } } diff --git a/TwoStepsAuthenticator/TimeAuthenticator.cs b/TwoStepsAuthenticator/TimeAuthenticator.cs index 8c42e41..24441fe 100644 --- a/TwoStepsAuthenticator/TimeAuthenticator.cs +++ b/TwoStepsAuthenticator/TimeAuthenticator.cs @@ -4,19 +4,22 @@ using System.Text; using System.Threading.Tasks; -namespace TwoStepsAuthenticator { - +namespace TwoStepsAuthenticator +{ + /// /// Implementation of rfc6238 Time-Based One-Time Password Algorithm /// - public class TimeAuthenticator : Authenticator { + public class TimeAuthenticator : Authenticator + { private static readonly Lazy DefaultUsedCodeManager = new Lazy(() => new UsedCodesManager()); private readonly Func NowFunc; private readonly IUsedCodesManager UsedCodeManager; private readonly int IntervalSeconds; - public TimeAuthenticator(IUsedCodesManager usedCodeManager = null, Func nowFunc = null, int intervalSeconds = 30) { + public TimeAuthenticator(IUsedCodesManager usedCodeManager = null, Func nowFunc = null, int intervalSeconds = 30) + { this.NowFunc = (nowFunc == null) ? () => DateTime.Now : nowFunc; this.UsedCodeManager = (usedCodeManager == null) ? DefaultUsedCodeManager.Value : usedCodeManager; this.IntervalSeconds = intervalSeconds; @@ -27,7 +30,8 @@ public TimeAuthenticator(IUsedCodesManager usedCodeManager = null, Func /// Shared Secret /// OTP - public string GetCode(string secret) { + public string GetCode(string secret) + { return GetCode(secret, NowFunc()); } @@ -37,7 +41,8 @@ public string GetCode(string secret) { /// Shared Secret /// Time to use as challenge /// OTP - public string GetCode(string secret, DateTime date) { + public string GetCode(string secret, DateTime date) + { return GetCodeInternal(secret, (ulong)GetInterval(date)); } @@ -46,11 +51,13 @@ public string GetCode(string secret, DateTime date) { /// /// Shared Secret /// OTP + /// The user /// true if code matches - public bool CheckCode(string secret, string code) { + public bool CheckCode(string secret, string code, object user) + { DateTime successfulTime = DateTime.MinValue; - return CheckCode(secret, code, out successfulTime); + return CheckCode(secret, code, user, out successfulTime); } /// @@ -58,23 +65,27 @@ public bool CheckCode(string secret, string code) { /// /// Shared Secret /// OTP + /// The user /// Matching time if successful /// true if code matches - public bool CheckCode(string secret, string code, out DateTime usedDateTime) { + public bool CheckCode(string secret, string code, object user, out DateTime usedDateTime) + { var baseTime = NowFunc(); DateTime successfulTime = DateTime.MinValue; // We need to do this in constant time var codeMatch = false; - for (int i = -2; i <= 1; i++) { + for (int i = -2; i <= 1; i++) + { var checkTime = baseTime.AddSeconds(IntervalSeconds * i); var checkInterval = GetInterval(checkTime); - if (ConstantTimeEquals(GetCode(secret, checkTime), code) && !UsedCodeManager.IsCodeUsed(checkInterval, code)) { + if (ConstantTimeEquals(GetCode(secret, checkTime), code) && (user == null || !UsedCodeManager.IsCodeUsed(checkInterval, code, user))) + { codeMatch = true; successfulTime = checkTime; - UsedCodeManager.AddCode(checkInterval, code); + UsedCodeManager.AddCode(checkInterval, code, user); } } @@ -82,7 +93,22 @@ public bool CheckCode(string secret, string code, out DateTime usedDateTime) { return codeMatch; } - private long GetInterval(DateTime dateTime) { + + /// + /// Checks if the passed code is valid. + /// + /// Shared Secret + /// OTP + /// true if code matches + public bool CheckCode(string secret, string code) + { + DateTime successfulTime = DateTime.MinValue; + + return CheckCode(secret, code, null, out successfulTime); + } + + private long GetInterval(DateTime dateTime) + { TimeSpan ts = (dateTime.ToUniversalTime() - new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc)); return (long)ts.TotalSeconds / IntervalSeconds; } diff --git a/TwoStepsAuthenticator/UsedCodesManager.cs b/TwoStepsAuthenticator/UsedCodesManager.cs index 13b3d61..dbd96a7 100644 --- a/TwoStepsAuthenticator/UsedCodesManager.cs +++ b/TwoStepsAuthenticator/UsedCodesManager.cs @@ -14,25 +14,28 @@ public class UsedCodesManager : IUsedCodesManager { internal sealed class UsedCode { - public UsedCode(long timestamp, String code) + public UsedCode(long timestamp, String code, object user) { this.UseDate = DateTime.Now; this.Code = code; this.Timestamp = timestamp; + this.User = user; } internal DateTime UseDate { get; private set; } internal long Timestamp { get; private set; } internal String Code { get; private set; } + internal object User { get; private set; } public override bool Equals(object obj) { - if (Object.ReferenceEquals(this, obj)) { + if (Object.ReferenceEquals(this, obj)) + { return true; } var other = obj as UsedCode; - return (other != null) ? this.Code.Equals(other.Code) && this.Timestamp.Equals(other.Timestamp) : false; + return (other != null) && this.Code.Equals(other.Code) && this.Timestamp.Equals(other.Timestamp) && this.User.Equals(other.User); } public override string ToString() { @@ -40,7 +43,7 @@ public override string ToString() } public override int GetHashCode() { - return Code.GetHashCode() + Timestamp.GetHashCode() * 17; + return Code.GetHashCode() + (Timestamp.GetHashCode() + User.GetHashCode() * 17) * 17; } } @@ -61,42 +64,44 @@ void cleaner_Elapsed(object sender, ElapsedEventArgs e) { var timeToClean = DateTime.Now.AddMinutes(-5); - try + try { rwlock.AcquireWriterLock(lockingTimeout); - while (codes.Count > 0 && codes.Peek().UseDate < timeToClean) { + while (codes.Count > 0 && codes.Peek().UseDate < timeToClean) + { codes.Dequeue(); } - } - finally + } + finally { rwlock.ReleaseWriterLock(); } } - public void AddCode(long timestamp, String code) + public void AddCode(long timestamp, String code, object user) { - try { + try + { rwlock.AcquireWriterLock(lockingTimeout); - codes.Enqueue(new UsedCode(timestamp, code)); - } - finally + codes.Enqueue(new UsedCode(timestamp, code, user)); + } + finally { rwlock.ReleaseWriterLock(); } } - public bool IsCodeUsed(long timestamp, String code) + public bool IsCodeUsed(long timestamp, String code, object user) { - try + try { rwlock.AcquireReaderLock(lockingTimeout); - return codes.Contains(new UsedCode(timestamp, code)); - } - finally + return codes.Contains(new UsedCode(timestamp, code, user)); + } + finally { rwlock.ReleaseReaderLock(); }