Browse Source

Implement "iss" support in the server stack and update the token validation logic to support issuers that don't end with a trailing slash

pull/1403/head
Kévin Chalet 4 years ago
parent
commit
a1215728db
  1. 4
      src/OpenIddict.Abstractions/OpenIddictConstants.cs
  2. 9
      src/OpenIddict.Abstractions/OpenIddictResources.resx
  3. 9
      src/OpenIddict.Abstractions/Primitives/OpenIddictResponse.cs
  4. 2
      src/OpenIddict.Client/OpenIddictClientConfiguration.cs
  5. 19
      src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs
  6. 2
      src/OpenIddict.Client/OpenIddictClientHandlers.Userinfo.cs
  7. 3
      src/OpenIddict.Client/OpenIddictClientHandlers.cs
  8. 13
      src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs
  9. 2
      src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs
  10. 2
      src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs
  11. 80
      src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs
  12. 7
      src/OpenIddict.Server/OpenIddictServerHandlers.Discovery.cs
  13. 2
      src/OpenIddict.Server/OpenIddictServerHandlers.cs
  14. 2
      src/OpenIddict.Validation/OpenIddictValidationConfiguration.cs
  15. 4
      src/OpenIddict.Validation/OpenIddictValidationHandlers.Introspection.cs
  16. 20
      src/OpenIddict.Validation/OpenIddictValidationHandlers.Protection.cs
  17. 7
      test/OpenIddict.Abstractions.Tests/Primitives/OpenIddictResponseTests.cs
  18. 117
      test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs
  19. 7
      test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.cs

4
src/OpenIddict.Abstractions/OpenIddictConstants.cs

@ -439,6 +439,10 @@ public static class OpenIddictConstants
public static readonly char[] Ampersand = { '&' };
public static readonly char[] Dash = { '-' };
public static readonly char[] Dot = { '.' };
public static readonly char[] EqualsSign = { '=' };
public static readonly char[] Hash = { '#' };
public static readonly char[] QuestionMark = { '?' };
public static readonly char[] Semicolon = { ';' };
public static readonly char[] Space = { ' ' };
}

9
src/OpenIddict.Abstractions/OpenIddictResources.resx

@ -1587,6 +1587,12 @@ To apply redirection responses, create a class implementing 'IOpenIddictClientHa
<data name="ID2133" xml:space="preserve">
<value>The '{0}' claim returned in the specified userinfo response/token doesn't match the expected value.</value>
</data>
<data name="ID2134" xml:space="preserve">
<value>Callback URLs cannot contain an "{0}" parameter.</value>
</data>
<data name="ID2135" xml:space="preserve">
<value>The '{0}' parameter must not include a '{1}' component.</value>
</data>
<data name="ID4000" xml:space="preserve">
<value>The '{0}' parameter shouldn't be null or empty at this point.</value>
</data>
@ -2132,6 +2138,9 @@ This may indicate that the hashed entry is corrupted or malformed.</value>
<data name="ID6180" xml:space="preserve">
<value>The redirection request was successfully validated.</value>
</data>
<data name="ID6181" xml:space="preserve">
<value>The authorization request was rejected because the '{Parameter}' contained a forbidden parameter: {Name}.</value>
</data>
<data name="ID8000" xml:space="preserve">
<value>https://documentation.openiddict.com/errors/{0}</value>
</data>

9
src/OpenIddict.Abstractions/Primitives/OpenIddictResponse.cs

@ -153,6 +153,15 @@ public class OpenIddictResponse : OpenIddictMessage
set => SetParameter(OpenIddictConstants.Parameters.IdToken, value);
}
/// <summary>
/// Gets or sets the "iss" parameter.
/// </summary>
public string? Iss
{
get => (string?) GetParameter(OpenIddictConstants.Parameters.Iss);
set => SetParameter(OpenIddictConstants.Parameters.Iss, value);
}
/// <summary>
/// Gets or sets the "refresh_token" parameter.
/// </summary>

2
src/OpenIddict.Client/OpenIddictClientConfiguration.cs

