search

apache / trafficcontrol

Apache Traffic Control is an Open Source implementation of a Content Delivery Network
https://trafficcontrol.apache.org/
Apache License 2.0
1.06k stars 342 forks source link

TO is mangling SSL server cert on update #3398

Closed jhowell-comcast closed 5 years ago

jhowell-comcast commented 5 years ago

Version: 3.0.0-9925.bd61e646.el7 When adding in a new server cert or updating an existing server cert, after pasting in the server cert + intermediate CA Chain into the server cert field and hitting update, the version number increments. TO logs show a POST action with a 200 response and no errors are thrown.

Leave the SSL page and come back to it for that DS, and the server cert field will only contain the server cert and last cert from the intermediate chain. The other certs from the intermediate chain have been removed.

Validated that this mangled cert is pushed to ATS servers and sometimes causes TR to do Null Pointer Exceptions when attempting to process the cert.

rawlinp commented 5 years ago

This might be due to the behavior of https://golang.org/pkg/crypto/x509/#Certificate.Verify:

Verify attempts to verify c by building one or more chains from c to a certificate in opts.Roots, using certificates in opts.Intermediates if needed. If successful, it returns one or more chains where the first element of the chain is c and the last element is from opts.Roots.

However, based on that wording, I'd expect it to possibly reorder/swap things around but always return a valid chain. Are the intermediates overlapping somehow?

jhowell-comcast commented 5 years ago

basically it's turning: 

Certificate[1]:
Owner: CN=speedtest.staging.xfinity.com, OU=Unified Communications, OU=Hosted by Comcast Corporation, OU=NSO, O=Comcast Corporation, STREET=1 Comcast Center, L=Philadelphia, ST=PA, OID.2.5.4.17=19103, C=US
Issuer: CN=COMODO RSA Organization Validation Secure Server CA, O=COMODO CA Limited, L=Salford, ST=Greater Manchester, C=GB
Valid from: Wed Feb 27 17:00:00 MST 2019 until: Sat Feb 27 16:59:59 MST 2021
  DNSName: speedtest.staging.xfinity.com
  DNSName: www.speedtest.staging.xfinity.com

Certificate[2]:
Owner: CN=COMODO RSA Organization Validation Secure Server CA, O=COMODO CA Limited, L=Salford, ST=Greater Manchester, C=GB
Issuer: CN=COMODO RSA Certification Authority, O=COMODO CA Limited, L=Salford, ST=Greater Manchester, C=GB
Valid from: Tue Feb 11 17:00:00 MST 2014 until: Sun Feb 11 16:59:59 MST 2029

Certificate[3]:
Owner: CN=COMODO RSA Certification Authority, O=COMODO CA Limited, L=Salford, ST=Greater Manchester, C=GB
Issuer: CN=AddTrust External CA Root, OU=AddTrust External TTP Network, O=AddTrust AB, C=SE
Valid from: Tue May 30 04:48:38 MDT 2000 until: Sat May 30 04:48:38 MDT 2020

Certificate[4]:
Owner: CN=AddTrust External CA Root, OU=AddTrust External TTP Network, O=AddTrust AB, C=SE
Issuer: CN=AddTrust External CA Root, OU=AddTrust External TTP Network, O=AddTrust AB, C=SE
Valid from: Tue May 30 04:48:38 MDT 2000 until: Sat May 30 04:48:38 MDT 2020

into:

Certificate[1]:
Owner: CN=speedtest.staging.xfinity.com, OU=Unified Communications, OU=Hosted by Comcast Corporation, OU=NSO, O=Comcast Corporation, STREET=1 Comcast Center, L=Philadelphia, ST=PA, OID.2.5.4.17=19103, C=US
Issuer: CN=COMODO RSA Organization Validation Secure Server CA, O=COMODO CA Limited, L=Salford, ST=Greater Manchester, C=GB
Valid from: Wed Feb 27 17:00:00 MST 2019 until: Sat Feb 27 16:59:59 MST 2021
  DNSName: speedtest.staging.xfinity.com
  DNSName: www.speedtest.staging.xfinity.com

Certificate[4]:
Owner: CN=AddTrust External CA Root, OU=AddTrust External TTP Network, O=AddTrust AB, C=SE
Issuer: CN=AddTrust External CA Root, OU=AddTrust External TTP Network, O=AddTrust AB, C=SE
Valid from: Tue May 30 04:48:38 MDT 2000 until: Sat May 30 04:48:38 MDT 2020
rob05c commented 5 years ago

I'm inclined to think we shouldn't be manipulating certs, that we should use them in the Router and Caches as the user gives them to us.

We should definitely warn the user, if they give us something we think is invalid or mis-ordered.

But it's always safer to not mangle things, unless we have an extremely high confidence that we're always right. I definitely don't have that confidence here, that the Go libraries always know the proper order (we've seen evidence that some clients require cert order backwards), or support all the valid cert types and algorithms.