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

DisableCertificateDownloads & AIA behaviour #59979

Closed
shaan1337 opened this issue Oct 5, 2021 · 19 comments
Closed

DisableCertificateDownloads & AIA behaviour #59979

shaan1337 opened this issue Oct 5, 2021 · 19 comments
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Security
Milestone

Comments

@shaan1337
Copy link

shaan1337 commented Oct 5, 2021

Description

I'm seeing the following behaviour on both windows & linux with the following configuration and wanted to confirm if it's expected.

Configuration:

  • Chain: Root CA -> Intermediate CA -> Leaf certificate
  • Leaf certificate with a valid Authority Information Access (AIA) URL pointing to the intermediate CA certificate (on a local test web server)
  • DisableCertificateDownloads = true in the chain policy
  • CustomTrustStore: Contains root CA
  • ExtraStore: Can contain the intermediate CA or not - the behaviour below is the same

Behaviour:

  • The AIA URL is always fetched from the webserver (unless it's in the cache on Windows)

  • The server needs to wait for the AIA certificate to be downloaded before sending the Server Hello. If there's any significant network delay and the HTTP client's timeout is low enough (~2 secs), the connection setup times out (tested on both windows & linux). The only way to prevent this from happening seems to be:

    • i) add the certificate manually to the user's store
    • ii) to remove the AIA entry from the certificate
    • iii) increase the http client timeout.
  • The downloaded AIA certificate is not used for validation (except if DisableCertificateDownloads = false)

  • On Windows, it's cached but not added to the certificate store (with DisableCertificateDownloads = false, it's added under Intermediate Certification Authorities)

  • On Linux, it's not cached and not added to the certificate store (with DisableCertificateDownloads = false, it's added under ~/.dotnet/corefx/cryptography/x509stores/ca)

  • Setting URLRetrievalTimeout to 1ms doesn't seem to have any effect on the AIA retrieval.

  1. The overall behaviour of DisableCertificateDownloads = true seems to be: "download the certificates but don't keep them in the certificate store and don't use them for validation".

  2. The overall behaviour of DisableCertificateDownloads = false seems to be: "download the certificates (if they are not already in the store (and not in the cert cache on windows)), keep them in the certificate store and use them for validation if any intermediate is missing from the chain".

The behaviour I initially expected was that the certificate should not be downloaded at all if DisableCertificateDownloads = true.

Configuration

  • Which version of .NET is the code running on?
    5.0.201

  • What OS and version, and what distro if applicable?
    Windows 10 & Ubuntu 18.04

  • What is the architecture (x64, x86, ARM, ARM64)?
    x64

Regression?

No

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Oct 5, 2021
@ghost
Copy link

ghost commented Oct 5, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

I'm seeing the following behaviour on both windows & linux with the following configuration and wanted to confirm if it's expected.

Configuration:

  • Chain: Root CA -> Intermediate CA -> Leaf certificate
  • Leaf certificate with a valid Authority Information Access (AIA) URL pointing to the intermediate CA certificate (on a local test web server)
  • DisableCertificateDownloads = true in the chain policy
  • CustomTrustStore: Contains root CA
  • ExtraStore: Can contain the intermediate CA or not - the behaviour below is the same

Behaviour:

  • The AIA URL is always fetched from the webserver (unless it's in the cache on Windows)
  • On Windows, it's cached but not added to the certificate store (with DisableCertificateDownloads = false, it's added under Intermediate Certification Authorities)
  • On Linux, it's not cached and not added to the certificate store (with DisableCertificateDownloads = false, it's added under ~/.dotnet/corefx/cryptography/x509stores/ca)
  • The downloaded AIA certificate is not used for validation (except if DisableCertificateDownloads = false)
  1. The overall behaviour of DisableCertificateDownloads seems to be: "download the certificates but don't keep them in the certificate store and don't use them for validation". Is it expected?

  2. Since the AIA URL is always downloaded (unless it's in the cache on Windows), any delay in fetching the certificate stalls the connection setup time and if the HTTP client's timeout is low enough, the connection setup times out (tested on both windows & linux). The only way to prevent this from happening seems to be: i) to remove the AIA entry from the certificate or ii) increase the http client timeout.

Configuration

  • Which version of .NET is the code running on?
    5.0.201

  • What OS and version, and what distro if applicable?
    Windows 10 & Ubuntu 18.04

  • What is the architecture (x64, x86, ARM, ARM64)?
    x64

Regression?

No

Author: shaan1337
Assignees: -
Labels:

area-System.Security, untriaged

Milestone: -

@bartonjs
Copy link
Member

bartonjs commented Oct 5, 2021