@ -65,7 +65,7 @@ public class OpenIddictClientConfiguration : IPostConfigureOptions<OpenIddictCli
if (!registration.MetadataAddress.IsAbsoluteUri)
{
var issuer = registration.Issuer;
if (issuer is null || !issuer.IsAbsoluteUri)
if (issuer is not { IsAbsoluteUri: true })
{
throw new InvalidOperationException(SR.GetResourceString(SR.ID0136));
}

19
src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs

@ -110,8 +110,23 @@ public static partial class OpenIddictClientHandlers
var parameters = registration!.TokenValidationParameters.Clone();
parameters.ValidIssuer ??= configuration.Issuer?.OriginalString;
parameters.ValidateIssuer = !string.IsNullOrEmpty(parameters.ValidIssuer);
parameters.ValidIssuers ??= configuration.Issuer switch
{
null => null,
// If the issuer URI doesn't contain any path/query/fragment, allow both http://www.fabrikam.com
// and http://www.fabrikam.com/ (the recommended URI representation) to be considered valid.
// See https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.3 for more information.
{ AbsolutePath: "/", Query.Length: 0, Fragment.Length: 0 } issuer => new[]
{
issuer.AbsoluteUri, // Uri.AbsoluteUri is normalized and always contains a trailing slash.
issuer.AbsoluteUri.Substring(0, issuer.AbsoluteUri.Length - 1)
},
Uri issuer => new[] { issuer.AbsoluteUri }
};
parameters.ValidateIssuer = parameters.ValidIssuers is not null;
// Combine the signing keys registered statically in the token validation parameters
// with the signing keys resolved from the OpenID Connect server configuration.

2
src/OpenIddict.Client/OpenIddictClientHandlers.Userinfo.cs

@ -129,7 +129,7 @@ public static partial class OpenIddictClientHandlers
// Resolve the issuer that will be attached to the claims created by this handler.
// Note: at this stage, the optional issuer extracted from the response is assumed
// to be valid, as it is guarded against unknown values by the ValidateIssuer handler.
var issuer = (string?) context.Response[Claims.Issuer] ?? configuration.Issuer!.OriginalString;
var issuer = (string?) context.Response[Claims.Issuer] ?? configuration.Issuer!.AbsoluteUri;
foreach (var parameter in context.Response.GetParameters())
{

3
src/OpenIddict.Client/OpenIddictClientHandlers.cs

@ -445,7 +445,8 @@ public static partial class OpenIddictClientHandlers
}
// If the two values don't match, this may indicate a mix-up attack attempt.
if (!string.Equals(issuer, context.Issuer.AbsoluteUri, StringComparison.Ordinal))
if (!Uri.TryCreate(issuer, UriKind.Absolute, out Uri? uri) ||
!uri.IsWellFormedOriginalString() || uri != configuration.Issuer)
{
context.Reject(
error: Errors.InvalidRequest,

13
src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs

@ -1226,6 +1226,19 @@ public class OpenIddictApplicationManager<TApplication> : IOpenIddictApplication
break;
}
// To prevent issuer fixation attacks where a malicious client would specify an "iss" parameter
// in the callback URL, ensure the query - if present - doesn't include an "iss" parameter.
if (!string.IsNullOrEmpty(uri.Query) && uri.Query.TrimStart(Separators.QuestionMark[0])
.Split(new[] { Separators.Ampersand[0], Separators.Semicolon[0] }, StringSplitOptions.RemoveEmptyEntries)
.Select(parameter => parameter.Split(Separators.EqualsSign, StringSplitOptions.RemoveEmptyEntries))
.Select(parts => parts[0] is string value ? Uri.UnescapeDataString(value) : null)
.Contains(Parameters.Iss, StringComparer.OrdinalIgnoreCase))
{
yield return new ValidationResult(SR.FormatID2134(Parameters.Iss));
break;
}
}
}

2
src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs

@ -240,7 +240,7 @@ public static partial class OpenIddictServerAspNetCoreHandlers
.AddFilter<RequireHttpRequest>()
.AddFilter<RequireTransportSecurityRequirementEnabled>()
.UseSingletonHandler<ValidateTransportSecurityRequirement>()
.SetOrder(InferEndpointType.Descriptor.Order + 1_000)
.SetOrder(InferIssuerFromHost.Descriptor.Order + 1_000)
.SetType(OpenIddictServerHandlerType.BuiltIn)
.Build();

2
src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs

@ -231,7 +231,7 @@ public static partial class OpenIddictServerOwinHandlers
.AddFilter<RequireOwinRequest>()
.AddFilter<RequireTransportSecurityRequirementEnabled>()
.UseSingletonHandler<ValidateTransportSecurityRequirement>()
.SetOrder(InferEndpointType.Descriptor.Order + 1_000)
.SetOrder(InferIssuerFromHost.Descriptor.Order + 1_000)
.SetType(OpenIddictServerHandlerType.BuiltIn)
.Build();

80
src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs

