From 57d01415199b8fe2fb693f145bfbebd8a9f7323a Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 26 Mar 2018 13:30:57 -0400 Subject: [PATCH] cert-checker: Ignore OCSP Must Staple certlint errs. (#3598) The upstream `certlint` package doesn't understand the RFC 7633 OCSP Must Staple PKIX Extension and flags its presence as an error. Until this is resolved upstream this commit updates `cmd/cert-checker` to ignore the error. The `TestCheckCert` unit test is updated to add an unsupported extension and the OCSP must staple extension to its test cert. Only the unsupported extension should be flagged as a problem. --- cmd/cert-checker/main.go | 16 +++++++++++++--- cmd/cert-checker/main_test.go | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go index 16681a8ee..4f517e03f 100644 --- a/cmd/cert-checker/main.go +++ b/cmd/cert-checker/main.go @@ -36,6 +36,9 @@ const ( good = "valid" bad = "invalid" + certlintCNError = "commonName field is deprecated" + certlintOCSPMustStapleError = "Certificate contains unknown extension (1.3.6.1.5.5.7.1.24)" + filenameLayout = "20060102" expectedValidityPeriod = time.Hour * 24 * 90 @@ -215,10 +218,17 @@ func (c *certChecker) checkCert(cert core.Certificate) (problems []string) { // would have if we omitted CommonName). There have been proposals at // CA/Browser Forum for an alternate contentless field whose purpose would // just be to make Subject non-empty, but so far they have not been - // successful. - if err.Error() != "commonName field is deprecated" { - problems = append(problems, err.Error()) + // successful. If the check error is `certlintCNError`, ignore it. + if err.Error() == certlintCNError { + continue } + // Certlint doesn't presently understand the RFC 7633 OCSP Must Staple + // extension. While this is unaddressed in the upstream library we ignore + // this error like we ignore `certlintCNError`. + if err.Error() == certlintOCSPMustStapleError { + continue + } + problems = append(problems, err.Error()) } } diff --git a/cmd/cert-checker/main_test.go b/cmd/cert-checker/main_test.go index d7150e115..74494e3b8 100644 --- a/cmd/cert-checker/main_test.go +++ b/cmd/cert-checker/main_test.go @@ -5,6 +5,7 @@ import ( "crypto/rsa" "crypto/x509" "crypto/x509/pkix" + "encoding/asn1" "fmt" "log" "math/big" @@ -141,6 +142,21 @@ func TestCheckCert(t *testing.T) { checker := newChecker(saDbMap, fc, pa, expectedValidityPeriod) + // Create a RFC 7633 OCSP Must Staple Extension. + // OID 1.3.6.1.5.5.7.1.24 + ocspMustStaple := pkix.Extension{ + Id: asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24}, + Critical: false, + Value: []uint8{0x30, 0x3, 0x2, 0x1, 0x5}, + } + + // Create a made up PKIX extension + imaginaryExtension := pkix.Extension{ + Id: asn1.ObjectIdentifier{1, 3, 3, 7}, + Critical: false, + Value: []uint8{0xC0, 0xFF, 0xEE}, + } + issued := checker.clock.Now().Add(-time.Hour * 24 * 45) goodExpiry := issued.Add(expectedValidityPeriod) serial := big.NewInt(1337) @@ -158,6 +174,7 @@ func TestCheckCert(t *testing.T) { KeyUsage: x509.KeyUsageDigitalSignature, OCSPServer: []string{"http://example.com/ocsp"}, IssuingCertificateURL: []string{"http://example.com/cert"}, + ExtraExtensions: []pkix.Extension{ocspMustStaple, imaginaryExtension}, } brokenCertDer, err := x509.CreateCertificate(rand.Reader, &rawCert, &rawCert, &testKey.PublicKey, testKey) test.AssertNotError(t, err, "Couldn't create certificate") @@ -186,6 +203,7 @@ func TestCheckCert(t *testing.T) { "Certificate has common name >64 characters long (65)": 1, "Policy Authority isn't willing to issue for '*.foodnotbombs.mil': Wildcard names not supported": 1, "commonName exeeding max lenght of 64": 1, + "Certificate contains unknown extension (1.3.3.7)": 1, } for _, p := range problems { _, ok := problemsMap[p] @@ -214,6 +232,7 @@ func TestCheckCert(t *testing.T) { rawCert.DNSNames = []string{"example-a.com"} rawCert.NotAfter = goodExpiry rawCert.BasicConstraintsValid = true + rawCert.ExtraExtensions = []pkix.Extension{ocspMustStaple} rawCert.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth} goodCertDer, err := x509.CreateCertificate(rand.Reader, &rawCert, &rawCert, &testKey.PublicKey, testKey) test.AssertNotError(t, err, "Couldn't create certificate")