The AIA URL is always fetched from the webserver (unless it's in the cache on Windows)

How are you measuring that? Unless we've had a serious regression that isn't the case, per our test rig.

@bartonjs
Copy link
Member

bartonjs commented Oct 5, 2021

Also, how are you performing your setup? X509Chain directly? SslStream? HttpClient? It may be that there's more than one chain walk involved, and we're missing some propagation of DisableCertificateDownloads.

@shaan1337
Copy link
Author

shaan1337 commented Oct 5, 2021

@bartonjs thanks for the quick response.

How are you measuring that? Unless we've had a serious regression that isn't the case, per our test rig.

By tailing access logs of the web server hosting the certificate in the AIA URL. For clearing the windows cache, i'm using certutil -URLCache <AIA url to delete> delete

Also, how are you performing your setup? X509Chain directly? SslStream? HttpClient? It may be that there's more than one chain walk involved, and we're missing some propagation of DisableCertificateDownloads.

It's X509Chain directly (the validation function is called via a RemoteCertificateValidationCallback in the http client's socket http handler). Sample code:

public static ValueTuple<bool, string> ValidateServerCertificateWithTrustedRootCerts(X509Certificate certificate,
    X509Chain chain, SslPolicyErrors sslPolicyErrors, Func<X509Certificate2Collection> trustedRootCertsSelector) {
    return ValidateCertificateWithTrustedRootCerts(certificate, chain, sslPolicyErrors, trustedRootCertsSelector, "server");
}

public static ValueTuple<bool, string> ValidateClientCertificateWithTrustedRootCerts(X509Certificate certificate,
    X509Chain chain, SslPolicyErrors sslPolicyErrors, Func<X509Certificate2Collection> trustedRootCertsSelector) {
    return ValidateCertificateWithTrustedRootCerts(certificate, chain, sslPolicyErrors, trustedRootCertsSelector, "client");
}

private static ValueTuple<bool, string> ValidateCertificateWithTrustedRootCerts(X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors, Func<X509Certificate2Collection> trustedRootCertsSelector, string certificateOrigin) {
    if (certificate == null)
        return (false, $"No certificate was provided by the {certificateOrigin}");

    var newChain = new X509Chain {
        ChainPolicy = {
            RevocationMode = X509RevocationMode.NoCheck,
            TrustMode = X509ChainTrustMode.CustomRootTrust,
            DisableCertificateDownloads = true
        }
    };

    var trustedRootCerts = trustedRootCertsSelector();
    if (trustedRootCerts != null) {
        foreach (var cert in trustedRootCerts) {
            if (cert.IssuerName.Name == cert.SubjectName.Name) {
                newChain.ChainPolicy.CustomTrustStore.Add(cert); //root
            } else {
                newChain.ChainPolicy.ExtraStore.Add(cert); //intermediate
            }
        }
    }

    newChain.Build(new X509Certificate2(certificate));
    var chainStatus = X509ChainStatusFlags.NoError;
    foreach (var status in newChain.ChainStatus) {
        chainStatus |= status.Status;
    }

    if (chainStatus == X509ChainStatusFlags.NoError)
        sslPolicyErrors &= ~SslPolicyErrors.RemoteCertificateChainErrors; //clear the RemoteCertificateChainErrors flag
    else
        sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors; //set the RemoteCertificateChainErrors flag

    if (sslPolicyErrors != SslPolicyErrors.None) {
        return (false, $"The certificate ({certificate.Subject}) provided by the {certificateOrigin} failed validation with the following error(s): {sslPolicyErrors.ToString()} ({chainStatus})");
    }

    return (true, null);
}

Http handler code:

var socketsHttpHandler = new SocketsHttpHandler {
    SslOptions = {
        RemoteCertificateValidationCallback = (sender, certificate, chain, errors) => {
            var (isValid, error) = serverCertValidator(certificate, chain, errors);
            if (!isValid && error != null) {
                Log.Error("Server certificate validation error: {e}", error);
            }

            return isValid;
        },
        LocalCertificateSelectionCallback = delegate {
            return clientCertificateSelector();
        }
    },
    PooledConnectionLifetime = ESConsts.HttpClientConnectionLifeTime
};

serverCertValidator points to ValidateServerCertificateWithTrustedRootCerts(cert, chain, errors, _trustedRootCertsSelector) in the above snippet and _trustedRootCertsSelector just points to an x509 certificate collection.

@bartonjs
Copy link
Member

bartonjs commented Oct 5, 2021

It's X509Chain directly (the validation function is called via a RemoteCertificateValidationCallback in the http client's socket http handler).

Ah. So, not X509Chain directly 😄.

By the time you're calling X509Chain there, it was already called by SslStream, so that it could pass in the prebuilt chain and know if the SslPolicyErrors value should contain the RemoteCertificateChainErrors bit, or not.

