Skip to content

Commit abef926

Browse files
cpuDaniel
andauthored
local/signer: use zmap/zlint v2.0.0, add filtering. (#1080)
This updates the CFSSL local signer ZLint pre-issuance linting integration to use v2.0.0. The existing signing profile configuration field "ignored_lints" is joined by a new field "ignored_lint_sources". This relies on features in the new 2.0.0 release and is useful for CAs that know certain classes of ZLint lints are never applicable (e.g. CABF EV guidelines, ETSI ESI, etc). Co-authored-by: Daniel <[email protected]>
1 parent 7c8e501 commit abef926

File tree

311 files changed

+13979
-3071
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

311 files changed

+13979
-3071
lines changed

config/config.go

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import (
1919
"github.com/cloudflare/cfssl/helpers"
2020
"github.com/cloudflare/cfssl/log"
2121
ocspConfig "github.com/cloudflare/cfssl/ocsp/config"
22-
"github.com/zmap/zlint/lints"
22+
// empty import of zlint/v2 required to have lints registered.
23+
_ "github.com/zmap/zlint/v2"
24+
"github.com/zmap/zlint/v2/lint"
2325
)
2426

2527
// A CSRWhitelist stores booleans for fields in the CSR. If a CSRWhitelist is
@@ -99,11 +101,12 @@ type SigningProfile struct {
99101
// 5 = all lint results except pass, notice and warn are considered errors
100102
// 6 = all lint results except pass, notice, warn and error are considered errors.
101103
// 7 = lint is performed, no lint results are treated as errors.
102-
LintErrLevel lints.LintStatus `json:"lint_error_level"`
103-
// IgnoredLints lists zlint lint names to ignore. Any lint results from
104-
// matching lints will be ignored no matter what the configured LintErrLevel
105-
// is.
106-
IgnoredLints []string `json:"ignored_lints"`
104+
LintErrLevel lint.LintStatus `json:"lint_error_level"`
105+
// ExcludeLints lists ZLint lint names to exclude from preissuance linting.
106+
ExcludeLints []string `json:"ignored_lints"`
107+
// ExcludeLintSources lists ZLint lint sources to exclude from preissuance
108+
// linting.
109+
ExcludeLintSources []string `json:"ignored_lint_sources"`
107110

108111
Policies []CertificatePolicy
109112
Expiry time.Duration
@@ -118,9 +121,11 @@ type SigningProfile struct {
118121
NameWhitelist *regexp.Regexp
119122
ExtensionWhitelist map[string]bool
120123
ClientProvidesSerialNumbers bool
121-
// IgnoredLintsMap is a bool map created from IgnoredLints when the profile is
122-
// loaded. It facilitates set membership testing.
123-
IgnoredLintsMap map[string]bool
124+
// LintRegistry is the collection of lints that should be used if
125+
// LintErrLevel is configured. By default all ZLint lints are used. If
126+
// ExcludeLints or ExcludeLintSources are set then this registry will be
127+
// filtered in populate() to exclude the named lints and lint sources.
128+
LintRegistry lint.Registry
124129
}
125130

126131
// UnmarshalJSON unmarshals a JSON string into an OID.
@@ -324,9 +329,38 @@ func (p *SigningProfile) populate(cfg *Config) error {
324329
p.ExtensionWhitelist[asn1.ObjectIdentifier(oid).String()] = true
325330
}
326331

327-
p.IgnoredLintsMap = map[string]bool{}
328-
for _, lintName := range p.IgnoredLints {
329-
p.IgnoredLintsMap[lintName] = true
332+
// By default perform any required preissuance linting with all ZLint lints.
333+
p.LintRegistry = lint.GlobalRegistry()
334+
335+
// If ExcludeLintSources are present in config build a lint.SourceList while
336+
// validating that no unknown sources were specified.
337+
var excludedSources lint.SourceList
338+
if len(p.ExcludeLintSources) > 0 {
339+
for _, sourceName := range p.ExcludeLintSources {
340+
var lintSource lint.LintSource
341+
lintSource.FromString(sourceName)
342+
if lintSource == lint.UnknownLintSource {
343+
return cferr.Wrap(cferr.PolicyError, cferr.InvalidPolicy,
344+
fmt.Errorf("failed to build excluded lint source list: unknown source %q",
345+
sourceName))
346+
}
347+
excludedSources = append(excludedSources, lintSource)
348+
}
349+
}
350+
351+
opts := lint.FilterOptions{
352+
ExcludeNames: p.ExcludeLints,
353+
ExcludeSources: excludedSources,
354+
}
355+
if !opts.Empty() {
356+
// If ExcludeLints or ExcludeLintSources were not empty then filter out the
357+
// lints we don't want to use for preissuance linting with this profile.
358+
filteredRegistry, err := p.LintRegistry.Filter(opts)
359+
if err != nil {
360+
return cferr.Wrap(cferr.PolicyError, cferr.InvalidPolicy,
361+
fmt.Errorf("failed to build filtered lint registry: %v", err))
362+
}
363+
p.LintRegistry = filteredRegistry
330364
}
331365

332366
return nil

config/config_test.go

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,11 @@ var validConfig = &Config{
5151
Expiry: expiry,
5252
},
5353
"valid-lint": {
54-
Usage: []string{"digital signature"},
55-
Expiry: expiry,
56-
LintErrLevel: 5,
57-
IgnoredLints: []string{"n_subject_common_name_included"},
54+
Usage: []string{"digital signature"},
55+
Expiry: expiry,
56+
LintErrLevel: 5,
57+
ExcludeLints: []string{"n_subject_common_name_included"},
58+
ExcludeLintSources: []string{"ETSI-ESI"},
5859
},
5960
},
6061
Default: &SigningProfile{
@@ -389,20 +390,32 @@ func TestParse(t *testing.T) {
389390

390391
}
391392

392-
func TestPopulateIgnoredLintsMap(t *testing.T) {
393-
lintName := "n_subject_common_name_included"
393+
func TestPopulateLintRegistry(t *testing.T) {
394+
excludedLintName := "n_subject_common_name_included"
395+
etsiLintName := "w_qcstatem_qctype_web"
394396
profile := &SigningProfile{
395-
ExpiryString: "300s",
396-
IgnoredLints: []string{lintName},
397+
ExpiryString: "300s",
398+
ExcludeLints: []string{excludedLintName},
399+
ExcludeLintSources: []string{"ETSI_ESI"},
397400
}
398401

399402
if err := profile.populate(nil); err != nil {
400403
t.Fatal("unexpected error from profile populate")
401404
}
402405

403-
if !profile.IgnoredLintsMap[lintName] {
404-
t.Errorf("expected to find lint %q in ignored lints map after populate()",
405-
lintName)
406+
// The LintRegistry shouldn't be nil.
407+
if profile.LintRegistry == nil {
408+
t.Errorf("expected to find non-nil lint registry after populate()")
409+
}
410+
411+
// The excluded lint shouldn't be found in the registry
412+
if l := profile.LintRegistry.ByName(excludedLintName); l != nil {
413+
t.Errorf("expected lint name %q to be filtered out, found %v", excludedLintName, l)
414+
}
415+
416+
// A lint from the excluded source category shouldn't be found in the registry.
417+
if l := profile.LintRegistry.ByName(etsiLintName); l != nil {
418+
t.Errorf("expected lint name %q to be filtered out, found %v", etsiLintName, l)
406419
}
407420
}
408421

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ require (
2323
github.com/pkg/errors v0.8.0 // indirect
2424
github.com/weppos/publicsuffix-go v0.5.0 // indirect
2525
github.com/ziutek/mymysql v1.5.4 // indirect
26-
github.com/zmap/zcrypto v0.0.0-20190729165852-9051775e6a2e
27-
github.com/zmap/zlint v0.0.0-20190806154020-fd021b4cfbeb
26+
github.com/zmap/zcrypto v0.0.0-20191112190257-7f2fe6faf8cf
27+
github.com/zmap/zlint/v2 v2.0.0
2828
golang.org/x/crypto v0.0.0-20200124225646-8b5121be2f68
2929
golang.org/x/lint v0.0.0-20190930215403-16217165b5de
30-
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3
30+
golang.org/x/net v0.0.0-20190620200207-3b0461eec859
3131
golang.org/x/text v0.3.2 // indirect
3232
)

go.sum

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,12 @@ github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs=
7676
github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0=
7777
github.com/zmap/rc2 v0.0.0-20131011165748-24b9757f5521/go.mod h1:3YZ9o3WnatTIZhuOtot4IcUfzoKVjUHqu6WALIyI0nE=
7878
github.com/zmap/zcertificate v0.0.0-20180516150559-0e3d58b1bac4/go.mod h1:5iU54tB79AMBcySS0R2XIyZBAVmeHranShAFELYx7is=
79-
github.com/zmap/zcrypto v0.0.0-20190729165852-9051775e6a2e h1:mvOa4+/DXStR4ZXOks/UsjeFdn5O5JpLUtzqk9U8xXw=
80-
github.com/zmap/zcrypto v0.0.0-20190729165852-9051775e6a2e/go.mod h1:w7kd3qXHh8FNaczNjslXqvFQiv5mMWRXlL9klTUAHc8=
81-
github.com/zmap/zlint v0.0.0-20190806154020-fd021b4cfbeb h1:vxqkjztXSaPVDc8FQCdHTaejm2x747f6yPbnu1h2xkg=
82-
github.com/zmap/zlint v0.0.0-20190806154020-fd021b4cfbeb/go.mod h1:29UiAJNsiVdvTBFCJW8e3q6dcDbOoPkhMgttOSCIMMY=
79+
github.com/zmap/zcrypto v0.0.0-20191112190257-7f2fe6faf8cf h1:Q9MiSA+G9DHe/TzG8pnycDn3HwpQuTygphu9M/7KYqU=
80+
github.com/zmap/zcrypto v0.0.0-20191112190257-7f2fe6faf8cf/go.mod h1:w7kd3qXHh8FNaczNjslXqvFQiv5mMWRXlL9klTUAHc8=
81+
github.com/zmap/zlint/v2 v2.0.0-rc4 h1:liUiMWqa52YUvSeSqGOJpqxGwISUi3bNkkmLQtNx7z4=
82+
github.com/zmap/zlint/v2 v2.0.0-rc4/go.mod h1:0jpqZ7cVjm8ABh/PTOp74MK50bPiN+HW+NjjESDxLVA=
83+
github.com/zmap/zlint/v2 v2.0.0 h1:Ve+1yR76LZhTXsxonKA35d5S8dIIW1pmIlr4ahrskhs=
84+
github.com/zmap/zlint/v2 v2.0.0/go.mod h1:0jpqZ7cVjm8ABh/PTOp74MK50bPiN+HW+NjjESDxLVA=
8385
golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
8486
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
8587
golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4 h1:HuIa8hRrWRSrqYzx1qI49NNxhdi2PrY7gxVSq1JjLDc=
@@ -91,6 +93,8 @@ golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHl
9193
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
9294
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 h1:0GoQqolDA55aaLxZyTzK/Y2ePZzZTUrRacwib7cNsYQ=
9395
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
96+
golang.org/x/net v0.0.0-20190620200207-3b0461eec859 h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI=
97+
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
9498
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
9599
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
96100
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

signer/local/local.go

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ import (
3434
"github.com/google/certificate-transparency-go/jsonclient"
3535

3636
zx509 "github.com/zmap/zcrypto/x509"
37-
"github.com/zmap/zlint"
38-
"github.com/zmap/zlint/lints"
37+
"github.com/zmap/zlint/v2"
38+
"github.com/zmap/zlint/v2/lint"
3939
"golang.org/x/net/context"
4040
)
4141

@@ -131,7 +131,7 @@ func NewSignerFromFile(caFile, caKeyFile string, policy *config.Signing) (*Signe
131131
// concrete zlint LintResults so that callers can further inspect the cause of
132132
// the failing lints.
133133
type LintError struct {
134-
ErrorResults map[string]lints.LintResult
134+
ErrorResults map[string]lint.LintResult
135135
}
136136

137137
func (e *LintError) Error() string {
@@ -140,14 +140,12 @@ func (e *LintError) Error() string {
140140
}
141141

142142
// lint performs pre-issuance linting of a given TBS certificate template when
143-
// the provided errLevel is > 0. Any lint results with a status higher than the
144-
// errLevel that isn't created by a lint in the ignoreMap will result in
145-
// a LintError being returned to the caller. Note that the template is provided
146-
// by-value and not by-reference. This is important as the lint function needs
147-
// to mutate the template's signature algorithm to match the lintPriv.
148-
func (s *Signer) lint(template x509.Certificate, errLevel lints.LintStatus, ignoreMap map[string]bool) error {
149-
// Always return nil when linting is disabled (lints.Reserved == 0).
150-
if errLevel == lints.Reserved {
143+
// the provided errLevel is > 0. Note that the template is provided by-value and
144+
// not by-reference. This is important as the lint function needs to mutate the
145+
// template's signature algorithm to match the lintPriv.
146+
func (s *Signer) lint(template x509.Certificate, errLevel lint.LintStatus, lintRegistry lint.Registry) error {
147+
// Always return nil when linting is disabled (lint.Reserved == 0).
148+
if errLevel == lint.Reserved {
151149
return nil
152150
}
153151
// without a lintPriv key to use to sign the tbsCertificate we can't lint it.
@@ -174,12 +172,9 @@ func (s *Signer) lint(template x509.Certificate, errLevel lints.LintStatus, igno
174172
if err != nil {
175173
return cferr.Wrap(cferr.CertificateError, cferr.ParseFailed, err)
176174
}
177-
errorResults := map[string]lints.LintResult{}
178-
results := zlint.LintCertificate(prelintCert)
175+
errorResults := map[string]lint.LintResult{}
176+
results := zlint.LintCertificateEx(prelintCert, lintRegistry)
179177
for name, res := range results.Results {
180-
if ignoreMap[name] {
181-
continue
182-
}
183178
if res.Status > errLevel {
184179
errorResults[name] = *res
185180
}
@@ -192,7 +187,7 @@ func (s *Signer) lint(template x509.Certificate, errLevel lints.LintStatus, igno
192187
return nil
193188
}
194189

195-
func (s *Signer) sign(template *x509.Certificate, lintErrLevel lints.LintStatus, lintIgnore map[string]bool) (cert []byte, err error) {
190+
func (s *Signer) sign(template *x509.Certificate, lintErrLevel lint.LintStatus, lintRegistry lint.Registry) (cert []byte, err error) {
196191
var initRoot bool
197192
if s.ca == nil {
198193
if !template.IsCA {
@@ -206,7 +201,7 @@ func (s *Signer) sign(template *x509.Certificate, lintErrLevel lints.LintStatus,
206201
initRoot = true
207202
}
208203

209-
if err := s.lint(*template, lintErrLevel, lintIgnore); err != nil {
204+
if err := s.lint(*template, lintErrLevel, lintRegistry); err != nil {
210205
return nil, err
211206
}
212207

@@ -454,7 +449,7 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
454449
var poisonExtension = pkix.Extension{Id: signer.CTPoisonOID, Critical: true, Value: []byte{0x05, 0x00}}
455450
var poisonedPreCert = certTBS
456451
poisonedPreCert.ExtraExtensions = append(safeTemplate.ExtraExtensions, poisonExtension)
457-
cert, err = s.sign(&poisonedPreCert, profile.LintErrLevel, profile.IgnoredLintsMap)
452+
cert, err = s.sign(&poisonedPreCert, profile.LintErrLevel, profile.LintRegistry)
458453
if err != nil {
459454
return
460455
}
@@ -499,7 +494,7 @@ func (s *Signer) Sign(req signer.SignRequest) (cert []byte, err error) {
499494
}
500495

501496
var signedCert []byte
502-
signedCert, err = s.sign(&certTBS, profile.LintErrLevel, profile.IgnoredLintsMap)
497+
signedCert, err = s.sign(&certTBS, profile.LintErrLevel, profile.LintRegistry)
503498
if err != nil {
504499
return nil, err
505500
}

signer/local/local_test.go

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232
"github.com/cloudflare/cfssl/log"
3333
"github.com/cloudflare/cfssl/signer"
3434
"github.com/google/certificate-transparency-go"
35-
"github.com/zmap/zlint/lints"
35+
"github.com/zmap/zlint/v2/lint"
3636
)
3737

3838
const (
@@ -1530,13 +1530,27 @@ func TestLint(t *testing.T) {
15301530
jankyTemplate.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}
15311531
jankyTemplate.IsCA = false
15321532

1533+
ignoredLintNameRegistry, err := lint.GlobalRegistry().Filter(lint.FilterOptions{
1534+
ExcludeNames: []string{"e_dnsname_not_valid_tld"},
1535+
})
1536+
if err != nil {
1537+
t.Fatalf("failed to construct ignoredLintNamesRegistry: %v", err)
1538+
}
1539+
1540+
ignoredLintSourcesRegistry, err := lint.GlobalRegistry().Filter(lint.FilterOptions{
1541+
ExcludeSources: lint.SourceList{lint.CABFBaselineRequirements},
1542+
})
1543+
if err != nil {
1544+
t.Fatalf("failed to construct ignoredLintSourcesRegistry: %v", err)
1545+
}
1546+
15331547
testCases := []struct {
15341548
name string
15351549
signer *Signer
1536-
lintErrLevel lints.LintStatus
1537-
ignoredLintMap map[string]bool
1550+
lintErrLevel lint.LintStatus
1551+
lintRegistry lint.Registry
15381552
expectedErr error
1539-
expectedErrResults map[string]lints.LintResult
1553+
expectedErrResults map[string]lint.LintResult
15401554
}{
15411555
{
15421556
name: "linting disabled",
@@ -1545,46 +1559,50 @@ func TestLint(t *testing.T) {
15451559
{
15461560
name: "signer without lint key",
15471561
signer: &Signer{},
1548-
lintErrLevel: lints.NA,
1562+
lintErrLevel: lint.NA,
15491563
expectedErr: errors.New(`{"code":2500,"message":"Private key is unavailable"}`),
15501564
},
15511565
{
15521566
name: "lint results above err level",
15531567
signer: lintSigner,
1554-
lintErrLevel: lints.Notice,
1568+
lintErrLevel: lint.Notice,
15551569
expectedErr: errors.New("pre-issuance linting found 2 error results"),
1556-
expectedErrResults: map[string]lints.LintResult{
1557-
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
1558-
"e_dnsname_not_valid_tld": lints.LintResult{Status: 6},
1570+
expectedErrResults: map[string]lint.LintResult{
1571+
"e_sub_cert_aia_does_not_contain_ocsp_url": lint.LintResult{Status: 6},
1572+
"e_dnsname_not_valid_tld": lint.LintResult{Status: 6},
15591573
},
15601574
},
15611575
{
15621576
name: "lint results below err level",
15631577
signer: lintSigner,
1564-
lintErrLevel: lints.Warn,
1578+
lintErrLevel: lint.Warn,
15651579
expectedErr: errors.New("pre-issuance linting found 2 error results"),
1566-
expectedErrResults: map[string]lints.LintResult{
1567-
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
1568-
"e_dnsname_not_valid_tld": lints.LintResult{Status: 6},
1580+
expectedErrResults: map[string]lint.LintResult{
1581+
"e_sub_cert_aia_does_not_contain_ocsp_url": lint.LintResult{Status: 6},
1582+
"e_dnsname_not_valid_tld": lint.LintResult{Status: 6},
15691583
},
15701584
},
15711585
{
1572-
name: "ignored lints, lint results above err level",
1586+
name: "ignored lint names, lint results above err level",
15731587
signer: lintSigner,
1574-
lintErrLevel: lints.Notice,
1575-
ignoredLintMap: map[string]bool{
1576-
"e_dnsname_not_valid_tld": true,
1577-
},
1578-
expectedErr: errors.New("pre-issuance linting found 1 error results"),
1579-
expectedErrResults: map[string]lints.LintResult{
1580-
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
1588+
lintErrLevel: lint.Notice,
1589+
lintRegistry: ignoredLintNameRegistry,
1590+
expectedErr: errors.New("pre-issuance linting found 1 error results"),
1591+
expectedErrResults: map[string]lint.LintResult{
1592+
"e_sub_cert_aia_does_not_contain_ocsp_url": lint.LintResult{Status: 6},
15811593
},
15821594
},
1595+
{
1596+
name: "ignored lint sources, lint results above err level",
1597+
signer: lintSigner,
1598+
lintErrLevel: lint.Notice,
1599+
lintRegistry: ignoredLintSourcesRegistry,
1600+
},
15831601
}
15841602

15851603
for _, tc := range testCases {
15861604
t.Run(tc.name, func(t *testing.T) {
1587-
err := tc.signer.lint(*jankyTemplate, tc.lintErrLevel, tc.ignoredLintMap)
1605+
err := tc.signer.lint(*jankyTemplate, tc.lintErrLevel, tc.lintRegistry)
15881606
if err != nil && tc.expectedErr == nil {
15891607
t.Errorf("Expected no err, got %#v", err)
15901608
} else if err == nil && tc.expectedErr != nil {

0 commit comments

Comments
 (0)