diff --git a/src/OpenIddict.Abstractions/OpenIddictResources.resx b/src/OpenIddict.Abstractions/OpenIddictResources.resx index a4e19c5a..023e228c 100644 --- a/src/OpenIddict.Abstractions/OpenIddictResources.resx +++ b/src/OpenIddict.Abstractions/OpenIddictResources.resx @@ -1346,6 +1346,15 @@ Alternatively, you can disable the token storage feature by calling 'services.Ad The '{0}' instance returned by CryptoConfig.CreateFromName() is not suitable for the requested operation. When registering a custom implementation of a cryptographic algorithm, make sure it inherits from the correct base type and uses the correct name (e.g "OpenIddict RSA Cryptographic Provider" implementations must derive from System.Security.Cryptography.RSA). + + The nonce cannot be resolved from the challenge context. + + + The nonce cannot be resolved from the sign-out context. + + + The nonce cannot be resolved from the state token. + The security token is missing. @@ -1832,6 +1841,12 @@ Alternatively, you can disable the token storage feature by calling 'services.Ad An unsupported response was returned by the remote authorization server. + + The correlation cookie is invalid or malformed. + + + The request forgery protection is missing or doesn't contain the expected value. + The '{0}' parameter shouldn't be null or empty at this point. @@ -1880,6 +1895,9 @@ Alternatively, you can disable the token storage feature by calling 'services.Ad The password shouldn't be null or empty at this point. + + The number of written bytes ({0}) doesn't match the expected value ({1}). + An error occurred while validating the token '{Token}'. @@ -2467,6 +2485,15 @@ This may indicate that the hashed entry is corrupted or malformed. The authorization request was rejected by the remote authorization server: {Response}. + + The request forgery protection is missing or doesn't contain the expected value, which may indicate a session fixation attack. + + + The nonce claim present in the frontchannel identity token doesn't contain the expected value, which may indicate a replay or token injection attack. + + + The nonce claim present in the backchannel identity doesn't contain the expected value, which may indicate a replay or token injection attack. + https://documentation.openiddict.com/errors/{0} diff --git a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs index 9729d3ee..e412ac54 100644 --- a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs +++ b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs @@ -4,6 +4,7 @@ * the license and the contributors participating to this project. */ +using System.Buffers.Binary; using System.Collections.Immutable; using System.ComponentModel; using System.Diagnostics; @@ -15,6 +16,7 @@ using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.IdentityModel.Tokens; using Microsoft.Net.Http.Headers; using Properties = OpenIddict.Client.AspNetCore.OpenIddictClientAspNetCoreConstants.Properties; @@ -36,7 +38,7 @@ public static partial class OpenIddictClientAspNetCoreHandlers /* * Authentication processing: */ - ValidateCorrelationCookie.Descriptor, + ResolveRequestForgeryProtection.Descriptor, ValidateEndpointUri.Descriptor, /* @@ -223,15 +225,14 @@ public static partial class OpenIddictClientAspNetCoreHandlers } /// - /// Contains the logic responsible for validating the correlation cookie that serves as a protection - /// against state token injection, forged requests, denial of service and session fixation attacks. + /// Contains the logic responsible for resolving the request forgery protection from the correlation cookie. /// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core. /// - public class ValidateCorrelationCookie : IOpenIddictClientHandler + public class ResolveRequestForgeryProtection : IOpenIddictClientHandler { private readonly IOptionsMonitor _options; - public ValidateCorrelationCookie(IOptionsMonitor options) + public ResolveRequestForgeryProtection(IOptionsMonitor options) => _options = options ?? throw new ArgumentNullException(nameof(options)); /// @@ -241,8 +242,8 @@ public static partial class OpenIddictClientAspNetCoreHandlers = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() .AddFilter() - .UseSingletonHandler() - .SetOrder(ValidateStateToken.Descriptor.Order + 500) + .UseSingletonHandler() + .SetOrder(ValidateRequestForgeryProtection.Descriptor.Order - 500) .SetType(OpenIddictClientHandlerType.BuiltIn) .Build(); @@ -261,36 +262,30 @@ public static partial class OpenIddictClientAspNetCoreHandlers var request = context.Transaction.GetHttpRequest() ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0114)); - // Resolve the request forgery protection from the state token principal. - var identifier = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection); - if (string.IsNullOrEmpty(identifier)) + // Resolve the nonce from the state token principal. + var nonce = context.StateTokenPrincipal.GetClaim(Claims.Private.Nonce); + if (string.IsNullOrEmpty(nonce)) { - throw new InvalidOperationException(SR.GetResourceString(SR.ID0339)); + throw new InvalidOperationException(SR.GetResourceString(SR.ID0354)); } // Resolve the cookie builder from the OWIN integration options. var builder = _options.CurrentValue.CookieBuilder; - // Compute the name of the cookie name based on the prefix set in the options - // and the random request forgery protection claim restored from the state. + // Compute the name of the cookie name based on the prefix and the random nonce. var name = new StringBuilder(builder.Name) .Append(Separators.Dot) - .Append(identifier) + .Append(nonce) .ToString(); - // Try to find the cookie matching the request forgery protection stored in the state. - // The correlation cookie serves as a binding mechanism ensuring that a state token - // stolen from an authorization response with the other parameters cannot be validly - // used without sending the matching correlation identifier used as the cookie name. - // - // If the cookie cannot be found, this may indicate that the authorization response - // is unsolicited and potentially malicious or be caused by an invalid or unadequate - // same-site configuration. + // Try to find the correlation cookie matching the nonce stored in the state. If the cookie + // cannot be found, this may indicate that the authorization response is unsolicited and + // potentially malicious or be caused by an invalid or unadequate same-site configuration. // // In any case, the authentication demand MUST be rejected as it's impossible to ensure // it's not an injection or session fixation attack without the correlation cookie. var value = request.Cookies[name]; - if (string.IsNullOrEmpty(value) || !string.Equals(value, "v1", StringComparison.Ordinal)) + if (string.IsNullOrEmpty(value)) { context.Reject( error: Errors.InvalidRequest, @@ -300,6 +295,49 @@ public static partial class OpenIddictClientAspNetCoreHandlers return default; } + try + { + // Extract the payload and validate the version marker. + var payload = Base64UrlEncoder.DecodeBytes(value); + if (payload.Length < (1 + sizeof(uint)) || payload[0] is not 0x01) + { + context.Reject( + error: Errors.InvalidRequest, + description: SR.GetResourceString(SR.ID2163), + uri: SR.FormatID8000(SR.ID2163)); + + return default; + } + + // Extract the length of the request forgery protection. + var length = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.AsSpan(1, sizeof(uint))); + if (length is 0 || length != (payload.Length - (1 + sizeof(uint)))) + { + context.Reject( + error: Errors.InvalidRequest, + description: SR.GetResourceString(SR.ID2163), + uri: SR.FormatID8000(SR.ID2163)); + + return default; + } + + // Note: since the correlation cookie is not protected against tampering, an unexpected + // value may be present in the cookie payload and this call may return a string whose + // length doesn't match the expected value. In any case, any tampering attempt will be + // detected when comparing the resolved value with the expected value stored in the state. + context.RequestForgeryProtection = Encoding.UTF8.GetString(payload, index: 5, length); + } + + catch + { + context.Reject( + error: Errors.InvalidRequest, + description: SR.GetResourceString(SR.ID2163), + uri: SR.FormatID8000(SR.ID2163)); + + return default; + } + // Return a response header asking the browser to delete the state cookie. // // Note: when deleting a cookie, the same options used when creating it MUST be specified. @@ -323,7 +361,7 @@ public static partial class OpenIddictClientAspNetCoreHandlers .AddFilter() .AddFilter() .UseSingletonHandler() - .SetOrder(ValidateCorrelationCookie.Descriptor.Order + 500) + .SetOrder(ResolveRequestForgeryProtection.Descriptor.Order + 500) .SetType(OpenIddictClientHandlerType.BuiltIn) .Build(); @@ -553,6 +591,11 @@ public static partial class OpenIddictClientAspNetCoreHandlers // a different protection strategy can remove this handler from the handlers list and add // a custom one using a different approach (e.g by storing the value in the session state). + if (string.IsNullOrEmpty(context.Nonce)) + { + throw new InvalidOperationException(SR.GetResourceString(SR.ID0352)); + } + if (string.IsNullOrEmpty(context.RequestForgeryProtection)) { throw new InvalidOperationException(SR.GetResourceString(SR.ID0343)); @@ -573,15 +616,29 @@ public static partial class OpenIddictClientAspNetCoreHandlers var options = builder.Build(response.HttpContext); options.Expires ??= context.StateTokenPrincipal.GetExpirationDate(); - // Compute a collision-resistant and hard-to-guess cookie name based on the prefix set - // in the options and the random request forgery protection claim generated earlier. + // Compute a collision-resistant and hard-to-guess cookie name using the nonce. var name = new StringBuilder(builder.Name) .Append(Separators.Dot) - .Append(context.RequestForgeryProtection) + .Append(context.Nonce) .ToString(); + // Create the cookie payload containing... + var count = Encoding.UTF8.GetByteCount(context.RequestForgeryProtection); + var payload = new byte[1 + sizeof(uint) + count]; + + // ... the version marker identifying the binary format used to create the payload (1 byte). + payload[0] = 0x01; + + // ... the length of the request forgery protection (4 bytes). + BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(1, sizeof(uint)), (uint) count); + + // ... the request forgery protection (variable length). + var written = Encoding.UTF8.GetBytes(s: context.RequestForgeryProtection, charIndex: 0, + charCount: context.RequestForgeryProtection.Length, bytes: payload, byteIndex: 5); + Debug.Assert(written == count, SR.FormatID4016(written, count)); + // Add the correlation cookie to the response headers. - response.Cookies.Append(name, "v1", options); + response.Cookies.Append(name, Base64UrlEncoder.Encode(payload), options); return default; } @@ -726,6 +783,11 @@ public static partial class OpenIddictClientAspNetCoreHandlers // a different protection strategy can remove this handler from the handlers list and add // a custom one using a different approach (e.g by storing the value in the session state). + if (string.IsNullOrEmpty(context.Nonce)) + { + throw new InvalidOperationException(SR.GetResourceString(SR.ID0353)); + } + if (string.IsNullOrEmpty(context.RequestForgeryProtection)) { throw new InvalidOperationException(SR.GetResourceString(SR.ID0344)); @@ -746,15 +808,29 @@ public static partial class OpenIddictClientAspNetCoreHandlers var options = builder.Build(response.HttpContext); options.Expires ??= context.StateTokenPrincipal.GetExpirationDate(); - // Compute a collision-resistant and hard-to-guess cookie name based on the prefix set - // in the options and the random request forgery protection claim generated earlier. + // Compute a collision-resistant and hard-to-guess cookie name using the nonce. var name = new StringBuilder(builder.Name) .Append(Separators.Dot) - .Append(context.RequestForgeryProtection) + .Append(context.Nonce) .ToString(); + // Create the cookie payload containing... + var count = Encoding.UTF8.GetByteCount(context.RequestForgeryProtection); + var payload = new byte[1 + sizeof(uint) + count]; + + // ... the version marker identifying the binary format used to create the payload (1 byte). + payload[0] = 0x01; + + // ... the length of the request forgery protection (4 bytes). + BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(1, sizeof(uint)), (uint) count); + + // ... the request forgery protection (variable length). + var written = Encoding.UTF8.GetBytes(s: context.RequestForgeryProtection, charIndex: 0, + charCount: context.RequestForgeryProtection.Length, bytes: payload, byteIndex: 5); + Debug.Assert(written == count, SR.FormatID4016(written, count)); + // Add the correlation cookie to the response headers. - response.Cookies.Append(name, "v1", options); + response.Cookies.Append(name, Base64UrlEncoder.Encode(payload), options); return default; } diff --git a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs index 7a7cb931..f5d8e5cc 100644 --- a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs +++ b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs @@ -4,6 +4,7 @@ * the license and the contributors participating to this project. */ +using System.Buffers.Binary; using System.Collections.Immutable; using System.ComponentModel; using System.Diagnostics; @@ -13,6 +14,7 @@ using System.Text; using System.Text.Json; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.IdentityModel.Tokens; using OpenIddict.Extensions; using Owin; using static OpenIddict.Client.Owin.OpenIddictClientOwinConstants; @@ -32,7 +34,7 @@ public static partial class OpenIddictClientOwinHandlers /* * Authentication processing: */ - ValidateCorrelationCookie.Descriptor, + ResolveRequestForgeryProtection.Descriptor, ValidateEndpointUri.Descriptor, /* @@ -222,15 +224,14 @@ public static partial class OpenIddictClientOwinHandlers } /// - /// Contains the logic responsible for validating the correlation cookie that serves as a - /// protection against state token injection, forged requests and session fixation attacks. + /// Contains the logic responsible for resolving the request forgery protection from the correlation cookie. /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. /// - public class ValidateCorrelationCookie : IOpenIddictClientHandler + public class ResolveRequestForgeryProtection : IOpenIddictClientHandler { private readonly IOptionsMonitor _options; - public ValidateCorrelationCookie(IOptionsMonitor options) + public ResolveRequestForgeryProtection(IOptionsMonitor options) => _options = options ?? throw new ArgumentNullException(nameof(options)); /// @@ -240,7 +241,7 @@ public static partial class OpenIddictClientOwinHandlers = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() .AddFilter() - .UseSingletonHandler() + .UseSingletonHandler() .SetOrder(ValidateStateToken.Descriptor.Order + 500) .SetType(OpenIddictClientHandlerType.BuiltIn) .Build(); @@ -260,11 +261,11 @@ public static partial class OpenIddictClientOwinHandlers var request = context.Transaction.GetOwinRequest() ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); - // Resolve the request forgery protection from the state token principal. - var identifier = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection); - if (string.IsNullOrEmpty(identifier)) + // Resolve the nonce from the state token principal. + var nonce = context.StateTokenPrincipal.GetClaim(Claims.Private.Nonce); + if (string.IsNullOrEmpty(nonce)) { - throw new InvalidOperationException(SR.GetResourceString(SR.ID0339)); + throw new InvalidOperationException(SR.GetResourceString(SR.ID0354)); } // Resolve the cookie manager and the cookie options from the OWIN integration options. @@ -272,26 +273,20 @@ public static partial class OpenIddictClientOwinHandlers _options.CurrentValue.CookieManager, _options.CurrentValue.CookieOptions); - // Compute the name of the cookie name based on the prefix set in the options - // and the random request forgery protection claim restored from the state. + // Compute the name of the cookie name based on the prefix and the random nonce. var name = new StringBuilder(_options.CurrentValue.CookieName) .Append(Separators.Dot) - .Append(identifier) + .Append(nonce) .ToString(); - // Try to find the cookie matching the request forgery protection stored in the state. - // The correlation cookie serves as a binding mechanism ensuring that a state token - // stolen from an authorization response with the other parameters cannot be validly - // used without sending the matching correlation identifier used as the cookie name. - // - // If the cookie cannot be found, this may indicate that the authorization response - // is unsolicited and potentially malicious or be caused by an invalid or unadequate - // same-site configuration. + // Try to find the correlation cookie matching the nonce stored in the state. If the cookie + // cannot be found, this may indicate that the authorization response is unsolicited and + // potentially malicious or be caused by an invalid or unadequate same-site configuration. // // In any case, the authentication demand MUST be rejected as it's impossible to ensure // it's not an injection or session fixation attack without the correlation cookie. var value = manager.GetRequestCookie(request.Context, name); - if (string.IsNullOrEmpty(value) || !string.Equals(value, "v1", StringComparison.Ordinal)) + if (string.IsNullOrEmpty(value)) { context.Reject( error: Errors.InvalidRequest, @@ -301,6 +296,49 @@ public static partial class OpenIddictClientOwinHandlers return default; } + try + { + // Extract the payload and validate the version marker. + var payload = Base64UrlEncoder.DecodeBytes(value); + if (payload.Length < (1 + sizeof(uint)) || payload[0] is not 0x01) + { + context.Reject( + error: Errors.InvalidRequest, + description: SR.GetResourceString(SR.ID2163), + uri: SR.FormatID8000(SR.ID2163)); + + return default; + } + + // Extract the length of the request forgery protection. + var length = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.AsSpan(1, sizeof(uint))); + if (length is 0 || length != (payload.Length - (1 + sizeof(uint)))) + { + context.Reject( + error: Errors.InvalidRequest, + description: SR.GetResourceString(SR.ID2163), + uri: SR.FormatID8000(SR.ID2163)); + + return default; + } + + // Note: since the correlation cookie is not protected against tampering, an unexpected + // value may be present in the cookie payload and this call may return a string whose + // length doesn't match the expected value. In any case, any tampering attempt will be + // detected when comparing the resolved value with the expected value stored in the state. + context.RequestForgeryProtection = Encoding.UTF8.GetString(payload, index: 5, length); + } + + catch + { + context.Reject( + error: Errors.InvalidRequest, + description: SR.GetResourceString(SR.ID2163), + uri: SR.FormatID8000(SR.ID2163)); + + return default; + } + // Return a response header asking the browser to delete the state cookie. // // Note: when deleting a cookie, the same options used when creating it MUST be specified. @@ -331,7 +369,7 @@ public static partial class OpenIddictClientOwinHandlers .AddFilter() .AddFilter() .UseSingletonHandler() - .SetOrder(ValidateCorrelationCookie.Descriptor.Order + 500) + .SetOrder(ResolveRequestForgeryProtection.Descriptor.Order + 500) .SetType(OpenIddictClientHandlerType.BuiltIn) .Build(); @@ -582,6 +620,11 @@ public static partial class OpenIddictClientOwinHandlers // a different protection strategy can remove this handler from the handlers list and add // a custom one using a different approach (e.g by storing the value in the session state). + if (string.IsNullOrEmpty(context.Nonce)) + { + throw new InvalidOperationException(SR.GetResourceString(SR.ID0352)); + } + if (string.IsNullOrEmpty(context.RequestForgeryProtection)) { throw new InvalidOperationException(SR.GetResourceString(SR.ID0343)); @@ -594,20 +637,34 @@ public static partial class OpenIddictClientOwinHandlers var response = context.Transaction.GetOwinRequest()?.Context.Response ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); - // Compute a collision-resistant and hard-to-guess cookie name based on the prefix set - // in the options and the random request forgery protection claim generated earlier. + // Compute a collision-resistant and hard-to-guess cookie name using the nonce. var name = new StringBuilder(_options.CurrentValue.CookieName) .Append(Separators.Dot) - .Append(context.RequestForgeryProtection) + .Append(context.Nonce) .ToString(); + // Create the cookie payload containing... + var count = Encoding.UTF8.GetByteCount(context.RequestForgeryProtection); + var payload = new byte[1 + sizeof(uint) + count]; + + // ... the version marker identifying the binary format used to create the payload (1 byte). + payload[0] = 0x01; + + // ... the length of the request forgery protection (4 bytes). + BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(1, sizeof(uint)), (uint) count); + + // ... the request forgery protection (variable length). + var written = Encoding.UTF8.GetBytes(s: context.RequestForgeryProtection, charIndex: 0, + charCount: context.RequestForgeryProtection.Length, bytes: payload, byteIndex: 5); + Debug.Assert(written == count, SR.FormatID4016(written, count)); + // Resolve the cookie manager and the cookie options from the OWIN integration options. var (manager, options) = ( _options.CurrentValue.CookieManager, _options.CurrentValue.CookieOptions); // Add the correlation cookie to the response headers. - manager.AppendResponseCookie(response.Context, name, "v1", new CookieOptions + manager.AppendResponseCookie(response.Context, name, Base64UrlEncoder.Encode(payload), new CookieOptions { Domain = options.Domain, HttpOnly = options.HttpOnly, @@ -781,6 +838,11 @@ public static partial class OpenIddictClientOwinHandlers // a different protection strategy can remove this handler from the handlers list and add // a custom one using a different approach (e.g by storing the value in the session state). + if (string.IsNullOrEmpty(context.Nonce)) + { + throw new InvalidOperationException(SR.GetResourceString(SR.ID0353)); + } + if (string.IsNullOrEmpty(context.RequestForgeryProtection)) { throw new InvalidOperationException(SR.GetResourceString(SR.ID0344)); @@ -793,20 +855,34 @@ public static partial class OpenIddictClientOwinHandlers var response = context.Transaction.GetOwinRequest()?.Context.Response ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); - // Compute a collision-resistant and hard-to-guess cookie name based on the prefix set - // in the options and the random request forgery protection claim generated earlier. + // Compute a collision-resistant and hard-to-guess cookie name using the nonce. var name = new StringBuilder(_options.CurrentValue.CookieName) .Append(Separators.Dot) - .Append(context.RequestForgeryProtection) + .Append(context.Nonce) .ToString(); + // Create the cookie payload containing... + var count = Encoding.UTF8.GetByteCount(context.RequestForgeryProtection); + var payload = new byte[1 + sizeof(uint) + count]; + + // ... the version marker identifying the binary format used to create the payload (1 byte). + payload[0] = 0x01; + + // ... the length of the request forgery protection (4 bytes). + BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(1, sizeof(uint)), (uint) count); + + // ... the request forgery protection (variable length). + var written = Encoding.UTF8.GetBytes(s: context.RequestForgeryProtection, charIndex: 0, + charCount: context.RequestForgeryProtection.Length, bytes: payload, byteIndex: 5); + Debug.Assert(written == count, SR.FormatID4016(written, count)); + // Resolve the cookie manager and the cookie options from the OWIN integration options. var (manager, options) = ( _options.CurrentValue.CookieManager, _options.CurrentValue.CookieOptions); // Add the correlation cookie to the response headers. - manager.AppendResponseCookie(response.Context, name, "v1", new CookieOptions + manager.AppendResponseCookie(response.Context, name, Base64UrlEncoder.Encode(payload), new CookieOptions { Domain = options.Domain, HttpOnly = options.HttpOnly, diff --git a/src/OpenIddict.Client/OpenIddictClientEvents.cs b/src/OpenIddict.Client/OpenIddictClientEvents.cs index 548b90ca..7175467d 100644 --- a/src/OpenIddict.Client/OpenIddictClientEvents.cs +++ b/src/OpenIddict.Client/OpenIddictClientEvents.cs @@ -304,6 +304,11 @@ public static partial class OpenIddictClientEvents /// public string? ResponseType { get; set; } + /// + /// Gets or sets the request forgery protection resolved from the user session, if applicable. + /// + public string? RequestForgeryProtection { get; set; } + /// /// Gets or sets the address of the token endpoint, if applicable. /// @@ -890,6 +895,11 @@ public static partial class OpenIddictClientEvents /// public string? TargetLinkUri { get; set; } + /// + /// Gets or sets the nonce that will be used for the sign-out demand, if applicable. + /// + public string? Nonce { get; set; } + /// /// Gets or sets the request forgery protection that will be stored in the state token, if applicable. /// Note: this value MUST NOT be user-defined or extracted from any request and MUST be random diff --git a/src/OpenIddict.Client/OpenIddictClientHandlers.cs b/src/OpenIddict.Client/OpenIddictClientHandlers.cs index 7b735cae..e5acbd48 100644 --- a/src/OpenIddict.Client/OpenIddictClientHandlers.cs +++ b/src/OpenIddict.Client/OpenIddictClientHandlers.cs @@ -7,6 +7,7 @@ using System.Collections.Immutable; using System.ComponentModel; using System.Diagnostics; +using System.Runtime.InteropServices; using System.Security.Claims; using System.Security.Cryptography; using System.Text; @@ -35,6 +36,7 @@ public static partial class OpenIddictClientHandlers ValidateStateToken.Descriptor, RedeemStateTokenEntry.Descriptor, ValidateStateTokenEndpointType.Descriptor, + ValidateRequestForgeryProtection.Descriptor, ResolveClientRegistrationFromStateToken.Descriptor, ValidateIssuerParameter.Descriptor, HandleFrontchannelErrorResponse.Descriptor, @@ -119,6 +121,7 @@ public static partial class OpenIddictClientHandlers AttachPostLogoutRedirectUri.Descriptor, EvaluateGeneratedLogoutTokens.Descriptor, AttachSignOutHostProperties.Descriptor, + AttachLogoutNonce.Descriptor, AttachLogoutRequestForgeryProtection.Descriptor, PrepareLogoutStateTokenPrincipal.Descriptor, GenerateLogoutStateToken.Descriptor, @@ -511,6 +514,76 @@ public static partial class OpenIddictClientHandlers } } + /// + /// Contains the logic responsible for validating the request forgery protection claim that serves as a + /// protection against state token injection, forged requests, denial of service and session fixation attacks. + /// + public class ValidateRequestForgeryProtection : IOpenIddictClientHandler + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictClientHandlerDescriptor Descriptor { get; } + = OpenIddictClientHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler() + .SetOrder(ValidateStateTokenEndpointType.Descriptor.Order + 1_000) + .SetType(OpenIddictClientHandlerType.BuiltIn) + .Build(); + + /// + public ValueTask HandleAsync(ProcessAuthenticationContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); + + // Resolve the request forgery protection from the state token principal. + var comparand = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection); + if (string.IsNullOrEmpty(comparand)) + { + throw new InvalidOperationException(SR.GetResourceString(SR.ID0339)); + } + + // The request forgery protection serves as a binding mechanism ensuring that a + // state token stolen from an authorization response with the other parameters + // cannot be validly used without also sending the matching correlation identifier. + // + // If the request forgery protection couldn't be resolved at this point or doesn't + // match the expected value, this may indicate that the authentication demand is + // unsolicited and potentially malicious (or caused by an invalid or unadequate + // same-site configuration, if the authentication demand was handled by a web server). + // + // In any case, the authentication demand MUST be rejected as it's impossible to ensure + // it's not an injection or session fixation attack without the correct "rfp" value. + if (string.IsNullOrEmpty(context.RequestForgeryProtection) || +#if SUPPORTS_TIME_CONSTANT_COMPARISONS + !CryptographicOperations.FixedTimeEquals( + left: MemoryMarshal.AsBytes(comparand.AsSpan()), + right: MemoryMarshal.AsBytes(context.RequestForgeryProtection.AsSpan()))) +#else + !Arrays.ConstantTimeAreEqual( + a: MemoryMarshal.AsBytes(comparand.AsSpan()).ToArray(), + b: MemoryMarshal.AsBytes(context.RequestForgeryProtection.AsSpan()).ToArray())) +#endif + { + context.Logger.LogWarning(SR.GetResourceString(SR.ID6209)); + + context.Reject( + error: Errors.InvalidRequest, + description: SR.GetResourceString(SR.ID2164), + uri: SR.FormatID8000(SR.ID2164)); + + return default; + } + + return default; + } + } + /// /// Contains the logic responsible for resolving the client registration /// based on the authorization server identity stored in the state token. @@ -524,7 +597,7 @@ public static partial class OpenIddictClientHandlers = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler() - .SetOrder(ValidateStateTokenEndpointType.Descriptor.Order + 1_000) + .SetOrder(ValidateRequestForgeryProtection.Descriptor.Order + 1_000) .Build(); /// @@ -1326,6 +1399,38 @@ public static partial class OpenIddictClientHandlers throw new ArgumentNullException(nameof(context)); } + // Note: the OpenID Connect specification relies on nonces as a way to detect and + // prevent replay attacks by binding the returned identity token(s) to a specific + // random value sent by the client application as part of the authorization request. + // + // When Proof Key for Code Exchange is not supported or not available, nonces can + // also be used to detect authorization code or identity token injection attacks. + // + // For more information, see https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes + // and https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.5.3.2. + // + // While OpenIddict fully implements nonce support, its implementation slightly + // differs from the implementation suggested by the OpenID Connect specification: + // + // - Nonces are used internally as unique, per-authorization flow identifiers and + // are always considered required when using an interactive flow, independently + // of whether the authorization flow is an OAuth 2.0-only or OpenID Connect flow. + // + // - Instead of being stored as separate cookies as suggested by the specification, + // nonces are used by the ASP.NET Core and OWIN hosts to build a unique value + // for the name of the correlation cookie used with state tokens to prevent CSRF, + // which reduces the number of cookies used by the OpenIddict client web hosts. + // + // - Nonces are attached to the authorization requests AND stored in the state + // tokens so that the nonces and the state tokens form a 1 <-> 1 relationship, + // which forces sending the matching state to be able to validate identity tokens. + // + // - Replay detection is implemented by invalidating state tokens the very first time + // they are presented at the redirection endpoint, even if the response indicates + // an errored authorization response (e.g if the authorization demand was denied). + // Since nonce validation depends on the value stored in the state token, marking + // state tokens as already redeemed is enough to prevent nonces from being replayed. + Debug.Assert(context.FrontchannelIdentityTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); @@ -1333,12 +1438,17 @@ public static partial class OpenIddictClientHandlers FrontchannelIdentityTokenNonce: context.FrontchannelIdentityTokenPrincipal.GetClaim(Claims.Nonce), StateTokenNonce: context.StateTokenPrincipal.GetClaim(Claims.Private.Nonce))) { - // If no nonce if no present in the state token (e.g because the authorization server doesn't - // support OpenID Connect and response_type=code was negotiated), bypass the validation logic. + // If no nonce is present in the state token, bypass the validation logic. case { StateTokenNonce: null or { Length: not > 0 } }: return default; - // If a nonce was found in the state token but is not present in the identity token, return an error. + // If the request was not an OpenID Connect request but an identity token + // was returned nethertheless, don't require a nonce to be present. + case { FrontchannelIdentityTokenNonce: null or { Length: not > 0 } } + when !context.StateTokenPrincipal.HasScope(Scopes.OpenId): + return default; + + // If the nonce is not present in the identity token, return an error. case { FrontchannelIdentityTokenNonce: null or { Length: not > 0 } }: context.Reject( error: Errors.InvalidRequest, @@ -1350,10 +1460,18 @@ public static partial class OpenIddictClientHandlers // If the two nonces don't match, return an error. case { FrontchannelIdentityTokenNonce: string left, StateTokenNonce: string right } when #if SUPPORTS_TIME_CONSTANT_COMPARISONS - !CryptographicOperations.FixedTimeEquals(Encoding.ASCII.GetBytes(left), Encoding.ASCII.GetBytes(right)): + !CryptographicOperations.FixedTimeEquals( + left: MemoryMarshal.AsBytes(left.AsSpan()), // The nonce in the identity token is already hashed. + right: MemoryMarshal.AsBytes(Base64UrlEncoder.Encode( + OpenIddictHelpers.ComputeSha256Hash(Encoding.UTF8.GetBytes(right))).AsSpan())): #else - !Arrays.ConstantTimeAreEqual(Encoding.ASCII.GetBytes(left), Encoding.ASCII.GetBytes(right)): + !Arrays.ConstantTimeAreEqual( + a: MemoryMarshal.AsBytes(left.AsSpan()).ToArray(), // The nonce in the identity token is already hashed. + b: MemoryMarshal.AsBytes(Base64UrlEncoder.Encode( + OpenIddictHelpers.ComputeSha256Hash(Encoding.UTF8.GetBytes(right))).AsSpan()).ToArray()): #endif + context.Logger.LogWarning(SR.GetResourceString(SR.ID6210)); + context.Reject( error: Errors.InvalidRequest, description: SR.FormatID2124(Claims.Nonce), @@ -1455,7 +1573,7 @@ public static partial class OpenIddictClientHandlers } } - static byte[] ComputeTokenHash(string algorithm, byte[] data) + static ReadOnlySpan ComputeTokenHash(string algorithm, string token) { // Resolve the hash algorithm associated with the signing algorithm and compute the token // hash. If an instance of the BCL hash algorithm cannot be resolved, throw an exception. @@ -1463,33 +1581,33 @@ public static partial class OpenIddictClientHandlers { SecurityAlgorithms.EcdsaSha256 or SecurityAlgorithms.HmacSha256 or SecurityAlgorithms.RsaSha256 or SecurityAlgorithms.RsaSsaPssSha256 - => OpenIddictHelpers.ComputeSha256Hash(data), + => OpenIddictHelpers.ComputeSha256Hash(Encoding.ASCII.GetBytes(token)), SecurityAlgorithms.EcdsaSha384 or SecurityAlgorithms.HmacSha384 or SecurityAlgorithms.RsaSha384 or SecurityAlgorithms.RsaSsaPssSha384 - => OpenIddictHelpers.ComputeSha384Hash(data), + => OpenIddictHelpers.ComputeSha384Hash(Encoding.ASCII.GetBytes(token)), SecurityAlgorithms.EcdsaSha512 or SecurityAlgorithms.HmacSha384 or SecurityAlgorithms.RsaSha512 or SecurityAlgorithms.RsaSsaPssSha512 - => OpenIddictHelpers.ComputeSha512Hash(data), + => OpenIddictHelpers.ComputeSha512Hash(Encoding.ASCII.GetBytes(token)), _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0293)) }; // Warning: only the left-most half of the access token and authorization code digest is used. // See http://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken for more information. - return Encoding.ASCII.GetBytes(Base64UrlEncoder.Encode(hash, 0, hash.Length / 2)); + return Base64UrlEncoder.Encode(hash, 0, hash.Length / 2).AsSpan(); } static bool ValidateTokenHash(string algorithm, string token, string hash) => #if SUPPORTS_TIME_CONSTANT_COMPARISONS CryptographicOperations.FixedTimeEquals( - left: Encoding.ASCII.GetBytes(hash), - right: ComputeTokenHash(algorithm, Encoding.ASCII.GetBytes(token))); + left: MemoryMarshal.AsBytes(hash.AsSpan()), + right: MemoryMarshal.AsBytes(ComputeTokenHash(algorithm, token))); #else Arrays.ConstantTimeAreEqual( - a: Encoding.ASCII.GetBytes(hash), - b: ComputeTokenHash(algorithm, Encoding.ASCII.GetBytes(token))); + a: MemoryMarshal.AsBytes(hash.AsSpan()).ToArray(), + b: MemoryMarshal.AsBytes(ComputeTokenHash(algorithm, token)).ToArray()); #endif return default; } @@ -2584,6 +2702,38 @@ public static partial class OpenIddictClientHandlers throw new ArgumentNullException(nameof(context)); } + // Note: the OpenID Connect specification relies on nonces as a way to detect and + // prevent replay attacks by binding the returned identity token(s) to a specific + // random value sent by the client application as part of the authorization request. + // + // When Proof Key for Code Exchange is not supported or not available, nonces can + // also be used to detect authorization code or identity token injection attacks. + // + // For more information, see https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes + // and https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-4.5.3.2. + // + // While OpenIddict fully implements nonce support, its implementation slightly + // differs from the implementation suggested by the OpenID Connect specification: + // + // - Nonces are used internally as unique, per-authorization flow identifiers and + // are always considered required when using an interactive flow, independently + // of whether the authorization flow is an OAuth 2.0-only or OpenID Connect flow. + // + // - Instead of being stored as separate cookies as suggested by the specification, + // nonces are used by the ASP.NET Core and OWIN hosts to build a unique value + // for the name of the correlation cookie used with state tokens to prevent CSRF, + // which reduces the number of cookies used by the OpenIddict client web hosts. + // + // - Nonces are attached to the authorization requests AND stored in the state + // tokens so that the nonces and the state tokens form a 1 <-> 1 relationship, + // which forces sending the matching state to be able to validate identity tokens. + // + // - Replay detection is implemented by invalidating state tokens the very first time + // they are presented at the redirection endpoint, even if the response indicates + // an errored authorization response (e.g if the authorization demand was denied). + // Since nonce validation depends on the value stored in the state token, marking + // state tokens as already redeemed is enough to prevent nonces from being replayed. + Debug.Assert(context.BackchannelIdentityTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); @@ -2591,12 +2741,17 @@ public static partial class OpenIddictClientHandlers BackchannelIdentityTokenNonce: context.BackchannelIdentityTokenPrincipal.GetClaim(Claims.Nonce), StateTokenNonce: context.StateTokenPrincipal.GetClaim(Claims.Private.Nonce))) { - // If no nonce if no present in the state token (e.g because the authorization server doesn't - // support OpenID Connect and response_type=code was negotiated), bypass the validation logic. + // If no nonce is present in the state token, bypass the validation logic. case { StateTokenNonce: null or { Length: not > 0 } }: return default; - // If a nonce was found in the state token but is not present in the identity token, return an error. + // If the request was not an OpenID Connect request but an identity token + // was returned nethertheless, don't require a nonce to be present. + case { BackchannelIdentityTokenNonce: null or { Length: not > 0 } } + when !context.StateTokenPrincipal.HasScope(Scopes.OpenId): + return default; + + // If the nonce is not present in the identity token, return an error. case { BackchannelIdentityTokenNonce: null or { Length: not > 0 } }: context.Reject( error: Errors.InvalidRequest, @@ -2608,10 +2763,18 @@ public static partial class OpenIddictClientHandlers // If the two nonces don't match, return an error. case { BackchannelIdentityTokenNonce: string left, StateTokenNonce: string right } when #if SUPPORTS_TIME_CONSTANT_COMPARISONS - !CryptographicOperations.FixedTimeEquals(Encoding.ASCII.GetBytes(left), Encoding.ASCII.GetBytes(right)): + !CryptographicOperations.FixedTimeEquals( + left: MemoryMarshal.AsBytes(left.AsSpan()), // The nonce in the identity token is already hashed. + right: MemoryMarshal.AsBytes(Base64UrlEncoder.Encode( + OpenIddictHelpers.ComputeSha256Hash(Encoding.UTF8.GetBytes(right))).AsSpan())): #else - !Arrays.ConstantTimeAreEqual(Encoding.ASCII.GetBytes(left), Encoding.ASCII.GetBytes(right)): + !Arrays.ConstantTimeAreEqual( + a: MemoryMarshal.AsBytes(left.AsSpan()).ToArray(), // The nonce in the identity token is already hashed. + b: MemoryMarshal.AsBytes(Base64UrlEncoder.Encode( + OpenIddictHelpers.ComputeSha256Hash(Encoding.UTF8.GetBytes(right))).AsSpan()).ToArray()): #endif + context.Logger.LogWarning(SR.GetResourceString(SR.ID6211)); + context.Reject( error: Errors.InvalidRequest, description: SR.FormatID2128(Claims.Nonce), @@ -2677,7 +2840,7 @@ public static partial class OpenIddictClientHandlers // Note: unlike frontchannel identity tokens, backchannel identity tokens are not expected to include // an authorization code hash as no authorization code is normally returned from the token endpoint. - static byte[] ComputeTokenHash(string algorithm, byte[] data) + static ReadOnlySpan ComputeTokenHash(string algorithm, string token) { // Resolve the hash algorithm associated with the signing algorithm and compute the token // hash. If an instance of the BCL hash algorithm cannot be resolved, throw an exception. @@ -2685,33 +2848,33 @@ public static partial class OpenIddictClientHandlers { SecurityAlgorithms.EcdsaSha256 or SecurityAlgorithms.HmacSha256 or SecurityAlgorithms.RsaSha256 or SecurityAlgorithms.RsaSsaPssSha256 - => OpenIddictHelpers.ComputeSha256Hash(data), + => OpenIddictHelpers.ComputeSha256Hash(Encoding.ASCII.GetBytes(token)), SecurityAlgorithms.EcdsaSha384 or SecurityAlgorithms.HmacSha384 or SecurityAlgorithms.RsaSha384 or SecurityAlgorithms.RsaSsaPssSha384 - => OpenIddictHelpers.ComputeSha384Hash(data), + => OpenIddictHelpers.ComputeSha384Hash(Encoding.ASCII.GetBytes(token)), SecurityAlgorithms.EcdsaSha512 or SecurityAlgorithms.HmacSha384 or SecurityAlgorithms.RsaSha512 or SecurityAlgorithms.RsaSsaPssSha512 - => OpenIddictHelpers.ComputeSha512Hash(data), + => OpenIddictHelpers.ComputeSha512Hash(Encoding.ASCII.GetBytes(token)), _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0293)) }; // Warning: only the left-most half of the access token and authorization code digest is used. // See http://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken for more information. - return Encoding.ASCII.GetBytes(Base64UrlEncoder.Encode(hash, 0, hash.Length / 2)); + return Base64UrlEncoder.Encode(hash, 0, hash.Length / 2).AsSpan(); } static bool ValidateTokenHash(string algorithm, string token, string hash) => #if SUPPORTS_TIME_CONSTANT_COMPARISONS CryptographicOperations.FixedTimeEquals( - left: Encoding.ASCII.GetBytes(hash), - right: ComputeTokenHash(algorithm, Encoding.ASCII.GetBytes(token))); + left: MemoryMarshal.AsBytes(hash.AsSpan()), + right: MemoryMarshal.AsBytes(ComputeTokenHash(algorithm, token))); #else Arrays.ConstantTimeAreEqual( - a: Encoding.ASCII.GetBytes(hash), - b: ComputeTokenHash(algorithm, Encoding.ASCII.GetBytes(token))); + a: MemoryMarshal.AsBytes(hash.AsSpan()).ToArray(), + b: MemoryMarshal.AsBytes(ComputeTokenHash(algorithm, token)).ToArray()); #endif return default; } @@ -4038,13 +4201,17 @@ public static partial class OpenIddictClientHandlers throw new ArgumentNullException(nameof(context)); } - // If the identity provider doesn't support OpenID Connect, don't attach a nonce. - if (!context.Configuration.ScopesSupported.Contains(Scopes.OpenId)) - { - return default; - } - // Generate a new crypto-secure random identifier that will be used as the nonce. + // + // Note: a nonce is always generated for interactive grants, independently of whether + // the request is an OpenID Connect request or not, as it's used to identify each + // authorization demand and is needed by the web hosts like ASP.NET Core and OWIN + // to resolve the name of the correlation cookie used to prevent forged requests. + // + // If the request is an OpenID Connect request, the nonce will also be hashed and + // attached to the authorization request so that the identity provider can bind + // the issued identity tokens to the generated value, which helps detect token + // replay (and authorization code injection attacks when PKCE is not available). context.Nonce = Base64UrlEncoder.Encode(OpenIddictHelpers.CreateRandomArray(size: 256)); return default; @@ -4133,6 +4300,9 @@ public static partial class OpenIddictClientHandlers else if (context.CodeChallengeMethod is CodeChallengeMethods.Sha256) { // Compute the SHA-256 hash of the code verifier and use it as the code challenge. + // + // Note: ASCII is deliberately used here, as it's the encoding required by the specification. + // For more information, see https://datatracker.ietf.org/doc/html/rfc7636#section-4.2. context.CodeChallenge = Base64UrlEncoder.Encode(OpenIddictHelpers.ComputeSha256Hash( Encoding.ASCII.GetBytes(context.CodeVerifier))); } @@ -4243,6 +4413,9 @@ public static partial class OpenIddictClientHandlers // Store the nonce in the state token so it can be later used to check whether // the nonce extracted from the identity token matches the generated value. + // + // Note: the nonce is also used by the ASP.NET Core and OWIN hosts as a way + // to uniquely identify the name of the correlation cookie used for antiforgery. principal.SetClaim(Claims.Private.Nonce, context.Nonce); // Store the requested scopes in the state token. @@ -4395,7 +4568,16 @@ public static partial class OpenIddictClientHandlers context.Request.Scope = string.Join(" ", context.Scopes); } - context.Request.Nonce = context.Nonce; + // If the request is an OpenID Connect request, attach the nonce as a parameter. + // + // Note: the nonce is always hashed before being sent, as recommended the specification. + // See https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes for more information. + if (context.Scopes.Contains(Scopes.OpenId) && !string.IsNullOrEmpty(context.Nonce)) + { + context.Request.Nonce = Base64UrlEncoder.Encode( + OpenIddictHelpers.ComputeSha256Hash(Encoding.UTF8.GetBytes(context.Nonce))); + } + context.Request.CodeChallenge = context.CodeChallenge; context.Request.CodeChallengeMethod = context.CodeChallengeMethod; @@ -4670,7 +4852,7 @@ public static partial class OpenIddictClientHandlers } /// - /// Contains the logic responsible for attaching a request forgery protection to the authorization request. + /// Contains the logic responsible for attaching a request forgery protection to the logout request. /// public class AttachLogoutRequestForgeryProtection : IOpenIddictClientHandler { @@ -4700,6 +4882,35 @@ public static partial class OpenIddictClientHandlers } } + /// + /// Contains the logic responsible for attaching a nonce to the logout request. + /// + public class AttachLogoutNonce : IOpenIddictClientHandler + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictClientHandlerDescriptor Descriptor { get; } + = OpenIddictClientHandlerDescriptor.CreateBuilder() + .UseSingletonHandler() + .SetOrder(AttachLogoutRequestForgeryProtection.Descriptor.Order + 1_000) + .Build(); + + /// + public ValueTask HandleAsync(ProcessSignOutContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + // Generate a new crypto-secure random identifier that will be used as the nonce. + context.Nonce = Base64UrlEncoder.Encode(OpenIddictHelpers.CreateRandomArray(size: 256)); + + return default; + } + } + /// /// Contains the logic responsible for preparing and attaching the claims principal /// used to generate the logout state token, if one is going to be returned. @@ -4713,7 +4924,7 @@ public static partial class OpenIddictClientHandlers = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler() - .SetOrder(AttachLogoutRequestForgeryProtection.Descriptor.Order + 1_000) + .SetOrder(AttachLogoutNonce.Descriptor.Order + 1_000) .SetType(OpenIddictClientHandlerType.BuiltIn) .Build(); @@ -4769,7 +4980,7 @@ public static partial class OpenIddictClientHandlers principal.SetClaim(Claims.AuthorizationServer, context.Issuer.AbsoluteUri); // Store the request forgery protection in the state token so it can be later used to - // ensure the authorization response sent to the redirection endpoint is not forged. + // ensure the logout response sent to the post-logout redirection endpoint is not forged. principal.SetClaim(Claims.RequestForgeryProtection, context.RequestForgeryProtection); // Store the optional return URL in the state token. @@ -4783,6 +4994,12 @@ public static partial class OpenIddictClientHandlers // Store the post_logout_redirect_uri to allow comparing to the actual redirection URL. principal.SetClaim(Claims.Private.PostLogoutRedirectUri, context.PostLogoutRedirectUri); + // Store the nonce in the state token. + // + // Note: the nonce is also used by the ASP.NET Core and OWIN hosts as a way + // to uniquely identify the name of the correlation cookie used for antiforgery. + principal.SetClaim(Claims.Private.Nonce, context.Nonce); + context.StateTokenPrincipal = principal; return default; diff --git a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs index d8ee6b4a..560e16ac 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs @@ -1402,7 +1402,7 @@ public class OpenIddictApplicationManager : IOpenIddictApplication payload[0] = 0x01; // Write the hashing algorithm version. - BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(1, 4), algorithm switch + BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(1, sizeof(uint)), algorithm switch { var name when name == HashAlgorithmName.SHA1 => 0, var name when name == HashAlgorithmName.SHA256 => 1, @@ -1412,10 +1412,10 @@ public class OpenIddictApplicationManager : IOpenIddictApplication }); // Write the iteration count of the algorithm. - BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(5, 8), (uint) iterations); + BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(5, sizeof(uint)), (uint) iterations); // Write the size of the salt. - BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(9, 12), (uint) salt.Length); + BinaryPrimitives.WriteUInt32BigEndian(payload.AsSpan(9, sizeof(uint)), (uint) salt.Length); // Write the salt. salt.CopyTo(payload.AsSpan(13)); @@ -1482,7 +1482,7 @@ public class OpenIddictApplicationManager : IOpenIddictApplication } // Read the hashing algorithm version. - var algorithm = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.Slice(1, 4)) switch + var algorithm = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.Slice(1, sizeof(uint))) switch { 0 => HashAlgorithmName.SHA1, 1 => HashAlgorithmName.SHA256, @@ -1492,10 +1492,10 @@ public class OpenIddictApplicationManager : IOpenIddictApplication }; // Read the iteration count of the algorithm. - var iterations = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.Slice(5, 8)); + var iterations = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.Slice(5, sizeof(uint))); // Read the size of the salt and ensure it's more than 128 bits. - var saltLength = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.Slice(9, 12)); + var saltLength = (int) BinaryPrimitives.ReadUInt32BigEndian(payload.Slice(9, sizeof(uint))); if (saltLength < 128 / 8) { return false; diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs index ce2e55fb..beca0e9e 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs @@ -6,6 +6,7 @@ using System.Collections.Immutable; using System.Diagnostics; +using System.Runtime.InteropServices; using System.Security.Claims; using System.Security.Cryptography; using System.Text; @@ -1548,11 +1549,11 @@ public static partial class OpenIddictServerHandlers var comparand = context.Principal.GetClaim(Claims.Private.CodeChallengeMethod) switch { // Note: when using the "plain" code challenge method, no hashing is actually performed. - // In this case, the raw ASCII bytes of the verifier are directly compared to the challenge. - CodeChallengeMethods.Plain => Encoding.ASCII.GetBytes(context.Request.CodeVerifier), + // In this case, the raw bytes of the verifier are directly compared to the challenge. + CodeChallengeMethods.Plain => context.Request.CodeVerifier, - CodeChallengeMethods.Sha256 => Encoding.ASCII.GetBytes(Base64UrlEncoder.Encode( - OpenIddictHelpers.ComputeSha256Hash(Encoding.ASCII.GetBytes(context.Request.CodeVerifier)))), + CodeChallengeMethods.Sha256 => Base64UrlEncoder.Encode( + OpenIddictHelpers.ComputeSha256Hash(Encoding.ASCII.GetBytes(context.Request.CodeVerifier))), null or { Length: 0 } => throw new InvalidOperationException(SR.GetResourceString(SR.ID0268)), @@ -1562,9 +1563,13 @@ public static partial class OpenIddictServerHandlers // Compare the verifier and the code challenge: if the two don't match, return an error. // Note: to prevent timing attacks, a time-constant comparer is always used. #if SUPPORTS_TIME_CONSTANT_COMPARISONS - if (!CryptographicOperations.FixedTimeEquals(comparand, Encoding.ASCII.GetBytes(challenge))) + if (!CryptographicOperations.FixedTimeEquals( + left: MemoryMarshal.AsBytes(comparand.AsSpan()), + right: MemoryMarshal.AsBytes(challenge.AsSpan()))) #else - if (!Arrays.ConstantTimeAreEqual(comparand, Encoding.ASCII.GetBytes(challenge))) + if (!Arrays.ConstantTimeAreEqual( + a: MemoryMarshal.AsBytes(comparand.AsSpan()).ToArray(), + b: MemoryMarshal.AsBytes(challenge.AsSpan()).ToArray())) #endif { context.Logger.LogInformation(SR.GetResourceString(SR.ID6092), Parameters.CodeVerifier); diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.cs index 2dae0613..d9c5c66e 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.cs @@ -2710,7 +2710,7 @@ public static partial class OpenIddictServerHandlers if (!string.IsNullOrEmpty(context.AccessToken)) { - var digest = ComputeHash(credentials, Encoding.ASCII.GetBytes(context.AccessToken)); + var digest = ComputeTokenHash(credentials, context.AccessToken); // Note: only the left-most half of the hash is used. // See http://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken @@ -2719,7 +2719,7 @@ public static partial class OpenIddictServerHandlers if (!string.IsNullOrEmpty(context.AuthorizationCode)) { - var digest = ComputeHash(credentials, Encoding.ASCII.GetBytes(context.AuthorizationCode)); + var digest = ComputeTokenHash(credentials, context.AuthorizationCode); // Note: only the left-most half of the hash is used. // See http://openid.net/specs/openid-connect-core-1_0.html#HybridIDToken @@ -2728,28 +2728,31 @@ public static partial class OpenIddictServerHandlers return default; - static byte[] ComputeHash(SigningCredentials credentials, byte[] data) => credentials switch + static byte[] ComputeTokenHash(SigningCredentials credentials, string token) => credentials switch { + // Note: ASCII is deliberately used here, as it's the encoding required by the specification. + // For more information, see https://openid.net/specs/openid-connect-core-1_0.html#CodeIDToken. + { Digest: SecurityAlgorithms.Sha256 or SecurityAlgorithms.Sha256Digest } or { Algorithm: SecurityAlgorithms.EcdsaSha256 or SecurityAlgorithms.EcdsaSha256Signature } or { Algorithm: SecurityAlgorithms.HmacSha256 or SecurityAlgorithms.HmacSha256Signature } or { Algorithm: SecurityAlgorithms.RsaSha256 or SecurityAlgorithms.RsaSha256Signature } or { Algorithm: SecurityAlgorithms.RsaSsaPssSha256 or SecurityAlgorithms.RsaSsaPssSha256Signature } - => OpenIddictHelpers.ComputeSha256Hash(data), + => OpenIddictHelpers.ComputeSha256Hash(Encoding.ASCII.GetBytes(token)), { Digest: SecurityAlgorithms.Sha384 or SecurityAlgorithms.Sha384Digest } or { Algorithm: SecurityAlgorithms.EcdsaSha384 or SecurityAlgorithms.EcdsaSha384Signature } or { Algorithm: SecurityAlgorithms.HmacSha384 or SecurityAlgorithms.HmacSha384Signature } or { Algorithm: SecurityAlgorithms.RsaSha384 or SecurityAlgorithms.RsaSha384Signature } or { Algorithm: SecurityAlgorithms.RsaSsaPssSha384 or SecurityAlgorithms.RsaSsaPssSha384Signature } - => OpenIddictHelpers.ComputeSha384Hash(data), + => OpenIddictHelpers.ComputeSha384Hash(Encoding.ASCII.GetBytes(token)), { Digest: SecurityAlgorithms.Sha512 or SecurityAlgorithms.Sha512Digest } or { Algorithm: SecurityAlgorithms.EcdsaSha512 or SecurityAlgorithms.EcdsaSha512Signature } or { Algorithm: SecurityAlgorithms.HmacSha512 or SecurityAlgorithms.HmacSha512Signature } or { Algorithm: SecurityAlgorithms.RsaSha512 or SecurityAlgorithms.RsaSha512Signature } or { Algorithm: SecurityAlgorithms.RsaSsaPssSha512 or SecurityAlgorithms.RsaSsaPssSha512Signature } - => OpenIddictHelpers.ComputeSha512Hash(data), + => OpenIddictHelpers.ComputeSha512Hash(Encoding.ASCII.GetBytes(token)), _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0267)) };