Browse Source

Update the documentation to clarify that state validation also helps mitigate authorization code injection attacks

pull/1435/head
Kévin Chalet 4 years ago
parent
commit
80f29a7f54
  1. 27
      src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs
  2. 7
      src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreOptions.cs
  3. 27
      src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs
  4. 11
      src/OpenIddict.Client.Owin/OpenIddictClientOwinOptions.cs
  5. 9
      src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Userinfo.cs
  6. 6
      src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.cs
  7. 6
      src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs

27
src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreHandlers.cs

@ -215,7 +215,8 @@ public static partial class OpenIddictClientAspNetCoreHandlers
} }
/// <summary> /// <summary>
/// Contains the logic responsible for validating the correlation cookie that serves as a CSRF protection. /// Contains the logic responsible for validating the correlation cookie that serves as a
/// protection against state token injection, forged requests and session fixation attacks.
/// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core. /// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core.
/// </summary> /// </summary>
public class ValidateCorrelationCookie : IOpenIddictClientHandler<ProcessAuthenticationContext> public class ValidateCorrelationCookie : IOpenIddictClientHandler<ProcessAuthenticationContext>
@ -255,8 +256,8 @@ public static partial class OpenIddictClientAspNetCoreHandlers
// Resolve the request forgery protection from the state token principal. // Resolve the request forgery protection from the state token principal.
// If the claim cannot be found, this means the protection was disabled // If the claim cannot be found, this means the protection was disabled
// using a custom event handler. In this case, bypass the validation. // using a custom event handler. In this case, bypass the validation.
var claim = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection); var identifier = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection);
if (string.IsNullOrEmpty(claim)) if (string.IsNullOrEmpty(identifier))
{ {
return default; return default;
} }
@ -268,15 +269,19 @@ public static partial class OpenIddictClientAspNetCoreHandlers
// and the random request forgery protection claim restored from the state. // and the random request forgery protection claim restored from the state.
var name = new StringBuilder(builder.Name) var name = new StringBuilder(builder.Name)
.Append(Separators.Dot) .Append(Separators.Dot)
.Append(claim) .Append(identifier)
.ToString(); .ToString();
// Try to find the cookie matching the request forgery protection stored in the state. // Try to find the cookie matching the request forgery protection stored in the state.
// //
// If the cookie cannot be found, this may indicate that the authorization response // 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 // is unsolicited and potentially malicious. This may also be caused by an unadequate
// same-site configuration. In any case, the authentication demand MUST be rejected // same-site configuration. The correlation cookie also serves as a binding mechanism
// as it's impossible to ensure it's not a session fixation attack without the cookie. // ensuring that a state token stolen from an authorization response with the other
// parameters cannot be validly used without sending the matching correlation identifier.
//
// 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]; var value = request.Cookies[name];
if (string.IsNullOrEmpty(value) || !string.Equals(value, "v1", StringComparison.Ordinal)) if (string.IsNullOrEmpty(value) || !string.Equals(value, "v1", StringComparison.Ordinal))
{ {
@ -347,7 +352,7 @@ public static partial class OpenIddictClientAspNetCoreHandlers
// abort the authorization flow by rejecting the token request that may be eventually sent // abort the authorization flow by rejecting the token request that may be eventually sent
// with the original redirect_uri, many servers are known to incorrectly implement this // with the original redirect_uri, many servers are known to incorrectly implement this
// redirect_uri validation logic. This check also offers limited protection as it cannot // redirect_uri validation logic. This check also offers limited protection as it cannot
// prevent the client credentials from being leaked to a malicious authorization server. // prevent the authorization code from being leaked to a malicious authorization server.
// By comparing the redirect_uri directly in the client, a first layer of protection is // By comparing the redirect_uri directly in the client, a first layer of protection is
// provided independently of whether the authorization server will enforce this check. // provided independently of whether the authorization server will enforce this check.
// //
@ -455,7 +460,8 @@ public static partial class OpenIddictClientAspNetCoreHandlers
} }
/// <summary> /// <summary>
/// Contains the logic responsible for creating a correlation cookie that serves as a CSRF countermeasure. /// Contains the logic responsible for creating a correlation cookie that serves as a
/// protection against state token injection, forged requests and session fixation attacks.
/// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core. /// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core.
/// </summary> /// </summary>
public class GenerateCorrelationCookie : IOpenIddictClientHandler<ProcessChallengeContext> public class GenerateCorrelationCookie : IOpenIddictClientHandler<ProcessChallengeContext>
@ -485,8 +491,8 @@ public static partial class OpenIddictClientAspNetCoreHandlers
throw new ArgumentNullException(nameof(context)); throw new ArgumentNullException(nameof(context));
} }
// Note: using a correlation cookie serves as an antiforgery protection as the request will // Note: using a correlation cookie serves as an injection/antiforgery protection as the request
// always be rejected if a cookie corresponding to the request forgery protection claim // 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 // 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 // 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 // a different protection strategy can set the request forgery protection claim to null or
@ -498,7 +504,6 @@ public static partial class OpenIddictClientAspNetCoreHandlers
} }
Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006));
Debug.Assert(!string.IsNullOrEmpty(context.StateToken), SR.GetResourceString(SR.ID4010));
// This handler only applies to ASP.NET Core requests. If the HTTP context cannot be resolved, // This handler only applies to ASP.NET Core requests. If the HTTP context cannot be resolved,
// this may indicate that the request was incorrectly processed by another server stack. // this may indicate that the request was incorrectly processed by another server stack.

