diff --git a/cmd/ceremony/cert.go b/cmd/ceremony/cert.go index ec6a59d11..44c33231a 100644 --- a/cmd/ceremony/cert.go +++ b/cmd/ceremony/cert.go @@ -336,7 +336,7 @@ func makeTemplate(randReader io.Reader, profile *certProfile, pubKey []byte, ct type failReader struct{} func (fr *failReader) Read([]byte) (int, error) { - return 0, errors.New("Empty reader used by x509.CreateCertificate") + return 0, errors.New("empty reader used by x509.CreateCertificate") } func generateCSR(profile *certProfile, signer crypto.Signer) ([]byte, error) { diff --git a/cmd/ceremony/crl.go b/cmd/ceremony/crl.go index fec17b3fd..7127c44a4 100644 --- a/cmd/ceremony/crl.go +++ b/cmd/ceremony/crl.go @@ -3,15 +3,18 @@ package notmain import ( "crypto" "crypto/x509" - "crypto/x509/pkix" "encoding/pem" "errors" + "fmt" "math/big" "time" + + "github.com/letsencrypt/boulder/crl/crl_x509" + "github.com/letsencrypt/boulder/linter" ) -func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nextUpdate time.Time, number int64, revokedCertificates []pkix.RevokedCertificate) ([]byte, error) { - template := &x509.RevocationList{ +func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nextUpdate time.Time, number int64, revokedCertificates []crl_x509.RevokedCertificate) ([]byte, error) { + template := &crl_x509.RevocationList{ RevokedCertificates: revokedCertificates, Number: big.NewInt(number), ThisUpdate: thisUpdate, @@ -33,12 +36,26 @@ func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nex return nil, errors.New("nextUpdate must be less than 12 months after thisUpdate") } + err := linter.CheckCRL(template, issuer, signer, []string{ + // We skip this lint because our ceremony tooling issues CRLs with validity + // periods up to 12 months, but the lint only allows up to 10 days (which + // is the limit for CRLs containing Subscriber Certificates). + "e_crl_validity_period", + // We skip this lint because it is only applicable for sharded/partitioned + // CRLs, which our Subscriber CRLs are, but our higher-level CRLs issued by + // this tool are not. + "e_crl_has_idp", + }) + if err != nil { + return nil, fmt.Errorf("crl failed pre-issuance lint: %w", err) + } + // x509.CreateRevocationList uses an io.Reader here for signing methods that require // a source of randomness. Since PKCS#11 based signing generates needed randomness // at the HSM we don't need to pass a real reader. Instead of passing a nil reader // we use one that always returns errors in case the internal usage of this reader // changes. - crlBytes, err := x509.CreateRevocationList(&failReader{}, template, issuer, signer) + crlBytes, err := crl_x509.CreateRevocationList(&failReader{}, template, issuer, signer) if err != nil { return nil, err } diff --git a/cmd/ceremony/crl_test.go b/cmd/ceremony/crl_test.go index 9da7a7d52..dac20a203 100644 --- a/cmd/ceremony/crl_test.go +++ b/cmd/ceremony/crl_test.go @@ -14,34 +14,33 @@ import ( "testing" "time" + "github.com/letsencrypt/boulder/crl/crl_x509" "github.com/letsencrypt/boulder/test" ) func TestGenerateCRLTimeBounds(t *testing.T) { - _, err := generateCRL(nil, nil, time.Time{}.Add(time.Hour), time.Time{}, 1, nil) + _, err := generateCRL(nil, nil, time.Now().Add(time.Hour), time.Now(), 1, nil) test.AssertError(t, err, "generateCRL did not fail") test.AssertEquals(t, err.Error(), "thisUpdate must be before nextUpdate") _, err = generateCRL(nil, &x509.Certificate{ - NotBefore: time.Time{}.Add(time.Hour), - NotAfter: time.Time{}, - }, time.Time{}, time.Time{}, 1, nil) + NotBefore: time.Now().Add(time.Hour), + NotAfter: time.Now(), + }, time.Now(), time.Now(), 1, nil) test.AssertError(t, err, "generateCRL did not fail") test.AssertEquals(t, err.Error(), "thisUpdate is before issuing certificate's notBefore") _, err = generateCRL(nil, &x509.Certificate{ - NotBefore: time.Time{}, - NotAfter: time.Time{}.Add(time.Hour * 2), - }, time.Time{}.Add(time.Hour), time.Time{}.Add(time.Hour*3), 1, nil) + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour * 2), + }, time.Now().Add(time.Hour), time.Now().Add(time.Hour*3), 1, nil) test.AssertError(t, err, "generateCRL did not fail") test.AssertEquals(t, err.Error(), "nextUpdate is after issuing certificate's notAfter") -} -func TestGenerateCRLLength(t *testing.T) { - _, err := generateCRL(nil, &x509.Certificate{ - NotBefore: time.Time{}, - NotAfter: time.Time{}.Add(time.Hour * 24 * 366), - }, time.Time{}, time.Time{}.Add(time.Hour*24*366), 1, nil) + _, err = generateCRL(nil, &x509.Certificate{ + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour * 24 * 370), + }, time.Now(), time.Now().Add(time.Hour*24*366), 1, nil) test.AssertError(t, err, "generateCRL did not fail") test.AssertEquals(t, err.Error(), "nextUpdate must be less than 12 months after thisUpdate") } @@ -62,17 +61,58 @@ func (p wrappedSigner) Public() crypto.PublicKey { return p.k.Public() } +func TestGenerateCRLLints(t *testing.T) { + k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + test.AssertNotError(t, err, "failed to generate test key") + + cert := &x509.Certificate{ + Subject: pkix.Name{CommonName: "asd"}, + SerialNumber: big.NewInt(7), + NotBefore: time.Now(), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + IsCA: true, + KeyUsage: x509.KeyUsageCRLSign, + SubjectKeyId: []byte{1, 2, 3}, + } + + certBytes, err := x509.CreateCertificate(rand.Reader, cert, cert, k.Public(), k) + test.AssertNotError(t, err, "failed to generate test cert") + cert, err = x509.ParseCertificate(certBytes) + test.AssertNotError(t, err, "failed to parse test cert") + + // This CRL should fail the following lints: + // - e_crl_has_idp (because our ceremony CRLs don't have the IDP extension) + // - e_crl_validity_period (because our ceremony CRLs are valid for a long time) + // - e_crl_acceptable_reason_codes (because 6 is forbidden) + // However, only the last of those should show up in the error message, + // because the first two should be explicitly removed from the lint registry + // by the ceremony tool. + six := 6 + _, err = generateCRL(&wrappedSigner{k}, cert, time.Now().Add(time.Hour), time.Now().Add(100*24*time.Hour), 1, []crl_x509.RevokedCertificate{ + { + SerialNumber: big.NewInt(12345), + ReasonCode: &six, + }, + }) + test.AssertError(t, err, "generateCRL did not fail") + test.AssertNotContains(t, err.Error(), "e_crl_has_idp") + test.AssertNotContains(t, err.Error(), "e_crl_validity_period") + test.AssertContains(t, err.Error(), "e_crl_acceptable_reason_codes") +} + func TestGenerateCRL(t *testing.T) { k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) test.AssertNotError(t, err, "failed to generate test key") template := &x509.Certificate{ - Subject: pkix.Name{CommonName: "asd"}, - SerialNumber: big.NewInt(7), - NotBefore: time.Time{}, - NotAfter: time.Time{}.Add(time.Hour * 3), - KeyUsage: x509.KeyUsageCRLSign, - SubjectKeyId: []byte{1, 2, 3}, + Subject: pkix.Name{CommonName: "asd"}, + SerialNumber: big.NewInt(7), + NotBefore: time.Now(), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCRLSign, + SubjectKeyId: []byte{1, 2, 3}, } certBytes, err := x509.CreateCertificate(rand.Reader, template, template, k.Public(), k) @@ -80,18 +120,18 @@ func TestGenerateCRL(t *testing.T) { cert, err := x509.ParseCertificate(certBytes) test.AssertNotError(t, err, "failed to parse test cert") - crlPEM, err := generateCRL(&wrappedSigner{k}, cert, time.Time{}.Add(time.Hour), time.Time{}.Add(time.Hour*2), 1, nil) + crlPEM, err := generateCRL(&wrappedSigner{k}, cert, time.Now().Add(time.Hour), time.Now().Add(time.Hour*2), 1, nil) test.AssertNotError(t, err, "generateCRL failed with valid profile") pemBlock, _ := pem.Decode(crlPEM) crlDER := pemBlock.Bytes // use crypto/x509 to check signature is valid and list is empty - goCRL, err := x509.ParseCRL(crlDER) + goCRL, err := x509.ParseRevocationList(crlDER) test.AssertNotError(t, err, "failed to parse CRL") - err = cert.CheckCRLSignature(goCRL) + err = goCRL.CheckSignatureFrom(cert) test.AssertNotError(t, err, "CRL signature check failed") - test.AssertEquals(t, len(goCRL.TBSCertList.RevokedCertificates), 0) + test.AssertEquals(t, len(goCRL.RevokedCertificates), 0) // fully parse the CRL to check that the version is correct, and that // it contains the CRL number extension containing the number we expect diff --git a/cmd/ceremony/main.go b/cmd/ceremony/main.go index 95800fabf..303d6e3ad 100644 --- a/cmd/ceremony/main.go +++ b/cmd/ceremony/main.go @@ -15,6 +15,7 @@ import ( "time" "github.com/letsencrypt/boulder/cmd" + "github.com/letsencrypt/boulder/crl/crl_x509" "github.com/letsencrypt/boulder/linter" "github.com/letsencrypt/boulder/pkcs11helpers" "github.com/letsencrypt/boulder/strictyaml" @@ -721,7 +722,7 @@ func crlCeremony(configBytes []byte) error { return fmt.Errorf("unable to parse crl-profile.next-update: %s", err) } - var revokedCertificates []pkix.RevokedCertificate + var revokedCertificates []crl_x509.RevokedCertificate for _, rc := range config.CRLProfile.RevokedCertificates { cert, err := loadCert(rc.CertificatePath) if err != nil { @@ -731,7 +732,7 @@ func crlCeremony(configBytes []byte) error { if err != nil { return fmt.Errorf("unable to parse crl-profile.revoked-certificates.revocation-date") } - revokedCert := pkix.RevokedCertificate{ + revokedCert := crl_x509.RevokedCertificate{ SerialNumber: cert.SerialNumber, RevocationTime: revokedAt, } diff --git a/cmd/crl-checker/main.go b/cmd/crl-checker/main.go index 3ec6c965b..d43f0037b 100644 --- a/cmd/crl-checker/main.go +++ b/cmd/crl-checker/main.go @@ -100,7 +100,14 @@ func main() { totalBytes += len(crl.Raw) - err = checker.Validate(crl, issuer, ageLimit) + zcrl, err := crl_x509.ParseRevocationList(crl.Raw) + if err != nil { + errCount += 1 + logger.Errf("parsing CRL %q failed: %s", u, err) + continue + } + + err = checker.Validate(zcrl, issuer, ageLimit) if err != nil { errCount += 1 logger.Errf("checking CRL %q failed: %s", u, err) diff --git a/crl/checker/checker.go b/crl/checker/checker.go index ac8858529..ad43506bd 100644 --- a/crl/checker/checker.go +++ b/crl/checker/checker.go @@ -8,9 +8,11 @@ import ( "sort" "time" + zlint_x509 "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3" + "github.com/letsencrypt/boulder/crl/crl_x509" "github.com/letsencrypt/boulder/linter" - crlint "github.com/letsencrypt/boulder/linter/lints/crl" ) // Validate runs the given CRL through our set of lints, ensures its signature @@ -18,7 +20,12 @@ import ( // less than ageLimit old. It returns an error if any of these conditions are // not met. func Validate(crl *crl_x509.RevocationList, issuer *x509.Certificate, ageLimit time.Duration) error { - err := linter.ProcessResultSet(crlint.LintCRL(crl)) + zcrl, err := zlint_x509.ParseRevocationList(crl.Raw) + if err != nil { + return fmt.Errorf("parsing CRL: %w", err) + } + + err = linter.ProcessResultSet(zlint.LintRevocationList(zcrl)) if err != nil { return fmt.Errorf("linting CRL: %w", err) } diff --git a/crl/checker/checker_test.go b/crl/checker/checker_test.go index 62380bc90..66918b0da 100644 --- a/crl/checker/checker_test.go +++ b/crl/checker/checker_test.go @@ -39,9 +39,15 @@ func TestValidate(t *testing.T) { test.AssertError(t, err, "validating crl from wrong issuer") test.AssertContains(t, err.Error(), "signature") - crl.Number = nil + crlFile, err = os.Open("../../linter/lints/cabf_br/testdata/crl_long_validity.pem") + test.AssertNotError(t, err, "opening test crl file") + crlPEM, err = io.ReadAll(crlFile) + test.AssertNotError(t, err, "reading test crl file") + crlDER, _ = pem.Decode(crlPEM) + crl, err = crl_x509.ParseRevocationList(crlDER.Bytes) + test.AssertNotError(t, err, "parsing test crl") err = Validate(crl, issuer, 100*365*24*time.Hour) - test.AssertError(t, err, "validaint crl with lint error") + test.AssertError(t, err, "validating crl with lint error") test.AssertContains(t, err.Error(), "linting") } diff --git a/linter/linter.go b/linter/linter.go index 7310ef9d4..22d535ef1 100644 --- a/linter/linter.go +++ b/linter/linter.go @@ -14,10 +14,11 @@ import ( "github.com/zmap/zlint/v3/lint" "github.com/letsencrypt/boulder/crl/crl_x509" - crllints "github.com/letsencrypt/boulder/linter/lints/crl" + _ "github.com/letsencrypt/boulder/linter/lints/cabf_br" _ "github.com/letsencrypt/boulder/linter/lints/chrome" _ "github.com/letsencrypt/boulder/linter/lints/cpcps" + _ "github.com/letsencrypt/boulder/linter/lints/rfc" ) var ErrLinting = fmt.Errorf("failed lint(s)") @@ -37,6 +38,15 @@ func Check(tbs *x509.Certificate, subjectPubKey crypto.PublicKey, realIssuer *x5 return err } +// CheckCRL is like Check, but for CRLs. +func CheckCRL(tbs *crl_x509.RevocationList, realIssuer *x509.Certificate, realSigner crypto.Signer, skipLints []string) error { + linter, err := New(realIssuer, realSigner, skipLints) + if err != nil { + return err + } + return linter.CheckCRL(tbs) +} + // Linter is capable of linting a to-be-signed (TBS) certificate. It does so by // signing that certificate with a throwaway private key and a fake issuer whose // public key matches the throwaway private key, and then running the resulting @@ -93,7 +103,7 @@ func (l Linter) CheckCRL(tbs *crl_x509.RevocationList) error { if err != nil { return err } - lintRes := crllints.LintCRL(crl) + lintRes := zlint.LintRevocationListEx(crl, l.registry) return ProcessResultSet(lintRes) } @@ -209,12 +219,12 @@ func ProcessResultSet(lintRes *zlint.ResultSet) error { return nil } -func makeLintCRL(tbs *crl_x509.RevocationList, issuer *x509.Certificate, signer crypto.Signer) (*crl_x509.RevocationList, error) { +func makeLintCRL(tbs *crl_x509.RevocationList, issuer *x509.Certificate, signer crypto.Signer) (*zlintx509.RevocationList, error) { lintCRLBytes, err := crl_x509.CreateRevocationList(rand.Reader, tbs, issuer, signer) if err != nil { return nil, err } - lintCRL, err := crl_x509.ParseRevocationList(lintCRLBytes) + lintCRL, err := zlintx509.ParseRevocationList(lintCRLBytes) if err != nil { return nil, err } diff --git a/linter/lints/cabf_br/lint_crl_acceptable_reason_codes.go b/linter/lints/cabf_br/lint_crl_acceptable_reason_codes.go new file mode 100644 index 000000000..13b63d2b4 --- /dev/null +++ b/linter/lints/cabf_br/lint_crl_acceptable_reason_codes.go @@ -0,0 +1,69 @@ +package cabfbr + +import ( + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints" +) + +type crlAcceptableReasonCodes struct{} + +/************************************************ +Baseline Requirements: 7.2.2.1: +The CRLReason indicated MUST NOT be unspecified (0). +The CRLReason MUST NOT be certificateHold (6). + +When the CRLReason code is not one of the following, then the reasonCode extension MUST NOT be provided: +- keyCompromise (RFC 5280 CRLReason #1); +- privilegeWithdrawn (RFC 5280 CRLReason #9); +- cessationOfOperation (RFC 5280 CRLReason #5); +- affiliationChanged (RFC 5280 CRLReason #3); or +- superseded (RFC 5280 CRLReason #4). +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_acceptable_reason_codes", + Description: "CRL entry Reason Codes must be 1, 3, 4, 5, or 9", + Citation: "BRs: 7.2.2.1", + Source: lint.CABFBaselineRequirements, + // We use the Mozilla Root Store Policy v2.8.1 effective date here + // because, although this lint enforces requirements from the BRs, those + // same requirements were in the MRSP first. + EffectiveDate: lints.MozillaPolicy281Date, + }, + Lint: NewCrlAcceptableReasonCodes, + }) +} + +func NewCrlAcceptableReasonCodes() lint.RevocationListLintInterface { + return &crlAcceptableReasonCodes{} +} + +func (l *crlAcceptableReasonCodes) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlAcceptableReasonCodes) Execute(c *x509.RevocationList) *lint.LintResult { + for _, rc := range c.RevokedCertificates { + if rc.ReasonCode == nil { + continue + } + switch *rc.ReasonCode { + case 1: // keyCompromise + case 3: // affiliationChanged + case 4: // superseded + case 5: // cessationOfOperation + case 9: // privilegeWithdrawn + continue + default: + return &lint.LintResult{ + Status: lint.Error, + Details: "CRLs MUST NOT include reasonCodes other than 1, 3, 4, 5, and 9", + } + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/cabf_br/lint_crl_acceptable_reason_codes_test.go b/linter/lints/cabf_br/lint_crl_acceptable_reason_codes_test.go new file mode 100644 index 000000000..1ab8f08ab --- /dev/null +++ b/linter/lints/cabf_br/lint_crl_acceptable_reason_codes_test.go @@ -0,0 +1,87 @@ +package cabfbr + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlAcceptableReasonCodes(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + // crl_good.pem contains a revocation entry with no reason code extension. + name: "good", + want: lint.Pass, + }, + { + name: "reason_0", + want: lint.Error, + wantSubStr: "MUST NOT include reasonCodes other than", + }, + { + name: "reason_1", + want: lint.Pass, + }, + { + name: "reason_2", + want: lint.Error, + wantSubStr: "MUST NOT include reasonCodes other than", + }, + { + name: "reason_3", + want: lint.Pass, + }, + { + name: "reason_4", + want: lint.Pass, + }, + { + name: "reason_5", + want: lint.Pass, + }, + { + name: "reason_6", + want: lint.Error, + wantSubStr: "MUST NOT include reasonCodes other than", + }, + { + name: "reason_8", + want: lint.Error, + wantSubStr: "MUST NOT include reasonCodes other than", + }, + { + name: "reason_9", + want: lint.Pass, + }, + { + name: "reason_10", + want: lint.Error, + wantSubStr: "MUST NOT include reasonCodes other than", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlAcceptableReasonCodes() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/cabf_br/lint_crl_no_critical_reason_codes.go b/linter/lints/cabf_br/lint_crl_no_critical_reason_codes.go new file mode 100644 index 000000000..c1950ab01 --- /dev/null +++ b/linter/lints/cabf_br/lint_crl_no_critical_reason_codes.go @@ -0,0 +1,51 @@ +package cabfbr + +import ( + "github.com/zmap/zcrypto/encoding/asn1" + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "github.com/zmap/zlint/v3/util" +) + +type crlCriticalReasonCodes struct{} + +/************************************************ +Baseline Requirements: 7.2.2.1: +If present, [the reasonCode] extension MUST NOT be marked critical. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_no_critical_reason_codes", + Description: "CRL entry reasonCode extension MUST NOT be marked critical", + Citation: "BRs: 7.2.2.1", + Source: lint.CABFBaselineRequirements, + EffectiveDate: util.CABFBRs_1_8_0_Date, + }, + Lint: NewCrlCriticalReasonCodes, + }) +} + +func NewCrlCriticalReasonCodes() lint.RevocationListLintInterface { + return &crlCriticalReasonCodes{} +} + +func (l *crlCriticalReasonCodes) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlCriticalReasonCodes) Execute(c *x509.RevocationList) *lint.LintResult { + reasonCodeOID := asn1.ObjectIdentifier{2, 5, 29, 21} // id-ce-reasonCode + for _, rc := range c.RevokedCertificates { + for _, ext := range rc.Extensions { + if ext.Id.Equal(reasonCodeOID) && ext.Critical { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRL entry reasonCode extension MUST NOT be marked critical", + } + } + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/cabf_br/lint_crl_no_critical_reason_codes_test.go b/linter/lints/cabf_br/lint_crl_no_critical_reason_codes_test.go new file mode 100644 index 000000000..8dc6d95fa --- /dev/null +++ b/linter/lints/cabf_br/lint_crl_no_critical_reason_codes_test.go @@ -0,0 +1,46 @@ +package cabfbr + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlCriticalReasonCodes(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "critical_reason", + want: lint.Error, + wantSubStr: "reasonCode extension MUST NOT be marked critical", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlCriticalReasonCodes() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/cabf_br/lint_crl_validity_period.go b/linter/lints/cabf_br/lint_crl_validity_period.go new file mode 100644 index 000000000..c40882cfa --- /dev/null +++ b/linter/lints/cabf_br/lint_crl_validity_period.go @@ -0,0 +1,59 @@ +package cabfbr + +import ( + "time" + + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "github.com/zmap/zlint/v3/util" +) + +type crlValidityPeriod struct{} + +/************************************************ +Baseline Requirements, Section 4.9.7: +For the status of Subscriber Certificates [...] the value of the nextUpdate +field MUST NOT be more than ten days beyond the value of the thisUpdate field. + +Although the validity period for CRLs covering the status of Subordinate CA +certificates is longer (up to 12 months), Boulder does not produce such CRLs, +so this lint only covers the Subscriber Certificate case. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_validity_period", + Description: "CRLs must have an acceptable validity period", + Citation: "BRs: 4.9.7", + Source: lint.CABFBaselineRequirements, + EffectiveDate: util.CABFBRs_1_2_1_Date, + }, + Lint: NewCrlValidityPeriod, + }) +} + +func NewCrlValidityPeriod() lint.RevocationListLintInterface { + return &crlValidityPeriod{} +} + +func (l *crlValidityPeriod) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlValidityPeriod) Execute(c *x509.RevocationList) *lint.LintResult { + validity := c.NextUpdate.Sub(c.ThisUpdate) + if validity <= 0 { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRL has NextUpdate at or before ThisUpdate", + } + } + if validity > 10*24*time.Hour { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRL has validity period greater than ten days", + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/cabf_br/lint_crl_validity_period_test.go b/linter/lints/cabf_br/lint_crl_validity_period_test.go new file mode 100644 index 000000000..44e5b6663 --- /dev/null +++ b/linter/lints/cabf_br/lint_crl_validity_period_test.go @@ -0,0 +1,51 @@ +package cabfbr + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlValidityPeriod(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "negative_validity", + want: lint.Error, + wantSubStr: "at or before", + }, + { + name: "long_validity", + want: lint.Error, + wantSubStr: "greater than ten days", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlValidityPeriod() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/crl/testdata/critical_reason.pem b/linter/lints/cabf_br/testdata/crl_critical_reason.pem similarity index 100% rename from linter/lints/crl/testdata/critical_reason.pem rename to linter/lints/cabf_br/testdata/crl_critical_reason.pem diff --git a/linter/lints/crl/testdata/good.pem b/linter/lints/cabf_br/testdata/crl_good.pem similarity index 100% rename from linter/lints/crl/testdata/good.pem rename to linter/lints/cabf_br/testdata/crl_good.pem diff --git a/linter/lints/crl/testdata/long_validity.pem b/linter/lints/cabf_br/testdata/crl_long_validity.pem similarity index 100% rename from linter/lints/crl/testdata/long_validity.pem rename to linter/lints/cabf_br/testdata/crl_long_validity.pem diff --git a/linter/lints/crl/testdata/negative_validity.pem b/linter/lints/cabf_br/testdata/crl_negative_validity.pem similarity index 100% rename from linter/lints/crl/testdata/negative_validity.pem rename to linter/lints/cabf_br/testdata/crl_negative_validity.pem diff --git a/linter/lints/crl/testdata/reason_0.pem b/linter/lints/cabf_br/testdata/crl_reason_0.pem similarity index 100% rename from linter/lints/crl/testdata/reason_0.pem rename to linter/lints/cabf_br/testdata/crl_reason_0.pem diff --git a/linter/lints/crl/testdata/good_old.pem b/linter/lints/cabf_br/testdata/crl_reason_1.pem similarity index 100% rename from linter/lints/crl/testdata/good_old.pem rename to linter/lints/cabf_br/testdata/crl_reason_1.pem diff --git a/linter/lints/crl/testdata/reason_10.pem b/linter/lints/cabf_br/testdata/crl_reason_10.pem similarity index 100% rename from linter/lints/crl/testdata/reason_10.pem rename to linter/lints/cabf_br/testdata/crl_reason_10.pem diff --git a/linter/lints/crl/testdata/reason_2.pem b/linter/lints/cabf_br/testdata/crl_reason_2.pem similarity index 100% rename from linter/lints/crl/testdata/reason_2.pem rename to linter/lints/cabf_br/testdata/crl_reason_2.pem diff --git a/linter/lints/crl/testdata/reason_3.pem b/linter/lints/cabf_br/testdata/crl_reason_3.pem similarity index 100% rename from linter/lints/crl/testdata/reason_3.pem rename to linter/lints/cabf_br/testdata/crl_reason_3.pem diff --git a/linter/lints/crl/testdata/reason_4.pem b/linter/lints/cabf_br/testdata/crl_reason_4.pem similarity index 100% rename from linter/lints/crl/testdata/reason_4.pem rename to linter/lints/cabf_br/testdata/crl_reason_4.pem diff --git a/linter/lints/crl/testdata/reason_5.pem b/linter/lints/cabf_br/testdata/crl_reason_5.pem similarity index 100% rename from linter/lints/crl/testdata/reason_5.pem rename to linter/lints/cabf_br/testdata/crl_reason_5.pem diff --git a/linter/lints/crl/testdata/reason_6.pem b/linter/lints/cabf_br/testdata/crl_reason_6.pem similarity index 100% rename from linter/lints/crl/testdata/reason_6.pem rename to linter/lints/cabf_br/testdata/crl_reason_6.pem diff --git a/linter/lints/crl/testdata/reason_8.pem b/linter/lints/cabf_br/testdata/crl_reason_8.pem similarity index 100% rename from linter/lints/crl/testdata/reason_8.pem rename to linter/lints/cabf_br/testdata/crl_reason_8.pem diff --git a/linter/lints/crl/testdata/reason_9.pem b/linter/lints/cabf_br/testdata/crl_reason_9.pem similarity index 100% rename from linter/lints/crl/testdata/reason_9.pem rename to linter/lints/cabf_br/testdata/crl_reason_9.pem diff --git a/linter/lints/common.go b/linter/lints/common.go index 819cea1b5..1cb9f931c 100644 --- a/linter/lints/common.go +++ b/linter/lints/common.go @@ -3,6 +3,8 @@ package lints import ( "time" + "github.com/zmap/zcrypto/encoding/asn1" + "github.com/zmap/zcrypto/x509/pkix" "github.com/zmap/zlint/v3/lint" ) @@ -19,5 +21,17 @@ const ( ) var ( - CPSV33Date = time.Date(2021, time.June, 8, 0, 0, 0, 0, time.UTC) + CPSV33Date = time.Date(2021, time.June, 8, 0, 0, 0, 0, time.UTC) + MozillaPolicy281Date = time.Date(2023, time.February, 15, 0, 0, 0, 0, time.UTC) ) + +// GetExtWithOID is a helper for several of our custom lints. It returns the +// extension with the given OID if it exists, or nil otherwise. +func GetExtWithOID(exts []pkix.Extension, oid asn1.ObjectIdentifier) *pkix.Extension { + for _, ext := range exts { + if ext.Id.Equal(oid) { + return &ext + } + } + return nil +} diff --git a/linter/lints/cpcps/lint_crl_has_idp.go b/linter/lints/cpcps/lint_crl_has_idp.go new file mode 100644 index 000000000..dff25e089 --- /dev/null +++ b/linter/lints/cpcps/lint_crl_has_idp.go @@ -0,0 +1,161 @@ +package cpcps + +import ( + "net/url" + + "github.com/zmap/zcrypto/encoding/asn1" + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "golang.org/x/crypto/cryptobyte" + cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" + + "github.com/letsencrypt/boulder/linter/lints" +) + +type crlHasIDP struct{} + +/************************************************ +Various root programs (and the BRs, after Ballot SC-063 passes) require that +sharded/partitioned CRLs have a specifically-encoded Issuing Distribution Point +extension. Since there's no way to tell from the CRL itself whether or not it +is sharded, we apply this lint universally to all CRLs, but as part of the Let's +Encrypt-specific suite of lints. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_has_idp", + Description: "Let's Encrypt issues sharded CRLs; therefore our CRLs must have an Issuing Distribution Point", + Citation: "", + Source: lints.LetsEncryptCPS, + EffectiveDate: lints.CPSV33Date, + }, + Lint: NewCrlHasIDP, + }) +} + +func NewCrlHasIDP() lint.RevocationListLintInterface { + return &crlHasIDP{} +} + +func (l *crlHasIDP) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlHasIDP) Execute(c *x509.RevocationList) *lint.LintResult { + idpOID := asn1.ObjectIdentifier{2, 5, 29, 28} // id-ce-issuingDistributionPoint + idpe := lints.GetExtWithOID(c.Extensions, idpOID) + if idpe == nil { + return &lint.LintResult{ + Status: lint.Warn, + Details: "CRL missing IDP", + } + } + if !idpe.Critical { + return &lint.LintResult{ + Status: lint.Error, + Details: "IDP MUST be critical", + } + } + + // Step inside the outer issuingDistributionPoint sequence to get access to + // its constituent fields, DistributionPoint and OnlyContainsUserCerts. + idpv := cryptobyte.String(idpe.Value) + if !idpv.ReadASN1(&idpv, cryptobyte_asn1.SEQUENCE) { + return &lint.LintResult{ + Status: lint.Warn, + Details: "Failed to read issuingDistributionPoint", + } + } + + // Ensure that the DistributionPoint is a reasonable URI. To get to the URI, + // we have to step inside the DistributionPointName, then step inside that's + // FullName, and finally read the singular SEQUENCE OF GeneralName element. + if !idpv.PeekASN1Tag(cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) { + return &lint.LintResult{ + Status: lint.Warn, + Details: "IDP should contain distributionPoint", + } + } + + var dpName cryptobyte.String + if !idpv.ReadASN1(&dpName, cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) { + return &lint.LintResult{ + Status: lint.Warn, + Details: "Failed to read IDP distributionPoint", + } + } + + if !dpName.ReadASN1(&dpName, cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) { + return &lint.LintResult{ + Status: lint.Warn, + Details: "Failed to read IDP distributionPoint fullName", + } + } + + uriBytes := make([]byte, 0) + if !dpName.ReadASN1Bytes(&uriBytes, cryptobyte_asn1.Tag(6).ContextSpecific()) { + return &lint.LintResult{ + Status: lint.Warn, + Details: "Failed to read IDP URI", + } + } + + uri, err := url.Parse(string(uriBytes)) + if err != nil { + return &lint.LintResult{ + Status: lint.Error, + Details: "Failed to parse IDP URI", + } + } + + if uri.Scheme != "http" { + return &lint.LintResult{ + Status: lint.Error, + Details: "IDP URI MUST use http scheme", + } + } + + if !dpName.Empty() { + return &lint.LintResult{ + Status: lint.Warn, + Details: "IDP should contain only one distributionPoint", + } + } + + // Ensure that OnlyContainsUserCerts is True. We have to read this boolean as + // a byte and ensure its value is 0xFF because cryptobyte.ReadASN1Boolean + // can't handle custom encoding rules like this field's [1] tag. + if !idpv.PeekASN1Tag(cryptobyte_asn1.Tag(1).ContextSpecific()) { + return &lint.LintResult{ + Status: lint.Warn, + Details: "IDP should contain onlyContainsUserCerts", + } + } + + onlyContainsUserCerts := make([]byte, 0) + if !idpv.ReadASN1Bytes(&onlyContainsUserCerts, cryptobyte_asn1.Tag(1).ContextSpecific()) { + return &lint.LintResult{ + Status: lint.Error, + Details: "Failed to read IDP onlyContainsUserCerts", + } + } + + if len(onlyContainsUserCerts) != 1 || onlyContainsUserCerts[0] != 0xFF { + return &lint.LintResult{ + Status: lint.Error, + Details: "IDP should set onlyContainsUserCerts: TRUE", + } + } + + // Ensure that no other fields are set. + if !idpv.Empty() { + return &lint.LintResult{ + Status: lint.Warn, + Details: "IDP should not contain fields other than distributionPoint and onlyContainsUserCerts", + } + } + + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/cpcps/lint_crl_has_idp_test.go b/linter/lints/cpcps/lint_crl_has_idp_test.go new file mode 100644 index 000000000..9c494cc8a --- /dev/null +++ b/linter/lints/cpcps/lint_crl_has_idp_test.go @@ -0,0 +1,65 @@ +package cpcps + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlHasIDP(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "no_idp", + want: lint.Warn, + }, + { + name: "idp_no_uri", + want: lint.Warn, + wantSubStr: "should contain distributionPoint", + }, + { + name: "idp_two_uris", + want: lint.Warn, + wantSubStr: "only one distributionPoint", + }, + { + name: "idp_no_usercerts", + want: lint.Warn, + wantSubStr: "should contain onlyContainsUserCerts", + }, + { + name: "idp_some_reasons", + want: lint.Warn, + wantSubStr: "should not contain fields other than", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlHasIDP() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/cpcps/lint_crl_has_no_aia.go b/linter/lints/cpcps/lint_crl_has_no_aia.go new file mode 100644 index 000000000..43f08976d --- /dev/null +++ b/linter/lints/cpcps/lint_crl_has_no_aia.go @@ -0,0 +1,51 @@ +package cpcps + +import ( + "github.com/zmap/zcrypto/encoding/asn1" + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints" +) + +type crlHasNoAIA struct{} + +/************************************************ +RFC 5280: 5.2.7 + +The requirements around the Authority Information Access extension are extensive. +Therefore we do not include one. +Conforming CRL issuers MUST include the nextUpdate field in all CRLs. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_has_no_aia", + Description: "Let's Encrypt does not include the CRL AIA extension", + Citation: "", + Source: lints.LetsEncryptCPS, + EffectiveDate: lints.CPSV33Date, + }, + Lint: NewCrlHasNoAIA, + }) +} + +func NewCrlHasNoAIA() lint.RevocationListLintInterface { + return &crlHasNoAIA{} +} + +func (l *crlHasNoAIA) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlHasNoAIA) Execute(c *x509.RevocationList) *lint.LintResult { + aiaOID := asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 1} // id-pe-authorityInfoAccess + if lints.GetExtWithOID(c.Extensions, aiaOID) != nil { + return &lint.LintResult{ + Status: lint.Notice, + Details: "CRL has an Authority Information Access url", + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/cpcps/lint_crl_has_no_aia_test.go b/linter/lints/cpcps/lint_crl_has_no_aia_test.go new file mode 100644 index 000000000..679bfe7ba --- /dev/null +++ b/linter/lints/cpcps/lint_crl_has_no_aia_test.go @@ -0,0 +1,46 @@ +package cpcps + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlHasNoAIA(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "aia", + want: lint.Notice, + wantSubStr: "Authority Information Access", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlHasNoAIA() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/cpcps/lint_crl_has_no_cert_issuers.go b/linter/lints/cpcps/lint_crl_has_no_cert_issuers.go new file mode 100644 index 000000000..61bed1fbb --- /dev/null +++ b/linter/lints/cpcps/lint_crl_has_no_cert_issuers.go @@ -0,0 +1,54 @@ +package cpcps + +import ( + "github.com/zmap/zcrypto/encoding/asn1" + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints" +) + +type crlHasNoCertIssuers struct{} + +/************************************************ +RFC 5280: 5.3.3 + +Section 5.3.3 defines the Certificate Issuer entry extension. The presence of +this extension means that the CRL is an "indirect CRL", including certificates +which were issued by a different issuer than the one issuing the CRL itself. +We do not issue indirect CRLs, so our CRL entries should not have this extension. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_has_no_cert_issuers", + Description: "Let's Encrypt does not issue indirect CRLs", + Citation: "", + Source: lints.LetsEncryptCPS, + EffectiveDate: lints.CPSV33Date, + }, + Lint: NewCrlHasNoCertIssuers, + }) +} + +func NewCrlHasNoCertIssuers() lint.RevocationListLintInterface { + return &crlHasNoCertIssuers{} +} + +func (l *crlHasNoCertIssuers) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlHasNoCertIssuers) Execute(c *x509.RevocationList) *lint.LintResult { + certIssuerOID := asn1.ObjectIdentifier{2, 5, 29, 29} // id-ce-certificateIssuer + for _, entry := range c.RevokedCertificates { + if lints.GetExtWithOID(entry.Extensions, certIssuerOID) != nil { + return &lint.LintResult{ + Status: lint.Notice, + Details: "CRL has an entry with a Certificate Issuer extension", + } + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/cpcps/lint_crl_has_no_cert_issuers_test.go b/linter/lints/cpcps/lint_crl_has_no_cert_issuers_test.go new file mode 100644 index 000000000..c2710ad58 --- /dev/null +++ b/linter/lints/cpcps/lint_crl_has_no_cert_issuers_test.go @@ -0,0 +1,45 @@ +package cpcps + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlHasNoCertIssuers(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "cert_issuer", + want: lint.Notice, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlHasNoCertIssuers() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/cpcps/lint_crl_is_not_delta.go b/linter/lints/cpcps/lint_crl_is_not_delta.go new file mode 100644 index 000000000..eaa588c44 --- /dev/null +++ b/linter/lints/cpcps/lint_crl_is_not_delta.go @@ -0,0 +1,65 @@ +package cpcps + +import ( + "github.com/zmap/zcrypto/encoding/asn1" + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints" +) + +type crlIsNotDelta struct{} + +/************************************************ +RFC 5280: 5.2.4 + +Section 5.2.4 defines a Delta CRL, and all the requirements that come with it. +These requirements are complex and do not serve our purpose, so we ensure that +we never issue a CRL which could be construed as a Delta CRL. + +RFC 5280: 5.2.6 + +Similarly, Section 5.2.6 defines the Freshest CRL extension, which is only +applicable in the case that the CRL is a Delta CRL. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_is_not_delta", + Description: "Let's Encrypt does not issue delta CRLs", + Citation: "", + Source: lints.LetsEncryptCPS, + EffectiveDate: lints.CPSV33Date, + }, + Lint: NewCrlIsNotDelta, + }) +} + +func NewCrlIsNotDelta() lint.RevocationListLintInterface { + return &crlIsNotDelta{} +} + +func (l *crlIsNotDelta) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlIsNotDelta) Execute(c *x509.RevocationList) *lint.LintResult { + deltaCRLIndicatorOID := asn1.ObjectIdentifier{2, 5, 29, 27} // id-ce-deltaCRLIndicator + if lints.GetExtWithOID(c.Extensions, deltaCRLIndicatorOID) != nil { + return &lint.LintResult{ + Status: lint.Notice, + Details: "CRL is a Delta CRL", + } + } + + freshestCRLOID := asn1.ObjectIdentifier{2, 5, 29, 46} // id-ce-freshestCRL + if lints.GetExtWithOID(c.Extensions, freshestCRLOID) != nil { + return &lint.LintResult{ + Status: lint.Notice, + Details: "CRL has a Freshest CRL url", + } + } + + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/cpcps/lint_crl_is_not_delta_test.go b/linter/lints/cpcps/lint_crl_is_not_delta_test.go new file mode 100644 index 000000000..23137d9d6 --- /dev/null +++ b/linter/lints/cpcps/lint_crl_is_not_delta_test.go @@ -0,0 +1,51 @@ +package cpcps + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlIsNotDelta(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "delta", + want: lint.Notice, + wantSubStr: "Delta", + }, + { + name: "freshest", + want: lint.Notice, + wantSubStr: "Freshest", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlIsNotDelta() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/crl/testdata/aia.pem b/linter/lints/cpcps/testdata/crl_aia.pem similarity index 100% rename from linter/lints/crl/testdata/aia.pem rename to linter/lints/cpcps/testdata/crl_aia.pem diff --git a/linter/lints/crl/testdata/cert_issuer.pem b/linter/lints/cpcps/testdata/crl_cert_issuer.pem similarity index 100% rename from linter/lints/crl/testdata/cert_issuer.pem rename to linter/lints/cpcps/testdata/crl_cert_issuer.pem diff --git a/linter/lints/crl/testdata/delta.pem b/linter/lints/cpcps/testdata/crl_delta.pem similarity index 100% rename from linter/lints/crl/testdata/delta.pem rename to linter/lints/cpcps/testdata/crl_delta.pem diff --git a/linter/lints/crl/testdata/freshest.pem b/linter/lints/cpcps/testdata/crl_freshest.pem similarity index 100% rename from linter/lints/crl/testdata/freshest.pem rename to linter/lints/cpcps/testdata/crl_freshest.pem diff --git a/linter/lints/cpcps/testdata/crl_good.pem b/linter/lints/cpcps/testdata/crl_good.pem new file mode 100644 index 000000000..8b383d0a0 --- /dev/null +++ b/linter/lints/cpcps/testdata/crl_good.pem @@ -0,0 +1,11 @@ +-----BEGIN X509 CRL----- +MIIBmDCCAR8CAQEwCgYIKoZIzj0EAwMwSTELMAkGA1UEBhMCWFgxFTATBgNVBAoT +DEJvdWxkZXIgVGVzdDEjMCEGA1UEAxMaKFRFU1QpIEVsZWdhbnQgRWxlcGhhbnQg +RTEXDTIyMTAxMDIwMTIwN1oXDTIyMTAxOTIwMTIwNlowKTAnAggDrlHbURVaPBcN +MjIxMDEwMTkxMjA3WjAMMAoGA1UdFQQDCgEBoHoweDAfBgNVHSMEGDAWgBQB2rt6 +yyUgjl551vmWQi8CQSkHvjARBgNVHRQECgIIFxzOPeSCumEwQgYDVR0cAQH/BDgw +NqAxoC+GLWh0dHA6Ly9jLmJvdWxkZXIudGVzdC82NjI4Mzc1NjkxMzU4ODI4OC8w +LmNybIEB/zAKBggqhkjOPQQDAwNnADBkAjAvDkIUnTYavJ6h8606MDyFh2uw/cF+ +OVnM4sE8nUdGy0XYg0hGfbR4MY+kRxRQayICMFeQPpcpIr0zgXpP6lUXU0rcLSva +tuaeQSVr24nGjZ7Py0vc94w0n7idZ8wje5+/Mw== +-----END X509 CRL----- diff --git a/linter/lints/crl/testdata/idp_no_uri.pem b/linter/lints/cpcps/testdata/crl_idp_no_uri.pem similarity index 100% rename from linter/lints/crl/testdata/idp_no_uri.pem rename to linter/lints/cpcps/testdata/crl_idp_no_uri.pem diff --git a/linter/lints/crl/testdata/idp_no_usercerts.pem b/linter/lints/cpcps/testdata/crl_idp_no_usercerts.pem similarity index 100% rename from linter/lints/crl/testdata/idp_no_usercerts.pem rename to linter/lints/cpcps/testdata/crl_idp_no_usercerts.pem diff --git a/linter/lints/crl/testdata/idp_some_reasons.pem b/linter/lints/cpcps/testdata/crl_idp_some_reasons.pem similarity index 100% rename from linter/lints/crl/testdata/idp_some_reasons.pem rename to linter/lints/cpcps/testdata/crl_idp_some_reasons.pem diff --git a/linter/lints/crl/testdata/idp_two_uris.pem b/linter/lints/cpcps/testdata/crl_idp_two_uris.pem similarity index 100% rename from linter/lints/crl/testdata/idp_two_uris.pem rename to linter/lints/cpcps/testdata/crl_idp_two_uris.pem diff --git a/linter/lints/crl/testdata/no_idp.pem b/linter/lints/cpcps/testdata/crl_no_idp.pem similarity index 100% rename from linter/lints/crl/testdata/no_idp.pem rename to linter/lints/cpcps/testdata/crl_no_idp.pem diff --git a/linter/lints/crl/lints.go b/linter/lints/crl/lints.go deleted file mode 100644 index cec8dd6f8..000000000 --- a/linter/lints/crl/lints.go +++ /dev/null @@ -1,659 +0,0 @@ -package crl - -import ( - "crypto/x509/pkix" - "encoding/asn1" - "errors" - "fmt" - "net/url" - "time" - - "github.com/zmap/zlint/v3" - "github.com/zmap/zlint/v3/lint" - "golang.org/x/crypto/cryptobyte" - cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" - - "github.com/letsencrypt/boulder/crl/crl_x509" -) - -const ( - utcTimeFormat = "YYMMDDHHMMSSZ" - generalizedTimeFormat = "YYYYMMDDHHMMSSZ" -) - -type crlLint func(*crl_x509.RevocationList) *lint.LintResult - -// registry is the collection of all known CRL lints. It is populated by this -// file's init(), and should not be touched by anything else on pain of races. -var registry map[string]crlLint - -func init() { - // NOTE TO DEVS: you MUST add your new lint function to this list or it - // WILL NOT be run. - registry = map[string]crlLint{ - "hasIssuerName": hasIssuerName, - "hasNextUpdate": hasNextUpdate, - "noEmptyRevokedCertificatesList": noEmptyRevokedCertificatesList, - "hasAKI": hasAKI, - "hasNumber": hasNumber, - "isNotDelta": isNotDelta, - "checkIDP": checkIDP, - "hasNoFreshest": hasNoFreshest, - "hasNoAIA": hasNoAIA, - "noZeroReasonCodes": noZeroReasonCodes, - "hasNoCertIssuers": hasNoCertIssuers, - "hasAcceptableValidity": hasAcceptableValidity, - "noCriticalReasons": noCriticalReasons, - "noCertificateHolds": noCertificateHolds, - "hasMozReasonCodes": hasMozReasonCodes, - "hasValidTimestamps": hasValidTimestamps, - } -} - -// getExtWithOID is a helper for several lints in this file. It returns the -// extension with the given OID if it exists, or nil otherwise. -func getExtWithOID(exts []pkix.Extension, oid asn1.ObjectIdentifier) *pkix.Extension { - for _, ext := range exts { - if ext.Id.Equal(oid) { - return &ext - } - } - return nil -} - -// LintCRL examines the given lint CRL, runs it through all of our checks, and -// returns a list of all failures -func LintCRL(lintCRL *crl_x509.RevocationList) *zlint.ResultSet { - rset := zlint.ResultSet{ - Version: 0, - Timestamp: time.Now().UnixNano(), - Results: make(map[string]*lint.LintResult), - } - - type namedResult struct { - Name string - Result *lint.LintResult - } - resChan := make(chan namedResult, len(registry)) - - for name, callable := range registry { - go func(name string, callable crlLint) { - resChan <- namedResult{name, callable(lintCRL)} - }(name, callable) - } - - for i := 0; i < len(registry); i++ { - res := <-resChan - switch res.Result.Status { - case lint.Notice: - rset.NoticesPresent = true - case lint.Warn: - rset.WarningsPresent = true - case lint.Error: - rset.ErrorsPresent = true - case lint.Fatal: - rset.FatalsPresent = true - } - rset.Results[res.Name] = res.Result - } - - return &rset -} - -// hasIssuerName checks RFC 5280, Section 5.1.2.3: -// The issuer field MUST contain a non-empty X.500 distinguished name (DN). -// This lint does not enforce that the issuer field complies with the rest of -// the encoding rules of a certificate issuer name, because it (perhaps wrongly) -// assumes that those were checked when the issuer was itself issued, and on all -// certificates issued by this CRL issuer. Also because there are just a lot of -// things to check there, and zlint doesn't expose a public helper for it. -func hasIssuerName(crl *crl_x509.RevocationList) *lint.LintResult { - if len(crl.Issuer.Names) == 0 { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRLs MUST have a non-empty issuer field", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// TODO(#6222): Write a lint which checks RFC 5280, Section 5.1.2.4 and 5.1.2.5: -// CRL issuers conforming to this profile MUST encode thisUpdate and nextUpdate -// as UTCTime for dates through the year 2049. UTCTime and GeneralizedTime -// values MUST be expressed in Greenwich Mean Time (Zulu) and MUST include -// seconds, even where the number of seconds is zero. - -// hasNextUpdate checks RFC 5280, Section 5.1.2.5: -// Conforming CRL issuers MUST include the nextUpdate field in all CRLs. -func hasNextUpdate(crl *crl_x509.RevocationList) *lint.LintResult { - if crl.NextUpdate.IsZero() { - return &lint.LintResult{ - Status: lint.Error, - Details: "Conforming CRL issuers MUST include the nextUpdate field in all CRLs", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// noEmptyRevokedCertificatesList checks RFC 5280, Section 5.1.2.6: -// When there are no revoked certificates, the revoked certificates list MUST be -// absent. -func noEmptyRevokedCertificatesList(crl *crl_x509.RevocationList) *lint.LintResult { - if crl.RevokedCertificates != nil && len(crl.RevokedCertificates) == 0 { - return &lint.LintResult{ - Status: lint.Error, - Details: "If the revokedCertificates list is empty, it must not be present", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// hasAKI checks RFC 5280, Section 5.2.1: -// Conforming CRL issuers MUST use the key identifier method, and MUST include -// this extension in all CRLs issued. -func hasAKI(crl *crl_x509.RevocationList) *lint.LintResult { - if len(crl.AuthorityKeyId) == 0 { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRLs MUST include the authority key identifier extension", - } - } - aki := cryptobyte.String(crl.AuthorityKeyId) - var akiBody cryptobyte.String - if !aki.ReadASN1(&akiBody, cryptobyte_asn1.SEQUENCE) { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRL has a malformed authority key identifier extension", - } - } - if !akiBody.PeekASN1Tag(cryptobyte_asn1.Tag(0).ContextSpecific()) { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRLs MUST use the key identifier method in the authority key identifier extension", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// hasNumber checks RFC 5280, Section 5.2.3: -// CRL issuers conforming to this profile MUST include this extension in all -// CRLs and MUST mark this extension as non-critical. Conforming CRL issuers -// MUST NOT use CRLNumber values longer than 20 octets. -func hasNumber(crl *crl_x509.RevocationList) *lint.LintResult { - if crl.Number == nil { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRLs MUST include the CRL number extension", - } - } - - crlNumberOID := asn1.ObjectIdentifier{2, 5, 29, 20} // id-ce-cRLNumber - ext := getExtWithOID(crl.Extensions, crlNumberOID) - if ext != nil && ext.Critical { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRL Number MUST NOT be marked critical", - } - } - - numBytes := crl.Number.Bytes() - if len(numBytes) > 20 || (len(numBytes) == 20 && numBytes[0]&0x80 != 0) { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRL Number MUST NOT be longer than 20 octets", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// isNotDelta checks that the CRL is not a Delta CRL. (RFC 5280, Section 5.2.4). -// There's no requirement against this, but Delta CRLs come with extra -// requirements we don't want to deal with. -func isNotDelta(crl *crl_x509.RevocationList) *lint.LintResult { - deltaCRLIndicatorOID := asn1.ObjectIdentifier{2, 5, 29, 27} // id-ce-deltaCRLIndicator - if getExtWithOID(crl.Extensions, deltaCRLIndicatorOID) != nil { - return &lint.LintResult{ - Status: lint.Notice, - Details: "CRL is a Delta CRL", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// checkIDP checks that the CRL does have an Issuing Distribution Point, that it -// is critical, that it contains a single http distributionPointName, that it -// asserts the onlyContainsUserCerts boolean, and that it does not contain any -// of the other fields. (RFC 5280, Section 5.2.5). -func checkIDP(crl *crl_x509.RevocationList) *lint.LintResult { - idpOID := asn1.ObjectIdentifier{2, 5, 29, 28} // id-ce-issuingDistributionPoint - idpe := getExtWithOID(crl.Extensions, idpOID) - if idpe == nil { - return &lint.LintResult{ - Status: lint.Warn, - Details: "CRL missing IDP", - } - } - if !idpe.Critical { - return &lint.LintResult{ - Status: lint.Error, - Details: "IDP MUST be critical", - } - } - - // Step inside the outer issuingDistributionPoint sequence to get access to - // its constituent fields, DistributionPoint and OnlyContainsUserCerts. - idpv := cryptobyte.String(idpe.Value) - if !idpv.ReadASN1(&idpv, cryptobyte_asn1.SEQUENCE) { - return &lint.LintResult{ - Status: lint.Warn, - Details: "Failed to read issuingDistributionPoint", - } - } - - // Ensure that the DistributionPoint is a reasonable URI. To get to the URI, - // we have to step inside the DistributionPointName, then step inside that's - // FullName, and finally read the singular SEQUENCE OF GeneralName element. - if !idpv.PeekASN1Tag(cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) { - return &lint.LintResult{ - Status: lint.Warn, - Details: "IDP should contain distributionPoint", - } - } - - var dpName cryptobyte.String - if !idpv.ReadASN1(&dpName, cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) { - return &lint.LintResult{ - Status: lint.Warn, - Details: "Failed to read IDP distributionPoint", - } - } - - if !dpName.ReadASN1(&dpName, cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) { - return &lint.LintResult{ - Status: lint.Warn, - Details: "Failed to read IDP distributionPoint fullName", - } - } - - uriBytes := make([]byte, 0) - if !dpName.ReadASN1Bytes(&uriBytes, cryptobyte_asn1.Tag(6).ContextSpecific()) { - return &lint.LintResult{ - Status: lint.Warn, - Details: "Failed to read IDP URI", - } - } - - uri, err := url.Parse(string(uriBytes)) - if err != nil { - return &lint.LintResult{ - Status: lint.Error, - Details: "Failed to parse IDP URI", - } - } - - if uri.Scheme != "http" { - return &lint.LintResult{ - Status: lint.Error, - Details: "IDP URI MUST use http scheme", - } - } - - if !dpName.Empty() { - return &lint.LintResult{ - Status: lint.Warn, - Details: "IDP should contain only one distributionPoint", - } - } - - // Ensure that OnlyContainsUserCerts is True. We have to read this boolean as - // a byte and ensure its value is 0xFF because cryptobyte.ReadASN1Boolean - // can't handle custom encoding rules like this field's [1] tag. - if !idpv.PeekASN1Tag(cryptobyte_asn1.Tag(1).ContextSpecific()) { - return &lint.LintResult{ - Status: lint.Warn, - Details: "IDP should contain onlyContainsUserCerts", - } - } - - onlyContainsUserCerts := make([]byte, 0) - if !idpv.ReadASN1Bytes(&onlyContainsUserCerts, cryptobyte_asn1.Tag(1).ContextSpecific()) { - return &lint.LintResult{ - Status: lint.Error, - Details: "Failed to read IDP onlyContainsUserCerts", - } - } - - if len(onlyContainsUserCerts) != 1 || onlyContainsUserCerts[0] != 0xFF { - return &lint.LintResult{ - Status: lint.Error, - Details: "IDP should set onlyContainsUserCerts: TRUE", - } - } - - // Ensure that no other fields are set. - if !idpv.Empty() { - return &lint.LintResult{ - Status: lint.Warn, - Details: "IDP should not contain fields other than distributionPoint and onlyContainsUserCerts", - } - } - - return &lint.LintResult{Status: lint.Pass} -} - -// hasNoFreshest checks that the CRL is does not have a Freshest CRL extension -// (RFC 5280, Section 5.2.6). There's no requirement against this, but Freshest -// CRL extensions (and the Delta CRLs they imply) come with extra requirements -// we don't want to deal with. -func hasNoFreshest(crl *crl_x509.RevocationList) *lint.LintResult { - freshestOID := asn1.ObjectIdentifier{2, 5, 29, 46} // id-ce-freshestCRL - if getExtWithOID(crl.Extensions, freshestOID) != nil { - return &lint.LintResult{ - Status: lint.Notice, - Details: "CRL has a Freshest CRL url", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// hasNoAIA checks that the CRL is does not have an Authority Information Access -// extension (RFC 5280, Section 5.2.7). There's no requirement against this, but -// AIAs come with extra requirements we don't want to deal with. -func hasNoAIA(crl *crl_x509.RevocationList) *lint.LintResult { - aiaOID := asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 1} // id-pe-authorityInfoAccess - if getExtWithOID(crl.Extensions, aiaOID) != nil { - return &lint.LintResult{ - Status: lint.Notice, - Details: "CRL has an Authority Information Access url", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// hasNoCertIssuers checks that the CRL does not have any entries with the -// Certificate Issuer extension (RFC 5280, Section 5.3.3). There is no -// requirement against this, but the presence of this extension would mean that -// the CRL includes certificates issued by an issuer other than the one signing -// the CRL itself, which we don't want to do. -func hasNoCertIssuers(crl *crl_x509.RevocationList) *lint.LintResult { - certIssuerOID := asn1.ObjectIdentifier{2, 5, 29, 29} // id-ce-certificateIssuer - for _, entry := range crl.RevokedCertificates { - if getExtWithOID(entry.Extensions, certIssuerOID) != nil { - return &lint.LintResult{ - Status: lint.Notice, - Details: "CRL has an entry with a Certificate Issuer extension", - } - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// hasAcceptableValidity checks Baseline Requirements, Section 4.9.7: -// The value of the nextUpdate field MUST NOT be more than ten days beyond the -// value of the thisUpdate field. -func hasAcceptableValidity(crl *crl_x509.RevocationList) *lint.LintResult { - validity := crl.NextUpdate.Sub(crl.ThisUpdate) - if validity <= 0 { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRL has NextUpdate at or before ThisUpdate", - } - } else if validity > 10*24*time.Hour { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRL has validity period greater than ten days", - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// noZeroReasonCodes checks Baseline Requirements, Section 7.2.2.1: -// The CRLReason indicated MUST NOT be unspecified (0). If the reason for -// revocation is unspecified, CAs MUST omit reasonCode entry extension, if -// allowed by the previous requirements. -// By extension, it therefore also checks RFC 5280, Section 5.3.1: -// The reason code CRL entry extension SHOULD be absent instead of using the -// unspecified (0) reasonCode value. -func noZeroReasonCodes(crl *crl_x509.RevocationList) *lint.LintResult { - for _, entry := range crl.RevokedCertificates { - if entry.ReasonCode != nil && *entry.ReasonCode == 0 { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRL entries MUST NOT contain the unspecified (0) reason code", - } - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// noCriticalReasons checks Baseline Requirements, Section 7.2.2.1: -// If present, [the reasonCode] extension MUST NOT be marked critical. -func noCriticalReasons(crl *crl_x509.RevocationList) *lint.LintResult { - reasonCodeOID := asn1.ObjectIdentifier{2, 5, 29, 21} // id-ce-reasonCode - for _, rc := range crl.RevokedCertificates { - for _, ext := range rc.Extensions { - if ext.Id.Equal(reasonCodeOID) && ext.Critical { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRL entry reasonCodes MUST NOT be critical", - } - } - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// noCertificateHolds checks Baseline Requirements, Section 7.2.2.1: -// The CRLReason MUST NOT be certificateHold (6). -func noCertificateHolds(crl *crl_x509.RevocationList) *lint.LintResult { - for _, entry := range crl.RevokedCertificates { - if entry.ReasonCode != nil && *entry.ReasonCode == 6 { - return &lint.LintResult{ - Status: lint.Error, - Details: "CRL entries MUST NOT use the certificateHold (6) reason code", - } - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// hasMozReasonCodes checks MRSP v2.8 Section 6.1.1: -// When the CRLReason code is not one of the following, then the reasonCode extension MUST NOT be provided: -// - keyCompromise (RFC 5280 CRLReason #1); -// - privilegeWithdrawn (RFC 5280 CRLReason #9); -// - cessationOfOperation (RFC 5280 CRLReason #5); -// - affiliationChanged (RFC 5280 CRLReason #3); or -// - superseded (RFC 5280 CRLReason #4). -func hasMozReasonCodes(crl *crl_x509.RevocationList) *lint.LintResult { - for _, rc := range crl.RevokedCertificates { - if rc.ReasonCode == nil { - continue - } - switch *rc.ReasonCode { - case 1: // keyCompromise - case 3: // affiliationChanged - case 4: // superseded - case 5: // cessationOfOperation - case 9: // privilegeWithdrawn - continue - default: - return &lint.LintResult{ - Status: lint.Error, - Details: "CRLs MUST NOT include reasonCodes other than 1, 3, 4, 5, and 9", - } - } - } - return &lint.LintResult{Status: lint.Pass} -} - -// hasValidTimestamps validates encoding of all CRL timestamp values as -// specified in section 4.1.2.5 of RFC5280. Timestamp values MUST be encoded as -// either UTCTime or a GeneralizedTime. -// -// UTCTime values MUST be expressed in Greenwich Mean Time (Zulu) and MUST -// include seconds (i.e., times are YYMMDDHHMMSSZ), even where the number of -// seconds is zero. See: -// https://www.rfc-editor.org/rfc/rfc5280.html#section-4.1.2.5.1 -// -// GeneralizedTime values MUST be expressed in Greenwich Mean Time (Zulu) and -// MUST include seconds (i.e., times are YYYYMMDDHHMMSSZ), even where the number -// of seconds is zero. GeneralizedTime values MUST NOT include fractional -// seconds. See: https://www.rfc-editor.org/rfc/rfc5280.html#section-4.1.2.5.2 -// -// Conforming applications MUST encode thisUpdate, nextUpdate, and cerficate -// validity timestamps prior to 2050 as UTCTime and GeneralizedTime there-after. -// See: -// - https://www.rfc-editor.org/rfc/rfc5280.html#section-5.1.2.4 -// - https://www.rfc-editor.org/rfc/rfc5280.html#section-5.1.2.5 -// - https://www.rfc-editor.org/rfc/rfc5280.html#section-5.1.2.6 -func hasValidTimestamps(crl *crl_x509.RevocationList) *lint.LintResult { - input := cryptobyte.String(crl.RawTBSRevocationList) - lintFail := lint.LintResult{ - Status: lint.Error, - Details: "Failed to re-parse tbsCertList during linting", - } - - // Read tbsCertList. - var tbs cryptobyte.String - if !input.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) { - return &lintFail - } - - // Skip (optional) version. - if !tbs.SkipOptionalASN1(cryptobyte_asn1.INTEGER) { - return &lintFail - } - - // Skip signature. - if !tbs.SkipASN1(cryptobyte_asn1.SEQUENCE) { - return &lintFail - } - - // Skip issuer. - if !tbs.SkipASN1(cryptobyte_asn1.SEQUENCE) { - return &lintFail - } - - // Read thisUpdate. - var thisUpdate cryptobyte.String - var thisUpdateTag cryptobyte_asn1.Tag - if !tbs.ReadAnyASN1Element(&thisUpdate, &thisUpdateTag) { - return &lintFail - } - - // Lint thisUpdate. - err := lintTimestamp(&thisUpdate, thisUpdateTag) - if err != nil { - return &lint.LintResult{Status: lint.Error, Details: err.Error()} - } - - // Peek (optional) nextUpdate. - if tbs.PeekASN1Tag(cryptobyte_asn1.UTCTime) || tbs.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime) { - // Read nextUpdate. - var nextUpdate cryptobyte.String - var nextUpdateTag cryptobyte_asn1.Tag - if !tbs.ReadAnyASN1Element(&nextUpdate, &nextUpdateTag) { - return &lintFail - } - - // Lint nextUpdate. - err = lintTimestamp(&nextUpdate, nextUpdateTag) - if err != nil { - return &lint.LintResult{Status: lint.Error, Details: err.Error()} - } - } - - // Peek (optional) revokedCertificates. - if tbs.PeekASN1Tag(cryptobyte_asn1.SEQUENCE) { - // Read sequence of revokedCertificate. - var revokedSeq cryptobyte.String - if !tbs.ReadASN1(&revokedSeq, cryptobyte_asn1.SEQUENCE) { - return &lintFail - } - - // Iterate over each revokedCertificate sequence. - for !revokedSeq.Empty() { - // Read revokedCertificate. - var certSeq cryptobyte.String - if !revokedSeq.ReadASN1Element(&certSeq, cryptobyte_asn1.SEQUENCE) { - return &lintFail - } - - if !certSeq.ReadASN1(&certSeq, cryptobyte_asn1.SEQUENCE) { - return &lintFail - } - - // Skip userCertificate (serial number). - if !certSeq.SkipASN1(cryptobyte_asn1.INTEGER) { - return &lintFail - } - - // Read revocationDate. - var revocationDate cryptobyte.String - var revocationDateTag cryptobyte_asn1.Tag - if !certSeq.ReadAnyASN1Element(&revocationDate, &revocationDateTag) { - return &lintFail - } - - // Lint revocationDate. - err = lintTimestamp(&revocationDate, revocationDateTag) - if err != nil { - return &lint.LintResult{Status: lint.Error, Details: err.Error()} - } - } - } - return &lint.LintResult{Status: lint.Pass} -} - -func lintTimestamp(der *cryptobyte.String, tag cryptobyte_asn1.Tag) error { - // Preserve the original timestamp for length checking. - derBytes := *der - var tsBytes cryptobyte.String - if !derBytes.ReadASN1(&tsBytes, tag) { - return errors.New("failed to read timestamp") - } - tsLen := len(string(tsBytes)) - - var parsedTime time.Time - switch tag { - case cryptobyte_asn1.UTCTime: - // Verify that the timestamp is properly formatted. - if tsLen != len(utcTimeFormat) { - return fmt.Errorf("timestamps encoded using UTCTime MUST be specified in the format %q", utcTimeFormat) - } - - if !der.ReadASN1UTCTime(&parsedTime) { - return errors.New("failed to read timestamp encoded using UTCTime") - } - - // Verify that the timestamp is prior to the year 2050. This should - // really never happen. - if parsedTime.Year() > 2049 { - return errors.New("ReadASN1UTCTime returned a UTCTime after 2049") - } - case cryptobyte_asn1.GeneralizedTime: - // Verify that the timestamp is properly formatted. - if tsLen != len(generalizedTimeFormat) { - return fmt.Errorf( - "timestamps encoded using GeneralizedTime MUST be specified in the format %q", generalizedTimeFormat, - ) - } - - if !der.ReadASN1GeneralizedTime(&parsedTime) { - return fmt.Errorf("failed to read timestamp encoded using GeneralizedTime") - } - - // Verify that the timestamp occurred after the year 2049. - if parsedTime.Year() < 2050 { - return errors.New("timestamps prior to 2050 MUST be encoded using UTCTime") - } - default: - return errors.New("unsupported time format") - } - - // Verify that the location is UTC. - if parsedTime.Location() != time.UTC { - return errors.New("time must be in UTC") - } - return nil -} diff --git a/linter/lints/crl/lints_test.go b/linter/lints/crl/lints_test.go deleted file mode 100644 index 9ecff364c..000000000 --- a/linter/lints/crl/lints_test.go +++ /dev/null @@ -1,311 +0,0 @@ -package crl - -import ( - "encoding/pem" - "os" - "testing" - - "github.com/letsencrypt/boulder/crl/crl_x509" - "github.com/letsencrypt/boulder/test" - "github.com/zmap/zlint/v3/lint" -) - -func loadPEMCRL(t *testing.T, filename string) *crl_x509.RevocationList { - t.Helper() - file, err := os.ReadFile(filename) - test.AssertNotError(t, err, "reading CRL file") - block, rest := pem.Decode(file) - test.AssertEquals(t, block.Type, "X509 CRL") - test.AssertEquals(t, len(rest), 0) - crl, err := crl_x509.ParseRevocationList(block.Bytes) - test.AssertNotError(t, err, "parsing CRL bytes") - return crl -} - -func TestHasIssuerName(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasIssuerName(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/no_issuer_name.pem") - res = hasIssuerName(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST have a non-empty issuer") -} - -func TestHasNextUpdate(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasNextUpdate(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/no_next_update.pem") - res = hasNextUpdate(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST include the nextUpdate") -} - -func TestNoEmptyRevokedCertificatesList(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := noEmptyRevokedCertificatesList(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/none_revoked.pem") - res = noEmptyRevokedCertificatesList(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/empty_revoked.pem") - res = noEmptyRevokedCertificatesList(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "must not be present") -} - -func TestHasAKI(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasAKI(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/no_aki.pem") - res = hasAKI(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST include the authority key identifier") - - crl = loadPEMCRL(t, "testdata/aki_name_and_serial.pem") - res = hasAKI(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST use the key identifier method") -} - -func TestHashNumber(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasNumber(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/no_number.pem") - res = hasNumber(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST include the CRL number") - - crl = loadPEMCRL(t, "testdata/critical_number.pem") - res = hasNumber(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT be marked critical") - - crl = loadPEMCRL(t, "testdata/long_number.pem") - res = hasNumber(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT be longer than 20 octets") -} - -func TestIsNotDelta(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := isNotDelta(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/delta.pem") - res = isNotDelta(crl) - test.AssertEquals(t, res.Status, lint.Notice) - test.AssertContains(t, res.Details, "Delta") -} - -func TestCheckIDP(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := checkIDP(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/no_idp.pem") - res = checkIDP(crl) - test.AssertEquals(t, res.Status, lint.Warn) - test.AssertContains(t, res.Details, "missing IDP") - - crl = loadPEMCRL(t, "testdata/idp_no_uri.pem") - res = checkIDP(crl) - test.AssertEquals(t, res.Status, lint.Warn) - test.AssertContains(t, res.Details, "should contain distributionPoint") - - crl = loadPEMCRL(t, "testdata/idp_two_uris.pem") - res = checkIDP(crl) - test.AssertEquals(t, res.Status, lint.Warn) - test.AssertContains(t, res.Details, "only one distributionPoint") - - crl = loadPEMCRL(t, "testdata/idp_no_usercerts.pem") - res = checkIDP(crl) - test.AssertEquals(t, res.Status, lint.Warn) - test.AssertContains(t, res.Details, "should contain onlyContainsUserCerts") - - crl = loadPEMCRL(t, "testdata/idp_some_reasons.pem") - res = checkIDP(crl) - test.AssertEquals(t, res.Status, lint.Warn) - test.AssertContains(t, res.Details, "should not contain fields other than") -} - -func TestHasNoFreshest(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasNoFreshest(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/freshest.pem") - res = hasNoFreshest(crl) - test.AssertEquals(t, res.Status, lint.Notice) - test.AssertContains(t, res.Details, "Freshest") -} - -func TestHasNoAIA(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasNoAIA(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/aia.pem") - res = hasNoAIA(crl) - test.AssertEquals(t, res.Status, lint.Notice) - test.AssertContains(t, res.Details, "Authority Information Access") -} - -func TestHasNoCertIssuers(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasNoCertIssuers(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/cert_issuer.pem") - res = hasNoCertIssuers(crl) - test.AssertEquals(t, res.Status, lint.Notice) - test.AssertContains(t, res.Details, "Certificate Issuer") -} - -func TestHasAcceptableValidity(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasAcceptableValidity(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/negative_validity.pem") - res = hasAcceptableValidity(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "at or before") - - crl = loadPEMCRL(t, "testdata/long_validity.pem") - res = hasAcceptableValidity(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "greater than ten days") -} - -func TestNoZeroReasonCodes(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := noZeroReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/reason_0.pem") - res = noZeroReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT contain the unspecified") -} - -func TestNoCriticalReasons(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := noCriticalReasons(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/critical_reason.pem") - res = noCriticalReasons(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "reasonCodes MUST NOT be critical") -} - -func TestNoCertificateHolds(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := noCertificateHolds(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/reason_6.pem") - res = noCertificateHolds(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT use the certificateHold") -} - -func TestHasMozReasonCodes(t *testing.T) { - // good.pem contains a revocation entry with no reason code extension. - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/reason_0.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT include reasonCodes other than") - - crl = loadPEMCRL(t, "testdata/reason_1.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/reason_2.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT include reasonCodes other than") - - crl = loadPEMCRL(t, "testdata/reason_3.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/reason_4.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/reason_5.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/reason_6.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT include reasonCodes other than") - - crl = loadPEMCRL(t, "testdata/reason_8.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT include reasonCodes other than") - - crl = loadPEMCRL(t, "testdata/reason_9.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - crl = loadPEMCRL(t, "testdata/reason_10.pem") - res = hasMozReasonCodes(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "MUST NOT include reasonCodes other than") -} - -func TestHasValidTimestamps(t *testing.T) { - crl := loadPEMCRL(t, "testdata/good.pem") - res := hasValidTimestamps(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - // Check that 'thisUpdate' of 'UTCTIME 500706164338Z' is considered valid. - crl = loadPEMCRL(t, "testdata/good_utctime_1950.pem") - res = hasValidTimestamps(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - // Check that 'thisUpdate' of 'GENERALIZEDTIME 20500706164338Z' is - // considered valid. - crl = loadPEMCRL(t, "testdata/good_gentime_2050.pem") - res = hasValidTimestamps(crl) - test.AssertEquals(t, res.Status, lint.Pass) - - // Check that 'thisUpdate' of 'GENERALIZEDTIME 20490706164338Z' (before - // 2050) is considered invalid. - crl = loadPEMCRL(t, "testdata/gentime_2049.pem") - res = hasValidTimestamps(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "timestamps prior to 2050 MUST be encoded using UTCTime") - - // Check that 'nextUpdate' of 'UTCTIME 2207061643Z' (missing seconds) is - // considered invalid. - crl = loadPEMCRL(t, "testdata/utctime_no_seconds.pem") - res = hasValidTimestamps(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "timestamps encoded using UTCTime MUST be specified in the format \"YYMMDDHHMMSSZ\"") - - // Check that 'revocationDate' of 'GENERALIZEDTIME 20490706154338Z' (before - // 2050) is considered invalid. - crl = loadPEMCRL(t, "testdata/gentime_revoked_2049.pem") - res = hasValidTimestamps(crl) - test.AssertEquals(t, res.Status, lint.Error) - test.AssertContains(t, res.Details, "timestamps prior to 2050 MUST be encoded using UTCTime") -} diff --git a/linter/lints/crl/testdata/idp.pem b/linter/lints/crl/testdata/idp.pem deleted file mode 100644 index 185e1faac..000000000 --- a/linter/lints/crl/testdata/idp.pem +++ /dev/null @@ -1,10 +0,0 @@ ------BEGIN X509 CRL----- -MIIBYTCB6QIBATAKBggqhkjOPQQDAzBJMQswCQYDVQQGEwJYWDEVMBMGA1UEChMM -Qm91bGRlciBUZXN0MSMwIQYDVQQDExooVEVTVCkgRWxlZ2FudCBFbGVwaGFudCBF -MRcNMjIwNzA2MTY0MzM4WhcNMjIwNzE1MTY0MzM4WjApMCcCCAOuUdtRFVo8Fw0y -MjA3MDYxNTQzMzhaMAwwCgYDVR0VBAMKAQGgRDBCMB8GA1UdIwQYMBaAFAHau3rL -JSCOXnnW+ZZCLwJBKQe+MBEGA1UdFAQKAggW/0sm37IYDzAMBgNVHRwEBTADgQH/ -MAoGCCqGSM49BAMDA2cAMGQCMFayE0WLrRoxaXzYbdPAi7AEEr53OIulDND4vPlN -0/A0RyJiIrgfXEPqsVCweqSoQQIwW7hgsE6Ke7wnxjuxc+jdK7iEyJxbbegQ0eYs -1lDH112u5l4UkOooPYThzlkcUdNC ------END X509 CRL----- diff --git a/linter/lints/crl/testdata/no_next_update.pem b/linter/lints/crl/testdata/no_next_update.pem deleted file mode 100644 index 83d86bd18..000000000 --- a/linter/lints/crl/testdata/no_next_update.pem +++ /dev/null @@ -1,9 +0,0 @@ ------BEGIN X509 CRL----- -MIIBRDCBzAIBATAKBggqhkjOPQQDAzBJMQswCQYDVQQGEwJYWDEVMBMGA1UEChMM -Qm91bGRlciBUZXN0MSMwIQYDVQQDExooVEVTVCkgRWxlZ2FudCBFbGVwaGFudCBF -MRcNMjIwNzA2MTY0MzM4WjApMCcCCAOuUdtRFVo8Fw0yMjA3MDYxNTQzMzhaMAww -CgYDVR0VBAMKAQGgNjA0MB8GA1UdIwQYMBaAFAHau3rLJSCOXnnW+ZZCLwJBKQe+ -MBEGA1UdFAQKAggW/0sm37IYDzAKBggqhkjOPQQDAwNnADBkAjBWshNFi60aMWl8 -2G3TwIuwBBK+dziLpQzQ+Lz5TdPwNEciYiK4H1xD6rFQsHqkqEECMFu4YLBOinu8 -J8Y7sXPo3Su4hMicW23oENHmLNZQx9ddruZeFJDqKD2E4c5ZHFHTQg== ------END X509 CRL----- diff --git a/linter/lints/crl/testdata/reason_1.pem b/linter/lints/crl/testdata/reason_1.pem deleted file mode 100644 index 0331fa9a8..000000000 --- a/linter/lints/crl/testdata/reason_1.pem +++ /dev/null @@ -1,10 +0,0 @@ ------BEGIN X509 CRL----- -MIIBUzCB2wIBATAKBggqhkjOPQQDAzBJMQswCQYDVQQGEwJYWDEVMBMGA1UEChMM -Qm91bGRlciBUZXN0MSMwIQYDVQQDExooVEVTVCkgRWxlZ2FudCBFbGVwaGFudCBF -MRcNMjIwNzA2MTY0MzM4WhcNMjIwNzE1MTY0MzM4WjApMCcCCAOuUdtRFVo8Fw0y -MjA3MDYxNTQzMzhaMAwwCgYDVR0VBAMKAQGgNjA0MB8GA1UdIwQYMBaAFAHau3rL -JSCOXnnW+ZZCLwJBKQe+MBEGA1UdFAQKAggW/0sm37IYDzAKBggqhkjOPQQDAwNn -ADBkAjBWshNFi60aMWl82G3TwIuwBBK+dziLpQzQ+Lz5TdPwNEciYiK4H1xD6rFQ -sHqkqEECMFu4YLBOinu8J8Y7sXPo3Su4hMicW23oENHmLNZQx9ddruZeFJDqKD2E -4c5ZHFHTQg== ------END X509 CRL----- diff --git a/linter/lints/rfc/lint_crl_has_aki.go b/linter/lints/rfc/lint_crl_has_aki.go new file mode 100644 index 000000000..58e7b5c00 --- /dev/null +++ b/linter/lints/rfc/lint_crl_has_aki.go @@ -0,0 +1,62 @@ +package rfc + +import ( + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "github.com/zmap/zlint/v3/util" + "golang.org/x/crypto/cryptobyte" + cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" +) + +type crlHasAKI struct{} + +/************************************************ +RFC 5280: 5.2.1 +Conforming CRL issuers MUST use the key identifier method, and MUST include this +extension in all CRLs issued. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_has_aki", + Description: "Conforming", + Citation: "RFC 5280: 5.2.1", + Source: lint.RFC5280, + EffectiveDate: util.RFC5280Date, + }, + Lint: NewCrlHasAKI, + }) +} + +func NewCrlHasAKI() lint.RevocationListLintInterface { + return &crlHasAKI{} +} + +func (l *crlHasAKI) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlHasAKI) Execute(c *x509.RevocationList) *lint.LintResult { + if len(c.AuthorityKeyId) == 0 { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRLs MUST include the authority key identifier extension", + } + } + aki := cryptobyte.String(c.AuthorityKeyId) + var akiBody cryptobyte.String + if !aki.ReadASN1(&akiBody, cryptobyte_asn1.SEQUENCE) { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRL has a malformed authority key identifier extension", + } + } + if !akiBody.PeekASN1Tag(cryptobyte_asn1.Tag(0).ContextSpecific()) { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRLs MUST use the key identifier method in the authority key identifier extension", + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/rfc/lint_crl_has_aki_test.go b/linter/lints/rfc/lint_crl_has_aki_test.go new file mode 100644 index 000000000..776727df4 --- /dev/null +++ b/linter/lints/rfc/lint_crl_has_aki_test.go @@ -0,0 +1,51 @@ +package rfc + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlHasAKI(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "no_aki", + want: lint.Error, + wantSubStr: "MUST include the authority key identifier", + }, + { + name: "aki_name_and_serial", + want: lint.Error, + wantSubStr: "MUST use the key identifier method", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlHasAKI() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/rfc/lint_crl_has_issuer_name.go b/linter/lints/rfc/lint_crl_has_issuer_name.go new file mode 100644 index 000000000..192d0ebd8 --- /dev/null +++ b/linter/lints/rfc/lint_crl_has_issuer_name.go @@ -0,0 +1,50 @@ +package rfc + +import ( + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "github.com/zmap/zlint/v3/util" +) + +type crlHasIssuerName struct{} + +/************************************************ +RFC 5280: 5.1.2.3 +The issuer field MUST contain a non-empty X.500 distinguished name (DN). + +This lint does not enforce that the issuer field complies with the rest of +the encoding rules of a certificate issuer name, because it (perhaps wrongly) +assumes that those were checked when the issuer was itself issued, and on all +certificates issued by this CRL issuer. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_has_issuer_name", + Description: "The CRL Issuer field MUST contain a non-empty X.500 distinguished name", + Citation: "RFC 5280: 5.1.2.3", + Source: lint.RFC5280, + EffectiveDate: util.RFC5280Date, + }, + Lint: NewCrlHasIssuerName, + }) +} + +func NewCrlHasIssuerName() lint.RevocationListLintInterface { + return &crlHasIssuerName{} +} + +func (l *crlHasIssuerName) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlHasIssuerName) Execute(c *x509.RevocationList) *lint.LintResult { + if len(c.Issuer.Names) == 0 { + return &lint.LintResult{ + Status: lint.Error, + Details: "The CRL Issuer field MUST contain a non-empty X.500 distinguished name", + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/rfc/lint_crl_has_issuer_name_test.go b/linter/lints/rfc/lint_crl_has_issuer_name_test.go new file mode 100644 index 000000000..ef6dcf38d --- /dev/null +++ b/linter/lints/rfc/lint_crl_has_issuer_name_test.go @@ -0,0 +1,46 @@ +package rfc + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlHasIssuerName(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "no_issuer_name", + want: lint.Error, + wantSubStr: "MUST contain a non-empty X.500 distinguished name", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlHasIssuerName() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/rfc/lint_crl_has_number.go b/linter/lints/rfc/lint_crl_has_number.go new file mode 100644 index 000000000..3120abd11 --- /dev/null +++ b/linter/lints/rfc/lint_crl_has_number.go @@ -0,0 +1,67 @@ +package rfc + +import ( + "github.com/zmap/zcrypto/encoding/asn1" + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "github.com/zmap/zlint/v3/util" + + "github.com/letsencrypt/boulder/linter/lints" +) + +type crlHasNumber struct{} + +/************************************************ +RFC 5280: 5.2.3 +CRL issuers conforming to this profile MUST include this extension in all CRLs +and MUST mark this extension as non-critical. Conforming CRL issuers MUST NOT +use CRLNumber values longer than 20 octets. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_has_number", + Description: "CRLs must have a well-formed CRL Number extension", + Citation: "RFC 5280: 5.2.3", + Source: lint.RFC5280, + EffectiveDate: util.RFC5280Date, + }, + Lint: NewCrlHasNumber, + }) +} + +func NewCrlHasNumber() lint.RevocationListLintInterface { + return &crlHasNumber{} +} + +func (l *crlHasNumber) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlHasNumber) Execute(c *x509.RevocationList) *lint.LintResult { + if c.Number == nil { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRLs MUST include the CRL number extension", + } + } + + crlNumberOID := asn1.ObjectIdentifier{2, 5, 29, 20} // id-ce-cRLNumber + ext := lints.GetExtWithOID(c.Extensions, crlNumberOID) + if ext != nil && ext.Critical { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRL Number MUST NOT be marked critical", + } + } + + numBytes := c.Number.Bytes() + if len(numBytes) > 20 || (len(numBytes) == 20 && numBytes[0]&0x80 != 0) { + return &lint.LintResult{ + Status: lint.Error, + Details: "CRL Number MUST NOT be longer than 20 octets", + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/rfc/lint_crl_has_number_test.go b/linter/lints/rfc/lint_crl_has_number_test.go new file mode 100644 index 000000000..a9225aeac --- /dev/null +++ b/linter/lints/rfc/lint_crl_has_number_test.go @@ -0,0 +1,56 @@ +package rfc + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlHasNumber(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "no_number", + want: lint.Error, + wantSubStr: "MUST include the CRL number", + }, + { + name: "critical_number", + want: lint.Error, + wantSubStr: "MUST NOT be marked critical", + }, + { + name: "long_number", + want: lint.Error, + wantSubStr: "MUST NOT be longer than 20 octets", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlHasNumber() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/rfc/lint_crl_has_valid_timestamps.go b/linter/lints/rfc/lint_crl_has_valid_timestamps.go new file mode 100644 index 000000000..0546d62c5 --- /dev/null +++ b/linter/lints/rfc/lint_crl_has_valid_timestamps.go @@ -0,0 +1,230 @@ +package rfc + +import ( + "errors" + "fmt" + "time" + + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "github.com/zmap/zlint/v3/util" + "golang.org/x/crypto/cryptobyte" + cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" +) + +const ( + utcTimeFormat = "YYMMDDHHMMSSZ" + generalizedTimeFormat = "YYYYMMDDHHMMSSZ" +) + +type crlHasValidTimestamps struct{} + +/************************************************ +RFC 5280: 5.1.2.4 +CRL issuers conforming to this profile MUST encode thisUpdate as UTCTime for +dates through the year 2049. CRL issuers conforming to this profile MUST encode +thisUpdate as GeneralizedTime for dates in the year 2050 or later. Conforming +applications MUST be able to process dates that are encoded in either UTCTime or +GeneralizedTime. + +Where encoded as UTCTime, thisUpdate MUST be specified and interpreted as +defined in Section 4.1.2.5.1. Where encoded as GeneralizedTime, thisUpdate MUST +be specified and interpreted as defined in Section 4.1.2.5.2. + +RFC 5280: 5.1.2.5 +CRL issuers conforming to this profile MUST encode nextUpdate as UTCTime for +dates through the year 2049. CRL issuers conforming to this profile MUST encode +nextUpdate as GeneralizedTime for dates in the year 2050 or later. Conforming +applications MUST be able to process dates that are encoded in either UTCTime or +GeneralizedTime. + +Where encoded as UTCTime, nextUpdate MUST be specified and interpreted as +defined in Section 4.1.2.5.1. Where encoded as GeneralizedTime, nextUpdate MUST +be specified and interpreted as defined in Section 4.1.2.5.2. + +RFC 5280: 5.1.2.6 +The time for revocationDate MUST be expressed as described in Section 5.1.2.4. + +RFC 5280: 4.1.2.5.1 +UTCTime values MUST be expressed in Greenwich Mean Time (Zulu) and MUST include +seconds (i.e., times are YYMMDDHHMMSSZ), even where the number of seconds is +zero. + +RFC 5280: 4.1.2.5.2 +GeneralizedTime values MUST be expressed in Greenwich Mean Time (Zulu) and MUST +include seconds (i.e., times are YYYYMMDDHHMMSSZ), even where the number of +seconds is zero. GeneralizedTime values MUST NOT include fractional seconds. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_has_valid_timestamps", + Description: "CRL thisUpdate, nextUpdate, and revocationDates must be properly encoded", + Citation: "RFC 5280: 5.1.2.4, 5.1.2.5, and 5.1.2.6", + Source: lint.RFC5280, + EffectiveDate: util.RFC5280Date, + }, + Lint: NewCrlHasValidTimestamps, + }) +} + +func NewCrlHasValidTimestamps() lint.RevocationListLintInterface { + return &crlHasValidTimestamps{} +} + +func (l *crlHasValidTimestamps) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlHasValidTimestamps) Execute(c *x509.RevocationList) *lint.LintResult { + input := cryptobyte.String(c.RawTBSRevocationList) + lintFail := lint.LintResult{ + Status: lint.Error, + Details: "Failed to re-parse tbsCertList during linting", + } + + // Read tbsCertList. + var tbs cryptobyte.String + if !input.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) { + return &lintFail + } + + // Skip (optional) version. + if !tbs.SkipOptionalASN1(cryptobyte_asn1.INTEGER) { + return &lintFail + } + + // Skip signature. + if !tbs.SkipASN1(cryptobyte_asn1.SEQUENCE) { + return &lintFail + } + + // Skip issuer. + if !tbs.SkipASN1(cryptobyte_asn1.SEQUENCE) { + return &lintFail + } + + // Read thisUpdate. + var thisUpdate cryptobyte.String + var thisUpdateTag cryptobyte_asn1.Tag + if !tbs.ReadAnyASN1Element(&thisUpdate, &thisUpdateTag) { + return &lintFail + } + + // Lint thisUpdate. + err := lintTimestamp(&thisUpdate, thisUpdateTag) + if err != nil { + return &lint.LintResult{Status: lint.Error, Details: err.Error()} + } + + // Peek (optional) nextUpdate. + if tbs.PeekASN1Tag(cryptobyte_asn1.UTCTime) || tbs.PeekASN1Tag(cryptobyte_asn1.GeneralizedTime) { + // Read nextUpdate. + var nextUpdate cryptobyte.String + var nextUpdateTag cryptobyte_asn1.Tag + if !tbs.ReadAnyASN1Element(&nextUpdate, &nextUpdateTag) { + return &lintFail + } + + // Lint nextUpdate. + err = lintTimestamp(&nextUpdate, nextUpdateTag) + if err != nil { + return &lint.LintResult{Status: lint.Error, Details: err.Error()} + } + } + + // Peek (optional) revokedCertificates. + if tbs.PeekASN1Tag(cryptobyte_asn1.SEQUENCE) { + // Read sequence of revokedCertificate. + var revokedSeq cryptobyte.String + if !tbs.ReadASN1(&revokedSeq, cryptobyte_asn1.SEQUENCE) { + return &lintFail + } + + // Iterate over each revokedCertificate sequence. + for !revokedSeq.Empty() { + // Read revokedCertificate. + var certSeq cryptobyte.String + if !revokedSeq.ReadASN1Element(&certSeq, cryptobyte_asn1.SEQUENCE) { + return &lintFail + } + + if !certSeq.ReadASN1(&certSeq, cryptobyte_asn1.SEQUENCE) { + return &lintFail + } + + // Skip userCertificate (serial number). + if !certSeq.SkipASN1(cryptobyte_asn1.INTEGER) { + return &lintFail + } + + // Read revocationDate. + var revocationDate cryptobyte.String + var revocationDateTag cryptobyte_asn1.Tag + if !certSeq.ReadAnyASN1Element(&revocationDate, &revocationDateTag) { + return &lintFail + } + + // Lint revocationDate. + err = lintTimestamp(&revocationDate, revocationDateTag) + if err != nil { + return &lint.LintResult{Status: lint.Error, Details: err.Error()} + } + } + } + return &lint.LintResult{Status: lint.Pass} +} + +func lintTimestamp(der *cryptobyte.String, tag cryptobyte_asn1.Tag) error { + // Preserve the original timestamp for length checking. + derBytes := *der + var tsBytes cryptobyte.String + if !derBytes.ReadASN1(&tsBytes, tag) { + return errors.New("failed to read timestamp") + } + tsLen := len(string(tsBytes)) + + var parsedTime time.Time + switch tag { + case cryptobyte_asn1.UTCTime: + // Verify that the timestamp is properly formatted. + if tsLen != len(utcTimeFormat) { + return fmt.Errorf("timestamps encoded using UTCTime MUST be specified in the format %q", utcTimeFormat) + } + + if !der.ReadASN1UTCTime(&parsedTime) { + return errors.New("failed to read timestamp encoded using UTCTime") + } + + // Verify that the timestamp is prior to the year 2050. This should + // really never happen. + if parsedTime.Year() > 2049 { + return errors.New("ReadASN1UTCTime returned a UTCTime after 2049") + } + case cryptobyte_asn1.GeneralizedTime: + // Verify that the timestamp is properly formatted. + if tsLen != len(generalizedTimeFormat) { + return fmt.Errorf( + "timestamps encoded using GeneralizedTime MUST be specified in the format %q", generalizedTimeFormat, + ) + } + + if !der.ReadASN1GeneralizedTime(&parsedTime) { + return fmt.Errorf("failed to read timestamp encoded using GeneralizedTime") + } + + // Verify that the timestamp occurred after the year 2049. + if parsedTime.Year() < 2050 { + return errors.New("timestamps prior to 2050 MUST be encoded using UTCTime") + } + default: + return errors.New("unsupported time format") + } + + // Verify that the location is UTC. + if parsedTime.Location() != time.UTC { + return errors.New("time must be in UTC") + } + return nil +} diff --git a/linter/lints/rfc/lint_crl_has_valid_timestamps_test.go b/linter/lints/rfc/lint_crl_has_valid_timestamps_test.go new file mode 100644 index 000000000..137ab89fa --- /dev/null +++ b/linter/lints/rfc/lint_crl_has_valid_timestamps_test.go @@ -0,0 +1,64 @@ +package rfc + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlHasValidTimestamps(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "good_utctime_1950", + want: lint.Pass, + }, + { + name: "good_gentime_2050", + want: lint.Pass, + }, + { + name: "gentime_2049", + want: lint.Error, + wantSubStr: "timestamps prior to 2050 MUST be encoded using UTCTime", + }, + { + name: "utctime_no_seconds", + want: lint.Error, + wantSubStr: "timestamps encoded using UTCTime MUST be specified in the format \"YYMMDDHHMMSSZ\"", + }, + { + name: "gentime_revoked_2049", + want: lint.Error, + wantSubStr: "timestamps prior to 2050 MUST be encoded using UTCTime", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlHasValidTimestamps() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/rfc/lint_crl_no_empty_revoked_certificates_list.go b/linter/lints/rfc/lint_crl_no_empty_revoked_certificates_list.go new file mode 100644 index 000000000..c1d865507 --- /dev/null +++ b/linter/lints/rfc/lint_crl_no_empty_revoked_certificates_list.go @@ -0,0 +1,47 @@ +package rfc + +import ( + "github.com/zmap/zcrypto/x509" + "github.com/zmap/zlint/v3/lint" + "github.com/zmap/zlint/v3/util" +) + +type crlNoEmptyRevokedCertsList struct{} + +/************************************************ +RFC 5280: 5.1.2.6 +When there are no revoked certificates, the revoked certificates list MUST be +absent. +************************************************/ + +func init() { + lint.RegisterRevocationListLint(&lint.RevocationListLint{ + LintMetadata: lint.LintMetadata{ + Name: "e_crl_no_empty_revoked_certificates_list", + Description: "When there are no revoked certificates, the revoked certificates list MUST be absent.", + Citation: "RFC 5280: 5.1.2.6", + Source: lint.RFC5280, + EffectiveDate: util.RFC5280Date, + }, + Lint: NewCrlNoEmptyRevokedCertsList, + }) +} + +func NewCrlNoEmptyRevokedCertsList() lint.RevocationListLintInterface { + return &crlNoEmptyRevokedCertsList{} +} + +func (l *crlNoEmptyRevokedCertsList) CheckApplies(c *x509.RevocationList) bool { + return true +} + +func (l *crlNoEmptyRevokedCertsList) Execute(c *x509.RevocationList) *lint.LintResult { + // TODO(#6741): Rewrite this lint because upstream does not make this distinction. + if c.RevokedCertificates != nil && len(c.RevokedCertificates) == 0 { + return &lint.LintResult{ + Status: lint.Error, + Details: "If the revokedCertificates list is empty, it must not be present", + } + } + return &lint.LintResult{Status: lint.Pass} +} diff --git a/linter/lints/rfc/lint_crl_no_empty_revoked_certificates_list_test.go b/linter/lints/rfc/lint_crl_no_empty_revoked_certificates_list_test.go new file mode 100644 index 000000000..d0361a812 --- /dev/null +++ b/linter/lints/rfc/lint_crl_no_empty_revoked_certificates_list_test.go @@ -0,0 +1,50 @@ +package rfc + +import ( + "fmt" + "strings" + "testing" + + "github.com/zmap/zlint/v3/lint" + + "github.com/letsencrypt/boulder/linter/lints/test" +) + +func TestCrlNoEmptyRevokedCertsList(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + want lint.LintStatus + wantSubStr string + }{ + { + name: "good", + want: lint.Pass, + }, + { + name: "none_revoked", + want: lint.Pass, + }, + { + name: "empty_revoked", + want: lint.Error, + wantSubStr: "must not be present", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + l := NewCrlNoEmptyRevokedCertsList() + c := test.LoadPEMCRL(t, fmt.Sprintf("testdata/crl_%s.pem", tc.name)) + r := l.Execute(c) + + if r.Status != tc.want { + t.Errorf("expected %q, got %q", tc.want, r.Status) + } + if !strings.Contains(r.Details, tc.wantSubStr) { + t.Errorf("expected %q, got %q", tc.wantSubStr, r.Details) + } + }) + } +} diff --git a/linter/lints/crl/testdata/aki_name_and_serial.pem b/linter/lints/rfc/testdata/crl_aki_name_and_serial.pem similarity index 100% rename from linter/lints/crl/testdata/aki_name_and_serial.pem rename to linter/lints/rfc/testdata/crl_aki_name_and_serial.pem diff --git a/linter/lints/crl/testdata/critical_number.pem b/linter/lints/rfc/testdata/crl_critical_number.pem similarity index 100% rename from linter/lints/crl/testdata/critical_number.pem rename to linter/lints/rfc/testdata/crl_critical_number.pem diff --git a/linter/lints/crl/testdata/empty_revoked.pem b/linter/lints/rfc/testdata/crl_empty_revoked.pem similarity index 100% rename from linter/lints/crl/testdata/empty_revoked.pem rename to linter/lints/rfc/testdata/crl_empty_revoked.pem diff --git a/linter/lints/crl/testdata/gentime_2049.pem b/linter/lints/rfc/testdata/crl_gentime_2049.pem similarity index 100% rename from linter/lints/crl/testdata/gentime_2049.pem rename to linter/lints/rfc/testdata/crl_gentime_2049.pem diff --git a/linter/lints/crl/testdata/gentime_revoked_2049.pem b/linter/lints/rfc/testdata/crl_gentime_revoked_2049.pem similarity index 100% rename from linter/lints/crl/testdata/gentime_revoked_2049.pem rename to linter/lints/rfc/testdata/crl_gentime_revoked_2049.pem diff --git a/linter/lints/rfc/testdata/crl_good.pem b/linter/lints/rfc/testdata/crl_good.pem new file mode 100644 index 000000000..8b383d0a0 --- /dev/null +++ b/linter/lints/rfc/testdata/crl_good.pem @@ -0,0 +1,11 @@ +-----BEGIN X509 CRL----- +MIIBmDCCAR8CAQEwCgYIKoZIzj0EAwMwSTELMAkGA1UEBhMCWFgxFTATBgNVBAoT +DEJvdWxkZXIgVGVzdDEjMCEGA1UEAxMaKFRFU1QpIEVsZWdhbnQgRWxlcGhhbnQg +RTEXDTIyMTAxMDIwMTIwN1oXDTIyMTAxOTIwMTIwNlowKTAnAggDrlHbURVaPBcN +MjIxMDEwMTkxMjA3WjAMMAoGA1UdFQQDCgEBoHoweDAfBgNVHSMEGDAWgBQB2rt6 +yyUgjl551vmWQi8CQSkHvjARBgNVHRQECgIIFxzOPeSCumEwQgYDVR0cAQH/BDgw +NqAxoC+GLWh0dHA6Ly9jLmJvdWxkZXIudGVzdC82NjI4Mzc1NjkxMzU4ODI4OC8w +LmNybIEB/zAKBggqhkjOPQQDAwNnADBkAjAvDkIUnTYavJ6h8606MDyFh2uw/cF+ +OVnM4sE8nUdGy0XYg0hGfbR4MY+kRxRQayICMFeQPpcpIr0zgXpP6lUXU0rcLSva +tuaeQSVr24nGjZ7Py0vc94w0n7idZ8wje5+/Mw== +-----END X509 CRL----- diff --git a/linter/lints/crl/testdata/good_gentime_2050.pem b/linter/lints/rfc/testdata/crl_good_gentime_2050.pem similarity index 100% rename from linter/lints/crl/testdata/good_gentime_2050.pem rename to linter/lints/rfc/testdata/crl_good_gentime_2050.pem diff --git a/linter/lints/crl/testdata/good_utctime_1950.pem b/linter/lints/rfc/testdata/crl_good_utctime_1950.pem similarity index 100% rename from linter/lints/crl/testdata/good_utctime_1950.pem rename to linter/lints/rfc/testdata/crl_good_utctime_1950.pem diff --git a/linter/lints/crl/testdata/long_number.pem b/linter/lints/rfc/testdata/crl_long_number.pem similarity index 100% rename from linter/lints/crl/testdata/long_number.pem rename to linter/lints/rfc/testdata/crl_long_number.pem diff --git a/linter/lints/crl/testdata/no_aki.pem b/linter/lints/rfc/testdata/crl_no_aki.pem similarity index 100% rename from linter/lints/crl/testdata/no_aki.pem rename to linter/lints/rfc/testdata/crl_no_aki.pem diff --git a/linter/lints/crl/testdata/no_issuer_name.pem b/linter/lints/rfc/testdata/crl_no_issuer_name.pem similarity index 100% rename from linter/lints/crl/testdata/no_issuer_name.pem rename to linter/lints/rfc/testdata/crl_no_issuer_name.pem diff --git a/linter/lints/crl/testdata/no_number.pem b/linter/lints/rfc/testdata/crl_no_number.pem similarity index 100% rename from linter/lints/crl/testdata/no_number.pem rename to linter/lints/rfc/testdata/crl_no_number.pem diff --git a/linter/lints/crl/testdata/none_revoked.pem b/linter/lints/rfc/testdata/crl_none_revoked.pem similarity index 100% rename from linter/lints/crl/testdata/none_revoked.pem rename to linter/lints/rfc/testdata/crl_none_revoked.pem diff --git a/linter/lints/crl/testdata/utctime_no_seconds.pem b/linter/lints/rfc/testdata/crl_utctime_no_seconds.pem similarity index 100% rename from linter/lints/crl/testdata/utctime_no_seconds.pem rename to linter/lints/rfc/testdata/crl_utctime_no_seconds.pem diff --git a/linter/lints/crl/testdata/README.md b/linter/lints/test/README.md similarity index 100% rename from linter/lints/crl/testdata/README.md rename to linter/lints/test/README.md diff --git a/linter/lints/test/helpers.go b/linter/lints/test/helpers.go new file mode 100644 index 000000000..55badf8be --- /dev/null +++ b/linter/lints/test/helpers.go @@ -0,0 +1,23 @@ +package test + +import ( + "encoding/pem" + "os" + "testing" + + "github.com/zmap/zcrypto/x509" + + "github.com/letsencrypt/boulder/test" +) + +func LoadPEMCRL(t *testing.T, filename string) *x509.RevocationList { + t.Helper() + file, err := os.ReadFile(filename) + test.AssertNotError(t, err, "reading CRL file") + block, rest := pem.Decode(file) + test.AssertEquals(t, block.Type, "X509 CRL") + test.AssertEquals(t, len(rest), 0) + crl, err := x509.ParseRevocationList(block.Bytes) + test.AssertNotError(t, err, "parsing CRL bytes") + return crl +}