Skip to content

Commit

Permalink
Add pkilint to CI via custom zlint (#7441)
Browse files Browse the repository at this point in the history 
Add a new "LintConfig" item to the CA's config, which can point to a
zlint configuration toml file. This allows lints to be configured, e.g.
to control the number of rounds of factorization performed by the Fermat
factorization lint.

Leverage this new config to create a new custom zlint which calls out to
a configured pkilint API endpoint. In config-next integration tests,
configure the lint to point at a new pkilint docker container.

This approach has three nice forward-looking features: we now have the
ability to configure any of our lints; it's easy to expand this
mechanism to lint CRLs when the pkilint API has support for that; and
it's easy to enable this new lint if we decide to stand up a pkilint
container in our production environment.

No production configuration changes are necessary at this time.

Fixes #7430
  • Loading branch information
@aarongable
aarongable authored Apr 30, 2024
1 parent 9f2a27e commit 939ac1b
Show file tree
Hide file tree
Showing 12 changed files with 324 additions and 113 deletions.
19 changes: 10 additions & 9 deletions ca/ca.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/jmhodges/clock"
"github.com/miekg/pkcs11"
"github.com/prometheus/client_golang/prometheus"
"github.com/zmap/zlint/v3/lint"
"golang.org/x/crypto/ocsp"
"google.golang.org/protobuf/types/known/timestamppb"

Expand Down Expand Up @@ -125,18 +126,18 @@ func makeIssuerMaps(issuers []*issuance.Issuer) (issuerMaps, error) {
return issuerMaps{issuersByAlg, issuersByNameID}, nil
}

// makeCertificateProfilesMap processes a list of certificate issuance profile
// configs and an option slice of zlint lint names to ignore into a set of
// pre-computed maps: 1) a human-readable name to the profile and 2) a unique
// hash over contents of the profile to the profile itself. It returns the maps
// or an error if a duplicate name or hash is found.
// makeCertificateProfilesMap processes a set of named certificate issuance
// profile configs into a two pre-computed maps: 1) a human-readable name to the
// profile and 2) a unique hash over contents of the profile to the profile
// itself. It returns the maps or an error if a duplicate name or hash is found.
// It also associates the given lint registry with each profile.
//
// The unique hash is used in the case of
// - RA instructs CA1 to issue a precertificate
// - CA1 returns the precertificate DER bytes and profile hash to the RA
// - RA instructs CA2 to issue a final certificate, but CA2 does not contain a
// profile corresponding to that hash and an issuance is prevented.
func makeCertificateProfilesMap(defaultName string, profiles map[string]issuance.ProfileConfig, ignoredLints []string) (certProfilesMaps, error) {
func makeCertificateProfilesMap(defaultName string, profiles map[string]issuance.ProfileConfig, lints lint.Registry) (certProfilesMaps, error) {
if len(profiles) <= 0 {
return certProfilesMaps{}, fmt.Errorf("must pass at least one certificate profile")
}
Expand All @@ -151,7 +152,7 @@ func makeCertificateProfilesMap(defaultName string, profiles map[string]issuance
profileByHash := make(map[[32]byte]*certProfileWithID, len(profiles))

for name, profileConfig := range profiles {
profile, err := issuance.NewProfile(profileConfig, ignoredLints)
profile, err := issuance.NewProfile(profileConfig, lints)
if err != nil {
return certProfilesMaps{}, err
}
Expand Down Expand Up @@ -205,8 +206,8 @@ func NewCertificateAuthorityImpl(
pa core.PolicyAuthority,
boulderIssuers []*issuance.Issuer,
defaultCertProfileName string,
ignoredCertProfileLints []string,
certificateProfiles map[string]issuance.ProfileConfig,
lints lint.Registry,
ecdsaAllowList *ECDSAAllowList,
certExpiry time.Duration,
certBackdate time.Duration,
Expand Down Expand Up @@ -238,7 +239,7 @@ func NewCertificateAuthorityImpl(
return nil, errors.New("must have at least one issuer")
}

certProfiles, err := makeCertificateProfilesMap(defaultCertProfileName, certificateProfiles, ignoredCertProfileLints)
certProfiles, err := makeCertificateProfilesMap(defaultCertProfileName, certificateProfiles, lints)
if err != nil {
return nil, err
}
Expand Down
129 changes: 67 additions & 62 deletions ca/ca_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
ctx509 "github.com/google/certificate-transparency-go/x509"
"github.com/jmhodges/clock"
"github.com/prometheus/client_golang/prometheus"
"github.com/zmap/zlint/v3/lint"
"google.golang.org/grpc"
"google.golang.org/protobuf/types/known/emptypb"

Expand All @@ -31,6 +32,7 @@ import (
"github.com/letsencrypt/boulder/features"
"github.com/letsencrypt/boulder/goodkey"
"github.com/letsencrypt/boulder/issuance"
"github.com/letsencrypt/boulder/linter"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/metrics"
"github.com/letsencrypt/boulder/must"
Expand Down Expand Up @@ -101,23 +103,23 @@ func mustRead(path string) []byte {
}

type testCtx struct {
pa core.PolicyAuthority
ocsp *ocspImpl
crl *crlImpl
defaultCertProfileName string
ignoredCertProfileLints []string
certProfiles map[string]issuance.ProfileConfig
certExpiry time.Duration
certBackdate time.Duration
serialPrefix int
maxNames int
boulderIssuers []*issuance.Issuer
keyPolicy goodkey.KeyPolicy
fc clock.FakeClock
stats prometheus.Registerer
signatureCount *prometheus.CounterVec
signErrorCount *prometheus.CounterVec
logger *blog.Mock
pa core.PolicyAuthority
ocsp *ocspImpl
crl *crlImpl
defaultCertProfileName string
lints lint.Registry
certProfiles map[string]issuance.ProfileConfig
certExpiry time.Duration
certBackdate time.Duration
serialPrefix int
maxNames int
boulderIssuers []*issuance.Issuer
keyPolicy goodkey.KeyPolicy
fc clock.FakeClock
stats prometheus.Registerer
signatureCount *prometheus.CounterVec
signErrorCount *prometheus.CounterVec
logger *blog.Mock
}

type mockSA struct {
Expand Down Expand Up @@ -217,6 +219,9 @@ func setup(t *testing.T) *testCtx {
Help: "A counter of signature errors labelled by error type",
}, []string{"type"})

lints, err := linter.NewRegistry([]string{"w_subject_common_name_included"})
test.AssertNotError(t, err, "Failed to create zlint registry")

ocsp, err := NewOCSPImpl(
boulderIssuers,
24*time.Hour,
Expand All @@ -243,23 +248,23 @@ func setup(t *testing.T) *testCtx {
test.AssertNotError(t, err, "Failed to create crl impl")

return &testCtx{
pa: pa,
ocsp: ocsp,
crl: crl,
defaultCertProfileName: "defaultBoulderCertificateProfile",
ignoredCertProfileLints: []string{"w_subject_common_name_included"},
certProfiles: certProfiles,
certExpiry: 8760 * time.Hour,
certBackdate: time.Hour,
serialPrefix: 17,
maxNames: 2,
boulderIssuers: boulderIssuers,
keyPolicy: keyPolicy,
fc: fc,
stats: metrics.NoopRegisterer,
signatureCount: signatureCount,
signErrorCount: signErrorCount,
logger: blog.NewMock(),
pa: pa,
ocsp: ocsp,
crl: crl,
defaultCertProfileName: "defaultBoulderCertificateProfile",
lints: lints,
certProfiles: certProfiles,
certExpiry: 8760 * time.Hour,
certBackdate: time.Hour,
serialPrefix: 17,
maxNames: 2,
boulderIssuers: boulderIssuers,
keyPolicy: keyPolicy,
fc: fc,
stats: metrics.NoopRegisterer,
signatureCount: signatureCount,
signErrorCount: signErrorCount,
logger: blog.NewMock(),
}
}

Expand Down Expand Up @@ -390,8 +395,8 @@ func issueCertificateSubTestSetup(t *testing.T, e *ECDSAAllowList) (*certificate
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
e,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -440,8 +445,8 @@ func TestNoIssuers(t *testing.T) {
testCtx.pa,
nil, // No issuers
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand All @@ -467,8 +472,8 @@ func TestMultipleIssuers(t *testing.T) {
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -547,8 +552,8 @@ func TestUnpredictableIssuance(t *testing.T) {
testCtx.pa,
boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -596,8 +601,8 @@ func TestUnpredictableIssuance(t *testing.T) {

func TestProfiles(t *testing.T) {
t.Parallel()
ctx := setup(t)
test.AssertEquals(t, len(ctx.certProfiles), 2)
testCtx := setup(t)
test.AssertEquals(t, len(testCtx.certProfiles), 2)

sa := &mockSA{}

Expand Down Expand Up @@ -672,10 +677,10 @@ func TestProfiles(t *testing.T) {
},
{
name: "default profiles from setup func",
profileConfigs: ctx.certProfiles,
profileConfigs: testCtx.certProfiles,
expectedProfiles: []nameToHash{
{
name: ctx.defaultCertProfileName,
name: testCtx.defaultCertProfileName,
hash: [32]byte{205, 182, 88, 236, 32, 18, 154, 120, 148, 194, 42, 215, 117, 140, 13, 169, 127, 196, 219, 67, 82, 36, 147, 67, 254, 117, 65, 112, 202, 60, 185, 9},
},
{
Expand Down Expand Up @@ -709,28 +714,28 @@ func TestProfiles(t *testing.T) {
tc := tc
// This is handled by boulder-ca, not the CA package.
if tc.defaultName == "" {
tc.defaultName = ctx.defaultCertProfileName
tc.defaultName = testCtx.defaultCertProfileName
}
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
tCA, err := NewCertificateAuthorityImpl(
sa,
ctx.pa,
ctx.boulderIssuers,
testCtx.pa,
testCtx.boulderIssuers,
tc.defaultName,
ctx.ignoredCertProfileLints,
tc.profileConfigs,
testCtx.lints,
nil,
ctx.certExpiry,
ctx.certBackdate,
ctx.serialPrefix,
ctx.maxNames,
ctx.keyPolicy,
ctx.logger,
ctx.stats,
ctx.signatureCount,
ctx.signErrorCount,
ctx.fc,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
testCtx.maxNames,
testCtx.keyPolicy,
testCtx.logger,
testCtx.stats,
testCtx.signatureCount,
testCtx.signErrorCount,
testCtx.fc,
)

if tc.expectedErrSubstr != "" {
Expand Down Expand Up @@ -862,8 +867,8 @@ func TestInvalidCSRs(t *testing.T) {
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -903,8 +908,8 @@ func TestRejectValidityTooLong(t *testing.T) {
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -1007,8 +1012,8 @@ func TestIssueCertificateForPrecertificate(t *testing.T) {
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -1078,8 +1083,8 @@ func TestIssueCertificateForPrecertificateWithSpecificCertificateProfile(t *test
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -1200,8 +1205,8 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down Expand Up @@ -1253,8 +1258,8 @@ func TestIssueCertificateForPrecertificateDuplicateSerial(t *testing.T) {
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down
2 changes: 1 addition & 1 deletion ca/ocsp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ func TestOCSP(t *testing.T) {
testCtx.pa,
testCtx.boulderIssuers,
testCtx.defaultCertProfileName,
testCtx.ignoredCertProfileLints,
testCtx.certProfiles,
testCtx.lints,
nil,
testCtx.certExpiry,
testCtx.certBackdate,
Expand Down
13 changes: 12 additions & 1 deletion cmd/boulder-ca/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/prometheus/client_golang/prometheus"
"github.com/zmap/zlint/v3/lint"

"github.com/letsencrypt/boulder/ca"
capb "github.com/letsencrypt/boulder/ca/proto"
Expand All @@ -19,6 +20,7 @@ import (
"github.com/letsencrypt/boulder/goodkey/sagoodkey"
bgrpc "github.com/letsencrypt/boulder/grpc"
"github.com/letsencrypt/boulder/issuance"
"github.com/letsencrypt/boulder/linter"
"github.com/letsencrypt/boulder/policy"
sapb "github.com/letsencrypt/boulder/sa/proto"
)
Expand Down Expand Up @@ -52,6 +54,7 @@ type Config struct {
// TODO(#7159): Make this required once all live configs are using it.
CRLProfile issuance.CRLProfileConfig `validate:"-"`
Issuers []issuance.IssuerConfig `validate:"min=1,dive"`
LintConfig string
IgnoredLints []string
}

Expand Down Expand Up @@ -234,6 +237,14 @@ func main() {
c.CA.Issuance.CertProfiles[c.CA.Issuance.DefaultCertificateProfileName] = c.CA.Issuance.Profile
}

lints, err := linter.NewRegistry(c.CA.Issuance.IgnoredLints)
cmd.FailOnError(err, "Failed to create zlint registry")
if c.CA.Issuance.LintConfig != "" {
lintconfig, err := lint.NewConfigFromFile(c.CA.Issuance.LintConfig)
cmd.FailOnError(err, "Failed to load zlint config file")
lints.SetConfiguration(lintconfig)
}

tlsConfig, err := c.CA.TLS.Load(scope)
cmd.FailOnError(err, "TLS config")

Expand Down Expand Up @@ -295,8 +306,8 @@ func main() {
pa,
issuers,
c.CA.Issuance.DefaultCertificateProfileName,
c.CA.Issuance.IgnoredLints,
c.CA.Issuance.CertProfiles,
lints,
ecdsaAllowList,
c.CA.Expiry.Duration,
c.CA.Backdate.Duration,
Expand Down
Loading

0 comments on commit 939ac1b

Please sign in to comment.