@ -56,7 +56,8 @@ public static partial class OpenIddictServerHandlers
*/
AttachRedirectUri.Descriptor,
InferResponseMode.Descriptor,
AttachResponseState.Descriptor);
AttachResponseState.Descriptor,
AttachIssuer.Descriptor);
/// <summary>
/// Contains the logic responsible of extracting authorization requests and invoking the corresponding event handlers.
@ -532,6 +533,27 @@ public static partial class OpenIddictServerHandlers
return default;
}
// To prevent issuer fixation attacks where a malicious client would specify an "iss" parameter
// in the redirect_uri, ensure the query - if present - doesn't include an "iss" parameter.
//
// Note: while OAuth 2.0 parameters are case-sentitive, the following check deliberately
// uses a case-insensitive comparison to ensure that all variations of "iss" are rejected.
if (!string.IsNullOrEmpty(uri.Query) && uri.Query.TrimStart(Separators.QuestionMark[0])
.Split(new[] { Separators.Ampersand[0], Separators.Semicolon[0] }, StringSplitOptions.RemoveEmptyEntries)
.Select(parameter => parameter.Split(Separators.EqualsSign, StringSplitOptions.RemoveEmptyEntries))
.Select(parts => parts[0] is string value ? Uri.UnescapeDataString(value) : null)
.Contains(Parameters.Iss, StringComparer.OrdinalIgnoreCase))
{
context.Logger.LogInformation(SR.GetResourceString(SR.ID6181), Parameters.RedirectUri, Parameters.Iss);
context.Reject(
error: Errors.InvalidRequest,
description: SR.FormatID2135(Parameters.RedirectUri, Parameters.Iss),
uri: SR.FormatID8000(SR.ID2135));
return default;
}
return default;
}
}
@ -1723,8 +1745,11 @@ public static partial class OpenIddictServerHandlers
throw new ArgumentNullException(nameof(context));
}
// Attach the request state to the authorization response.
if (string.IsNullOrEmpty(context.Response.State))
// If the user agent is expected to be redirected to the client application, attach the request
// state to the authorization response to help the client mitigate CSRF/session fixation attacks.
//
// Note: don't override the state if one was already attached to the response instance.
if (!string.IsNullOrEmpty(context.RedirectUri) && string.IsNullOrEmpty(context.Response.State))
{
context.Response.State = context.Request?.State;
}
@ -1732,5 +1757,54 @@ public static partial class OpenIddictServerHandlers
return default;
}
}
/// <summary>
/// Contains the logic responsible of attaching an "iss" parameter
/// containing the address of the authorization server to the response.
/// </summary>
public class AttachIssuer : IOpenIddictServerHandler<ApplyAuthorizationResponseContext>
{
/// <summary>
/// Gets the default descriptor definition assigned to this handler.
/// </summary>
public static OpenIddictServerHandlerDescriptor Descriptor { get; }
= OpenIddictServerHandlerDescriptor.CreateBuilder<ApplyAuthorizationResponseContext>()
.UseSingletonHandler<AttachIssuer>()
.SetOrder(AttachResponseState.Descriptor.Order + 1_000)
.SetType(OpenIddictServerHandlerType.BuiltIn)
.Build();
/// <inheritdoc/>
public ValueTask HandleAsync(ApplyAuthorizationResponseContext context)
{
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}
// If the user agent is expected to be redirected to the client application, attach the
// issuer address to the authorization response to help the client detect mix-up attacks.
//
// Note: this applies to all authorization responses, whether they represent valid or errored responses.
// For more information, see https://datatracker.ietf.org/doc/html/draft-ietf-oauth-iss-auth-resp-05.
if (!string.IsNullOrEmpty(context.RedirectUri))
{
// At this stage, throw an exception if the issuer cannot be retrieved.
if (context.Issuer is not { IsAbsoluteUri: true })
{
throw new InvalidOperationException(SR.GetResourceString(SR.ID0023));
}
// Note: don't override the issuer if one was already attached to the response instance.
if (string.IsNullOrEmpty(context.Response.Iss))
{
context.Response.Iss = context.Issuer.AbsoluteUri;
}
}
return default;
}
}
}
}

7
src/OpenIddict.Server/OpenIddictServerHandlers.Discovery.cs

