diff options
Diffstat (limited to 'src')
14 files changed, 148 insertions, 16 deletions
diff --git a/src/DotNetOpenAuth.BuildTasks/DotNetOpenAuth.BuildTasks.csproj b/src/DotNetOpenAuth.BuildTasks/DotNetOpenAuth.BuildTasks.csproj index 7993ed5..179c825 100644 --- a/src/DotNetOpenAuth.BuildTasks/DotNetOpenAuth.BuildTasks.csproj +++ b/src/DotNetOpenAuth.BuildTasks/DotNetOpenAuth.BuildTasks.csproj @@ -1,5 +1,6 @@ <?xml version="1.0" encoding="utf-8"?> <Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.props))\EnlistmentInfo.props" Condition=" '$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.props))' != '' " /> <PropertyGroup> <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> @@ -161,11 +162,5 @@ </BootstrapperPackage> </ItemGroup> <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" /> - <!-- To modify your build process, add your task inside one of the targets below and uncomment it. - Other similar extension points exist, see Microsoft.Common.targets. - <Target Name="BeforeBuild"> - </Target> - <Target Name="AfterBuild"> - </Target> - --> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.targets))\EnlistmentInfo.targets" Condition=" '$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.targets))' != '' " /> </Project>
\ No newline at end of file diff --git a/src/DotNetOpenAuth.Test/DotNetOpenAuth.Test.csproj b/src/DotNetOpenAuth.Test/DotNetOpenAuth.Test.csproj index 596c499..f4d0bb3 100644 --- a/src/DotNetOpenAuth.Test/DotNetOpenAuth.Test.csproj +++ b/src/DotNetOpenAuth.Test/DotNetOpenAuth.Test.csproj @@ -1,5 +1,6 @@ <?xml version="1.0" encoding="utf-8"?> <Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.props))\EnlistmentInfo.props" Condition=" '$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.props))' != '' " /> <PropertyGroup> <ProjectRoot Condition="'$(ProjectRoot)' == ''">$(MSBuildProjectDirectory)\..\..\</ProjectRoot> <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> @@ -391,4 +392,5 @@ </ItemGroup> <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" /> <Import Project="$(ProjectRoot)tools\DotNetOpenAuth.targets" /> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.targets))\EnlistmentInfo.targets" Condition=" '$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.targets))' != '' " /> </Project>
\ No newline at end of file diff --git a/src/DotNetOpenAuth.Test/Messaging/Bindings/StandardExpirationBindingElementTests.cs b/src/DotNetOpenAuth.Test/Messaging/Bindings/StandardExpirationBindingElementTests.cs index 9ba433d..e0c2de6 100644 --- a/src/DotNetOpenAuth.Test/Messaging/Bindings/StandardExpirationBindingElementTests.cs +++ b/src/DotNetOpenAuth.Test/Messaging/Bindings/StandardExpirationBindingElementTests.cs @@ -6,6 +6,8 @@ namespace DotNetOpenAuth.Test.Messaging.Bindings { using System; + + using DotNetOpenAuth.Configuration; using DotNetOpenAuth.Messaging; using DotNetOpenAuth.Messaging.Bindings; using DotNetOpenAuth.Test.Mocks; @@ -30,10 +32,22 @@ namespace DotNetOpenAuth.Test.Messaging.Bindings { this.ParameterizedReceiveProtectedTest(DateTime.UtcNow, false); } + [TestCase] + public void VerifyFutureTimestampWithinClockSkewIsAccepted() { + this.Channel = CreateChannel(MessageProtections.Expiration); + this.ParameterizedReceiveProtectedTest(DateTime.UtcNow + DotNetOpenAuthSection.Configuration.Messaging.MaximumClockSkew - TimeSpan.FromSeconds(1), false); + } + [TestCase, ExpectedException(typeof(ExpiredMessageException))] - public void VerifyBadTimestampIsRejected() { + public void VerifyOldTimestampIsRejected() { this.Channel = CreateChannel(MessageProtections.Expiration); this.ParameterizedReceiveProtectedTest(DateTime.UtcNow - StandardExpirationBindingElement.MaximumMessageAge - TimeSpan.FromSeconds(1), false); } + + [TestCase, ExpectedException(typeof(ProtocolException))] + public void VerifyFutureTimestampIsRejected() { + this.Channel = CreateChannel(MessageProtections.Expiration); + this.ParameterizedReceiveProtectedTest(DateTime.UtcNow + DotNetOpenAuthSection.Configuration.Messaging.MaximumClockSkew + TimeSpan.FromSeconds(1), false); + } } } diff --git a/src/DotNetOpenAuth.Test/Messaging/MessagingUtilitiesTests.cs b/src/DotNetOpenAuth.Test/Messaging/MessagingUtilitiesTests.cs index 1e0ede5..2c2da64 100644 --- a/src/DotNetOpenAuth.Test/Messaging/MessagingUtilitiesTests.cs +++ b/src/DotNetOpenAuth.Test/Messaging/MessagingUtilitiesTests.cs @@ -8,6 +8,7 @@ namespace DotNetOpenAuth.Test.Messaging { using System; using System.Collections.Generic; using System.Collections.Specialized; + using System.Diagnostics; using System.IO; using System.Net; using System.Text.RegularExpressions; @@ -227,5 +228,67 @@ namespace DotNetOpenAuth.Test.Messaging { string roundTripped = MessagingUtilities.Decrypt(cipher, key); Assert.AreEqual(PlainText, roundTripped); } + + /// <summary> + /// Verifies that the time-independent string equality check works accurately. + /// </summary> + [TestCase] + public void EqualsConstantTime() { + this.EqualsConstantTimeHelper(null, null); + this.EqualsConstantTimeHelper(null, string.Empty); + this.EqualsConstantTimeHelper(string.Empty, string.Empty); + this.EqualsConstantTimeHelper(string.Empty, "a"); + this.EqualsConstantTimeHelper(null, "a"); + this.EqualsConstantTimeHelper("a", "a"); + this.EqualsConstantTimeHelper("a", "A"); + this.EqualsConstantTimeHelper("A", "A"); + this.EqualsConstantTimeHelper("ab", "ab"); + this.EqualsConstantTimeHelper("ab", "b"); + } + + /// <summary> + /// Verifies that EqualsConstantTime actually has the same execution time regardless of how well a value matches. + /// </summary> + [TestCase, Category("Performance")] + public void EqualsConstantTimeIsActuallyConstantTime() { + string expected = new string('A', 5000); + string totalmismatch = new string('B', 5000); + string almostmatch = new string('A', 4999) + 'B'; + + const int Iterations = 4000; + var totalMismatchTimer = new Stopwatch(); + totalMismatchTimer.Start(); + for (int i = 0; i < Iterations; i++) { + MessagingUtilities.EqualsConstantTime(expected, totalmismatch); + } + totalMismatchTimer.Stop(); + + var almostMatchTimer = new Stopwatch(); + almostMatchTimer.Start(); + for (int i = 0; i < Iterations; i++) { + MessagingUtilities.EqualsConstantTime(expected, almostmatch); + } + almostMatchTimer.Stop(); + + const double ToleranceFactor = 0.06; + long averageTimeTicks = (totalMismatchTimer.ElapsedTicks + almostMatchTimer.ElapsedTicks) / 2; + var tolerableDifference = TimeSpan.FromTicks((long)(averageTimeTicks * ToleranceFactor)); + var absoluteDifference = TimeSpan.FromTicks(Math.Abs(totalMismatchTimer.ElapsedTicks - almostMatchTimer.ElapsedTicks)); + double actualFactor = (double)absoluteDifference.Ticks / averageTimeTicks; + Assert.IsTrue(absoluteDifference <= tolerableDifference, "A total mismatch took {0} but a near match took {1}, which is too different to be indistinguishable. The tolerable difference is {2} but the actual difference is {3}. This represents a difference of {4}%, beyond the tolerated {5}%.", totalMismatchTimer.Elapsed, almostMatchTimer.Elapsed, tolerableDifference, absoluteDifference, Math.Round(actualFactor * 100), Math.Round(ToleranceFactor * 100)); + Console.WriteLine("A total mismatch took {0} and a near match took {1}. The tolerable difference is {2}, and the actual difference is {3}. This represents a difference of {4}%, within the tolerated {5}%.", totalMismatchTimer.Elapsed, almostMatchTimer.Elapsed, tolerableDifference, absoluteDifference, Math.Round(actualFactor * 100), Math.Round(ToleranceFactor * 100)); + Console.WriteLine("The equality test execution time difference was only {0}%, within the tolerable {1}%", Math.Round(100 * actualFactor), Math.Round(ToleranceFactor * 100)); + } + + /// <summary> + /// Verifies that the time-independent string equality check works for a given pair of strings. + /// </summary> + /// <param name="value1">The first value.</param> + /// <param name="value2">The second value.</param> + private void EqualsConstantTimeHelper(string value1, string value2) { + bool expected = string.Equals(value1, value2, StringComparison.Ordinal); + Assert.AreEqual(expected, MessagingUtilities.EqualsConstantTime(value1, value2)); + Assert.AreEqual(expected, MessagingUtilities.EqualsConstantTime(value2, value1)); + } } } diff --git a/src/DotNetOpenAuth.Test/OAuth/ChannelElements/OAuthChannelTests.cs b/src/DotNetOpenAuth.Test/OAuth/ChannelElements/OAuthChannelTests.cs index dd6738f..54ed37e 100644 --- a/src/DotNetOpenAuth.Test/OAuth/ChannelElements/OAuthChannelTests.cs +++ b/src/DotNetOpenAuth.Test/OAuth/ChannelElements/OAuthChannelTests.cs @@ -359,7 +359,7 @@ namespace DotNetOpenAuth.Test.OAuth.ChannelElements { { "Name", "Andrew" }, { "Location", "http://hostb/pathB" }, { "Timestamp", XmlConvert.ToString(DateTime.UtcNow, XmlDateTimeSerializationMode.Utc) }, - { "realm" , "someValue" }, + { "realm", "someValue" }, }; IProtocolMessage requestMessage = this.channel.ReadFromRequest(CreateHttpRequestInfo(scheme, fields)); Assert.IsNotNull(requestMessage); diff --git a/src/DotNetOpenAuth/DotNetOpenAuth.csproj b/src/DotNetOpenAuth/DotNetOpenAuth.csproj index 1287b61..f402c95 100644 --- a/src/DotNetOpenAuth/DotNetOpenAuth.csproj +++ b/src/DotNetOpenAuth/DotNetOpenAuth.csproj @@ -1,7 +1,7 @@ <?xml version="1.0" encoding="utf-8"?> <Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.props))\EnlistmentInfo.props" Condition=" '$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.props))' != '' " /> <PropertyGroup> - <ProjectRoot Condition="'$(ProjectRoot)' == ''">$(MSBuildProjectDirectory)\..\..\</ProjectRoot> <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> <CodeContractsAssemblyMode>1</CodeContractsAssemblyMode> @@ -860,4 +860,5 @@ http://opensource.org/licenses/ms-pl.html </Target> <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" /> <Import Project="$(ProjectRoot)tools\DotNetOpenAuth.targets" /> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.targets))\EnlistmentInfo.targets" Condition=" '$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), EnlistmentInfo.targets))' != '' " /> </Project>
\ No newline at end of file diff --git a/src/DotNetOpenAuth/Messaging/Bindings/StandardExpirationBindingElement.cs b/src/DotNetOpenAuth/Messaging/Bindings/StandardExpirationBindingElement.cs index ddfa88a..4396c16 100644 --- a/src/DotNetOpenAuth/Messaging/Bindings/StandardExpirationBindingElement.cs +++ b/src/DotNetOpenAuth/Messaging/Bindings/StandardExpirationBindingElement.cs @@ -83,11 +83,19 @@ namespace DotNetOpenAuth.Messaging.Bindings { if (expiringMessage != null) { // Yes the UtcCreationDate is supposed to always be in UTC already, // but just in case a given message failed to guarantee that, we do it here. - DateTime expirationDate = expiringMessage.UtcCreationDate.ToUniversalTimeSafe() + MaximumMessageAge; + DateTime creationDate = expiringMessage.UtcCreationDate.ToUniversalTimeSafe(); + DateTime expirationDate = creationDate + MaximumMessageAge; if (expirationDate < DateTime.UtcNow) { throw new ExpiredMessageException(expirationDate, expiringMessage); } + // Mitigate HMAC attacks (just guessing the signature until they get it) by + // disallowing post-dated messages. + ErrorUtilities.VerifyProtocol( + creationDate <= DateTime.UtcNow + DotNetOpenAuthSection.Configuration.Messaging.MaximumClockSkew, + MessagingStrings.MessageTimestampInFuture, + creationDate); + return MessageProtections.Expiration; } diff --git a/src/DotNetOpenAuth/Messaging/MessagingStrings.Designer.cs b/src/DotNetOpenAuth/Messaging/MessagingStrings.Designer.cs index e34dc03..f600330 100644 --- a/src/DotNetOpenAuth/Messaging/MessagingStrings.Designer.cs +++ b/src/DotNetOpenAuth/Messaging/MessagingStrings.Designer.cs @@ -322,6 +322,15 @@ namespace DotNetOpenAuth.Messaging { } /// <summary> + /// Looks up a localized string similar to This message has a timestamp of {0}, which is beyond the allowable clock skew for in the future.. + /// </summary> + internal static string MessageTimestampInFuture { + get { + return ResourceManager.GetString("MessageTimestampInFuture", resourceCulture); + } + } + + /// <summary> /// Looks up a localized string similar to A non-empty string was expected.. /// </summary> internal static string NonEmptyStringExpected { diff --git a/src/DotNetOpenAuth/Messaging/MessagingStrings.resx b/src/DotNetOpenAuth/Messaging/MessagingStrings.resx index d7d0b01..7f9d91b 100644 --- a/src/DotNetOpenAuth/Messaging/MessagingStrings.resx +++ b/src/DotNetOpenAuth/Messaging/MessagingStrings.resx @@ -312,4 +312,7 @@ <data name="EncoderInstantiationFailed" xml:space="preserve"> <value>Unable to instantiate the message part encoder/decoder type {0}.</value> </data> -</root>
\ No newline at end of file + <data name="MessageTimestampInFuture" xml:space="preserve"> + <value>This message has a timestamp of {0}, which is beyond the allowable clock skew for in the future.</value> + </data> +</root> diff --git a/src/DotNetOpenAuth/Messaging/MessagingUtilities.cs b/src/DotNetOpenAuth/Messaging/MessagingUtilities.cs index 1860770..9dbd1b9 100644 --- a/src/DotNetOpenAuth/Messaging/MessagingUtilities.cs +++ b/src/DotNetOpenAuth/Messaging/MessagingUtilities.cs @@ -682,6 +682,43 @@ namespace DotNetOpenAuth.Messaging { } /// <summary> + /// Compares to string values for ordinal equality in such a way that its execution time does not depend on how much of the value matches. + /// </summary> + /// <param name="value1">The first value.</param> + /// <param name="value2">The second value.</param> + /// <returns>A value indicating whether the two strings share ordinal equality.</returns> + /// <remarks> + /// In signature equality checks, a difference in execution time based on how many initial characters match MAY + /// be used as an attack to figure out the expected signature. It is therefore important to make a signature + /// equality check's execution time independent of how many characters match the expected value. + /// See http://codahale.com/a-lesson-in-timing-attacks/ for more information. + /// </remarks> + internal static bool EqualsConstantTime(string value1, string value2) { + // If exactly one value is null, they don't match. + if (value1 == null ^ value2 == null) { + return false; + } + + // If both values are null (since if one is at this point then they both are), it's a match. + if (value1 == null) { + return true; + } + + if (value1.Length != value2.Length) { + return false; + } + + // This looks like a pretty crazy way to compare values, but it provides a constant time equality check, + // and is more resistant to compiler optimizations than simply setting a boolean flag and returning the boolean after the loop. + int result = 0; + for (int i = 0; i < value1.Length; i++) { + result |= value1[i] ^ value2[i]; + } + + return result == 0; + } + + /// <summary> /// Adds a set of HTTP headers to an <see cref="HttpResponse"/> instance, /// taking care to set some headers to the appropriate properties of /// <see cref="HttpResponse" /> diff --git a/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementBase.cs b/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementBase.cs index fdf6e08..cb81139 100644 --- a/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementBase.cs +++ b/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementBase.cs @@ -273,7 +273,7 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { Contract.Requires<ArgumentNullException>(message != null); string signature = this.GetSignature(message); - return message.Signature == signature; + return MessagingUtilities.EqualsConstantTime(message.Signature, signature); } /// <summary> diff --git a/src/DotNetOpenAuth/OAuth/Messages/AuthorizedTokenRequest.cs b/src/DotNetOpenAuth/OAuth/Messages/AuthorizedTokenRequest.cs index 1228290..02c6c1d 100644 --- a/src/DotNetOpenAuth/OAuth/Messages/AuthorizedTokenRequest.cs +++ b/src/DotNetOpenAuth/OAuth/Messages/AuthorizedTokenRequest.cs @@ -42,7 +42,7 @@ namespace DotNetOpenAuth.OAuth.Messages { public string VerificationCode { get; set; } /// <summary> - /// Gets or sets the unauthorized Request Token used to obtain authorization. + /// Gets or sets the authorized Request Token used to obtain authorization. /// </summary> [MessagePart("oauth_token", IsRequired = true)] internal string RequestToken { get; set; } diff --git a/src/DotNetOpenAuth/OpenId/ChannelElements/SigningBindingElement.cs b/src/DotNetOpenAuth/OpenId/ChannelElements/SigningBindingElement.cs index 3f4a998..30310ac 100644 --- a/src/DotNetOpenAuth/OpenId/ChannelElements/SigningBindingElement.cs +++ b/src/DotNetOpenAuth/OpenId/ChannelElements/SigningBindingElement.cs @@ -134,7 +134,7 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { Association association = this.GetSpecificAssociation(signedMessage); if (association != null) { string signature = this.GetSignature(signedMessage, association); - if (!string.Equals(signedMessage.Signature, signature, StringComparison.Ordinal)) { + if (!MessagingUtilities.EqualsConstantTime(signedMessage.Signature, signature)) { Logger.Bindings.Error("Signature verification failed."); throw new InvalidSignatureException(message); } diff --git a/src/DotNetOpenAuth/OpenId/Identifier.cs b/src/DotNetOpenAuth/OpenId/Identifier.cs index 36ec784..305976a 100644 --- a/src/DotNetOpenAuth/OpenId/Identifier.cs +++ b/src/DotNetOpenAuth/OpenId/Identifier.cs @@ -36,7 +36,7 @@ namespace DotNetOpenAuth.OpenId { /// <summary> /// Gets the original string that was normalized to create this Identifier. /// </summary> - public string OriginalString { get; private set; } + internal string OriginalString { get; private set; } /// <summary> /// Gets the Identifier in the form in which it should be serialized. |