Support intermediate CAs

[pquentin]

I tried to address #3.

It does not seem to work, though. Test with:

import trustme

ca = trustme.CA()
ca = ca.create_child_ca()
server_cert = ca.issue_server_cert(u"test-host.example.org")

server_cert.private_key_and_cert_chain_pem.write_to_path("server.pem")

When commenting the ca = ca.create_child_ca() line, openssl verify -untrusted server.pem server.pem says:

server.pem: O = "trustme v0.4.0+dev", OU = Testing server cert #XukT8k24PBXQBSqv
error 20 at 0 depth lookup:unable to get local issuer certificate

But with the create_child_ca line, we get:

server.pem: O = "trustme v0.4.0+dev", OU = Testing CA #D2vixR6eI2g5N2k5
error 19 at 1 depth lookup:self signed certificate in certificate chain

I have no idea what I'm doing: what's the problem? I did try to make
sure to only self-sign the root CA and not including it in the
server.pem file.

[codecov]

Codecov Report

Merging #30 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #30   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines         255    275   +20     
  Branches       14     18    +4     
=====================================
+ Hits          255    275   +20

Flag	Coverage Δ
#linux	100% <100%> (ø)	⬆️
#windows	100% <100%> (ø)	⬆️

Impacted Files	Coverage Δ
tests/test_trustme.py	100% <100%> (ø)	⬆️
trustme/__init__.py	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5e3db4...6a634cf. Read the comment docs.

[njsmith]

(Also needs tests of course.)

[njsmith]

I think here you want self._private_key, not sign_key, because this fills in the parts of the cert that say "this is the key that I use to sign things"?

[pquentin]

@njsmith Thanks! That was the issue. I'll make sure to add tests, yes, thanks for the reminder.

[pquentin]

(I've pushed my progress here, but I still need to dig in the test failures, so it's not worth reviewing as the last commit will contain obvious mistakes.)

[pquentin]

So I don't really know how to debug this. The certificate chain seems to be valid. To check, use this new test script:

import trustme

ca = trustme.CA()
ca = ca.create_child_ca()
server_cert = ca.issue_server_cert(u"test-host.example.org")

ca.cert_pem.write_to_path("ca.pem")
server_cert.private_key_and_cert_chain_pem.write_to_path("server.pem")

Running python3 t.py && openssl verify -CAfile ca.pem server.pem gives:

server.pem: OK

Yay!

However the test client from the end-to-end test still says: ssl.SSLError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:719), even in the stdlib case where SSLContext.load_cert_chain() accepts certificates like our server.pem above.

Any advice on a general strategy to debug this test failure?

[njsmith]

@pquentin I'm afraid I do not know of any general strategy. If we really get stuck on something, my general strategy will probably be "beg Alex Gaynor or Paul Kehrer for help".

In this case though I tried doing some general poking around to make sure things looked basically reasonable, and noticed something odd. When I run your script, I get out ca.pem and server.pem files. ca.pem has a single certificate, and server.pem has a private key + 2 certificates, as expected. But... it looks like one of the certificates in server.pem matches the certificate in ca.pem, which I think is not expected? If you do diff -u ca.pem server.pem it doesn't show any deleted lines, just added lines.

(Actually I didn't try the diff -u trick first. The first thing I tried was running

openssl x509 -in ca.pem -text
openssl x509 -in server.pem -text

and I noticed that the random name that trustme uses for the CA seemed to match, which seemed weird since the server cert is supposed to be issued by the intermediate CA, not the root CA...)

...and actually now I'm looking at the test script again, and it doesn't look quite right :-). It throws away the root CA object, and write out the intermediate CA as ca.pem... so here's a new version of t.py:

import trustme

root_ca = trustme.CA()
child_ca = root_ca.create_child_ca()
server_cert = child_ca.issue_server_cert(u"test-host.example.org")

root_ca.cert_pem.write_to_path("ca.pem")
server_cert.private_key_and_cert_chain_pem.write_to_path("server.pem")

Now I get:

~/trustme$ openssl verify -CAfile ca.pem server.pem
O = "trustme v0.4.0+dev", OU = Testing server cert #_iL9SOq4pQhLuTD7
error 20 at 0 depth lookup: unable to get local issuer certificate
error server.pem: verification failed

Annoyingly, openssl x509 -text doesn't show the whole cert chain, just the first cert... if we take the second cert blob out of server.pem and put it in its own file, then I see (among a bunch of other stuff):

        Issuer: O = "trustme v0.4.0+dev", OU = Testing CA #f7LCUmRUgXwqgdR0
        Validity
            Not Before: Jan  1 00:00:00 2000 GMT
            Not After : Jan  1 00:00:00 3000 GMT
        Subject: O = "trustme v0.4.0+dev", OU = Testing CA #f7LCUmRUgXwqgdR0