7
src/OpenIddict.Client.AspNetCore/OpenIddictClientAspNetCoreOptions.cs

@ -40,14 +40,15 @@ public class OpenIddictClientAspNetCoreOptions : AuthenticationSchemeOptions
public bool EnableStatusCodePagesIntegration { get; set; } public bool EnableStatusCodePagesIntegration { get; set; }
/// <summary> /// <summary>
/// Gets or sets the cookie builder used to create the cookies that are /// Gets or sets the cookie builder used to create the cookies that are used to
/// used to protect against forged requests/session fixation attacks. /// bind authorization responses with their original request and help mitigate
/// authorization code injection, forged requests and session fixation attacks.
/// </summary> /// </summary>
public CookieBuilder CookieBuilder { get; set; } = new() public CookieBuilder CookieBuilder { get; set; } = new()
{ {
HttpOnly = true, HttpOnly = true,
IsEssential = true, IsEssential = true,
Name = "OpenIddict.Client.RequestForgeryProtection", Name = "OpenIddict.Client.State",
SameSite = SameSiteMode.None, SameSite = SameSiteMode.None,
SecurePolicy = CookieSecurePolicy.Always // Note: same-site=none requires using HTTPS. SecurePolicy = CookieSecurePolicy.Always // Note: same-site=none requires using HTTPS.
}; };

27
src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs

