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

CT enforcement seems incorrect #479

Open
Tracked by #1
briansmith opened this issue Feb 9, 2021 · 3 comments
Open
Tracked by #1

CT enforcement seems incorrect #479

briansmith opened this issue Feb 9, 2021 · 3 comments

Comments

@briansmith
Copy link
Contributor

In the code there is logic like this:

        // 3. Verify any included SCTs.
        match (self.server_cert.scts.as_ref(), sess.config.ct_logs) {
            (Some(scts), Some(logs)) => {
                verify::verify_scts(&self.server_cert.cert_chain[0], now, scts, logs)?;
            }
            (_, _) => {}
        }

If the server doesn't supply SCTs then Rustls doesn't enforce that the certificate has been logged, even if the user configured CT logs. Without additional APIs for controlling it at a finer-grained level, the user configuring CT logs should force Rustls to ensure every certificate has been logged, and if the server doesn't send any SCTs then Rustls shouldn't accept the certificate.

@briansmith
Copy link
Contributor Author

Further it should be possible to enforce CT for the client certificate, but Rustls only implements CT checks on the client side (for the server's certificate).

@ctz
Copy link
Member

ctz commented Feb 21, 2021

Back when I was initially doing this work, I found that support for SCT stapling in TLS was pretty patchy; compared to embedding the SCT list in a certificate extension. So rejecting a handshake because the SCT was inside the certificate rather than stapled seemed not very helpful.

@briansmith
Copy link
Contributor Author

Back when I was initially doing this work, I found that support for SCT stapling in TLS was pretty patchy; compared to embedding the SCT list in a certificate extension. So rejecting a handshake because the SCT was inside the certificate rather than stapled seemed not very helpful.

Right. That's why I think fixing this depends on fixing #480, which I've started to do in PR #526. The certificate verifier needs both the stapled SCTs (if any) and the SCTs in the certificate (if any). It also needs the list of trusted logs which isn't necessarily available to the application using Rustls; it might be managed within the operating system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants