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

SslStream.IsMutuallyAuthenticated returns wrong value when cached creds are used #65563

Closed
omghb opened this issue Feb 18, 2022 · 11 comments · Fixed by #79128 or #79601
Closed

SslStream.IsMutuallyAuthenticated returns wrong value when cached creds are used #65563

omghb opened this issue Feb 18, 2022 · 11 comments · Fixed by #79128 or #79601

Comments

@omghb
Copy link

omghb commented Feb 18, 2022

Description

We use SslStream to connect with a device which allows to be configured for mutual authentication with client certificates. For that we use AuthenticateAsClientAsync. After calling this method we check the IsMutuallyAuthenticated property the way it is mentioned in the docs:

...Check the IsMutuallyAuthenticated property to determine whether mutual authentication occurred.

see Remarks section

This works fine so far. But subsequent connect with authenticate to the same device (server) now results in that the property IsMutuallyAuthenticated returns true even when this is not correct anymore.

This wrong behavior comes from the caching which is done by the SslStream as it is mentioned in the docs:

Note:
The Framework caches SSL sessions as they are created and attempts to reuse a cached session for a new request, if possible. When attempting to reuse an SSL session, the Framework uses the first element of ClientCertificates (if there is one), or tries to reuse an anonymous sessions if ClientCertificates is empty.

see Remarks section

Reproduction Steps

  1. Connect to a server via SslStream which requires mutual authentication via client certificate.
  2. Call AuthenticateAsClientAsync and provide the client certificate
  3. Verify that IsMutuallyAuthenticated returns true
  4. Disconnect
  • Note: Keep the client running
  1. Restart and reconfigure the server so that it does not require a mutual authentication anymore
  2. Connect to a server via SslStream
  3. Call AuthenticateAsClientAsync and provide the client certificate
  4. Verify what IsMutuallyAuthenticated returns now (see Expected and Actual behavior)

Expected behavior

In step 8:
8. IsMutuallyAuthenticated should return false

Actual behavior

In step 8:
8. IsMutuallyAuthenticated returns true

Although a TLS 1.2 connection was established without mutual authentication.

Regression?

I can reproduce this issue with:

  • .NET 6
  • .NET 5
  • .NET Framework 4.8

Known Workarounds

This not-recommended workaround solves the issue:

  • Call the following method before starting connecting to the server.
private static void ClearCache()
{
    var sslAssembly = Assembly.GetAssembly(typeof(SslStream));
    var sslSessionCacheClass = sslAssembly.GetType("System.Net.Security.SslSessionsCache");
    var cachedCredsInfo = sslSessionCacheClass.GetField("s_cachedCreds", BindingFlags.NonPublic | BindingFlags.Static);
    var cachedCreds = cachedCredsInfo.GetValue(null);
    cachedCreds.GetType().GetMethod("Clear", BindingFlags.Public | BindingFlags.Instance).Invoke(cachedCreds, null);
}

Configuration

  • Windows 10 20H2

Other information

Maybe this is related: #65539

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net.Security untriaged New issue has not been triaged by the area owner labels Feb 18, 2022
@ghost
Copy link

ghost commented Feb 18, 2022

Tagging subscribers to this area: @dotnet/ncl, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

We use SslStream to connect with a device which allows to be configured for mutual authentication with client certificates. For that we use AuthenticateAsClientAsync. After calling this method we check the IsMutuallyAuthenticated property the way it is mentioned in the docs:

...Check the IsMutuallyAuthenticated property to determine whether mutual authentication occurred.

see Remarks section

This works fine so far. But subsequent connect with authenticate to the same device (server) now results in that the property IsMutuallyAuthenticated returns true even when this is not correct anymore.

This wrong behavior comes from the caching which is done by the SslStream as it is mentioned in the docs:

Note:
The Framework caches SSL sessions as they are created and attempts to reuse a cached session for a new request, if possible. When attempting to reuse an SSL session, the Framework uses the first element of ClientCertificates (if there is one), or tries to reuse an anonymous sessions if ClientCertificates is empty.

see Remarks section

