Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support client cert negotation #33264

Merged
merged 19 commits into from
Jun 15, 2021
8 changes: 7 additions & 1 deletion src/Servers/Kestrel/Core/src/ClientCertificateMode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ public enum ClientCertificateMode
/// <summary>
/// A client certificate will be requested, and the client must provide a valid certificate for authentication to succeed.
/// </summary>
RequireCertificate
RequireCertificate,

/// <summary>
/// A client certificate is not required and will not be requested from clients at the start of the connection.
/// It may be requested by the application later.
/// </summary>
DelayCertificate,
}
}
19 changes: 11 additions & 8 deletions src/Servers/Kestrel/Core/src/Internal/SniOptionsSelector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ public SniOptionsSelector(

if (clientCertificateMode != ClientCertificateMode.NoCertificate)
{
sslOptions.ClientCertificateRequired = true;
sslOptions.ClientCertificateRequired = clientCertificateMode == ClientCertificateMode.AllowCertificate
BrennanConroy marked this conversation as resolved.
Show resolved Hide resolved
|| clientCertificateMode == ClientCertificateMode.RequireCertificate;
sslOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) =>
HttpsConnectionMiddleware.RemoteCertificateValidationCallback(
clientCertificateMode, fallbackHttpsOptions.ClientCertificateValidation, certificate, chain, sslPolicyErrors);
Expand All @@ -94,7 +95,7 @@ public SniOptionsSelector(
httpProtocols = HttpsConnectionMiddleware.ValidateAndNormalizeHttpProtocols(httpProtocols, logger);
HttpsConnectionMiddleware.ConfigureAlpn(sslOptions, httpProtocols);

var sniOptions = new SniOptions(sslOptions, httpProtocols);
var sniOptions = new SniOptions(sslOptions, httpProtocols, clientCertificateMode);

if (name.Equals(WildcardHost, StringComparison.Ordinal))
{
Expand All @@ -112,7 +113,7 @@ public SniOptionsSelector(
}
}

public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, string serverName)
public (SslServerAuthenticationOptions, ClientCertificateMode) GetOptions(ConnectionContext connection, string serverName)
{
SniOptions? sniOptions = null;

Expand Down Expand Up @@ -172,14 +173,14 @@ public SslServerAuthenticationOptions GetOptions(ConnectionContext connection, s
_onAuthenticateCallback(connection, sslOptions);
}

return sslOptions;
return (sslOptions, sniOptions.ClientCertificateMode);
}

public static ValueTask<SslServerAuthenticationOptions> OptionsCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken)
public static ValueTask<(SslServerAuthenticationOptions, ClientCertificateMode)> OptionsCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken)
{
var sniOptionsSelector = (SniOptionsSelector)state;
var options = sniOptionsSelector.GetOptions(connection, clientHelloInfo.ServerName);
return new ValueTask<SslServerAuthenticationOptions>(options);
var (options, clientCertificateMode) = sniOptionsSelector.GetOptions(connection, clientHelloInfo.ServerName);
return new ValueTask<(SslServerAuthenticationOptions, ClientCertificateMode)>((options, clientCertificateMode));
}