@ -377,7 +377,7 @@ public static partial class OpenIddictServerHandlers
}
// At this stage, throw an exception if the issuer cannot be retrieved.
if (issuer is null || !issuer.IsAbsoluteUri)
if (issuer is not { IsAbsoluteUri: true })
{
throw new InvalidOperationException(SR.GetResourceString(SR.ID0023));
}
@ -746,6 +746,11 @@ public static partial class OpenIddictServerHandlers
context.Metadata[Metadata.RequestParameterSupported] = false;
context.Metadata[Metadata.RequestUriParameterSupported] = false;
// As of 3.2.0, OpenIddict automatically returns an "iss" parameter containing its own address as
// part of authorization responses to help clients mitigate mix-up attacks. For more information,
// see https://datatracker.ietf.org/doc/html/draft-ietf-oauth-iss-auth-resp-05.
context.Metadata[Metadata.AuthorizationResponseIssParameterSupported] = true;
return default;
}
}

2
src/OpenIddict.Server/OpenIddictServerHandlers.cs

@ -2953,7 +2953,7 @@ public static partial class OpenIddictServerHandlers
}
// At this stage, throw an exception if the issuer cannot be retrieved.
if (issuer is null || !issuer.IsAbsoluteUri)
if (issuer is not { IsAbsoluteUri: true })
{
throw new InvalidOperationException(SR.GetResourceString(SR.ID0023));
}

2
src/OpenIddict.Validation/OpenIddictValidationConfiguration.cs