Reproduction Steps

  1. Connect to a server via SslStream which requires mutual authentication via client certificate.
  2. Call AuthenticateAsClientAsync and provide the client certificate
  3. Verify that IsMutuallyAuthenticated returns true
  4. Disconnect
  • Note: Keep the client running
  1. Restart and reconfigure the server so that it does not require a mutual authentication anymore
  2. Connect to a server via SslStream
  3. Call AuthenticateAsClientAsync and provide the client certificate
  4. Verify what IsMutuallyAuthenticated returns now (see Expected and Actual behavior)

Expected behavior

In step 8:
8. IsMutuallyAuthenticated should return false

Actual behavior

In step 8:
8. IsMutuallyAuthenticated returns true

Although a TLS 1.2 connection was established without mutual authentication.

Regression?

I can reproduce this issue with:

  • .NET 6
  • .NET 5
  • .NET Framework 4.8

Known Workarounds

This not-recommended workaround solves the issue:

  • Call the following method before starting connecting to the server.
private static void ClearCache()
{
    var sslAssembly = Assembly.GetAssembly(typeof(SslStream));
    var sslSessionCacheClass = sslAssembly.GetType("System.Net.Security.SslSessionsCache");
    var cachedCredsInfo = sslSessionCacheClass.GetField("s_cachedCreds", BindingFlags.NonPublic | BindingFlags.Static);
    var cachedCreds = cachedCredsInfo.GetValue(null);
    cachedCreds.GetType().GetMethod("Clear", BindingFlags.Public | BindingFlags.Instance).Invoke(cachedCreds, null);
}

Configuration

No response

Other information

Maybe this is related: #65539

Author: omghb
Assignees: -
Labels:

area-System.Net.Security, untriaged

Milestone: -

@wfurt
Copy link
Member

wfurt commented Feb 18, 2022

I'm not sure this is truly wrong. If given session is created using TLS resume e.g. without full handshake it inherits everything from previous one. That may include peer's certificates. And therefore the authentication status reflects that - the authentication occured previously.

We should investigate @rzikm to fully understand this. If anything, we may expose API to prevent TLS resume in cases when client wants to use different certificate for each session.

@rzikm
Copy link
Member

rzikm commented Feb 18, 2022

I'm not sure this is truly wrong. If given session is created using TLS resume e.g. without full handshake it inherits everything from previous one. That may include peer's certificates. And therefore the authentication status reflects that - the authentication occured previously.

Although not directly relevant, is a use case where client uses the same certificate to connect to a different server relevant? Because if the second server does not request client auth we arrive at situation where client believes that the connection is mutually authenticated but server (correctly) assumes otherwise.

Could we get target endpoint/hostname into the key used in SSL session cache?

@wfurt
Copy link
Member

wfurt commented Feb 18, 2022

We probably can. But connecting to different TargetHost should prevent TLS resume. And if I remember right we look for certificate at given session, not credentials.

@rzikm
Copy link
Member

rzikm commented Feb 18, 2022

Sorry, I probably confused something. I remembered the PoC TLS resumption PR you had for Linux and somehow assumed this wasn't implemented yet on Windows either.

Regarding the issue here, I think if the second connection was established using a TLS Session Ticket (i.e. without full TLS handshake), then IsMutuallyAuthenticated should return the same value as at the end of previous session (both on server and client, I am not sure if we have a test for it right now). So if it is indeed the case here, then this is not a bug but a feature. We might want to consider API for disabling session resumption though, and possibly improve the documentation.

@wfurt
Copy link
Member

wfurt commented Feb 18, 2022

Windows can do it for very long time since the handshake is not handled in process and the decision depends on Schannel.
This also allows to possibly resume TLS across processes AFAIK.

@wfurt
Copy link
Member

wfurt commented Mar 3, 2022

Since you touch this recently, can you take a look @rzikm to see if there is work and what the impact is?

@rzikm
Copy link
Member

rzikm commented Mar 16, 2022

Looks to be real bug, I checked with wireshark and the there is no resumption in that case . It can be even reproduced in a single process by changing the server certificate:

Minimal repro
using System.Net;
using System.Net.Sockets;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;

X509Certificate2 clientCert = new X509Certificate2(@"C:\Users\radekzikmund\Downloads\TestDataCertificates\testclienteku.contoso.com.pfx", "PLACEHOLDER");
X509Certificate2 serverCert = new X509Certificate2(@"C:\Users\radekzikmund\Downloads\TestDataCertificates\testservereku.contoso.com.pfx", "PLACEHOLDER");
X509Certificate2 serverCert2 = new X509Certificate2(@"C:\Users\radekzikmund\Downloads\TestDataCertificates\testselfsignedservereku.contoso.com.pfx", "PLACEHOLDER");

