Make CERT_CORRECT actually correct.
CERT_CORRECT in X509Certificate2Test is not correct at all. It contains
a slew of encoding mistakes:
- BOOLEAN { FALSE } is not a valid parameter for dsa-with-sha1. Per
RFC3279, the parameters should be omitted.
- dsa-with-sha1 is not a valid SPKI OID. It's a signature OID. The DSA
SPKI OID is different and needs DSA parameters.
- The body of the SPKI is not a DSA public key.
- A GeneralName using the directoryName CHOICE arm should use explicit
tagging, not implicit tagging. This is a weird corner case of ASN.1
that a lot of folks get wrong.
- 0x01 is not a valid BOOLEAN in DER. The correct encoding of TRUE is
0xff.
- An authorityKeyIdentifier is a moderately complex structure, not a
BOOLEAN.
- A subjectKeyIdentifier is an OCTET STRING, not a BOOLEAN.
- A policyMappings is a moderately complex structure, not a BOOLEAN.
The test is checking that the basicConstraints extension is correctly
parsed, but that isn't a great test when the certificate is invalid.
X.509 is an extensible format, so which syntax errors are rejected
when is a complicated question. (E.g.,
https://github.com/google/conscrypt/issues/916). It happened that
Conscrypt did not reject the certificate today, but it also would have
been valid for Conscrypt to reject it. A recent issue in Conscrypt
coincidentally tripped this test bug. The Conscrypt fix will have a
unit test closer to the code, so we can fix this test cert.
This CL fixes the test certificate to test what it's actually aiming
to test. I've manually reworked the mistakes using
https://github.com/google/der-ascii and replaced the DSA OIDs with
ECDSA so that a correct key is a bit more compact. (The test only parses
basicConstraints, so it does not care.) The signature itself is still
not valid, but the test does not check signatures, so this is fine.
I've also applied the same fixes to CERT_TAMPERED, which appears to be a
slightly tweaked version of CERT_CORRECT. (Although CERT_TAMPERED
appears to have some intentional syntax errors that break the parse.)
Test: Treehugger
Bug: 171808928
Change-Id: Ide6b2e46f4eb172d33b266759a76ea063f5c989e
1 file changed