diff options
Diffstat (limited to 'src')
5 files changed, 110 insertions, 60 deletions
diff --git a/src/DotNetOpenAuth.Core/Messaging/MessagingUtilities.cs b/src/DotNetOpenAuth.Core/Messaging/MessagingUtilities.cs index b26deeb..42ba18e 100644 --- a/src/DotNetOpenAuth.Core/Messaging/MessagingUtilities.cs +++ b/src/DotNetOpenAuth.Core/Messaging/MessagingUtilities.cs @@ -290,6 +290,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> + public 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> /// Clears any existing elements in a collection and fills the collection with a given set of values. /// </summary> /// <typeparam name="T">The type of value kept in the collection.</typeparam> @@ -868,43 +905,6 @@ 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.OAuth2.AuthorizationServer/OAuth2/AuthorizationServer.cs b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/AuthorizationServer.cs index 9696402..c7a1a23 100644 --- a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/AuthorizationServer.cs +++ b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/AuthorizationServer.cs @@ -63,7 +63,7 @@ namespace DotNetOpenAuth.OAuth2 { if (message.ResponseType == EndUserAuthorizationResponseType.AuthorizationCode) { // Clients with no secrets can only request implicit grant types. var client = this.AuthorizationServerServices.GetClientOrThrow(message.ClientIdentifier); - ErrorUtilities.VerifyProtocol(!string.IsNullOrEmpty(client.Secret), Protocol.EndUserAuthorizationRequestErrorCodes.UnauthorizedClient); + ErrorUtilities.VerifyProtocol(client.HasNonEmptySecret, Protocol.EndUserAuthorizationRequestErrorCodes.UnauthorizedClient); } } diff --git a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ChannelElements/MessageValidationBindingElement.cs b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ChannelElements/MessageValidationBindingElement.cs index be4f70d..23dcbf5 100644 --- a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ChannelElements/MessageValidationBindingElement.cs +++ b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ChannelElements/MessageValidationBindingElement.cs @@ -81,9 +81,8 @@ namespace DotNetOpenAuth.OAuth2.ChannelElements { var authenticatedClientRequest = message as AuthenticatedClientRequestBase; if (authenticatedClientRequest != null) { var client = this.AuthorizationServer.GetClientOrThrow(authenticatedClientRequest.ClientIdentifier); - string secret = client.Secret; - AuthServerUtilities.TokenEndpointVerify(!string.IsNullOrEmpty(secret), Protocol.AccessTokenRequestErrorCodes.UnauthorizedClient); // an empty secret is not allowed for client authenticated calls. - AuthServerUtilities.TokenEndpointVerify(MessagingUtilities.EqualsConstantTime(secret, authenticatedClientRequest.ClientSecret), Protocol.AccessTokenRequestErrorCodes.InvalidClient, AuthServerStrings.ClientSecretMismatch); + AuthServerUtilities.TokenEndpointVerify(client.HasNonEmptySecret, Protocol.AccessTokenRequestErrorCodes.UnauthorizedClient); // an empty secret is not allowed for client authenticated calls. + AuthServerUtilities.TokenEndpointVerify(client.IsValidClientSecret(authenticatedClientRequest.ClientSecret), Protocol.AccessTokenRequestErrorCodes.InvalidClient, AuthServerStrings.ClientSecretMismatch); if (clientCredentialOnly != null) { clientCredentialOnly.CredentialsValidated = true; diff --git a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ClientDescription.cs b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ClientDescription.cs index 76c3ea6..1ec9789 100644 --- a/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ClientDescription.cs +++ b/src/DotNetOpenAuth.OAuth2.AuthorizationServer/OAuth2/ClientDescription.cs @@ -9,6 +9,7 @@ namespace DotNetOpenAuth.OAuth2 { using System.Collections.Generic; using System.Linq; using System.Text; + using DotNetOpenAuth.Messaging; /// <summary> /// A default implementation of the <see cref="IClientDescription"/> interface. @@ -20,6 +21,11 @@ namespace DotNetOpenAuth.OAuth2 { private readonly Func<Uri, bool> isCallbackAllowed; /// <summary> + /// The client's secret, if any. + /// </summary> + private readonly string secret; + + /// <summary> /// Initializes a new instance of the <see cref="ClientDescription"/> class. /// </summary> /// <param name="secret">The secret.</param> @@ -27,18 +33,13 @@ namespace DotNetOpenAuth.OAuth2 { /// <param name="clientType">Type of the client.</param> /// <param name="isCallbackAllowed">A delegate that determines whether the callback is allowed.</param> public ClientDescription(string secret, Uri defaultCallback, ClientType clientType, Func<Uri, bool> isCallbackAllowed = null) { - this.Secret = secret; + this.secret = secret; this.DefaultCallback = defaultCallback; this.ClientType = clientType; this.isCallbackAllowed = isCallbackAllowed; } /// <summary> - /// Gets the client secret. - /// </summary> - public string Secret { get; private set; } - - /// <summary> /// Gets the callback to use when an individual authorization request /// does not include an explicit callback URI. /// </summary> @@ -53,6 +54,13 @@ namespace DotNetOpenAuth.OAuth2 { public ClientType ClientType { get; private set; } /// <summary> + /// Gets a value indicating whether a non-empty secret is registered for this client. + /// </summary> + public bool HasNonEmptySecret { + get { return !string.IsNullOrEmpty(this.secret); } + } + + /// <summary> /// Determines whether a callback URI included in a client's authorization request /// is among those allowed callbacks for the registered client. /// </summary> @@ -67,5 +75,24 @@ namespace DotNetOpenAuth.OAuth2 { return EqualityComparer<Uri>.Default.Equals(this.DefaultCallback, callback); } + + #region IClientDescription Members + + /// <summary> + /// Checks whether the specified client secret is correct. + /// </summary> + /// <param name="secret">The secret obtained from the client.</param> + /// <returns><c>true</c> if the secret matches the one in the authorization server's record for the client; <c>false</c> otherwise.</returns> + /// <remarks> + /// All string equality checks, whether checking secrets or their hashes, + /// should be done using <see cref="MessagingUtilites.EqualsConstantTime"/> to mitigate timing attacks. + /// </remarks> + public bool IsValidClientSecret(string secret) { + Requires.NotNullOrEmpty(secret, "secret"); + + return MessagingUtilities.EqualsConstantTime(secret, this.secret); + } + + #endregion } } diff --git a/src/DotNetOpenAuth.OAuth2.ClientAuthorization/OAuth2/IClientDescription.cs b/src/DotNetOpenAuth.OAuth2.ClientAuthorization/OAuth2/IClientDescription.cs index d30151b..bcef28b 100644 --- a/src/DotNetOpenAuth.OAuth2.ClientAuthorization/OAuth2/IClientDescription.cs +++ b/src/DotNetOpenAuth.OAuth2.ClientAuthorization/OAuth2/IClientDescription.cs @@ -15,11 +15,6 @@ namespace DotNetOpenAuth.OAuth2 { [ContractClass(typeof(IClientDescriptionContract))] public interface IClientDescription { /// <summary> - /// Gets the client secret. - /// </summary> - string Secret { get; } - - /// <summary> /// Gets the callback to use when an individual authorization request /// does not include an explicit callback URI. /// </summary> @@ -32,6 +27,11 @@ namespace DotNetOpenAuth.OAuth2 { ClientType ClientType { get; } /// <summary> + /// Gets a value indicating whether a non-empty secret is registered for this client. + /// </summary> + bool HasNonEmptySecret { get; } + + /// <summary> /// Determines whether a callback URI included in a client's authorization request /// is among those allowed callbacks for the registered client. /// </summary> @@ -56,6 +56,17 @@ namespace DotNetOpenAuth.OAuth2 { /// </para> /// </remarks> bool IsCallbackAllowed(Uri callback); + + /// <summary> + /// Checks whether the specified client secret is correct. + /// </summary> + /// <param name="secret">The secret obtained from the client.</param> + /// <returns><c>true</c> if the secret matches the one in the authorization server's record for the client; <c>false</c> otherwise.</returns> + /// <remarks> + /// All string equality checks, whether checking secrets or their hashes, + /// should be done using <see cref="MessagingUtilites.EqualsConstantTime"/> to mitigate timing attacks. + /// </remarks> + bool IsValidClientSecret(string secret); } /// <summary> @@ -66,14 +77,6 @@ namespace DotNetOpenAuth.OAuth2 { #region IClientDescription Members /// <summary> - /// Gets the client secret. - /// </summary> - /// <value></value> - string IClientDescription.Secret { - get { throw new NotImplementedException(); } - } - - /// <summary> /// Gets the type of the client. /// </summary> ClientType IClientDescription.ClientType { @@ -95,6 +98,13 @@ namespace DotNetOpenAuth.OAuth2 { } /// <summary> + /// Gets a value indicating whether a non-empty secret is registered for this client. + /// </summary> + bool IClientDescription.HasNonEmptySecret { + get { throw new NotImplementedException(); } + } + + /// <summary> /// Determines whether a callback URI included in a client's authorization request /// is among those allowed callbacks for the registered client. /// </summary> @@ -108,6 +118,20 @@ namespace DotNetOpenAuth.OAuth2 { throw new NotImplementedException(); } + /// <summary> + /// Checks whether the specified client secret is correct. + /// </summary> + /// <param name="secret">The secret obtained from the client.</param> + /// <returns><c>true</c> if the secret matches the one in the authorization server's record for the client; <c>false</c> otherwise.</returns> + /// <remarks> + /// All string equality checks, whether checking secrets or their hashes, + /// should be done using <see cref="MessagingUtilites.EqualsConstantTime"/> to mitigate timing attacks. + /// </remarks> + bool IClientDescription.IsValidClientSecret(string secret) { + Requires.NotNullOrEmpty(secret, "secret"); + throw new NotImplementedException(); + } + #endregion } } |