TcpListener listener = new TcpListener(new IPEndPoint(IPAddress.Loopback, 0));
listener.Start();

Console.WriteLine(listener.Server.LocalEndPoint);
async Task Run(X509Certificate2 clientCert, X509Certificate2 serverCert, bool requireClientCert)
{
    Console.WriteLine($"clientCert: {clientCert.Subject}");
    Console.WriteLine($"serverCert: {serverCert.Subject}");
    Console.WriteLine($"requireClientCert: {requireClientCert}");
    TcpClient tcpClient = new TcpClient();
    TcpClient tcpServer;

    {
        var clientConnect = tcpClient.ConnectAsync((IPEndPoint)listener.LocalEndpoint);
        tcpServer = await listener.AcceptTcpClientAsync();
        await clientConnect;
    }

    using SslStream client = new SslStream(tcpClient.GetStream());
    using SslStream server = new SslStream(tcpServer.GetStream());

    var clientAuthTask = client.AuthenticateAsClientAsync(new SslClientAuthenticationOptions
    {
        TargetHost = "SomeTargetHost",
        ClientCertificates = new X509Certificate2Collection(clientCert),
        RemoteCertificateValidationCallback = delegate { return true; },
        EnabledSslProtocols = System.Security.Authentication.SslProtocols.Tls12
    });

    var serverAuthTask = server.AuthenticateAsServerAsync(new SslServerAuthenticationOptions
    {
        ClientCertificateRequired = requireClientCert,
        ServerCertificate = serverCert,
        RemoteCertificateValidationCallback = delegate { return true; },
        EnabledSslProtocols = System.Security.Authentication.SslProtocols.Tls12
    });

    await Task.WhenAll(clientAuthTask, serverAuthTask);

    Console.WriteLine($"client.IsMutuallyAuthenticated: {client.IsMutuallyAuthenticated}");
    Console.WriteLine($"server.IsMutuallyAuthenticated: {server.IsMutuallyAuthenticated}");
    Console.WriteLine();
}

await Run(clientCert, serverCert, true);
await Run(clientCert, serverCert2, false);

listener.Stop();

Example output:

127.0.0.1:64043
clientCert: CN=testclienteku.contoso.com
serverCert: CN=testservereku.contoso.com
requireClientCert: True
client.IsMutuallyAuthenticated: True
server.IsMutuallyAuthenticated: True

clientCert: CN=testclienteku.contoso.com
serverCert: CN=testselfsignedservereku.contoso.com
requireClientCert: False
client.IsMutuallyAuthenticated: True
server.IsMutuallyAuthenticated: False

It seems to me that we need to somehow detect if the client actually sent the certificate. Perhaps by querying the security context after the handshake finishes? And, if needed, clear the SecureChannel._selectedClientCertificate.

Note that the issue affects all platforms, not only Windows.

@karelz karelz added bug and removed untriaged New issue has not been triaged by the area owner labels Mar 31, 2022
@karelz
Copy link
Member

karelz commented Mar 31, 2022

Triage:

  • It's a bug
  • Narrow scenario - requires remote server to be restarted with different configuration.

@karelz karelz added this to the Future milestone Mar 31, 2022
@omghb
Copy link
Author

omghb commented May 16, 2022

  • Narrow scenario - requires remote server to be restarted with different configuration.

@karelz: This is a common scenario for us. The remote server is an embedded device that can be configured by the customer to require a client certificate. Additionally, the customer can disable the require client certificate feature via hardware reset.

We run into this issue when our client software (Windows, .NET 6) keeps running while the “require client certificate” feature gets disabled. When we re-connect to the device the IsMutuallyAuthenticated property returns true and we use this information to inform the user about the security status. Unfortunately, this information is wrong because of the caching in .NET.

@wfurt wfurt modified the milestones: Future, 8.0.0 Nov 16, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Dec 2, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Dec 13, 2022
@wfurt
Copy link
Member

wfurt commented Dec 13, 2022

reopening for Android

@wfurt wfurt reopened this Dec 13, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Dec 13, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Dec 16, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.