internal static SslServerAuthenticationOptions CloneSslOptions(SslServerAuthenticationOptions sslOptions) =>
Expand All @@ -200,14 +201,16 @@ internal static SslServerAuthenticationOptions CloneSslOptions(SslServerAuthenti

private class SniOptions
{
public SniOptions(SslServerAuthenticationOptions sslOptions, HttpProtocols httpProtocols)
public SniOptions(SslServerAuthenticationOptions sslOptions, HttpProtocols httpProtocols, ClientCertificateMode clientCertificateMode)
{
SslOptions = sslOptions;
HttpProtocols = httpProtocols;
ClientCertificateMode = clientCertificateMode;
}

public SslServerAuthenticationOptions SslOptions { get; }
public HttpProtocols HttpProtocols { get; }
public ClientCertificateMode ClientCertificateMode { get; }
}

private class LongestStringFirstComparer : IComparer<string>
Expand Down
26 changes: 25 additions & 1 deletion src/Servers/Kestrel/Core/src/Internal/TlsConnectionFeature.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.AspNetCore.Connections.Features;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core.Features;
using Microsoft.AspNetCore.Server.Kestrel.Https;

namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
{
Expand All @@ -25,6 +26,7 @@ internal class TlsConnectionFeature : ITlsConnectionFeature, ITlsApplicationProt
private int? _hashStrength;
private ExchangeAlgorithmType? _keyExchangeAlgorithm;
private int? _keyExchangeStrength;
private Task<X509Certificate2?>? _clientCertTask;

public TlsConnectionFeature(SslStream sslStream)
{
Expand All @@ -36,6 +38,8 @@ public TlsConnectionFeature(SslStream sslStream)
_sslStream = sslStream;
}

internal ClientCertificateMode ClientCertificateMode { get; set; }

public X509Certificate2? ClientCertificate
{
get
Expand Down Expand Up @@ -99,7 +103,27 @@ public int KeyExchangeStrength

public Task<X509Certificate2?> GetClientCertificateAsync(CancellationToken cancellationToken)
{
return Task.FromResult(ClientCertificate);
// Only try once per connection
if (_clientCertTask != null)
{
return _clientCertTask;
}

if (ClientCertificate != null
|| ClientCertificateMode != ClientCertificateMode.DelayCertificate
// Delayed client cert negotiation is not allowed on HTTP/2 (or HTTP/3, but that's implemented elsewhere).
|| _sslStream.NegotiatedApplicationProtocol == SslApplicationProtocol.Http2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a test to verify we aren't renegotiating when NegotiatedApplicationProtocol is HTTP/2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in the HTTP/2 HttpClient interop tests. That's one of the only places we test HTTP/2 over TLS.

{
return _clientCertTask = Task.FromResult(ClientCertificate);
}

return _clientCertTask = GetClientCertificateAsyncCore(cancellationToken);

async Task<X509Certificate2?> GetClientCertificateAsyncCore(CancellationToken cancellationToken)
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
{
await _sslStream.NegotiateClientCertificateAsync(cancellationToken);
return ClientCertificate;
}
}

private static X509Certificate2? ConvertToX509Certificate2(X509Certificate? certificate)
Expand Down
13 changes: 10 additions & 3 deletions src/Servers/Kestrel/Core/src/ListenOptionsHttpsExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
using System.IO;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Connections;
using Microsoft.AspNetCore.Server.Kestrel.Core;
using Microsoft.AspNetCore.Server.Kestrel.Https;
using Microsoft.AspNetCore.Server.Kestrel.Https.Internal;
Expand Down Expand Up @@ -258,10 +261,14 @@ public static ListenOptions UseHttps(this ListenOptions listenOptions, ServerOpt
{
// HttpsOptionsCallback is an internal delegate that is just the ServerOptionsSelectionCallback + a ConnectionContext parameter.
// Given that ConnectionContext will eventually be replaced by System.Net.Connections, it doesn't make much sense to make the HttpsOptionsCallback delegate public.
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
HttpsOptionsCallback adaptedCallback = (connection, stream, clientHelloInfo, state, cancellationToken) =>
serverOptionsSelectionCallback(stream, clientHelloInfo, state, cancellationToken);
return listenOptions.UseHttps(GetTlsOptionsAsync, state, handshakeTimeout);

return listenOptions.UseHttps(adaptedCallback, state, handshakeTimeout);
async ValueTask<(SslServerAuthenticationOptions, ClientCertificateMode)> GetTlsOptionsAsync(
ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken)
{
var tlsOptions = await serverOptionsSelectionCallback(stream, clientHelloInfo, state, cancellationToken);
return new (tlsOptions, ClientCertificateMode.DelayCertificate);
}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

namespace Microsoft.AspNetCore.Server.Kestrel.Https.Internal
{
internal delegate ValueTask<SslServerAuthenticationOptions> HttpsOptionsCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken);
internal delegate ValueTask<(SslServerAuthenticationOptions, ClientCertificateMode)> HttpsOptionsCallback(ConnectionContext connection, SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken);

internal class HttpsConnectionMiddleware
{
Expand Down Expand Up @@ -148,6 +148,8 @@ public async Task OnConnectionAsync(ConnectionContext context)
var sslStream = sslDuplexPipe.Stream;

var feature = new Core.Internal.TlsConnectionFeature(sslStream);
// Set the mode if options were used. If the callback is used it will set the mode later.
feature.ClientCertificateMode = _options?.ClientCertificateMode ?? ClientCertificateMode.NoCertificate;
context.Features.Set<ITlsConnectionFeature>(feature);
context.Features.Set<ITlsHandshakeFeature>(feature);
context.Features.Set<ITlsApplicationProtocolFeature>(feature);
Expand Down Expand Up @@ -321,7 +323,8 @@ private Task DoOptionsBasedHandshakeAsync(ConnectionContext context, SslStream s
ServerCertificate = _serverCertificate,
ServerCertificateContext = _serverCertificateContext,
ServerCertificateSelectionCallback = selector,
ClientCertificateRequired = _options.ClientCertificateMode != ClientCertificateMode.NoCertificate,
ClientCertificateRequired = _options.ClientCertificateMode == ClientCertificateMode.AllowCertificate
|| _options.ClientCertificateMode == ClientCertificateMode.RequireCertificate,
EnabledSslProtocols = _options.SslProtocols,
CertificateRevocationCheckMode = _options.CheckCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck,
};
Expand Down Expand Up @@ -424,7 +427,8 @@ private static async ValueTask<SslServerAuthenticationOptions> ServerOptionsCall
feature.HostName = clientHelloInfo.ServerName;
context.Features.Set(sslStream);

var sslOptions = await middleware._httpsOptionsCallback!(context, sslStream, clientHelloInfo, middleware._httpsOptionsCallbackState!, cancellationToken);
var (sslOptions, clientCertificateMode) = await middleware._httpsOptionsCallback!(context, sslStream, clientHelloInfo, middleware._httpsOptionsCallbackState!, cancellationToken);
feature.ClientCertificateMode = clientCertificateMode;

KestrelEventSource.Log.TlsHandshakeStart(context, sslOptions);

Expand Down
1 change: 1 addition & 0 deletions src/Servers/Kestrel/Core/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
*REMOVED*~static Microsoft.AspNetCore.Hosting.ListenOptionsHttpsExtensions.UseHttps(this Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions listenOptions, string fileName, string password) -> Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions
*REMOVED*~static Microsoft.AspNetCore.Hosting.ListenOptionsHttpsExtensions.UseHttps(this Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions listenOptions, string fileName, string password, System.Action<Microsoft.AspNetCore.Server.Kestrel.Https.HttpsConnectionAdapterOptions> configureOptions) -> Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions
*REMOVED*~static Microsoft.AspNetCore.Server.Kestrel.Https.CertificateLoader.LoadFromStoreCert(string subject, string storeName, System.Security.Cryptography.X509Certificates.StoreLocation storeLocation, bool allowInvalid) -> System.Security.Cryptography.X509Certificates.X509Certificate2
Microsoft.AspNetCore.Server.Kestrel.Https.ClientCertificateMode.DelayCertificate = 3 -> Microsoft.AspNetCore.Server.Kestrel.Https.ClientCertificateMode
static Microsoft.AspNetCore.Hosting.ListenOptionsHttpsExtensions.UseHttps(this Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions! listenOptions, Microsoft.AspNetCore.Server.Kestrel.Https.HttpsConnectionAdapterOptions! httpsOptions) -> Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions!
static Microsoft.AspNetCore.Hosting.ListenOptionsHttpsExtensions.UseHttps(this Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions! listenOptions, System.Action<Microsoft.AspNetCore.Server.Kestrel.Https.HttpsConnectionAdapterOptions!>! configureOptions) -> Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions!
static Microsoft.AspNetCore.Hosting.ListenOptionsHttpsExtensions.UseHttps(this Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions! listenOptions, System.Net.Security.ServerOptionsSelectionCallback! serverOptionsSelectionCallback, object! state) -> Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions!
Expand Down
Loading