@ -212,7 +212,8 @@ public static partial class OpenIddictClientOwinHandlers
} }
/// <summary> /// <summary>
/// Contains the logic responsible for validating the correlation cookie that serves as a CSRF protection. /// Contains the logic responsible for validating the correlation cookie that serves as a
/// protection against state token injection, forged requests and session fixation attacks.
/// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN.
/// </summary> /// </summary>
public class ValidateCorrelationCookie : IOpenIddictClientHandler<ProcessAuthenticationContext> public class ValidateCorrelationCookie : IOpenIddictClientHandler<ProcessAuthenticationContext>
@ -252,8 +253,8 @@ public static partial class OpenIddictClientOwinHandlers
// Resolve the request forgery protection from the state token principal. // Resolve the request forgery protection from the state token principal.
// If the claim cannot be found, this means the protection was disabled // If the claim cannot be found, this means the protection was disabled
// using a custom event handler. In this case, bypass the validation. // using a custom event handler. In this case, bypass the validation.
var claim = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection); var identifier = context.StateTokenPrincipal.GetClaim(Claims.RequestForgeryProtection);
if (string.IsNullOrEmpty(claim)) if (string.IsNullOrEmpty(identifier))
{ {
return default; return default;
} }
@ -267,15 +268,19 @@ public static partial class OpenIddictClientOwinHandlers
// and the random request forgery protection claim restored from the state. // and the random request forgery protection claim restored from the state.
var name = new StringBuilder(_options.CurrentValue.CookieName) var name = new StringBuilder(_options.CurrentValue.CookieName)
.Append(Separators.Dot) .Append(Separators.Dot)
.Append(claim) .Append(identifier)
.ToString(); .ToString();
// Try to find the cookie matching the request forgery protection stored in the state. // Try to find the cookie matching the request forgery protection stored in the state.
// //
// If the cookie cannot be found, this may indicate that the authorization response // 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 // is unsolicited and potentially malicious. This may also be caused by an unadequate
// same-site configuration. In any case, the authentication demand MUST be rejected // same-site configuration. The correlation cookie also serves as a binding mechanism
// as it's impossible to ensure it's not a session fixation attack without the cookie. // ensuring that a state token stolen from an authorization response with the other
// parameters cannot be validly used without sending the matching correlation identifier.
//
// 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); var value = manager.GetRequestCookie(request.Context, name);
if (string.IsNullOrEmpty(value) || !string.Equals(value, "v1", StringComparison.Ordinal)) if (string.IsNullOrEmpty(value) || !string.Equals(value, "v1", StringComparison.Ordinal))
{ {
@ -353,7 +358,7 @@ public static partial class OpenIddictClientOwinHandlers
// abort the authorization flow by rejecting the token request that may be eventually sent // abort the authorization flow by rejecting the token request that may be eventually sent
// with the original redirect_uri, many servers are known to incorrectly implement this // with the original redirect_uri, many servers are known to incorrectly implement this
// redirect_uri validation logic. This check also offers limited protection as it cannot // redirect_uri validation logic. This check also offers limited protection as it cannot
// prevent the client credentials from being leaked to a malicious authorization server. // prevent the authorization code from being leaked to a malicious authorization server.
// By comparing the redirect_uri directly in the client, a first layer of protection is // By comparing the redirect_uri directly in the client, a first layer of protection is
// provided independently of whether the authorization server will enforce this check. // provided independently of whether the authorization server will enforce this check.
// //
@ -454,7 +459,8 @@ public static partial class OpenIddictClientOwinHandlers
} }
/// <summary> /// <summary>
/// Contains the logic responsible for creating a correlation cookie that serves as a CSRF countermeasure. /// Contains the logic responsible for creating a correlation cookie that serves as a
/// protection against state token injection, forged requests and session fixation attacks.
/// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN.
/// </summary> /// </summary>
public class GenerateCorrelationCookie : IOpenIddictClientHandler<ProcessChallengeContext> public class GenerateCorrelationCookie : IOpenIddictClientHandler<ProcessChallengeContext>
@ -484,8 +490,8 @@ public static partial class OpenIddictClientOwinHandlers
throw new ArgumentNullException(nameof(context)); throw new ArgumentNullException(nameof(context));
} }
// Note: using a correlation cookie serves as an antiforgery protection as the request will // Note: using a correlation cookie serves as an injection/antiforgery protection as the request
// always be rejected if a cookie corresponding to the request forgery protection claim // 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 // 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 // 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 // a different protection strategy can set the request forgery protection claim to null or
@ -497,7 +503,6 @@ public static partial class OpenIddictClientOwinHandlers
} }
Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006));
Debug.Assert(!string.IsNullOrEmpty(context.StateToken), SR.GetResourceString(SR.ID4010));
// This handler only applies to OWIN requests. If the HTTP context cannot be resolved, // This handler only applies to OWIN requests. If the HTTP context cannot be resolved,
// this may indicate that the request was incorrectly processed by another server stack. // this may indicate that the request was incorrectly processed by another server stack.

11
src/OpenIddict.Client.Owin/OpenIddictClientOwinOptions.cs