It sort of sounds like we might need options on SslStream/HttpClient to let you say "I'm going to build the chain myself in the callback... don't do anything for it".... though we'd need to figure out how name validation figures into that flow.

@vcsjones
Copy link
Member

vcsjones commented Oct 5, 2021

By the time you're calling X509Chain there, it was already called by SslStream, so that it could pass in the prebuilt chain and know if the SslPolicyErrors value should contain the RemoteCertificateChainErrors bit, or not.

I think the thing to do today is to specify offline: true when creating your own SslStreamCertificateContext.Create. The context can be set on SslServerAuthenticationOptions.ServerCertificateContext.

@bartonjs
Copy link
Member

bartonjs commented Oct 5, 2021

I think the thing to do today is to specify offline: true when creating your own SslStreamCertificateContext.Create.

I believe that only applies for building the context around the server identity cert in server mode. I don't think it applies for a client verifying the server (or a server verifying a client auth cert), and therefore that it doesn't extend out to "when used by HttpClient", since that's always a TLS client role.

@shaan1337
Copy link
Author

By the time you're calling X509Chain there, it was already called by SslStream, so that it could pass in the prebuilt chain and know if the SslPolicyErrors value should contain the RemoteCertificateChainErrors bit, or not.

Ohh okay, that makes complete sense now thanks! 😄 I always wondered how it was possible that it was passing SslPolicyErrors without doing a prior chain validation and finally thought maybe it was only doing other checks like name mismatch, etc.

Yup the dontValidateChain option would be great. I guess the same applies to CRL checks then with X509RevocationMode.NoCheck? But I've not tested it.

The main issue around it is that it slows down the connection setup time, so a validateChainOffline option could also work.

I think the thing to do today is to specify offline: true when creating your own SslStreamCertificateContext.Create.

Thanks, I'll have a look at the APIs to confirm if it applies to the http client as well.

@vcsjones
Copy link
Member

vcsjones commented Oct 6, 2021

@shaan1337

I don't think it applies for a client verifying the server (or a server verifying a client auth cert), and therefore that it doesn't extend out to "when used by HttpClient", since that's always a TLS client role.

@bartonjs might be right here, it may not give you exactly what you're looking for, either.

@shaan1337
Copy link
Author

shaan1337 commented Oct 6, 2021

after a peek at the code, it looks like revocation check settings are overridden here with the setting from _sslAuthenticationOptions to avoid doing revocation checks during the first chain validation

chain.ChainPolicy.RevocationMode = _sslAuthenticationOptions.CertificateRevocationCheckMode;

So, if a DisableCertificateDownloads setting is added to SslAuthenticationOptions, something similar could be done here

it would fix it for HttpClient but i'm not sure about other use cases

@bartonjs bartonjs added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Security and removed area-System.Security untriaged New issue has not been triaged by the area owner labels Oct 7, 2021
@ghost
Copy link

ghost commented Oct 7, 2021

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

Issue Details

Description

I'm seeing the following behaviour on both windows & linux with the following configuration and wanted to confirm if it's expected.

Configuration:

  • Chain: Root CA -> Intermediate CA -> Leaf certificate
  • Leaf certificate with a valid Authority Information Access (AIA) URL pointing to the intermediate CA certificate (on a local test web server)
  • DisableCertificateDownloads = true in the chain policy
  • CustomTrustStore: Contains root CA
  • ExtraStore: Can contain the intermediate CA or not - the behaviour below is the same

Behaviour:

  • The AIA URL is always fetched from the webserver (unless it's in the cache on Windows)

  • The server needs to wait for the AIA certificate to be downloaded before sending the Server Hello. If there's any significant network delay and the HTTP client's timeout is low enough (~2 secs), the connection setup times out (tested on both windows & linux). The only way to prevent this from happening seems to be:

    • i) add the certificate manually to the user's store
    • ii) to remove the AIA entry from the certificate
    • iii) increase the http client timeout.
  • The downloaded AIA certificate is not used for validation (except if DisableCertificateDownloads = false)

  • On Windows, it's cached but not added to the certificate store (with DisableCertificateDownloads = false, it's added under Intermediate Certification Authorities)

  • On Linux, it's not cached and not added to the certificate store (with DisableCertificateDownloads = false, it's added under ~/.dotnet/corefx/cryptography/x509stores/ca)

  • Setting URLRetrievalTimeout to 1ms doesn't seem to have any effect on the AIA retrieval.

  1. The overall behaviour of DisableCertificateDownloads = true seems to be: "download the certificates but don't keep them in the certificate store and don't use them for validation".

  2. The overall behaviour of DisableCertificateDownloads = false seems to be: "download the certificates (if they are not already in the store (and not in the cert cache on windows)), keep them in the certificate store and use them for validation if any intermediate is missing from the chain".

