Skip to content

Commit ac88671

Browse files
authored
cert-checker: configure issuers for CP/CPS lints (#8927)
Add a new config field to cert-checker: `issuerCerts`, identical to the config field of the same name in the RA. Use this field to look up the intermediate CA certificate which issued each cert that cert-checker checks, and to add that issuer to our custom linter config, just like the CA and ceremony tool already do (as of #8923). This will allow the cert-checker to successfully run our upcoming custom CP/CPS lints, which require that the issuer be configured. To facilitate this new functionality, slightly refactor `linter.NewRegistryWithConfig` so that it is usable by both the ceremony tool and cert-checker.
1 parent d551d4b commit ac88671

5 files changed

Lines changed: 79 additions & 20 deletions

File tree

cmd/ceremony/main.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,14 @@ func postIssuanceLinting(fc, issuer, existing *x509.Certificate, skipLints []str
9494
if err != nil {
9595
return fmt.Errorf("unable to create lint config: %s", err)
9696
}
97-
registry, err := linter.NewRegistryWithConfig(skipLints, lintConfig)
97+
registry, err := linter.NewRegistry(skipLints)
9898
if err != nil {
9999
return fmt.Errorf("unable to create zlint registry: %s", err)
100100
}
101+
registry, err = linter.ConfigureRegistry(registry, lintConfig)
102+
if err != nil {
103+
return fmt.Errorf("unable to configure zlint registry: %s", err)
104+
}
101105
lintRes := zlint.LintCertificateEx(parsed, registry)
102106
err = linter.ProcessResultSet(lintRes)
103107
if err != nil {

cmd/cert-checker/main.go

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/letsencrypt/boulder/goodkey"
3333
"github.com/letsencrypt/boulder/goodkey/sagoodkey"
3434
"github.com/letsencrypt/boulder/identifier"
35+
"github.com/letsencrypt/boulder/issuance"
3536
"github.com/letsencrypt/boulder/linter"
3637
blog "github.com/letsencrypt/boulder/log"
3738
"github.com/letsencrypt/boulder/policy"
@@ -125,7 +126,9 @@ type certChecker struct {
125126
issuedReport report
126127
checkPeriod time.Duration
127128
acceptableValidityDurations map[time.Duration]bool
129+
issuers map[string]*issuance.Certificate
128130
lints lint.Registry
131+
lintConfig linter.Config
129132
logger blog.Logger
130133
}
131134

@@ -135,7 +138,9 @@ func newChecker(saDbMap certDB,
135138
kp goodkey.KeyPolicy,
136139
period time.Duration,
137140
avd map[time.Duration]bool,
141+
issuers map[string]*issuance.Certificate,
138142
lints lint.Registry,
143+
lintConfig linter.Config,
139144
logger blog.Logger,
140145
) certChecker {
141146
precertGetter := func(ctx context.Context, serial string) ([]byte, error) {
@@ -154,7 +159,9 @@ func newChecker(saDbMap certDB,
154159
clock: clk,
155160
checkPeriod: period,
156161
acceptableValidityDurations: avd,
162+
issuers: issuers,
157163
lints: lints,
164+
lintConfig: lintConfig,
158165
logger: logger,
159166
}
160167
}
@@ -373,8 +380,29 @@ func (c *certChecker) checkCert(ctx context.Context, cert *corepb.Certificate) (
373380
sans = append(sans, ip.String())
374381
}
375382

383+
// Configure zlint.
384+
lintConfig := c.lintConfig
385+
if len(c.issuers) > 0 {
386+
issuer, ok := c.issuers[parsedCert.Issuer.CommonName]
387+
if !ok {
388+
problems = append(problems, fmt.Sprintf("Unrecognized issuer: %q", parsedCert.Issuer.CommonName))
389+
return nil, problems
390+
}
391+
392+
lintConfig, err = c.lintConfig.WithIssuer(issuer.Certificate)
393+
if err != nil {
394+
problems = append(problems, "Couldn't configure lints with issuer")
395+
return nil, problems
396+
}
397+
}
398+
registry, err := linter.ConfigureRegistry(c.lints, lintConfig)
399+
if err != nil {
400+
problems = append(problems, "Couldn't create lint registry")
401+
return nil, problems
402+
}
403+
376404
// Run zlint checks.
377-
results := zlint.LintCertificateEx(parsedCert, c.lints)
405+
results := zlint.LintCertificateEx(parsedCert, registry)
378406
for name, res := range results.Results {
379407
if res.Status <= lint.Pass {
380408
continue
@@ -592,6 +620,12 @@ type Config struct {
592620
// configured to submit SCTs to test logs.
593621
CTIncludeTestLogs bool
594622

623+
// IssuerCerts are paths to all intermediate certificates which may have
624+
// been used to issue certificates in the last 90 days. These are used to
625+
// configure our CP/CPS-specific lints.
626+
// TODO(#5492): Change this to `"min=1,dive,required"`
627+
IssuerCerts []string `validate:"omitempty"`
628+
595629
Features features.Config
596630
}
597631
PA cmd.PAConfig
@@ -699,10 +733,18 @@ func main() {
699733

700734
lints, err := linter.NewRegistry(config.CertChecker.IgnoredLints)
701735
cmd.FailOnError(err, "Failed to create zlint registry")
736+
737+
lintConfig := linter.Config{}
702738
if config.CertChecker.LintConfig != "" {
703-
lintconfig, err := lint.NewConfigFromFile(config.CertChecker.LintConfig)
739+
lintConfig, err = linter.LoadConfigFile(config.CertChecker.LintConfig)
704740
cmd.FailOnError(err, "Failed to load zlint config file")
705-
lints.SetConfiguration(lintconfig)
741+
}
742+
743+
issuers := make(map[string]*issuance.Certificate)
744+
for _, issuerCertPath := range config.CertChecker.IssuerCerts {
745+
issuer, err := issuance.LoadCertificate(issuerCertPath)
746+
cmd.FailOnError(err, "Failed to load issuer cert file")
747+
issuers[issuer.Subject.CommonName] = issuer
706748
}
707749

708750
checker := newChecker(
@@ -712,7 +754,9 @@ func main() {
712754
kp,
713755
config.CertChecker.CheckPeriod.Duration,
714756
acceptableValidityDurations,
757+
issuers,
715758
lints,
759+
lintConfig,
716760
logger,
717761
)
718762
fmt.Fprintf(os.Stderr, "# Getting certificates issued in the last %s\n", config.CertChecker.CheckPeriod)

cmd/cert-checker/main_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func init() {
7272
}
7373

7474
func BenchmarkCheckCert(b *testing.B) {
75-
checker := newChecker(nil, clock.New(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock())
75+
checker := newChecker(nil, clock.New(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
7676
testKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
7777
expiry := time.Now().AddDate(0, 0, 1)
7878
serial := big.NewInt(1337)
@@ -108,7 +108,7 @@ func TestCheckWildcardCert(t *testing.T) {
108108

109109
testKey, _ := rsa.GenerateKey(rand.Reader, 2048)
110110
fc := clock.NewFake()
111-
checker := newChecker(saDbMap, fc, pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock())
111+
checker := newChecker(saDbMap, fc, pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
112112
issued := checker.clock.Now().Add(-time.Minute)
113113
goodExpiry := issued.Add(testValidityDuration - time.Second)
114114
serial := big.NewInt(1337)
@@ -151,7 +151,7 @@ func TestCheckCertReturnsSANs(t *testing.T) {
151151
defer func() {
152152
saCleanup()
153153
}()
154-
checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock())
154+
checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
155155

156156
certPEM, err := os.ReadFile("testdata/quite_invalid.pem")
157157
if err != nil {
@@ -218,7 +218,7 @@ func TestCheckCert(t *testing.T) {
218218
t.Run(tc.name, func(t *testing.T) {
219219
testKey, _ := tc.key.genKey()
220220

221-
checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock())
221+
checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
222222

223223
// Create a RFC 7633 OCSP Must Staple Extension.
224224
// OID 1.3.6.1.5.5.7.1.24
@@ -340,7 +340,7 @@ func TestGetAndProcessCerts(t *testing.T) {
340340
fc.Set(fc.Now().Add(time.Hour))
341341

342342
mocklog := blog.NewMock()
343-
checker := newChecker(saDbMap, fc, pa, kp, time.Hour, testValidityDurations, nil, mocklog)
343+
checker := newChecker(saDbMap, fc, pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, mocklog)
344344
sa, err := sa.NewSQLStorageAuthority(saDbMap, saDbMap, nil, 0, fc, blog.NewMock(), metrics.NoopRegisterer)
345345
test.AssertNotError(t, err, "Couldn't create SA to insert certificates")
346346
saCleanUp := test.ResetBoulderTestDatabase(t)
@@ -426,7 +426,7 @@ func (db mismatchedCountDB) SelectOne(_ context.Context, holder any, _ string, _
426426
func TestGetCertsEmptyResults(t *testing.T) {
427427
saDbMap, err := sa.DBMapForTest(vars.DBConnSA)
428428
test.AssertNotError(t, err, "Couldn't connect to database")
429-
checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock())
429+
checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
430430
checker.dbMap = mismatchedCountDB{}
431431

432432
batchSize = 3
@@ -453,7 +453,7 @@ func (db emptyDB) SelectOne(_ context.Context, holder any, _ string, _ ...any) e
453453
// expected if the DB finds no certificates to match the SELECT query and
454454
// should return an error.
455455
func TestGetCertsNullResults(t *testing.T) {
456-
checker := newChecker(emptyDB{}, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock())
456+
checker := newChecker(emptyDB{}, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
457457

458458
err := checker.getCerts(context.Background())
459459
test.AssertError(t, err, "Should have gotten error from empty DB")
@@ -499,7 +499,7 @@ func TestGetCertsLate(t *testing.T) {
499499
clk := clock.NewFake()
500500
db := &lateDB{issuedTime: clk.Now().Add(-time.Hour)}
501501
checkPeriod := 24 * time.Hour
502-
checker := newChecker(db, clk, pa, kp, checkPeriod, testValidityDurations, nil, blog.NewMock())
502+
checker := newChecker(db, clk, pa, kp, checkPeriod, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
503503

504504
err := checker.getCerts(context.Background())
505505
test.AssertNotError(t, err, "getting certs")
@@ -560,7 +560,7 @@ func TestIgnoredLint(t *testing.T) {
560560
err = loglist.InitLintList("../../test/ct-test-srv/log_list.json", false)
561561
test.AssertNotError(t, err, "failed to load ct log list")
562562
testKey, _ := rsa.GenerateKey(rand.Reader, 2048)
563-
checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock())
563+
checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
564564
serial := big.NewInt(1337)
565565

566566
x509OID, err := x509.OIDFromInts([]uint64{1, 2, 3})
@@ -642,7 +642,7 @@ func TestIgnoredLint(t *testing.T) {
642642
}
643643

644644
func TestPrecertCorrespond(t *testing.T) {
645-
checker := newChecker(nil, clock.New(), pa, kp, time.Hour, testValidityDurations, nil, blog.NewMock())
645+
checker := newChecker(nil, clock.New(), pa, kp, time.Hour, testValidityDurations, nil, nil, linter.Config{}, blog.NewMock())
646646
checker.getPrecert = func(_ context.Context, _ string) ([]byte, error) {
647647
return []byte("hello"), nil
648648
}

linter/config.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,15 @@ func (r configuredRegistry) GetConfiguration() lint.Configuration {
106106
return r.config
107107
}
108108

109-
// NewRegistryWithConfig is like NewRegistry, but the returned registry also
110-
// carries the contents of the given Config.
111-
func NewRegistryWithConfig(skipLints []string, config Config) (lint.Registry, error) {
112-
reg, err := NewRegistry(skipLints)
113-
if err != nil {
114-
return nil, err
109+
// ConfigureRegistry combines the given registry with the given config. This is
110+
// useful for reusing the same registry (which can be expensive to build) with
111+
// many different configs.
112+
func ConfigureRegistry(reg lint.Registry, config Config) (lint.Registry, error) {
113+
if reg == nil {
114+
// Normally zlint would replace a nil registry with the global registry on
115+
// its own, but of course a configuredRegistry *isn't* nil even if the
116+
// registry it wraps is, so we need to do this fallback ourselves.
117+
reg = lint.GlobalRegistry()
115118
}
116119
lintConfig, err := config.build()
117120
if err != nil {

test/config-next/cert-checker.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@
1414
"1080h",
1515
"160h"
1616
],
17+
"issuerCerts": [
18+
"test/certs/webpki/int-rsa-a.cert.pem",
19+
"test/certs/webpki/int-rsa-b.cert.pem",
20+
"test/certs/webpki/int-rsa-c.cert.pem",
21+
"test/certs/webpki/int-ecdsa-a.cert.pem",
22+
"test/certs/webpki/int-ecdsa-b.cert.pem",
23+
"test/certs/webpki/int-ecdsa-c.cert.pem"
24+
],
1725
"lintConfig": "test/config-next/zlint.toml",
1826
"ignoredLints": [
1927
"w_ext_subject_key_identifier_missing_sub_cert",

0 commit comments

Comments
 (0)