Skip to content

Commit

Permalink
local/signer: use zmap/zlint v2.0.0, add filtering. (#1080)
Browse files Browse the repository at this point in the history 
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 <cpu@letsencrypt.org>
  • Loading branch information
@cpu
cpu and Daniel authored Mar 24, 2020
1 parent 7c8e501 commit abef926
Show file tree
Hide file tree
Showing 311 changed files with 13,979 additions and 3,071 deletions.
58 changes: 46 additions & 12 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import (
"github.com/cloudflare/cfssl/helpers"
"github.com/cloudflare/cfssl/log"
ocspConfig "github.com/cloudflare/cfssl/ocsp/config"
"github.com/zmap/zlint/lints"
// empty import of zlint/v2 required to have lints registered.
_ "github.com/zmap/zlint/v2"
"github.com/zmap/zlint/v2/lint"
)

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

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

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

p.IgnoredLintsMap = map[string]bool{}
for _, lintName := range p.IgnoredLints {
p.IgnoredLintsMap[lintName] = true
// By default perform any required preissuance linting with all ZLint lints.
p.LintRegistry = lint.GlobalRegistry()

// If ExcludeLintSources are present in config build a lint.SourceList while
// validating that no unknown sources were specified.
var excludedSources lint.SourceList
if len(p.ExcludeLintSources) > 0 {
for _, sourceName := range p.ExcludeLintSources {
var lintSource lint.LintSource
lintSource.FromString(sourceName)
if lintSource == lint.UnknownLintSource {
return cferr.Wrap(cferr.PolicyError, cferr.InvalidPolicy,
fmt.Errorf("failed to build excluded lint source list: unknown source %q",
sourceName))
}
excludedSources = append(excludedSources, lintSource)
}
}

opts := lint.FilterOptions{
ExcludeNames: p.ExcludeLints,
ExcludeSources: excludedSources,
}
if !opts.Empty() {
// If ExcludeLints or ExcludeLintSources were not empty then filter out the
// lints we don't want to use for preissuance linting with this profile.
filteredRegistry, err := p.LintRegistry.Filter(opts)
if err != nil {
return cferr.Wrap(cferr.PolicyError, cferr.InvalidPolicy,
fmt.Errorf("failed to build filtered lint registry: %v", err))
}
p.LintRegistry = filteredRegistry
}

return nil
Expand Down
35 changes: 24 additions & 11 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ var validConfig = &Config{
Expiry: expiry,
},
"valid-lint": {
Usage: []string{"digital signature"},
Expiry: expiry,
LintErrLevel: 5,
IgnoredLints: []string{"n_subject_common_name_included"},
Usage: []string{"digital signature"},
Expiry: expiry,
LintErrLevel: 5,
ExcludeLints: []string{"n_subject_common_name_included"},
ExcludeLintSources: []string{"ETSI-ESI"},
},
},
Default: &SigningProfile{
Expand Down Expand Up @@ -389,20 +390,32 @@ func TestParse(t *testing.T) {

}

func TestPopulateIgnoredLintsMap(t *testing.T) {
lintName := "n_subject_common_name_included"
func TestPopulateLintRegistry(t *testing.T) {
excludedLintName := "n_subject_common_name_included"
etsiLintName := "w_qcstatem_qctype_web"
profile := &SigningProfile{
ExpiryString: "300s",
IgnoredLints: []string{lintName},
ExpiryString: "300s",
ExcludeLints: []string{excludedLintName},
ExcludeLintSources: []string{"ETSI_ESI"},
}

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

if !profile.IgnoredLintsMap[lintName] {
t.Errorf("expected to find lint %q in ignored lints map after populate()",
lintName)
// The LintRegistry shouldn't be nil.
if profile.LintRegistry == nil {
t.Errorf("expected to find non-nil lint registry after populate()")
}

// The excluded lint shouldn't be found in the registry
if l := profile.LintRegistry.ByName(excludedLintName); l != nil {
t.Errorf("expected lint name %q to be filtered out, found %v", excludedLintName, l)
}

// A lint from the excluded source category shouldn't be found in the registry.
if l := profile.LintRegistry.ByName(etsiLintName); l != nil {
t.Errorf("expected lint name %q to be filtered out, found %v", etsiLintName, l)
}
}

Expand Down
6 changes: 3 additions & 3 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ require (
github.com/pkg/errors v0.8.0 // indirect
github.com/weppos/publicsuffix-go v0.5.0 // indirect
github.com/ziutek/mymysql v1.5.4 // indirect
github.com/zmap/zcrypto v0.0.0-20190729165852-9051775e6a2e
github.com/zmap/zlint v0.0.0-20190806154020-fd021b4cfbeb
github.com/zmap/zcrypto v0.0.0-20191112190257-7f2fe6faf8cf
github.com/zmap/zlint/v2 v2.0.0
golang.org/x/crypto v0.0.0-20200124225646-8b5121be2f68
golang.org/x/lint v0.0.0-20190930215403-16217165b5de
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3
golang.org/x/net v0.0.0-20190620200207-3b0461eec859
golang.org/x/text v0.3.2 // indirect
)
12 changes: 8 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,12 @@ github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs=
github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0=
github.com/zmap/rc2 v0.0.0-20131011165748-24b9757f5521/go.mod h1:3YZ9o3WnatTIZhuOtot4IcUfzoKVjUHqu6WALIyI0nE=
github.com/zmap/zcertificate v0.0.0-20180516150559-0e3d58b1bac4/go.mod h1:5iU54tB79AMBcySS0R2XIyZBAVmeHranShAFELYx7is=
github.com/zmap/zcrypto v0.0.0-20190729165852-9051775e6a2e h1:mvOa4+/DXStR4ZXOks/UsjeFdn5O5JpLUtzqk9U8xXw=
github.com/zmap/zcrypto v0.0.0-20190729165852-9051775e6a2e/go.mod h1:w7kd3qXHh8FNaczNjslXqvFQiv5mMWRXlL9klTUAHc8=
github.com/zmap/zlint v0.0.0-20190806154020-fd021b4cfbeb h1:vxqkjztXSaPVDc8FQCdHTaejm2x747f6yPbnu1h2xkg=
github.com/zmap/zlint v0.0.0-20190806154020-fd021b4cfbeb/go.mod h1:29UiAJNsiVdvTBFCJW8e3q6dcDbOoPkhMgttOSCIMMY=
github.com/zmap/zcrypto v0.0.0-20191112190257-7f2fe6faf8cf h1:Q9MiSA+G9DHe/TzG8pnycDn3HwpQuTygphu9M/7KYqU=
github.com/zmap/zcrypto v0.0.0-20191112190257-7f2fe6faf8cf/go.mod h1:w7kd3qXHh8FNaczNjslXqvFQiv5mMWRXlL9klTUAHc8=
github.com/zmap/zlint/v2 v2.0.0-rc4 h1:liUiMWqa52YUvSeSqGOJpqxGwISUi3bNkkmLQtNx7z4=
github.com/zmap/zlint/v2 v2.0.0-rc4/go.mod h1:0jpqZ7cVjm8ABh/PTOp74MK50bPiN+HW+NjjESDxLVA=
github.com/zmap/zlint/v2 v2.0.0 h1:Ve+1yR76LZhTXsxonKA35d5S8dIIW1pmIlr4ahrskhs=
github.com/zmap/zlint/v2 v2.0.0/go.mod h1:0jpqZ7cVjm8ABh/PTOp74MK50bPiN+HW+NjjESDxLVA=
golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4 h1:HuIa8hRrWRSrqYzx1qI49NNxhdi2PrY7gxVSq1JjLDc=
Expand All @@ -91,6 +93,8 @@ golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHl
golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 h1:0GoQqolDA55aaLxZyTzK/Y2ePZzZTUrRacwib7cNsYQ=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859 h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand Down
35 changes: 15 additions & 20 deletions signer/local/local.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ import (
"github.com/google/certificate-transparency-go/jsonclient"

zx509 "github.com/zmap/zcrypto/x509"
"github.com/zmap/zlint"
"github.com/zmap/zlint/lints"
"github.com/zmap/zlint/v2"
"github.com/zmap/zlint/v2/lint"
"golang.org/x/net/context"
)

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

func (e *LintError) Error() string {
Expand All @@ -140,14 +140,12 @@ func (e *LintError) Error() string {
}

// lint performs pre-issuance linting of a given TBS certificate template when
// the provided errLevel is > 0. Any lint results with a status higher than the
// errLevel that isn't created by a lint in the ignoreMap will result in
// a LintError being returned to the caller. Note that the template is provided
// by-value and not by-reference. This is important as the lint function needs
// to mutate the template's signature algorithm to match the lintPriv.
func (s *Signer) lint(template x509.Certificate, errLevel lints.LintStatus, ignoreMap map[string]bool) error {
// Always return nil when linting is disabled (lints.Reserved == 0).
if errLevel == lints.Reserved {
// the provided errLevel is > 0. Note that the template is provided by-value and
// not by-reference. This is important as the lint function needs to mutate the
// template's signature algorithm to match the lintPriv.
func (s *Signer) lint(template x509.Certificate, errLevel lint.LintStatus, lintRegistry lint.Registry) error {
// Always return nil when linting is disabled (lint.Reserved == 0).
if errLevel == lint.Reserved {
return nil
}
// without a lintPriv key to use to sign the tbsCertificate we can't lint it.
Expand All @@ -174,12 +172,9 @@ func (s *Signer) lint(template x509.Certificate, errLevel lints.LintStatus, igno
if err != nil {
return cferr.Wrap(cferr.CertificateError, cferr.ParseFailed, err)
}
errorResults := map[string]lints.LintResult{}
results := zlint.LintCertificate(prelintCert)
errorResults := map[string]lint.LintResult{}
results := zlint.LintCertificateEx(prelintCert, lintRegistry)
for name, res := range results.Results {
if ignoreMap[name] {
continue
}
if res.Status > errLevel {
errorResults[name] = *res
}
Expand All @@ -192,7 +187,7 @@ func (s *Signer) lint(template x509.Certificate, errLevel lints.LintStatus, igno
return nil
}

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

if err := s.lint(*template, lintErrLevel, lintIgnore); err != nil {
if err := s.lint(*template, lintErrLevel, lintRegistry); err != nil {
return nil, err
}

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

var signedCert []byte
signedCert, err = s.sign(&certTBS, profile.LintErrLevel, profile.IgnoredLintsMap)
signedCert, err = s.sign(&certTBS, profile.LintErrLevel, profile.LintRegistry)
if err != nil {
return nil, err
}
Expand Down
62 changes: 40 additions & 22 deletions signer/local/local_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
"github.com/cloudflare/cfssl/log"
"github.com/cloudflare/cfssl/signer"
"github.com/google/certificate-transparency-go"
"github.com/zmap/zlint/lints"
"github.com/zmap/zlint/v2/lint"
)

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

ignoredLintNameRegistry, err := lint.GlobalRegistry().Filter(lint.FilterOptions{
ExcludeNames: []string{"e_dnsname_not_valid_tld"},
})
if err != nil {
t.Fatalf("failed to construct ignoredLintNamesRegistry: %v", err)
}

ignoredLintSourcesRegistry, err := lint.GlobalRegistry().Filter(lint.FilterOptions{
ExcludeSources: lint.SourceList{lint.CABFBaselineRequirements},
})
if err != nil {
t.Fatalf("failed to construct ignoredLintSourcesRegistry: %v", err)
}

testCases := []struct {
name string
signer *Signer
lintErrLevel lints.LintStatus
ignoredLintMap map[string]bool
lintErrLevel lint.LintStatus
lintRegistry lint.Registry
expectedErr error
expectedErrResults map[string]lints.LintResult
expectedErrResults map[string]lint.LintResult
}{
{
name: "linting disabled",
Expand All @@ -1545,46 +1559,50 @@ func TestLint(t *testing.T) {
{
name: "signer without lint key",
signer: &Signer{},
lintErrLevel: lints.NA,
lintErrLevel: lint.NA,
expectedErr: errors.New(`{"code":2500,"message":"Private key is unavailable"}`),
},
{
name: "lint results above err level",
signer: lintSigner,
lintErrLevel: lints.Notice,
lintErrLevel: lint.Notice,
expectedErr: errors.New("pre-issuance linting found 2 error results"),
expectedErrResults: map[string]lints.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
"e_dnsname_not_valid_tld": lints.LintResult{Status: 6},
expectedErrResults: map[string]lint.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lint.LintResult{Status: 6},
"e_dnsname_not_valid_tld": lint.LintResult{Status: 6},
},
},
{
name: "lint results below err level",
signer: lintSigner,
lintErrLevel: lints.Warn,
lintErrLevel: lint.Warn,
expectedErr: errors.New("pre-issuance linting found 2 error results"),
expectedErrResults: map[string]lints.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
"e_dnsname_not_valid_tld": lints.LintResult{Status: 6},
expectedErrResults: map[string]lint.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lint.LintResult{Status: 6},
"e_dnsname_not_valid_tld": lint.LintResult{Status: 6},
},
},
{
name: "ignored lints, lint results above err level",
name: "ignored lint names, lint results above err level",
signer: lintSigner,
lintErrLevel: lints.Notice,
ignoredLintMap: map[string]bool{
"e_dnsname_not_valid_tld": true,
},
expectedErr: errors.New("pre-issuance linting found 1 error results"),
expectedErrResults: map[string]lints.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lints.LintResult{Status: 6},
lintErrLevel: lint.Notice,
lintRegistry: ignoredLintNameRegistry,
expectedErr: errors.New("pre-issuance linting found 1 error results"),
expectedErrResults: map[string]lint.LintResult{
"e_sub_cert_aia_does_not_contain_ocsp_url": lint.LintResult{Status: 6},
},
},
{
name: "ignored lint sources, lint results above err level",
signer: lintSigner,
lintErrLevel: lint.Notice,
lintRegistry: ignoredLintSourcesRegistry,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := tc.signer.lint(*jankyTemplate, tc.lintErrLevel, tc.ignoredLintMap)
err := tc.signer.lint(*jankyTemplate, tc.lintErrLevel, tc.lintRegistry)
if err != nil && tc.expectedErr == nil {
t.Errorf("Expected no err, got %#v", err)
} else if err == nil && tc.expectedErr != nil {
Expand Down
Loading

0 comments on commit abef926

Please sign in to comment.