chromium / badssl.com

:lock: Memorable site for testing clients against bad SSL configs.
https://badssl.com
Apache License 2.0
2.78k stars 186 forks source link

Provide a subdomain with a reversed certificate chain #443

Closed g-andrade closed 3 years ago

g-andrade commented 4 years ago

Problem

Disorderly certificate chains are a common mistake when setting up web servers and other kinds of TLS-protected services.

Common browsers are clever enough to walk such certificate chains.

However, a strict TLS implementation like Erlang/OTP's does not allow for certificate chains that are not hierarchically ordered without consumer-provided implementations to achieve it.

Providing a subdomain with a disorderly certificate chain would make testing such code easier.

Solution

This PR proposes the addition of a new subdomain, ~disorderly-chain.badssl.com~ reversed-chain.badssl.com, whose certificate chain is declared in reverse order, with the intermediate CA placed before the subdomain certificate.

This ensures that any TLS implementations testing their behaviour against the new subdomain need to know how to walk a ~disorderly~ reversed certificate chain.

Resolves #433 ; Closes #405 .

googlebot commented 4 years ago

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

g-andrade commented 4 years ago

I cherry-picked @omahabrad 's fix from PR #433 in order to make docker builds work, hence the CLA warning.

@omahabrad : do you consent to it? If you do, please leave a comment with "@googlebot I consent."

omahabrad commented 4 years ago

@googlebot I consent.

googlebot commented 4 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

g-andrade commented 4 years ago

This is fairly nitpicky, but might I recommend misordered-chain instead of disorderlychanin? The former is more specific, and might be less confusing to people who aren't as familiar with adjectives (rather than adverbs) that end in "-ly".

Fair enough :-) I was undecided between "disordered", "unordered" and "disorderly" - I missed "misordered" entirely.

In the case of a chain containing 2 certs, it would also be accurate to call it reversed-chain. If you want to test that a client can handle chains that don't go strictly in ancestor order or child order, an additional intermediate would be useful. I don't know if it's easy to get that from a real CA.

You're right; it's not much of a generic case with only two certificates up the chain, just a simple reversal. I'll look into more complex scenarios.

christhompson commented 4 years ago

+1 to "misordered-chain" wording for the multiple-intermediate case. Worst case we could make it test-only if we don't think we can get a publicly trusted certificate easily for this.

reversed-chain could use a normal certificate, but I seem to recall that nginx will complain if the leaf and intermediate are in the wrong order (key mismatch error). I haven't pulled this branch locally yet to test -- does this start up as expected?

christhompson commented 4 years ago

Also, if we add this (test-only or with a real cert if we can get one) I think it would resolve Issue #374

g-andrade commented 3 years ago

This is fairly nitpicky, but might I recommend misordered-chain instead of disorderly

Fixed on ab30874.

g-andrade commented 3 years ago

I haven't pulled this branch locally yet to test -- does this start up as expected?

Sorry for the unreasonably later answer - it does! I'm able to run make serve and load https://badssl.test locally, on the browser (and https://misorderly-chain.badssl.test as well.)

g-andrade commented 3 years ago

In the case of a chain containing 2 certs, it would also be accurate to call it reversed-chain. If you want to test that a client can handle chains that don't go strictly in ancestor order or child order, an additional intermediate would be useful. I don't know if it's easy to get that from a real CA.

A fair point; with that in mind, I've once more renamed the new subdomain - this time to reversed-chain.

Would it be good to merge as is? I'm very sorry for not having picked this PR up for quite some time - other things took priority since then.

paulo-ferraz-oliveira commented 3 years ago

Hi.

We could benefit from this change. Any chance it gets merged soon?

Thanks.

paulo-ferraz-oliveira commented 3 years ago

Bump.

lgarron commented 3 years ago

@christhompson I could merge, but I can't deploy. Do you think it would be worth setting up automatic deploys?

We could set up SSH or pull-based deploys using GitHub Actions fairly easily now, and either auto-deploy master or give project maintainers permissions to trigger a deploy.

christhompson commented 3 years ago

Sorry for the delay on this one. I can merge and get this deployed.

I also filed #462 to track automating deployment, since it's something I've been thinking about occasionally for a while now.

christhompson commented 3 years ago

Double-checking this today, it looks like this maybe isn't testing what we intended: the subdomain config is serving the reversed cert chain but currently using the BadSSL intermediate in the chain, and using the private key of the intermediate instead of a leaf cert. This means that it will trigger a NET::ERR_CERT_AUTHORITY_INVALID error in Chrome (and similar in Safari/Firefox).

If we fix this to build a chain using the main leaf cert (wildcard-main.crt) and the publicly-trusted intermediate, and then serve using the leaf-main.key private key, then nginx (well, OpenSSL under the hood) will refuse to start due to the cert/key mismatch:

nginx: [emerg] SSL_CTX_use_PrivateKey_file("/etc/keys/leaf-main.key") failed (SSL: error:0B080074:x509 certificate routines:X509_check_private_key:key values mismatch)

Which is good for nginx/openssl users in general, but bad for us trying to intentionally hold it wrong and serve a reversed cert chain. I did some spelunking in nginx and OpenSSL docs, but it doesn't look like there's an easy way to have it load the chain correctly but then serve it incorrectly.

lgarron commented 3 years ago

Which is good for nginx/openssl users in general, but bad for us trying to intentionally hold it wrong and serve a reversed cert chain. I did some spelunking in nginx and OpenSSL docs, but it doesn't look like there's an easy way to have it load the chain correctly but then serve it incorrectly.

Can we hand off a port to another process?

christhompson commented 3 years ago

Yeah, we could probably set up something else to handle this case -- did you have an alternative in mind?

christhompson commented 3 years ago

For now I've removed this from the dashboard and homepage to avoid confusion (#470).

g-andrade commented 3 years ago

Oh well, it is what it is...

But why does it work locally and not when deployed, since the Docker container is also running nginx?

I've just re-checked that it works locally.

Firstly I get the warning about an unknown issuer (as expected):

Screenshot at 2021-04-03 00-37-47

And once I bypass that, the page is correctly loaded:

Screenshot at 2021-04-03 00-38-01

(But it wasn't working for me in production, either.)

Is there some fundamental difference I'm missing between the production and local chains? (Aside from the issuer being known in the former.) [And sorry if this is a dumb question.]

christhompson commented 2 years ago

Not a dumb question: the reason this "works" in test is that nginx-includes/subdomain-reversed-chain.conf specifies

 ssl_certificate {{ site.cert-path }}/subdomain-reversed-chain.pem;
 ssl_certificate_key /etc/keys/ca-intermediate.key;

And in the test build we do have the CA intermediate key sitting around and so the config passes the key match test.