quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.85k stars 394 forks source link

Server Certificate verification does not verify certificate signed by intermediate CA #1203

Closed vlvicente closed 1 year ago

vlvicente commented 3 years ago

When setting a client connection with certificate verification, the server certificate is only trusted if it is signed by a root CA or if we load the intermediate CA which signed the server cert. Shouldn't the lib be able to verify the certificate as stated by the rustls crate:

You do not need to provide anything other than a set of root certificates to trust. Certificate verification cannot be turned off or disabled in the main API.

The error is:

16:14:16 [WARN] Sending fatal alert BadCertificate
ERROR: failed to connect: the cryptographic handshake failed: error 42: invalid certificate: UnknownIssuer

I've tested this behavior by loading a set of root CA and using the native-certificates set as default feature in the crate. In my example I'm using a certificate signed by Sectigo RSA Domain Validation Secure Server CA which was emmited by the root USERTrust RSA Certification Authority but quinn only verifies the certificate if I load the Sectigo certificate.

Note that when using TcpStream and rustls::Stream to make a simple https call, the certificate is verified without setting the intermediate CA.

djc commented 3 years ago

That is a surprising result. Which version of Quinn are you testing? What version of rustls are you using for your TCP test? Can you show us the code you use to configure the Quinn client?

You might also check this against current main, which is still on rustls 0.19 (like the release), but changed the way TLS gets configured in advance of the upgrade to rustls 0.20 which defers more of the configuration directly to rustls.

vlvicente commented 3 years ago

I'm using the quinn version 0.7.2 in my tests.
For the TCP test I used both rustls 0.18.0 and 0.20.0, both being able to stablish a tls connection without certificate errors.

Regarding the Quinn client configuration, I've tried these two approaches. The first one loading the firefox certstore in the "ca-bundle.crt" file and in the second one using the TLS_ROOT_SERVER certificates. Note that in both these examples, the native-certificates were being used by default.

Now, if I append the sectigo certificate to my "ca-bundle.crt" file, the first examples manages to verify the certificate and stablish a tls connection. 1.

pub fn set_client_config() -> ClientConfig {

    let mut cfg = ClientConfigBuilder::default();
    cfg.protocols(ALPN_QUICK_HTTP);

    let mut cfg = cfg.build();
    let roots = RootCertStore::empty();

    let ca_bundle = CACert::get("ca-bundle.crt").unwrap();
    let ca_bundle = ca_bundle.data.to_vec();
    let ca_bundle_pem = CertificateChain::from_pem(&ca_bundle).unwrap();

    let mut ca_bundle_iter = ca_bundle_pem.iter();
    for cert in ca_bundle_iter {
        let cert = cert.to_owned();
        let cert = Certificate::from(cert);
        cfg.add_certificate_authority(cert);
    }

    cfg
}

2.

pub fn set_client_config() -> ClientConfig {
    let mut cfg = ClientConfig::default();
    let tls_cfg =
        std::sync::Arc::get_mut(&mut cfg.crypto).unwrap();
    let protocol= b"hq-29".to_owned();
    let protocol = protocol.to_vec();
    tls_cfg.root_store.add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS);
    tls_cfg.set_protocols(&vec!(protocol));
    cfg
}

I'll check against the current main and let you know the results.

Edit (update): Tests with quinn main branch:

  1. using TLS_SERVER_ROOTS:

    pub fn set_client_config() -> ClientConfig {
    
    let mut cfg = ClientConfig::with_native_roots();
    let tls_cfg =
        std::sync::Arc::get_mut(&mut cfg.crypto).unwrap();
    
    tls_cfg.root_store.add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS);
    
    let mut cfg_builder = ClientConfigBuilder::new(cfg);
    
    cfg_builder.protocols(ALPN_QUICK_HTTP);
    
    cfg_builder.build()
    }
  2. Using only native-certificates

    pub fn set_client_config() -> ClientConfig {
     let mut cfg = ClientConfig::default();
     let tls_cfg =
         std::sync::Arc::get_mut(&mut cfg.crypto).unwrap();
     let protocol= b"hq-29".to_owned();
     let protocol = protocol.to_vec();
     tls_cfg.root_store.add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS);
     tls_cfg.set_protocols(&vec!(protocol));
     cfg
    }
  3. Loading firefox certificate store + TLS_SERVER_ROOTS + native-certificates

    pub fn set_client_config() -> ClientConfig {
    
    let ca_bundle = CACert::get("ca-bundle.crt").unwrap();
    let ca_bundle = ca_bundle.data.to_vec();
    let ca_bundle_pem = CertificateChain::from_pem(&ca_bundle).unwrap();
    
    let ca_bundle_iter = ca_bundle_pem.iter();
    
    let mut certs = Vec::new();
    for cert in ca_bundle_iter {
        let cert = cert.to_owned();
        let cert = Certificate::from(cert);
        certs.push(cert);
    }
    let mut cfg = ClientConfig::with_root_certificates(certs).unwrap();
    let tls_cfg =
        std::sync::Arc::get_mut(&mut cfg.crypto).unwrap();
    tls_cfg.root_store.add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS);
    
    let mut cfg_builder = ClientConfigBuilder::new(cfg);
    
    cfg_builder.protocols(ALPN_QUICK_HTTP);
    
    cfg_builder.build()
    }

All the examples above result in the same error, unless I add the sectigo certificate as stated before:

10:59:08 [WARN] Sending fatal alert BadCertificate
ERROR: failed to connect: the cryptographic handshake failed: error 42: invalid certificate: UnknownIssuer
Ralith commented 3 years ago

let mut cfg = ClientConfig::default();

ClientConfig::default doesn't exist on the main branch; it was removed in https://github.com/quinn-rs/quinn/pull/1194.

quinn leaves certificate verification entirely up to rustls, so there must be a difference in your rustls configuration. You can specify an arbitrary rustls configuration by constructing a ClientConfig directly (e.g. ClientConfig { transport: Default::default, crypto: your_rustls_config_here }). Try using the exact same rustls configuration that you've verified over TCP ~(but note that versions must contain only rustls::ProtocolVersion::TLSv1_3).~

Ralith commented 2 years ago

@vvicente-adyta are you still having issues with this?

vlvicente commented 1 year ago

Solved long ago... did not remember to close here... For what I remember the problem was in webpki lib and was solved in some version upgrade.