Don't include root certificates returned by Sectigo in CA bundle

[nlopez]

EDGE-2152 We began including the root certificate in the Sectigo chain on Feb 9, 2023, and this caused trouble for a customer's reverse proxy config (EDGES-11).

# Comparison of cert chains downloaded from Lemur for star.agent.datadoghq.com-2022-07-27 (sectigo-old)
❯ find sectigo-old -name "*.pem" | xargs -I {} openssl x509 -noout -issuer -subject -serial -in {}
issuer= /C=US/ST=New Jersey/L=Jersey City/O=The USERTRUST Network/CN=USERTrust RSA Certification Authority
subject= /C=GB/ST=Greater Manchester/L=Salford/O=Sectigo Limited/CN=Sectigo RSA Domain Validation Secure Server CA
serial=7D5B5126B476BA11DB74160BBC530DA7
issuer= /C=GB/ST=Greater Manchester/L=Salford/O=Comodo CA Limited/CN=AAA Certificate Services
subject= /C=US/ST=New Jersey/L=Jersey City/O=The USERTRUST Network/CN=USERTrust RSA Certification Authority
serial=3972443AF922B751D7D36C10DD313595

# versus SAN-WILDCARD.agent.datadoghq.com-SectigoRSADomainValidationSecureServerCA-20230209-20240209 (sectigo-new)
❯ find sectigo-new -name "*.pem" | xargs -I {} openssl x509 -noout -issuer -subject -serial -in {}
issuer= /C=US/ST=New Jersey/L=Jersey City/O=The USERTRUST Network/CN=USERTrust RSA Certification Authority
subject= /C=GB/ST=Greater Manchester/L=Salford/O=Sectigo Limited/CN=Sectigo RSA Domain Validation Secure Server CA
serial=7D5B5126B476BA11DB74160BBC530DA7
issuer= /C=GB/ST=Greater Manchester/L=Salford/O=Comodo CA Limited/CN=AAA Certificate Services
subject= /C=US/ST=New Jersey/L=Jersey City/O=The USERTRUST Network/CN=USERTrust RSA Certification Authority
serial=3972443AF922B751D7D36C10DD313595
issuer= /C=GB/ST=Greater Manchester/L=Salford/O=Comodo CA Limited/CN=AAA Certificate Services
subject= /C=GB/ST=Greater Manchester/L=Salford/O=Comodo CA Limited/CN=AAA Certificate Services
serial=01

Because we didn't previously include the root, I'm comfortable reverting to this configuration. In general, the server sending the root is optional, and clients are supposed to have roots in their trust store prior to connecting to the server (and specifically should not trust any root provided by the server itself.)

One of our customer's issue comes from the fact that when their MITM proxy re-signs certificates, it does so at the signature strength of the weakest signature algorithm in the chain. Even though our Sectigo cert chains up to a cert with a stronger sig alg than SHA1, the presence of the AAA Certificate Services root sent by the server takes precedence. This behavior is admittedly a weird edge case, but the combination of not including the root being:

1. the more correct behavior¹²
2. the behavior prior to Feb 9, 2023
3. less likely to cause issues with middleboxes

makes me want to make this change.

Tests updated to verify this:

❯ py.test -v lemur/plugins/lemur_sectigo
==================================================================================== test session starts ====================================================================================
platform darwin -- Python 3.9.9, pytest-7.2.0, pluggy-1.0.0 -- /Users/nick.lopez/src/github.com/DataDog/lemur/env/bin/python3
cachedir: .pytest_cache
rootdir: /Users/nick.lopez/src/github.com/DataDog/lemur, configfile: setup.cfg
plugins: mock-3.10.0, requests-mock-1.10.0, Faker-18.4.0, ddtrace-0.53.0, flask-1.2.0
collected 2 items

lemur/plugins/lemur_sectigo/tests/test_plugin.py::TestSectigoIssuerPlugin::test_create_certificate PASSED                                                                             [ 50%]
lemur/plugins/lemur_sectigo/tests/test_plugin.py::TestSectigoIssuerPlugin::test_determine_certificate_term PASSED                                                                     [100%]

===================================================================================== 2 passed in 0.94s =====================================================================================

Footnotes

1. https://security.stackexchange.com/a/260380 ↩

2. https://www.rfc-editor.org/rfc/rfc5246 the self-signed certificate that specifies the root certificate authority MAY be omitted from the chain, under the assumption that the remote end must already possess it in order to validate it in any case. ↩

[bobmshannon]

It's good this defensive check is in place otherwise it could inadvertently remove a valid intermediate certificate we would need if the API changes in the future or doesn't return a root certificate in some cases.

[bobmshannon]

This change seems fine, but let's make sure the customer understands the implications of this change before rolling this out. In particular, there may still be some edge cases where a client can anchor trust against AAA Certificate Services instead of the newer cross-signed root.

[nlopez]

Sounds good. I added some more detail in my latest internal reply to their ZD ticket.