diff --git a/ca/ca.go b/ca/ca.go index 87a6fc52c70..d58f5ddd39b 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -9,7 +9,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" - "encoding/gob" "encoding/hex" "errors" "fmt" @@ -195,7 +194,7 @@ func makeIssuerMaps(issuers []*issuance.Issuer) (issuerMaps, error) { // - 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) (certProfilesMaps, error) { +func makeCertificateProfilesMap(defaultName string, profiles map[string]*issuance.ProfileConfigNew) (certProfilesMaps, error) { if len(profiles) <= 0 { return certProfilesMaps{}, fmt.Errorf("must pass at least one certificate profile") } @@ -215,20 +214,7 @@ func makeCertificateProfilesMap(defaultName string, profiles map[string]*issuanc return certProfilesMaps{}, err } - // gob can only encode exported fields, of which an issuance.Profile has - // none. However, since we're already in a loop iteration having access - // to the issuance.ProfileConfig used to generate the issuance.Profile, - // we'll generate the hash from that. - var encodedProfile bytes.Buffer - enc := gob.NewEncoder(&encodedProfile) - err = enc.Encode(profileConfig) - if err != nil { - return certProfilesMaps{}, err - } - if len(encodedProfile.Bytes()) <= 0 { - return certProfilesMaps{}, fmt.Errorf("certificate profile encoding returned 0 bytes") - } - hash := sha256.Sum256(encodedProfile.Bytes()) + hash := profile.Hash() withID := certProfileWithID{ name: name, @@ -237,7 +223,6 @@ func makeCertificateProfilesMap(defaultName string, profiles map[string]*issuanc } profilesByName[name] = &withID - _, found := profilesByHash[hash] if found { return certProfilesMaps{}, fmt.Errorf("duplicate certificate profile hash %d", hash) @@ -256,7 +241,7 @@ func NewCertificateAuthorityImpl( pa core.PolicyAuthority, boulderIssuers []*issuance.Issuer, defaultCertProfileName string, - certificateProfiles map[string]*issuance.ProfileConfig, + certificateProfiles map[string]*issuance.ProfileConfigNew, serialPrefix byte, maxNames int, keyPolicy goodkey.KeyPolicy, diff --git a/ca/ca_test.go b/ca/ca_test.go index c96de987669..8d7d4d6f50f 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -102,7 +102,7 @@ type testCtx struct { ocsp *ocspImpl crl *crlImpl defaultCertProfileName string - certProfiles map[string]*issuance.ProfileConfig + certProfiles map[string]*issuance.ProfileConfigNew serialPrefix byte maxNames int boulderIssuers []*issuance.Issuer @@ -153,14 +153,14 @@ func setup(t *testing.T) *testCtx { err = pa.LoadHostnamePolicyFile("../test/hostname-policy.yaml") test.AssertNotError(t, err, "Couldn't set hostname policy") - certProfiles := make(map[string]*issuance.ProfileConfig, 0) - certProfiles["legacy"] = &issuance.ProfileConfig{ + certProfiles := make(map[string]*issuance.ProfileConfigNew, 0) + certProfiles["legacy"] = &issuance.ProfileConfigNew{ AllowMustStaple: true, MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 90}, MaxValidityBackdate: config.Duration{Duration: time.Hour}, IgnoredLints: []string{"w_subject_common_name_included"}, } - certProfiles["modern"] = &issuance.ProfileConfig{ + certProfiles["modern"] = &issuance.ProfileConfigNew{ AllowMustStaple: true, OmitCommonName: true, OmitKeyEncipherment: true, @@ -546,7 +546,7 @@ func TestMakeCertificateProfilesMap(t *testing.T) { testCtx := setup(t) test.AssertEquals(t, len(testCtx.certProfiles), 2) - testProfile := issuance.ProfileConfig{ + testProfile := issuance.ProfileConfigNew{ AllowMustStaple: false, MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 90}, MaxValidityBackdate: config.Duration{Duration: time.Hour}, @@ -560,7 +560,7 @@ func TestMakeCertificateProfilesMap(t *testing.T) { testCases := []struct { name string defaultName string - profileConfigs map[string]*issuance.ProfileConfig + profileConfigs map[string]*issuance.ProfileConfigNew expectedErrSubstr string expectedProfiles []nameToHash }{ @@ -571,13 +571,13 @@ func TestMakeCertificateProfilesMap(t *testing.T) { }, { name: "no profiles", - profileConfigs: map[string]*issuance.ProfileConfig{}, + profileConfigs: map[string]*issuance.ProfileConfigNew{}, expectedErrSubstr: "at least one certificate profile", }, { name: "no profile matching default name", defaultName: "default", - profileConfigs: map[string]*issuance.ProfileConfig{ + profileConfigs: map[string]*issuance.ProfileConfigNew{ "notDefault": &testProfile, }, expectedErrSubstr: "profile object was not found for that name", @@ -585,7 +585,7 @@ func TestMakeCertificateProfilesMap(t *testing.T) { { name: "duplicate hash", defaultName: "default", - profileConfigs: map[string]*issuance.ProfileConfig{ + profileConfigs: map[string]*issuance.ProfileConfigNew{ "default": &testProfile, "default2": &testProfile, }, @@ -594,7 +594,7 @@ func TestMakeCertificateProfilesMap(t *testing.T) { { name: "empty profile config", defaultName: "empty", - profileConfigs: map[string]*issuance.ProfileConfig{ + profileConfigs: map[string]*issuance.ProfileConfigNew{ "empty": {}, }, expectedProfiles: []nameToHash{ diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index d7ab60ccec3..a4e60fced4f 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -40,7 +40,7 @@ type Config struct { // One of the profile names must match the value of // DefaultCertificateProfileName or boulder-ca will fail to start. - CertProfiles map[string]*issuance.ProfileConfig `validate:"dive,keys,alphanum,min=1,max=32,endkeys,required_without=Profile,structonly"` + CertProfiles map[string]*issuance.ProfileConfigNew `validate:"dive,keys,alphanum,min=1,max=32,endkeys,required_without=Profile,structonly"` // TODO(#7159): Make this required once all live configs are using it. CRLProfile issuance.CRLProfileConfig `validate:"-"` diff --git a/issuance/cert.go b/issuance/cert.go index 0c97b1b84c5..43064c25495 100644 --- a/issuance/cert.go +++ b/issuance/cert.go @@ -6,9 +6,11 @@ import ( "crypto/ecdsa" "crypto/rand" "crypto/rsa" + "crypto/sha256" "crypto/x509" "crypto/x509/pkix" "encoding/asn1" + "encoding/gob" "encoding/json" "errors" "fmt" @@ -28,7 +30,26 @@ import ( "github.com/letsencrypt/boulder/precert" ) -// ProfileConfig describes the certificate issuance constraints for all issuers. +// ProfileConfig is a subset of ProfileConfigNew used for hashing. +// +// Deprecated: Use ProfileConfigNew instead. +// +// This struct exists for backwards-compatibility purposes when generating hashes +// of profile configs. +// +// The CA uses a hash of the gob encoding of ProfileConfig to ensure precert +// and final cert issuance use the exact same profile settings. Gob encodes all +// fields, including zero values, which means adding fields immediately changes all +// hashes, causing a deployability problem. It also encodes the struct name. +// +// To solve the deployability problem, we're switching to ASN.1 encoding. However, +// while deploying that we still need the ability to hash old configs the same way +// they've always been hashed. So this struct (with the same name it always had) +// gets hashed, only when `ProfileConfigNew.IncludeCRLDistributionPoints` (the +// newly added field) is false. +// +// Note that gob encodes the names of structs, not just their fields, so we needed +// to retain the name as well. type ProfileConfig struct { // AllowMustStaple, when false, causes all IssuanceRequests which specify the // OCSP Must Staple extension to be rejected. @@ -72,6 +93,82 @@ type ProfileConfig struct { Policies []PolicyConfig `validate:"-"` } +// ProfileConfigNew describes the certificate issuance constraints for all issuers. +// +// See ProfileConfig for why this is called "New". +// +// This struct gets hashed in the CA to allow matching up precert and final cert +// issuance by the exact profile config. We compute the hash over an ASN.1 encoding +// because ASN.1 encoding has a canonical form and can omit optional fields (which +// allows for gracefully adding new fields without changing the hash of existing +// profile configs). This struct does not get embedded into any certs, CRLs, or +// other objects, and does not get signed; it's only used internally. +// +// Note: even though these fields have encoding instructions (tag:N), they will +// be encoded in the order they appear in the struct, so do not reorder them. +type ProfileConfigNew struct { + // AllowMustStaple, when false, causes all IssuanceRequests which specify the + // OCSP Must Staple extension to be rejected. + AllowMustStaple bool `asn1:"tag:1,optional"` + + // OmitCommonName causes the CN field to be excluded from the resulting + // certificate, regardless of its inclusion in the IssuanceRequest. + OmitCommonName bool `asn1:"tag:2,optional"` + // OmitKeyEncipherment causes the keyEncipherment bit to be omitted from the + // Key Usage field of all certificates (instead of only from ECDSA certs). + OmitKeyEncipherment bool `asn1:"tag:3,optional"` + // OmitClientAuth causes the id-kp-clientAuth OID (TLS Client Authentication) + // to be omitted from the EKU extension. + OmitClientAuth bool `asn1:"tag:4,optional"` + // OmitSKID causes the Subject Key Identifier extension to be omitted. + OmitSKID bool `asn1:"tag:5,optional"` + // IncludeCRLDistributionPoints causes the CRLDistributionPoints extension to + // be added to all certificates issued by this profile. + IncludeCRLDistributionPoints bool `asn1:"tag:6,optional"` + + MaxValidityPeriod config.Duration `asn1:"tag:7,optional"` + MaxValidityBackdate config.Duration `asn1:"tag:8,optional"` + + // LintConfig is a path to a zlint config file, which can be used to control + // the behavior of zlint's "customizable lints". + LintConfig string `asn1:"tag:9,optional"` + // IgnoredLints is a list of lint names that we know will fail for this + // profile, and which we know it is safe to ignore. + IgnoredLints []string `asn1:"tag:10,optional"` +} + +func (pcn ProfileConfigNew) Hash() ([32]byte, error) { + var encodedBytes []byte + var err error + if !pcn.IncludeCRLDistributionPoints { + old := ProfileConfig{ + AllowMustStaple: pcn.AllowMustStaple, + AllowCTPoison: false, + AllowSCTList: false, + AllowCommonName: false, + OmitCommonName: pcn.OmitCommonName, + OmitKeyEncipherment: pcn.OmitKeyEncipherment, + OmitClientAuth: pcn.OmitClientAuth, + OmitSKID: pcn.OmitSKID, + MaxValidityPeriod: pcn.MaxValidityPeriod, + MaxValidityBackdate: pcn.MaxValidityBackdate, + LintConfig: pcn.LintConfig, + IgnoredLints: pcn.IgnoredLints, + Policies: nil, + } + var encoded bytes.Buffer + enc := gob.NewEncoder(&encoded) + err = enc.Encode(old) + encodedBytes = encoded.Bytes() + } else { + encodedBytes, err = asn1.Marshal(pcn) + } + if err != nil { + return [32]byte{}, err + } + return sha256.Sum256(encodedBytes), nil +} + // PolicyConfig describes a policy type PolicyConfig struct { OID string `validate:"required"` @@ -89,10 +186,12 @@ type Profile struct { maxValidity time.Duration lints lint.Registry + + hash [32]byte } // NewProfile converts the profile config into a usable profile. -func NewProfile(profileConfig *ProfileConfig) (*Profile, error) { +func NewProfile(profileConfig *ProfileConfigNew) (*Profile, error) { // The Baseline Requirements, Section 7.1.2.7, says that the notBefore time // must be "within 48 hours of the time of signing". We can be even stricter. if profileConfig.MaxValidityBackdate.Duration >= 24*time.Hour { @@ -113,6 +212,11 @@ func NewProfile(profileConfig *ProfileConfig) (*Profile, error) { lints.SetConfiguration(lintconfig) } + hash, err := profileConfig.Hash() + if err != nil { + return nil, err + } + sp := &Profile{ allowMustStaple: profileConfig.AllowMustStaple, omitCommonName: profileConfig.OmitCommonName, @@ -122,11 +226,16 @@ func NewProfile(profileConfig *ProfileConfig) (*Profile, error) { maxBackdate: profileConfig.MaxValidityBackdate.Duration, maxValidity: profileConfig.MaxValidityPeriod.Duration, lints: lints, + hash: hash, } return sp, nil } +func (p *Profile) Hash() [32]byte { + return p.hash +} + // GenerateValidity returns a notBefore/notAfter pair bracketing the input time, // based on the profile's configured backdate and validity. func (p *Profile) GenerateValidity(now time.Time) (time.Time, time.Time) { diff --git a/issuance/cert_test.go b/issuance/cert_test.go index 80f8c5d4674..ca8386146fc 100644 --- a/issuance/cert_test.go +++ b/issuance/cert_test.go @@ -11,12 +11,14 @@ import ( "crypto/x509/pkix" "encoding/asn1" "encoding/base64" + "fmt" "testing" "time" ct "github.com/google/certificate-transparency-go" "github.com/jmhodges/clock" + "github.com/letsencrypt/boulder/config" "github.com/letsencrypt/boulder/ctpolicy/loglist" "github.com/letsencrypt/boulder/linter" "github.com/letsencrypt/boulder/test" @@ -779,7 +781,7 @@ func TestMismatchedProfiles(t *testing.T) { // Create a new profile that differs slightly (no common name) pc = defaultProfileConfig() - pc.AllowCommonName = false + pc.OmitCommonName = false test.AssertNotError(t, err, "building test lint registry") noCNProfile, err := NewProfile(pc) test.AssertNotError(t, err, "NewProfile failed") @@ -809,3 +811,51 @@ func TestMismatchedProfiles(t *testing.T) { test.AssertError(t, err, "preparing final cert issuance") test.AssertContains(t, err.Error(), "precert does not correspond to linted final cert") } + +func TestProfileHash(t *testing.T) { + // A profile without IncludeCRLDistributionPoints. + // Hash calculated over the gob encoding of the old `ProfileConfig`. + profile := ProfileConfigNew{ + IncludeCRLDistributionPoints: false, + AllowMustStaple: true, + OmitCommonName: true, + OmitKeyEncipherment: false, + OmitClientAuth: false, + OmitSKID: true, + MaxValidityPeriod: config.Duration{Duration: time.Hour}, + MaxValidityBackdate: config.Duration{Duration: time.Second}, + LintConfig: "example/config.toml", + IgnoredLints: []string{"one", "two"}, + } + hash, err := profile.Hash() + if err != nil { + t.Fatalf("hashing %+v: %s", profile, err) + } + expectedHash := "f6b5766141fdc066824e781347095ffb3c86fa97a174e21123a323a93b078f46" + if expectedHash != fmt.Sprintf("%x", hash) { + t.Errorf("%+v.Hash()=%x, want %s", profile, hash, expectedHash) + } + + // A profile _with_ IncludeCRLDistributionPoints. + // Hash calculated over the ASN.1 encoding of the `ProfileConfigNew`. + profile = ProfileConfigNew{ + IncludeCRLDistributionPoints: true, + AllowMustStaple: true, + OmitCommonName: true, + OmitKeyEncipherment: false, + OmitClientAuth: false, + OmitSKID: true, + MaxValidityPeriod: config.Duration{Duration: time.Hour}, + MaxValidityBackdate: config.Duration{Duration: time.Second}, + LintConfig: "example/config.toml", + IgnoredLints: []string{"one", "two"}, + } + hash, err = profile.Hash() + if err != nil { + t.Fatalf("hashing %+v: %s", profile, err) + } + expectedHash = "d2a6c9f0aa37d2ac0b15476cb6e0ae9b98ba59b1321d8d6da26efc620581c53d" + if expectedHash != fmt.Sprintf("%x", hash) { + t.Errorf("%+v.Hash()=%x, want %s", profile, hash, expectedHash) + } +} diff --git a/issuance/issuer_test.go b/issuance/issuer_test.go index 39e409fa059..46f23d38579 100644 --- a/issuance/issuer_test.go +++ b/issuance/issuer_test.go @@ -22,8 +22,8 @@ import ( "github.com/letsencrypt/boulder/test" ) -func defaultProfileConfig() *ProfileConfig { - return &ProfileConfig{ +func defaultProfileConfig() *ProfileConfigNew { + return &ProfileConfigNew{ AllowMustStaple: true, MaxValidityPeriod: config.Duration{Duration: time.Hour}, MaxValidityBackdate: config.Duration{Duration: time.Hour},