The behaviour I initially expected was that the certificate should not be downloaded at all if DisableCertificateDownloads = true.

Configuration

  • Which version of .NET is the code running on?
    5.0.201

  • What OS and version, and what distro if applicable?
    Windows 10 & Ubuntu 18.04

  • What is the architecture (x64, x86, ARM, ARM64)?
    x64

Regression?

No

Author: shaan1337
Assignees: -
Labels:

api-suggestion, area-System.Net.Security

Milestone: -

@bartonjs
Copy link
Member

bartonjs commented Oct 7, 2021

Reassigned to System.Net.Security since this is about SslStream (and HttpClient) not having a way of disabling the chain build, or setting DisableCertificatedDownloads, during a remote certificate verification.

@wfurt
Copy link
Member

wfurt commented Oct 7, 2021

This is conceptually similar to #40423 and #35839.
I'm wondering it it would be better / more flexible to add while X509Chain to the SslOptions @bartonjs.
With that, users can set what ever policy and setting chain allows (or will in the future)
I worry that adding just bool may not be always sufficient.

@bartonjs
Copy link
Member

bartonjs commented Oct 7, 2021

Assuming you mean X509ChainPolicy, the problems are:

  1. If the caller provides a policy and doesn't ask for the right EKUs is that their problem, or ours?
  2. X509ChainPolicy doesn't (currently) have a notion of "use 'now' when you evaluate the chain", the verification time is the object creation time or whatever explicit value was set... so depending on the lifetime of the object it will start accepting certs that should be expired and/or rejecting certs that were too recently issued.
  3. What opinions do we want to have regarding post-call mutation?

One option would be to accept a policy as input, but make a copy of it. In that copy we can insert-if-not-present the appropriate EKU type... or only do that if it's empty... or whatever. We could also then set the validation time to be "now" right before calling the chain build each time; and/or teach X509ChainPolicy that DateTime.MinValue should always be treated as "now" when evaluating the chain.

I think I sort of like the copy+fixup... it'd also make it easy to specify custom roots, without needing to write the callback.

@wfurt
Copy link
Member

wfurt commented May 12, 2022

We already have split for Client & Server options. Since this is advanced use I would feel OK to let user choose and provide good example in API docs. We conceptually have three choices:

  • accept what user provides without further check. It feels unlikely client cert would have correct server name and vice versa so any misconfiguration is likely to fail anyway IMHO.
  • add them automatically based on the option type. (or do that only if not set)
  • throw InvalidParameter if missing not set correctly.

as far as 3: we already have issue like this with certificates (and possibly other objects). It is also not clear if that would thread safe. That could be also possibly clarified explicitly.

I don't have any strong feeling about the NOW. It feels like it would be nice to reuse the object without need to copy/create it over and over. But we already do copy for other options so I think it would be ok - certainly for now.

It would work for this and #59944. ChainPolicy was also suggested on #40423. While we may not bypass the validation completely, disabling AIA would go long way IMHO.

namespace System.Net.Security
{
    public class SslServerAuthenticationOptions
    {
        ....
+       X509ChainPolicy? ValidationPolicy;
    }
    public class SslClientAuthenticationOptions
    {
        ....
+       X509ChainPolicy? ValidationPolicy;
    }
}

SocketsHttpHandler has SslClientAuthenticationOptions exposed. Generic HttpClientHandler does not, but I don't see easy way around it.
For Kestrel, @davidfowl asked for Cancelation in #35839 but ability to set timeout may suffice.

@MihaZupan MihaZupan added the untriaged New issue has not been triaged by the area owner label Jun 6, 2022
@karelz
Copy link
Member

karelz commented Jun 7, 2022

Triage: We had number of similar reports and related problems. Would be nice to solve it in 7.0 if possible.

@karelz karelz added this to the 7.0.0 milestone Jun 7, 2022
@karelz karelz removed the untriaged New issue has not been triaged by the area owner label Jun 7, 2022
@davidfowl
Copy link
Member

Related #35839

@wfurt
Copy link
Member

wfurt commented Jun 7, 2022

yes. This will not let you cancel but X509ChainPolicy allows you to set timeout.

@wfurt
Copy link
Member

wfurt commented Jul 15, 2022

Client and server now have option to pass in X509ChainPolicy via CertificateChainPolicy in ssl options. If present, chain constructed inside SslStream will respect the setting including DisableCertificateDownloads.

@wfurt wfurt closed this as completed Jul 15, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Security
Projects
None yet
Development

No branches or pull requests

7 participants