Implement user-defined certificate strings through torrc (part of the proposal 179 efforts)
Implement torrc options that allow users to specify their own values in certificate fields (like 'CN', 'C', 'ST', 'O', the lifetime of the cert., and the serial number).
This is not documented in proposal 179.
Activity
changed milestone to %Tor: unspecified
This is based on the same messed-up master as #4548 (moved) was, and is all mixed up with #4548 (moved) changes. Next time I'd prefer have something where it's easy to tell which actual commits I'm supposed to review. As near as I can tell here, it's supposed to be everything in bug4549 starting with fb8b31c3fb26d03.
Notes on review:
- wrt the tor_tls_set_custom_cert_strings interface: There is a great quote from Alan Perlis: "If you have a procedure with ten parameters, you probably missed some." These arguments probably want to turn into a single argument of type tor_cert_cfg_t, to be defined in tortls.h
- Customizeable start and end time and serial number: these are probably a mistake. Serial number needs to be different for every generated cert with the same issuer; start time and end time seem like they're begging you to have your Tor not work because you made your own cert expire. Instead let's go with certs that expire after a year or something; that seems to be the standard lifetime.
- tor_X509_name_new now has tons of copy-and-paste code. That's not so good; let's use a function or a macro to make it shorter.
- There's no need to use BN_pseudo_rand to get a random number; just use Tor's crypto_rand_int or whatever it's called.
- The documentation for tor_decorate_cert makes no sense to me. How does stuff "float around" ? What does it mean to "decorate" a cert? Which certs get decorated? Why is this a separate function?
- Looks like the covert-channel-in-the-serialnumber junk is still in here too; it'll need to come out.
Questions/ideas:
- It looks like you apply the issuer name stuff to be the name of EVERY issuer, and the subject name to be subject name on EVERY cert? That isn't right, if so. This stuff only wants to apply to the initial presented-in-cleartext cert, which now wants to be different from the cert chain presented after in the v2 or v3 handshake. The cert chain probably wants to stay unchanged.
- How have you tested this?
- Maybe we should have the ability to store these and the corresponding link keys in the datadir. If so, that should probably get done along with the "load a custom cert from disk" work.
Trac:
Parent: N/A to #3972 (moved)-
Replying to nickm:
- Customizeable start and end time and serial number: these are probably a mistake. Serial number needs to be different for every generated cert with the same issuer; start time and end time seem like they're begging you to have your Tor not work because you made your own cert expire. Instead let's go with certs that expire after a year or something; that seems to be the standard lifetime.
You think so? I see the validity options as a poor man's solution to #4390 (moved)#comment:1 and a much better way to do "start time fuzzing".
We can enforce correct use by doing appropriate checks. I'm already checking that
CertEndDate>CertStartDate. We can also check thatCertEndDateandCertStartDateare bigger than time(NULL). We can also add a LOG_WARN message in the startup that "the options you set are advanced options.".Of course, they should also be considered advanced options (as the rest of the TLSCert* options are). My point is that if at some point censors find a way to detect our serial numbers, and don't have a way to check that a relay's serial number has been the same, bridge operators can circumvent blocking by setting their own SNs. The same goes for validity times.
I don't see us losing much by implementing this, and we increase our versatility.
- tor_X509_name_new now has tons of copy-and-paste code. That's not so good; let's use a function or a macro to make it shorter.
- There's no need to use BN_pseudo_rand to get a random number; just use Tor's crypto_rand_int or whatever it's called.
- The documentation for tor_decorate_cert makes no sense to me. How does stuff "float around" ? What does it mean to "decorate" a cert? Which certs get decorated? Why is this a separate function?
- Looks like the covert-channel-in-the-serialnumber junk is still in here too; it'll need to come out.
Questions/ideas:
- It looks like you apply the issuer name stuff to be the name of EVERY issuer, and the subject name to be subject name on EVERY cert? That isn't right, if so. This stuff only wants to apply to the initial presented-in-cleartext cert, which now wants to be different from the cert chain presented after in the v2 or v3 handshake. The cert chain probably wants to stay unchanged.
I agree, I'll try to implement it properly.. I did it like this, because I saw the
cname,cname_signstuff getting applied to all certificates, and I didn't want to complicate the implementation.- How have you tested this?
I ran wireshark and checked out the TLS certificates. I also ran a local relay/client and made sure that nothing is screwed up in normal Tor operation. I'll do more checks in the latter scenario.
- Maybe we should have the ability to store these and the corresponding link keys in the datadir. If so, that should probably get done along with the "load a custom cert from disk" work.
Yes, this can be done along with #4550 (moved).
-
Replying to asn:
Replying to nickm:
- Customizeable start and end time and serial number: these are probably a mistake. Serial number needs to be different for every generated cert with the same issuer; start time and end time seem like they're begging you to have your Tor not work because you made your own cert expire. Instead let's go with certs that expire after a year or something; that seems to be the standard lifetime.
You think so?
I don't say stuff I don't think on trac tickets :)
I see the validity options as a poor man's solution to #4390 (moved)#comment:1 and a much better way to do "start time fuzzing".
We can get a much better solution by keeping certs around on disk, so we actually have start/end times that mean something.
We can enforce correct use by doing appropriate checks. I'm already checking that
CertEndDate>CertStartDate. We can also check thatCertEndDateandCertStartDateare bigger than time(NULL). We can also add a LOG_WARN message in the startup that "the options you set are advanced options.".Of course, they should also be considered advanced options (as the rest of the TLSCert* options are). My point is that if at some point censors find a way to detect our serial numbers, and don't have a way to check that a relay's serial number has been the same, bridge operators can circumvent blocking by setting their own SNs. The same goes for validity times.
Probably we should look into how self-signed certs' serial numbers are generated, and do ours like that instead. AFAICT, openssl's tools for making certs don't ask you for your own serial number: they instead just generate a random 64-bit value.
I don't see us losing much by implementing this, and we increase our versatility.
I do. It will make our certs follow a very weird pattern (changing every day, but having the same start and end time and serial number each time), and will make nodes suddenly stop working when the end date passes without people reconfiguring their Tor, and will force people to understand what an X509 serial number is.
Also, once we do have a real solution to this problem (probably involving storing the same cert to disk and using it longer-term), we'll still be stuck with the code we added to support these options indefinitely.
- It looks like you apply the issuer name stuff to be the name of EVERY issuer, and the subject name to be subject name on EVERY cert? That isn't right, if so. This stuff only wants to apply to the initial presented-in-cleartext cert, which now wants to be different from the cert chain presented after in the v2 or v3 handshake. The cert chain probably wants to stay unchanged.
I agree, I'll try to implement it properly.. I did it like this, because I saw the
cname,cname_signstuff getting applied to all certificates, and I didn't want to complicate the implementation.- How have you tested this?
I ran wireshark and checked out the TLS certificates. I also ran a local relay/client and made sure that nothing is screwed up in normal Tor operation. I'll do more checks in the latter scenario.
Okay. in the next round of tests, make sure that the 0.2.2 Tor clients and servers can interoperate with clients and servers running this patched code.
-
Replying to nickm:
- It looks like you apply the issuer name stuff to be the name of EVERY issuer, and the subject name to be subject name on EVERY cert? That isn't right, if so. This stuff only wants to apply to the initial presented-in-cleartext cert, which now wants to be different from the cert chain presented after in the v2 or v3 handshake. The cert chain probably wants to stay unchanged.
Hm, what about the v1 handshake? There, the 'presented-in-cleartext certificate' has to be passed along with the 'identity certificate'. This means that in the v1 case, we have to customize the 'identity certificate' with the user-defined certificate strings so that the certificate chain remains valid with:
cleartext_cert.issuerDNs == id_cert.issuerDNs == id_cert.subjectDNsI don't consider this necessarily wrong, since it seems logical from a PKI PoV.
If we do this, the 'link certificate' is equal to the 'presented-in-cleartext certificate', since we will also need to customize the 'link certificate' to form a valid certificate chain with the customized 'identity certificate' during the v2 renegotiation. This probably means that we don't need an extra 'presented-in-cleartext certificate' and we can simply customize our 'link certificate'.
If we don't want to do this, we will have to either kill the v1 handshake (we probably don't like this), or we will have to change the logic with which we create our certificate chains since OpenSSL will not present a non-valid certificate chain during the SSL handshake [0]. Both solutions seem much dirtier than customizing all the certificates that need to be customized.
I believe that if a user has provided certificate strings, we have to customize all our SSL context certificates so that they make sense from a PKI point of view (and so that all of our handshakes work correctly.). What do you think?
[0]: Instead of doing X509_STORE_add_cert() we will have to do SSL_CTX_add_extra_chain_cert(), so that the invalid cert. chain gets presented during v1. And in the case of a v2, we will have to dive into the guts of OpenSSL and remove the cert from the SSL context ourselves, before re-adding it in the renegotiation stage. Sounds very ugly. http://osdir.com/ml/encryption.openssl.cvs/2003-02/msg00032.html
-
Replying to asn:
Replying to nickm:
- It looks like you apply the issuer name stuff to be the name of EVERY issuer, and the subject name to be subject name on EVERY cert? That isn't right, if so. This stuff only wants to apply to the initial presented-in-cleartext cert, which now wants to be different from the cert chain presented after in the v2 or v3 handshake. The cert chain probably wants to stay unchanged.
Hm, what about the v1 handshake? There, the 'presented-in-cleartext certificate' has to be passed along with the 'identity certificate'. This means that in the v1 case, we have to customize the 'identity certificate' with the user-defined certificate strings so that the certificate chain remains valid with:
cleartext_cert.issuerDNs == id_cert.issuerDNs == id_cert.subjectDNsI don't consider this necessarily wrong, since it seems logical from a PKI PoV.
If we do this, the 'link certificate' is equal to the 'presented-in-cleartext certificate', since we will also need to customize the 'link certificate' to form a valid certificate chain with the customized 'identity certificate' during the v2 renegotiation. This probably means that we don't need an extra 'presented-in-cleartext certificate' and we can simply customize our 'link certificate'.
If we don't want to do this, we will have to either kill the v1 handshake (we probably don't like this), or we will have to change the logic with which we create our certificate chains since OpenSSL will not present a non-valid certificate chain during the SSL handshake [0]. Both solutions seem much dirtier than customizing all the certificates that need to be customized.
I believe that if a user has provided certificate strings, we have to customize all our SSL context certificates so that they make sense from a PKI point of view (and so that all of our handshakes work correctly.). What do you think?
Ah, okay. I think I'm convinced that all the certs probably need to have new names in them, if we're going to keep v1 alive (which we are for now). But part of my original point stands, though: when there's a 2-cert chain, we need to have the issuerDN of the link cert match with the subjectDN of the identity cert. I believe that your original patch did that oddly, and instead gave all certs the same issuerDN and all certs the same subjectDN.
So here's what I'd suggest:
Let the user configure an issuerDN "I" and a subjectDN "S".
When there's just one cert presented in plaintext, it should probably look self-signed. So it should have issuerDN=subjectDN=S. When there are two certificates, the link cert should have issuerDN=I and subjectDN=S, and the identity cert needs issuerDN=subjectDN=I.
Does that work out?
-
Replying to nickm:
So here's what I'd suggest:
Let the user configure an issuerDN "I" and a subjectDN "S".
When there's just one cert presented in plaintext, it should probably look self-signed. So it should have issuerDN=subjectDN=S. When there are two certificates, the link cert should have issuerDN=I and subjectDN=S, and the identity cert needs issuerDN=subjectDN=I.
Does that work out?
I think it's doable. It will require us to define two extra certificates. One 'self-signed presented-in-cleartext' certificate to be used in v2/v3, and one 'signed-by-the-identity-certificate presented-in-cleartext' certificate to be used in v1.
I'll try to do it today.
-
Nick, do you have any tips on testing the v2 handshake?
I'm trying to peak into the v2 handshake by using s_client(1SSL) and renegotiating, but I can't seem to be able to trigger the server sending me his certificate chain. I always get a single certificate out of the server during the renegotiation, and no certificate request.
My
s_clientcommand is:openssl s_client -cert ./le_cert.pem -cipher ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES256-SHA:DHE-DSS-AES256-SHA:ECDH-RSA-AES256-SHA:ECDH-ECDSA-AES256-SHA:AES256-SHA:ECDHE-ECDSA-RC4-SHA:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-RC4-SHA:ECDHE-RSA-AES128-SHA:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA:ECDH-RSA-RC4-SHA:ECDH-RSA-AES128-SHA:ECDH-ECDSA-RC4-SHA:ECDH-ECDSA-AES128-SHA:RC4-MD5:RC4-SHA:AES128-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:EDH-DSS-DES-CBC3-SHA:ECDH-RSA-DES-CBC3-SHA:ECDH-ECDSA-DES-CBC3-SHA:DES-CBC3-SHA -msg -showcerts -connect localhost:6666 -
At the time, all I had in mind was testing it with 0.2.2.x as a client and making sure it worked.
Does that s_client line work with 0.2.2.x as a server, btw? If so, there could be a problem, possibly related to #4587 (moved) .
-
Replying to nickm:
At the time, all I had in mind was testing it with 0.2.2.x as a client and making sure it worked.
It seems to work, but I would like to make sure that everything looks as expected inside the renegotiation.
Does that s_client line work with 0.2.2.x as a server, btw? If so, there could be a problem, possibly related to #4587 (moved) .
That
s_clientline does not work with any kind of a relay. I tried random relays off the consensus, and maint-0.2.2 as well. -
Found the bug of comment:11. Filed it at #4591 (moved).
-
Pushed my progress in branch
bug4549_progress.It seems to work with all link protocols, but I'll do more testing tomorrow.
I haven't done SIGHUP support yet and manual page documentation (both are already done in
bug4549.).I won't have too many hours to work on this tomorrow, but I'll try to test it and add SIGHUP-support and manual documentation.
-
Ugh. The "BUG BUG BUG BUG BUG BUG" thing makes me really nervous here. As you suspected the "too big" commit does indeed do a whole lot of stuff in a way that makes it hard to review. I will try to make sense of the last few commits here, and figure out whether to take this, but it might need to slip to 0.2.4 at this point unless it suddenly all looks great when I next examine it.
-
Replying to nickm:
Yeah, I'm really sorry, but this area of the code has gotten hairy enough, and this patch has gotten tricky enough, that I think we really need to postpone it to 0.2.4.x.
OK. I'll clean up the branch. I'll update this ticket when it's ready.
mentioned in issue #4550 (moved)
mentioned in issue #4587 (moved)
mentioned in issue #7145 (moved)
moved to tpo/core/tor#4549 (closed)