From 00aa55851155eef1266a596fd97965f3ab6d03aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Mon, 26 Sep 2022 18:18:43 +0200 Subject: [PATCH] Update the GenerateLoginCorrelationCookie handler to throw an exception if the request forgery protection was set to null --- .../OpenIddictResources.resx | 6 +++++ .../OpenIddictClientAspNetCoreHandlers.cs | 22 ++++++++++--------- .../OpenIddictClientOwinHandlers.cs | 22 ++++++++++--------- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/OpenIddict.Abstractions/OpenIddictResources.resx b/src/OpenIddict.Abstractions/OpenIddictResources.resx index f71a0ea4..c844411d 100644 --- a/src/OpenIddict.Abstractions/OpenIddictResources.resx +++ b/src/OpenIddict.Abstractions/OpenIddictResources.resx @@ -1325,6 +1325,12 @@ Alternatively, you can disable the token storage feature by calling 'services.Ad Identical issuers cannot be used in multiple client registrations. + + The request forgery protection claim cannot be resolved from the challenge context. + + + The request forgery protection claim cannot be resolved from the sign-out context. + The security token is missing. diff --git a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs index b3171bc4..0b9f33ea 100644 --- a/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs +++ b/src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs @@ -279,12 +279,13 @@ public static partial class OpenIddictClientAspNetCoreHandlers .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. This may also be caused by an unadequate - // same-site configuration. The correlation cookie also 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. + // 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. @@ -525,6 +526,7 @@ public static partial class OpenIddictClientAspNetCoreHandlers public static OpenIddictClientHandlerDescriptor Descriptor { get; } = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() + .AddFilter() .AddFilter() .UseSingletonHandler() .SetOrder(AttachChallengeParameters.Descriptor.Order + 1_000) @@ -543,12 +545,12 @@ public static partial class OpenIddictClientAspNetCoreHandlers // will always be rejected if a cookie corresponding to the request forgery protection claim // persisted in the state token cannot be found. This protection is considered essential // in OpenIddict and cannot be disabled via the options. Applications that prefer implementing - // a different protection strategy can set the request forgery protection claim to null or - // remove this handler from the handlers list and add a custom one using a different approach. + // 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.RequestForgeryProtection)) { - return default; + throw new InvalidOperationException(SR.GetResourceString(SR.ID0343)); } Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); @@ -711,12 +713,12 @@ public static partial class OpenIddictClientAspNetCoreHandlers // will always be rejected if a cookie corresponding to the request forgery protection claim // persisted in the state token cannot be found. This protection is considered essential // in OpenIddict and cannot be disabled via the options. Applications that prefer implementing - // a different protection strategy can set the request forgery protection claim to null or - // remove this handler from the handlers list and add a custom one using a different approach. + // 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.RequestForgeryProtection)) { - return default; + throw new InvalidOperationException(SR.GetResourceString(SR.ID0344)); } Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); diff --git a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs index e3662273..d153d04d 100644 --- a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs +++ b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs @@ -280,12 +280,13 @@ public static partial class OpenIddictClientOwinHandlers .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. This may also be caused by an unadequate - // same-site configuration. The correlation cookie also 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. + // 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. @@ -555,6 +556,7 @@ public static partial class OpenIddictClientOwinHandlers public static OpenIddictClientHandlerDescriptor Descriptor { get; } = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() + .AddFilter() .AddFilter() .UseSingletonHandler() .SetOrder(AttachChallengeParameters.Descriptor.Order + 1_000) @@ -573,12 +575,12 @@ public static partial class OpenIddictClientOwinHandlers // will always be rejected if a cookie corresponding to the request forgery protection claim // persisted in the state token cannot be found. This protection is considered essential // in OpenIddict and cannot be disabled via the options. Applications that prefer implementing - // a different protection strategy can set the request forgery protection claim to null or - // remove this handler from the handlers list and add a custom one using a different approach. + // 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.RequestForgeryProtection)) { - return default; + throw new InvalidOperationException(SR.GetResourceString(SR.ID0343)); } Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); @@ -768,12 +770,12 @@ public static partial class OpenIddictClientOwinHandlers // will always be rejected if a cookie corresponding to the request forgery protection claim // persisted in the state token cannot be found. This protection is considered essential // in OpenIddict and cannot be disabled via the options. Applications that prefer implementing - // a different protection strategy can set the request forgery protection claim to null or - // remove this handler from the handlers list and add a custom one using a different approach. + // 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.RequestForgeryProtection)) { - return default; + throw new InvalidOperationException(SR.GetResourceString(SR.ID0344)); } Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006));