@ -45,13 +45,16 @@ public class OpenIddictClientOwinOptions : AuthenticationOptions
public ICookieManager CookieManager { get; set; } = new CookieManager(); public ICookieManager CookieManager { get; set; } = new CookieManager();
/// <summary> /// <summary>
/// Gets or sets the name of the cookie used to protect against forged requests/session fixation attacks. /// Gets or sets the name of the correlation cookie used to bind authorization
/// responses with their original request and help mitigate authorization
/// code injection, forged requests and session fixation attacks.
/// </summary> /// </summary>
public string CookieName { get; set; } = "OpenIddict.Client.RequestForgeryProtection"; public string CookieName { get; set; } = "OpenIddict.Client.State";
/// <summary> /// <summary>
/// Gets or sets the cookie options used to create the cookies that are /// Gets or sets the cookie options used to create the cookies that are used to
/// used to protect against forged requests/session fixation attacks. /// bind authorization responses with their original request and help mitigate
/// authorization code injection, forged requests and session fixation attacks.
/// </summary> /// </summary>
public CookieOptions CookieOptions { get; set; } = new() public CookieOptions CookieOptions { get; set; } = new()
{ {

9
src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.Userinfo.cs

@ -23,6 +23,7 @@ public static partial class OpenIddictClientSystemNetHttpHandlers
*/ */
PrepareGetHttpRequest<PrepareUserinfoRequestContext>.Descriptor, PrepareGetHttpRequest<PrepareUserinfoRequestContext>.Descriptor,
AttachBearerAccessToken.Descriptor, AttachBearerAccessToken.Descriptor,
AttachQueryStringParameters<PrepareUserinfoRequestContext>.Descriptor,
SendHttpRequest<ApplyUserinfoRequestContext>.Descriptor, SendHttpRequest<ApplyUserinfoRequestContext>.Descriptor,
DisposeHttpRequest<ApplyUserinfoRequestContext>.Descriptor, DisposeHttpRequest<ApplyUserinfoRequestContext>.Descriptor,
@ -44,7 +45,7 @@ public static partial class OpenIddictClientSystemNetHttpHandlers
= OpenIddictClientHandlerDescriptor.CreateBuilder<PrepareUserinfoRequestContext>() = OpenIddictClientHandlerDescriptor.CreateBuilder<PrepareUserinfoRequestContext>()
.AddFilter<RequireHttpMetadataAddress>() .AddFilter<RequireHttpMetadataAddress>()
.UseSingletonHandler<AttachBearerAccessToken>() .UseSingletonHandler<AttachBearerAccessToken>()
.SetOrder(AttachFormParameters<PrepareUserinfoRequestContext>.Descriptor.Order - 1000) .SetOrder(AttachQueryStringParameters<PrepareUserinfoRequestContext>.Descriptor.Order - 500)
.SetType(OpenIddictClientHandlerType.BuiltIn) .SetType(OpenIddictClientHandlerType.BuiltIn)
.Build(); .Build();
@ -97,6 +98,12 @@ public static partial class OpenIddictClientSystemNetHttpHandlers
throw new ArgumentNullException(nameof(context)); throw new ArgumentNullException(nameof(context));
} }
// Don't overwrite the response if one was already provided.
if (context.Response is not null || !string.IsNullOrEmpty(context.UserinfoToken))
{
return;
}
// This handler only applies to System.Net.Http requests. If the HTTP response cannot be resolved, // This handler only applies to System.Net.Http requests. If the HTTP response cannot be resolved,
// this may indicate that the request was incorrectly processed by another client stack. // this may indicate that the request was incorrectly processed by another client stack.
var response = context.Transaction.GetHttpResponseMessage() ?? var response = context.Transaction.GetHttpResponseMessage() ??

6
src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.cs

@ -345,6 +345,12 @@ public static partial class OpenIddictClientSystemNetHttpHandlers
throw new ArgumentNullException(nameof(context)); throw new ArgumentNullException(nameof(context));
} }
// Don't overwrite the response if one was already provided.
if (context.Transaction.Response is not null)
{
return;
}
// This handler only applies to System.Net.Http requests. If the HTTP response cannot be resolved, // This handler only applies to System.Net.Http requests. If the HTTP response cannot be resolved,
// this may indicate that the request was incorrectly processed by another client stack. // this may indicate that the request was incorrectly processed by another client stack.
var response = context.Transaction.GetHttpResponseMessage() ?? var response = context.Transaction.GetHttpResponseMessage() ??

6
src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs

@ -344,6 +344,12 @@ public static partial class OpenIddictValidationSystemNetHttpHandlers
throw new ArgumentNullException(nameof(context)); throw new ArgumentNullException(nameof(context));
} }
// Don't overwrite the response if one was already provided.
if (context.Transaction.Response is not null)
{
return;
}
// This handler only applies to System.Net.Http requests. If the HTTP response cannot be resolved, // This handler only applies to System.Net.Http requests. If the HTTP response cannot be resolved,
// this may indicate that the request was incorrectly processed by another client stack. // this may indicate that the request was incorrectly processed by another client stack.
var response = context.Transaction.GetHttpResponseMessage() ?? var response = context.Transaction.GetHttpResponseMessage() ??

Loading…
Cancel
Save