@ -109,7 +109,7 @@ public class OpenIddictValidationConfiguration : IPostConfigureOptions<OpenIddic
if (!options.MetadataAddress.IsAbsoluteUri)
{
var issuer = options.Issuer;
if (issuer is null || !issuer.IsAbsoluteUri)
if (issuer is not { IsAbsoluteUri: true })
{
throw new InvalidOperationException(SR.GetResourceString(SR.ID0136));
}

4
src/OpenIddict.Validation/OpenIddictValidationHandlers.Introspection.cs

@ -378,8 +378,8 @@ public static partial class OpenIddictValidationHandlers
// Note: at this stage, the optional issuer extracted from the response is assumed
// to be valid, as it is guarded against unknown values by the ValidateIssuer handler.
var issuer = (string?) context.Response[Claims.Issuer] ??
configuration?.Issuer?.OriginalString ??
context.Issuer?.OriginalString ?? ClaimsIdentity.DefaultIssuer;
configuration?.Issuer?.AbsoluteUri ??
context.Issuer?.AbsoluteUri ?? ClaimsIdentity.DefaultIssuer;
foreach (var parameter in context.Response.GetParameters())
{

20
src/OpenIddict.Validation/OpenIddictValidationHandlers.Protection.cs

@ -70,8 +70,24 @@ public static partial class OpenIddictValidationHandlers
// Clone the token validation parameters and set the issuer using the value found in the
// OpenID Connect server configuration (that can be static or retrieved using discovery).
var parameters = context.Options.TokenValidationParameters.Clone();
parameters.ValidIssuer ??= configuration.Issuer?.OriginalString;
parameters.ValidateIssuer = !string.IsNullOrEmpty(parameters.ValidIssuer);
parameters.ValidIssuers ??= configuration.Issuer switch
{
null => null,
// If the issuer URI doesn't contain any path/query/fragment, allow both http://www.fabrikam.com
// and http://www.fabrikam.com/ (the recommended URI representation) to be considered valid.
// See https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.3 for more information.
{ AbsolutePath: "/", Query.Length: 0, Fragment.Length: 0 } issuer => new[]
{
issuer.AbsoluteUri, // Uri.AbsoluteUri is normalized and always contains a trailing slash.
issuer.AbsoluteUri.Substring(0, issuer.AbsoluteUri.Length - 1)
},
Uri issuer => new[] { issuer.AbsoluteUri }
};
parameters.ValidateIssuer = parameters.ValidIssuers is not null;
// Combine the signing keys registered statically in the token validation parameters
// with the signing keys resolved from the OpenID Connect server configuration.

7
test/OpenIddict.Abstractions.Tests/Primitives/OpenIddictResponseTests.cs

@ -71,6 +71,13 @@ public class OpenIddictResponseTests
/* value: */ new OpenIddictParameter("802A3E3E-DCCA-4EFC-89FA-7D82FE8C27E4")
};
yield return new object[]
{
/* property: */ nameof(OpenIddictResponse.Iss),
/* name: */ Parameters.Iss,
/* value: */ new OpenIddictParameter("802A3E3E-DCCA-4EFC-89FA-7D82FE8C27E4")
};
yield return new object[]
{
/* property: */ nameof(OpenIddictResponse.RefreshToken),

117
test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs

@ -241,6 +241,41 @@ public abstract partial class OpenIddictServerIntegrationTests
Assert.Equal(SR.FormatID8000(message), response.ErrorUri);
}
[Theory]
[InlineData("http://www.fabrikam.com/path?iss")]
[InlineData("http://www.fabrikam.com/path?iss=value")]
[InlineData("http://www.fabrikam.com/path?;iss")]
[InlineData("http://www.fabrikam.com/path?;iss=value")]
[InlineData("http://www.fabrikam.com/path?&iss")]
[InlineData("http://www.fabrikam.com/path?&iss=value")]
[InlineData("http://www.fabrikam.com/path?state;iss")]
[InlineData("http://www.fabrikam.com/path?state;iss=value")]
[InlineData("http://www.fabrikam.com/path?state&iss")]
[InlineData("http://www.fabrikam.com/path?state&iss=value")]
[InlineData("http://www.fabrikam.com/path?state=abc;iss")]
[InlineData("http://www.fabrikam.com/path?state=abc;iss=value")]
[InlineData("http://www.fabrikam.com/path?state=abc&iss")]
[InlineData("http://www.fabrikam.com/path?state=abc&iss=value")]
public async Task ValidateAuthorizationRequest_RedirectUriWithIssuerParameterCausesAnError(string address)
{
// Arrange
await using var server = await CreateServerAsync(options => options.EnableDegradedMode());
await using var client = await server.CreateClientAsync();
// Act
var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest
{
ClientId = "Fabrikam",
RedirectUri = address,
Scope = Scopes.OpenId
});
// Assert
Assert.Equal(Errors.InvalidRequest, response.Error);
Assert.Equal(SR.FormatID2135(Parameters.RedirectUri, Parameters.Iss), response.ErrorDescription);
Assert.Equal(SR.FormatID8000(SR.ID2135), response.ErrorUri);
}
[Fact]
public async Task ValidateAuthorizationRequest_MissingResponseTypeCausesAnError()
{
@ -2335,4 +2370,86 @@ public abstract partial class OpenIddictServerIntegrationTests
// Assert
Assert.Equal("custom_state", response.State);
}
[Theory]
[InlineData(null)]
[InlineData("http://www.fabrikam.com/")]
public async Task ApplyAuthorizationResponse_SetsIssuerParameter(string issuer)
{
// Arrange
await using var server = await CreateServerAsync(options =>
{
options.EnableDegradedMode();
if (!string.IsNullOrEmpty(issuer))
{
options.SetIssuer(new Uri(issuer, UriKind.Absolute));
}
options.AddEventHandler<HandleAuthorizationRequestContext>(builder =>
builder.UseInlineHandler(context =>
{
context.Principal = new ClaimsPrincipal(new ClaimsIdentity("Bearer"))
.SetClaim(Claims.Subject, "Bob le Magnifique");
return default;
}));
});
await using var client = await server.CreateClientAsync();
// Act
var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest
{
ClientId = "Fabrikam",
RedirectUri = "http://www.fabrikam.com/path",
ResponseType = ResponseTypes.Code,
State = "af0ifjsldkj"
});
// Assert
Assert.Equal(issuer is not null ? issuer : "http://localhost/", response.Iss);
}
[Fact]
public async Task ApplyAuthorizationResponse_DoesNotOverrideIssuerSetByApplicationCode()
{
// Arrange
await using var server = await CreateServerAsync(options =>
{
options.EnableDegradedMode();
options.SetIssuer(new Uri("http://www.fabrikam.com/", UriKind.Absolute));
options.AddEventHandler<HandleAuthorizationRequestContext>(builder =>
builder.UseInlineHandler(context =>
{
context.Principal = new ClaimsPrincipal(new ClaimsIdentity("Bearer"))
.SetClaim(Claims.Subject, "Bob le Magnifique");
return default;
}));
options.AddEventHandler<ApplyAuthorizationResponseContext>(builder =>
builder.UseInlineHandler(context =>
{
context.Response.Iss = "http://www.contoso.com/";
return default;
}));
});
await using var client = await server.CreateClientAsync();
// Act
var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest
{
ClientId = "Fabrikam",
RedirectUri = "http://www.fabrikam.com/path",
ResponseType = ResponseTypes.Code,
State = "af0ifjsldkj"
});
// Assert
Assert.Equal("http://www.contoso.com/", response.Iss);
}
}

7
test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.cs

@ -1201,7 +1201,12 @@ public abstract partial class OpenIddictServerIntegrationTests
});
// Assert
Assert.Equal(0, response.Count);
Assert.Null(response.AccessToken);
Assert.Null(response.Code);
Assert.Null(response.DeviceCode);
Assert.Null(response.IdToken);
Assert.Null(response.RefreshToken);
Assert.Null(response.UserCode);
}
[Theory]

Loading…
Cancel
Save