So it looks like the intermediate CA cert is self-signed, instead of being signed by the root CA?

[pquentin]

Thank you @njsmith, this is incredibly useful, as always! Thanks for sharing your process, too: I now realize that I should have tried those things.

[pquentin]

@njsmith So the intermediate CA was signed by the root CA, but the issuer name was incorrectly set to the intermediate CA subject name. It turns out that this was the actual issue in the tests, which are now fixed!

I also added a commit that adds required extensions for CAs, as noted in https://security.stackexchange.com/a/176602/182464 (I created an account there to upvote the answer.)

However I still get this with the fixed t.py script:

server.pem: O = "trustme v0.4.0+dev", OU = Testing CA #ppQXPPpyekdUC4Rn
error 2 at 1 depth lookup:unable to get issuer certificate

I'll have to find out why.

[njsmith]

Did a bit of reading on this, and found: "This extension MUST be present and MUST be marked critical. Bit positions for keyCertSign and cRLSign MUST be set. If the Private Key is used for signing OCSP responses, then the digitalSignature bit MUST be set." – https://cabforum.org/baseline-requirements-certificate-contents/

So I think we should have crl_sign=True?

[pquentin]

Thanks, done. (The openssl issue remains, I still need to look into it.)

[njsmith]

Pasting from private chat:

ughhh
I think it's just openssl verify being super annoying
try this: openssl verify -CAfile ca.pem -untrusted server.pem server.pem
I think what that means is: "using ca.pem as a source of trusted CA certificates, and using server.pem as a source of untrusted intermediate certificates, verify the first certificate you find in server.pem"
i.e. openssl verify doesn't actually read a chain out of the file you give it, even though that's how ever other piece of software deals with chains

I get:

~/trustme$ openssl verify -CAfile ca.pem -untrusted server.pem server.pem
server.pem: OK

Or even better, let's add -show_chain:

~/trustme$ openssl verify -show_chain -CAfile ca.pem -untrusted server.pem server.pem 
server.pem: OK
Chain:
depth=0: O = "trustme v0.4.0+dev", OU = Testing server cert #ZaGiS-6N0KCjy_BK (untrusted)
depth=1: O = "trustme v0.4.0+dev", OU = Testing CA #qEzIbceq27OcWW52 (untrusted)
depth=2: O = "trustme v0.4.0+dev", OU = Testing CA #f-18wqEgBKhTGN_k

[njsmith]

Okay since this seems to actually be working now, let me do a more nitpicky review!

The actual code looks fine to me.

• Needs a news entry. Actually, probably needs 2 news entries: one for the intermediate cert support, and one for the new keyUsage stuff.

• Do you want to include a convenient way to create a server cert with a missing intermediate? (Could be a separate PR, but IIUC that's what you actually want but it seems... actually kind of non-trivial with trustme's API currently. Maybe broken_server_cert = working_server_cert.with_intermediates_missing()?)

[njsmith]

This test feels a bit trivial... maybe that's OK, it's just a smoke test? But maybe we should at least check a few basic attributes, like that server_cert's chain has two entries, the second one matches the child ca's chain, that sort of thing?

[njsmith]

(And thank you for your patience working through all this nonsense!)

[pquentin]

Thank you, @njsmith, for your patience and for clearing all those roadblocks for me.

openssl verify -CAfile ca.pem -untrusted server.pem -show_chain -purpose sslserver -verify_hostname test-host.example.org server.pem is an even better check, because it checks the extensions (basic constraints, key usage and extended key usage) and hostname.

I added more checks, made sure to decrease the path length, and added the news entries. Can you improve the wording of the docstring and news entries? And anything else you'd like to improve.

[pquentin]

(Oh and I'd prefer to leave broken_server_cert = working_server_cert.with_intermediates_missing() for another PR: I need to remove Python 2.6 support from urllib3 before I can depend on trustme there.)

[njsmith]

Looks good to me!

I'd prefer to leave broken_server_cert = working_server_cert.with_intermediates_missing() for another PR

makes sense

[pquentin]

Maybe this comment will avoid me losing half an hour the next time I try to understand why -show_chain does not work on my machine: it requires OpenSSL 1.1!

With Homebrew, you need to explicitly install it with brew install openssl@1.1, and then use something like echo 'export PATH="/usr/local/opt/openssl@1.1/bin:$PATH"' >> ~/.zshrc because Homebrew refuses to override OpenSSL from macOS so brew link openssl@1.1 won't work.