diff --git a/cmd/ceremony/main.go b/cmd/ceremony/main.go index 0ee69aa9fb7..1a8a390457f 100644 --- a/cmd/ceremony/main.go +++ b/cmd/ceremony/main.go @@ -94,10 +94,14 @@ func postIssuanceLinting(fc, issuer, existing *x509.Certificate, skipLints []str if err != nil { return fmt.Errorf("unable to create lint config: %s", err) } - registry, err := linter.NewRegistryWithConfig(skipLints, lintConfig) + registry, err := linter.NewRegistry(skipLints) if err != nil { return fmt.Errorf("unable to create zlint registry: %s", err) } + registry, err = linter.ConfigureRegistry(registry, lintConfig) + if err != nil { + return fmt.Errorf("unable to configure zlint registry: %s", err) + } lintRes := zlint.LintCertificateEx(parsed, registry) err = linter.ProcessResultSet(lintRes) if err != nil { diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go index 4e8ba80aa56..e327df76828 100644 --- a/cmd/cert-checker/main.go +++ b/cmd/cert-checker/main.go @@ -32,6 +32,7 @@ import ( "github.com/letsencrypt/boulder/goodkey" "github.com/letsencrypt/boulder/goodkey/sagoodkey" "github.com/letsencrypt/boulder/identifier" + "github.com/letsencrypt/boulder/issuance" "github.com/letsencrypt/boulder/linter" blog "github.com/letsencrypt/boulder/log" "github.com/letsencrypt/boulder/policy" @@ -125,7 +126,9 @@ type certChecker struct { issuedReport report checkPeriod time.Duration acceptableValidityDurations map[time.Duration]bool + issuers map[string]*issuance.Certificate lints lint.Registry + lintConfig linter.Config logger blog.Logger } @@ -135,7 +138,9 @@ func newChecker(saDbMap certDB, kp goodkey.KeyPolicy, period time.Duration, avd map[time.Duration]bool, + issuers map[string]*issuance.Certificate, lints lint.Registry, + lintConfig linter.Config, logger blog.Logger, ) certChecker { precertGetter := func(ctx context.Context, serial string) ([]byte, error) { @@ -154,7 +159,9 @@ func newChecker(saDbMap certDB, clock: clk, checkPeriod: period, acceptableValidityDurations: avd, + issuers: issuers, lints: lints, + lintConfig: lintConfig, logger: logger, } } @@ -373,8 +380,29 @@ func (c *certChecker) checkCert(ctx context.Context, cert *corepb.Certificate) ( sans = append(sans, ip.String()) } + // Configure zlint. + lintConfig := c.lintConfig + if len(c.issuers) > 0 { + issuer, ok := c.issuers[parsedCert.Issuer.CommonName] + if !ok { + problems = append(problems, fmt.Sprintf("Unrecognized issuer: %q", parsedCert.Issuer.CommonName)) + return nil, problems + } + + lintConfig, err = c.lintConfig.WithIssuer(issuer.Certificate) + if err != nil { + problems = append(problems, "Couldn't configure lints with issuer") + return nil, problems + } + } + registry, err := linter.ConfigureRegistry(c.lints, lintConfig) + if err != nil { + problems = append(problems, "Couldn't create lint registry") + return nil, problems + } + // Run zlint checks. - results := zlint.LintCertificateEx(parsedCert, c.lints) + results := zlint.LintCertificateEx(parsedCert, registry) for name, res := range results.Results { if res.Status <= lint.Pass { continue @@ -592,6 +620,12 @@ type Config struct { // configured to submit SCTs to test logs. CTIncludeTestLogs bool + // IssuerCerts are paths to all intermediate certificates which may have + // been used to issue certificates in the last 90 days. These are used to + // configure our CP/CPS-specific lints. + // TODO(#5492): Change this to `"min=1,dive,required"` + IssuerCerts []string `validate:"omitempty"` + Features features.Config } PA cmd.PAConfig @@ -699,10 +733,18 @@ func main() { lints, err := linter.NewRegistry(config.CertChecker.IgnoredLints) cmd.FailOnError(err, "Failed to create zlint registry") + + lintConfig := linter.Config{} if config.CertChecker.LintConfig != "" { - lintconfig, err := lint.NewConfigFromFile(config.CertChecker.LintConfig) + lintConfig, err = linter.LoadConfigFile(config.CertChecker.LintConfig) cmd.FailOnError(err, "Failed to load zlint config file") - lints.SetConfiguration(lintconfig) + } + + issuers := make(map[string]*issuance.Certificate) + for _, issuerCertPath := range config.CertChecker.IssuerCerts { + issuer, err := issuance.LoadCertificate(issuerCertPath) + cmd.FailOnError(err, "Failed to load issuer cert file") + issuers[issuer.Subject.CommonName] = issuer } checker := newChecker( @@ -712,7 +754,9 @@ func main() { kp, config.CertChecker.CheckPeriod.Duration, acceptableValidityDurations, + issuers, lints, + lintConfig, logger, ) fmt.Fprintf(os.Stderr, "# Getting certificates issued in the last %s\n", config.CertChecker.CheckPeriod) diff --git a/cmd/cert-checker/main_test.go b/cmd/cert-checker/main_test.go index 47d49994d33..9721e164ce5 100644 --- a/cmd/cert-checker/main_test.go +++ b/cmd/cert-checker/main_test.go @@ -72,7 +72,7 @@ func init() { } func BenchmarkCheckCert(b *testing.B) { - checker := newChecker(nil, clock.New(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(nil, clock.New(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) testKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) expiry := time.Now().AddDate(0, 0, 1) serial := big.NewInt(1337) @@ -108,7 +108,7 @@ func TestCheckWildcardCert(t *testing.T) { testKey, _ := rsa.GenerateKey(rand.Reader, 2048) fc := clock.NewFake() - checker := newChecker(saDbMap, fc, pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(saDbMap, fc, pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) issued := checker.clock.Now().Add(-time.Minute) goodExpiry := issued.Add(testValidityDuration - time.Second) serial := big.NewInt(1337) @@ -151,7 +151,7 @@ func TestCheckCertReturnsSANs(t *testing.T) { defer func() { saCleanup() }() - checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) certPEM, err := os.ReadFile("testdata/quite_invalid.pem") if err != nil { @@ -218,7 +218,7 @@ func TestCheckCert(t *testing.T) { t.Run(tc.name, func(t *testing.T) { testKey, _ := tc.key.genKey() - checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) // Create a RFC 7633 OCSP Must Staple Extension. // OID 1.3.6.1.5.5.7.1.24 @@ -340,7 +340,7 @@ func TestGetAndProcessCerts(t *testing.T) { fc.Set(fc.Now().Add(time.Hour)) mocklog := blog.NewMock() - checker := newChecker(saDbMap, fc, pa, kp, time.Hour, testValidityDurations, nil, mocklog) + checker := newChecker(saDbMap, fc, pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, mocklog) sa, err := sa.NewSQLStorageAuthority(saDbMap, saDbMap, nil, 0, fc, blog.NewMock(), metrics.NoopRegisterer) test.AssertNotError(t, err, "Couldn't create SA to insert certificates") saCleanUp := test.ResetBoulderTestDatabase(t) @@ -426,7 +426,7 @@ func (db mismatchedCountDB) SelectOne(_ context.Context, holder any, _ string, _ func TestGetCertsEmptyResults(t *testing.T) { saDbMap, err := sa.DBMapForTest(vars.DBConnSA) test.AssertNotError(t, err, "Couldn't connect to database") - checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) checker.dbMap = mismatchedCountDB{} batchSize = 3 @@ -453,7 +453,7 @@ func (db emptyDB) SelectOne(_ context.Context, holder any, _ string, _ ...any) e // expected if the DB finds no certificates to match the SELECT query and // should return an error. func TestGetCertsNullResults(t *testing.T) { - checker := newChecker(emptyDB{}, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(emptyDB{}, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) err := checker.getCerts(context.Background()) test.AssertError(t, err, "Should have gotten error from empty DB") @@ -499,7 +499,7 @@ func TestGetCertsLate(t *testing.T) { clk := clock.NewFake() db := &lateDB{issuedTime: clk.Now().Add(-time.Hour)} checkPeriod := 24 * time.Hour - checker := newChecker(db, clk, pa, kp, checkPeriod, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(db, clk, pa, kp, checkPeriod, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) err := checker.getCerts(context.Background()) test.AssertNotError(t, err, "getting certs") @@ -560,7 +560,7 @@ func TestIgnoredLint(t *testing.T) { err = loglist.InitLintList("../../test/ct-test-srv/log_list.json", false) test.AssertNotError(t, err, "failed to load ct log list") testKey, _ := rsa.GenerateKey(rand.Reader, 2048) - checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) serial := big.NewInt(1337) x509OID, err := x509.OIDFromInts([]uint64{1, 2, 3}) @@ -642,7 +642,7 @@ func TestIgnoredLint(t *testing.T) { } func TestPrecertCorrespond(t *testing.T) { - checker := newChecker(nil, clock.New(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock()) + checker := newChecker(nil, clock.New(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock()) checker.getPrecert = func(_ context.Context, _ string) ([]byte, error) { return []byte("hello"), nil } diff --git a/linter/config.go b/linter/config.go index 4e49e99c7a4..6ea5da03c96 100644 --- a/linter/config.go +++ b/linter/config.go @@ -106,12 +106,15 @@ func (r configuredRegistry) GetConfiguration() lint.Configuration { return r.config } -// NewRegistryWithConfig is like NewRegistry, but the returned registry also -// carries the contents of the given Config. -func NewRegistryWithConfig(skipLints []string, config Config) (lint.Registry, error) { - reg, err := NewRegistry(skipLints) - if err != nil { - return nil, err +// ConfigureRegistry combines the given registry with the given config. This is +// useful for reusing the same registry (which can be expensive to build) with +// many different configs. +func ConfigureRegistry(reg lint.Registry, config Config) (lint.Registry, error) { + if reg == nil { + // Normally zlint would replace a nil registry with the global registry on + // its own, but of course a configuredRegistry *isn't* nil even if the + // registry it wraps is, so we need to do this fallback ourselves. + reg = lint.GlobalRegistry() } lintConfig, err := config.build() if err != nil { diff --git a/test/config-next/cert-checker.json b/test/config-next/cert-checker.json index e046cccc08c..6c463003a14 100644 --- a/test/config-next/cert-checker.json +++ b/test/config-next/cert-checker.json @@ -14,6 +14,14 @@ "1080h", "160h" ], + "issuerCerts": [ + "test/certs/webpki/int-rsa-a.cert.pem", + "test/certs/webpki/int-rsa-b.cert.pem", + "test/certs/webpki/int-rsa-c.cert.pem", + "test/certs/webpki/int-ecdsa-a.cert.pem", + "test/certs/webpki/int-ecdsa-b.cert.pem", + "test/certs/webpki/int-ecdsa-c.cert.pem" + ], "lintConfig": "test/config-next/zlint.toml", "ignoredLints": [ "w_ext_subject_key_identifier_missing_sub_cert",