Simplify profile config hashing (#8081)

Remove the backwards-compatible profile hashing code. It is no longer necessary, since all deployed profile configs now set IncludeCRLDistributionPoints to true and set the UnsplitIssuance flag to true. Catch up the CA and crl-updater configs to match config-next and what is actively deployed in prod. Part of https://github.com/letsencrypt/boulder/issues/8039 Part of https://github.com/letsencrypt/boulder/issues/8059
2025-03-26 17:59:18 -05:00 · 2025-03-26 17:59:18 -05:00 · 8b1a87ea8d
parent c881ce1f94
commit 8b1a87ea8d
10 changed files with 46 additions and 166 deletions
--- a/ca/ca.go
+++ b/ca/ca.go
@ -200,7 +200,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(profiles map[string]*issuance.ProfileConfigNew) (certProfilesMaps, error) {
+func makeCertificateProfilesMap(profiles map[string]*issuance.ProfileConfig) (certProfilesMaps, error) {
 	if len(profiles) <= 0 {
 		return certProfilesMaps{}, fmt.Errorf("must pass at least one certificate profile")
 	}
@ -241,7 +241,7 @@ func NewCertificateAuthorityImpl(
 	sctService rapb.SCTProviderClient,
 	pa core.PolicyAuthority,
 	boulderIssuers []*issuance.Issuer,
-	certificateProfiles map[string]*issuance.ProfileConfigNew,
+	certificateProfiles map[string]*issuance.ProfileConfig,
 	serialPrefix byte,
 	maxNames int,
 	keyPolicy goodkey.KeyPolicy,
--- a/ca/ca_test.go
+++ b/ca/ca_test.go
@ -102,7 +102,7 @@ type testCtx struct {
 	pa             core.PolicyAuthority
 	ocsp           *ocspImpl
 	crl            *crlImpl
-	certProfiles   map[string]*issuance.ProfileConfigNew
+	certProfiles   map[string]*issuance.ProfileConfig
 	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.ProfileConfigNew, 0)
-	certProfiles["legacy"] = &issuance.ProfileConfigNew{
+	certProfiles := make(map[string]*issuance.ProfileConfig, 0)
+	certProfiles["legacy"] = &issuance.ProfileConfig{
 		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.ProfileConfigNew{
+	certProfiles["modern"] = &issuance.ProfileConfig{
 		AllowMustStaple:     true,
 		OmitCommonName:      true,
 		OmitKeyEncipherment: true,
@ -552,7 +552,7 @@ func TestMakeCertificateProfilesMap(t *testing.T) {
 	testCtx := setup(t)
 	test.AssertEquals(t, len(testCtx.certProfiles), 2)

-	testProfile := issuance.ProfileConfigNew{
+	testProfile := issuance.ProfileConfig{
 		AllowMustStaple:     false,
 		MaxValidityPeriod:   config.Duration{Duration: time.Hour * 24 * 90},
 		MaxValidityBackdate: config.Duration{Duration: time.Hour},
@ -565,7 +565,7 @@ func TestMakeCertificateProfilesMap(t *testing.T) {

 	testCases := []struct {
 		name              string
-		profileConfigs    map[string]*issuance.ProfileConfigNew
+		profileConfigs    map[string]*issuance.ProfileConfig
 		expectedErrSubstr string
 		expectedProfiles  []nameToHash
 	}{
@ -576,12 +576,12 @@ func TestMakeCertificateProfilesMap(t *testing.T) {
 		},
 		{
 			name:              "no profiles",
-			profileConfigs:    map[string]*issuance.ProfileConfigNew{},
+			profileConfigs:    map[string]*issuance.ProfileConfig{},
 			expectedErrSubstr: "at least one certificate profile",
 		},
 		{
 			name: "duplicate hash",
-			profileConfigs: map[string]*issuance.ProfileConfigNew{
+			profileConfigs: map[string]*issuance.ProfileConfig{
 				"default":  &testProfile,
 				"default2": &testProfile,
 			},
@ -589,13 +589,13 @@ func TestMakeCertificateProfilesMap(t *testing.T) {
 		},
 		{
 			name: "empty profile config",
-			profileConfigs: map[string]*issuance.ProfileConfigNew{
+			profileConfigs: map[string]*issuance.ProfileConfig{
 				"empty": {},
 			},
 			expectedProfiles: []nameToHash{
 				{
 					name: "empty",
-					hash: [32]byte{0x25, 0x27, 0x72, 0xa1, 0xaf, 0x95, 0xfe, 0xc7, 0x32, 0x78, 0x38, 0x97, 0xd0, 0xf1, 0x83, 0x92, 0xc3, 0xac, 0x60, 0x91, 0x68, 0x4f, 0x22, 0xb6, 0x57, 0x2f, 0x89, 0x1a, 0x54, 0xe5, 0xd8, 0xa3},
+					hash: [32]byte{0xe4, 0xf6, 0xd, 0xa, 0xa6, 0xd7, 0xf3, 0xd3, 0xb6, 0xa6, 0x49, 0x4b, 0x1c, 0x86, 0x1b, 0x99, 0xf6, 0x49, 0xc6, 0xf9, 0xec, 0x51, 0xab, 0xaf, 0x20, 0x1b, 0x20, 0xf2, 0x97, 0x32, 0x7c, 0x95},
 				},
 			},
 		},
@ -605,11 +605,11 @@ func TestMakeCertificateProfilesMap(t *testing.T) {
 			expectedProfiles: []nameToHash{
 				{
 					name: "legacy",
-					hash: [32]byte{0x44, 0xc5, 0xbc, 0x73, 0x8, 0x95, 0xba, 0x4c, 0x13, 0x12, 0xc4, 0xc, 0x5d, 0x77, 0x2f, 0x54, 0xf8, 0x54, 0x1, 0xb8, 0x84, 0xaf, 0x6c, 0x58, 0x74, 0x6, 0xac, 0xda, 0x3e, 0x37, 0xfc, 0x88},
+					hash: [32]byte{0xb7, 0xd9, 0x7e, 0xfc, 0x5a, 0xdd, 0xc7, 0xfe, 0xc, 0xea, 0xed, 0x7b, 0x8c, 0xf5, 0x4, 0x57, 0x71, 0x97, 0x42, 0x80, 0xbe, 0x4d, 0x14, 0xa, 0x35, 0x9a, 0x89, 0xc3, 0x7a, 0x57, 0x41, 0xb7},
 				},
 				{
 					name: "modern",
-					hash: [32]byte{0x58, 0x7, 0xea, 0x3a, 0x85, 0xcd, 0xf9, 0xd1, 0x7a, 0x9a, 0x59, 0x76, 0xfc, 0x92, 0xea, 0x1b, 0x69, 0x54, 0xe4, 0xbe, 0xcf, 0xe3, 0x91, 0xfa, 0x85, 0x4, 0xbf, 0x1f, 0x55, 0x97, 0x2c, 0x8b},
+					hash: [32]byte{0x2e, 0x82, 0x9b, 0xe4, 0x4d, 0xac, 0xfc, 0x2d, 0x83, 0xbf, 0x62, 0xe5, 0xe1, 0x50, 0xe8, 0xba, 0xd2, 0x66, 0x1a, 0xb3, 0xf2, 0xe7, 0xb5, 0xf2, 0x24, 0x94, 0x1f, 0x83, 0xc6, 0x57, 0xe, 0x58},
 				},
 			},
 		},
--- a/cmd/boulder-ca/main.go
+++ b/cmd/boulder-ca/main.go
@ -46,7 +46,7 @@ type Config struct {

 			// One of the profile names must match the value of ra.defaultProfileName
 			// or large amounts of issuance will fail.
-			CertProfiles map[string]*issuance.ProfileConfigNew `validate:"dive,keys,alphanum,min=1,max=32,endkeys,required_without=Profile,structonly"`
+			CertProfiles map[string]*issuance.ProfileConfig `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:"-"`
--- a/issuance/cert.go
+++ b/issuance/cert.go
@ -10,7 +10,6 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/asn1"
-	"encoding/gob"
 	"encoding/json"
 	"errors"
 	"fmt"
@ -30,72 +29,7 @@ import (
 	"github.com/letsencrypt/boulder/precert"
 )

-// 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.
-	AllowMustStaple bool
-	// AllowCTPoison has no effect.
-	// Deprecated: We will always allow the CT Poison extension because it is
-	// mandated for Precertificates.
-	AllowCTPoison bool
-	// AllowSCTList has no effect.
-	// Deprecated: We intend to include SCTs in all final Certificates for the
-	// foreseeable future.
-	AllowSCTList bool
-	// AllowCommonName has no effect.
-	// Deprecated: Rather than rejecting IssuanceRequests which include a common
-	// name, we would prefer to simply drop the CN. Use `OmitCommonName` instead.
-	AllowCommonName bool
-
-	// OmitCommonName causes the CN field to be excluded from the resulting
-	// certificate, regardless of its inclusion in the IssuanceRequest.
-	OmitCommonName bool
-	// OmitKeyEncipherment causes the keyEncipherment bit to be omitted from the
-	// Key Usage field of all certificates (instead of only from ECDSA certs).
-	OmitKeyEncipherment bool
-	// OmitClientAuth causes the id-kp-clientAuth OID (TLS Client Authentication)
-	// to be omitted from the EKU extension.
-	OmitClientAuth bool
-	// OmitSKID causes the Subject Key Identifier extension to be omitted.
-	OmitSKID bool
-
-	MaxValidityPeriod   config.Duration
-	MaxValidityBackdate config.Duration
-
-	// LintConfig is a path to a zlint config file, which can be used to control
-	// the behavior of zlint's "customizable lints".
-	LintConfig string
-	// 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
-
-	// Deprecated: we do not respect this field.
-	Policies []PolicyConfig `validate:"-"`
-}
-
-// ProfileConfigNew describes the certificate issuance constraints for all issuers.
-//
-// See ProfileConfig for why this is called "New".
+// ProfileConfig describes the certificate issuance constraints for all issuers.
 //
 // 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
@ -106,7 +40,7 @@ type ProfileConfig struct {
 //
 // 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 {
+type ProfileConfig struct {
 	// AllowMustStaple, when false, causes all IssuanceRequests which specify the
 	// OCSP Must Staple extension to be rejected.
 	AllowMustStaple bool `asn1:"tag:1,optional"`
@ -137,32 +71,8 @@ type ProfileConfigNew struct {
 	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)
-	}
+func (pcn ProfileConfig) hash() ([32]byte, error) {
+	encodedBytes, err := asn1.Marshal(pcn)
 	if err != nil {
 		return [32]byte{}, err
 	}
@ -193,7 +103,7 @@ type Profile struct {
 }

 // NewProfile converts the profile config into a usable profile.
-func NewProfile(profileConfig *ProfileConfigNew) (*Profile, error) {
+func NewProfile(profileConfig *ProfileConfig) (*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 {
@ -214,7 +124,7 @@ func NewProfile(profileConfig *ProfileConfigNew) (*Profile, error) {
 		lints.SetConfiguration(lintconfig)
 	}

-	hash, err := profileConfig.Hash()
+	hash, err := profileConfig.hash()
 	if err != nil {
 		return nil, err
 	}
--- a/issuance/cert_test.go
+++ b/issuance/cert_test.go
@ -870,32 +870,9 @@ func TestMismatchedProfiles(t *testing.T) {
 }

 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{
+	profile := ProfileConfig{
 		IncludeCRLDistributionPoints: true,
 		AllowMustStaple:              true,
 		OmitCommonName:               true,
@ -907,11 +884,11 @@ func TestProfileHash(t *testing.T) {
 		LintConfig:                   "example/config.toml",
 		IgnoredLints:                 []string{"one", "two"},
 	}
-	hash, err = profile.Hash()
+	hash, err := profile.hash()
 	if err != nil {
 		t.Fatalf("hashing %+v: %s", profile, err)
 	}
-	expectedHash = "d2a6c9f0aa37d2ac0b15476cb6e0ae9b98ba59b1321d8d6da26efc620581c53d"
+	expectedHash := "d2a6c9f0aa37d2ac0b15476cb6e0ae9b98ba59b1321d8d6da26efc620581c53d"
 	if expectedHash != fmt.Sprintf("%x", hash) {
 		t.Errorf("%+v.Hash()=%x, want %s", profile, hash, expectedHash)
 	}
--- a/issuance/issuer_test.go
+++ b/issuance/issuer_test.go
@ -22,8 +22,8 @@ import (
 	"github.com/letsencrypt/boulder/test"
 )

-func defaultProfileConfig() *ProfileConfigNew {
-	return &ProfileConfigNew{
+func defaultProfileConfig() *ProfileConfig {
+	return &ProfileConfig{
 		AllowMustStaple:     true,
 		MaxValidityPeriod:   config.Duration{Duration: time.Hour},
 		MaxValidityBackdate: config.Duration{Duration: time.Hour},
--- a/test/config-next/ca.json
+++ b/test/config-next/ca.json
@ -55,17 +55,7 @@
 			"certProfiles": {
 				"legacy": {
 					"allowMustStaple": true,
-					"maxValidityPeriod": "7776000s",
-					"maxValidityBackdate": "1h5m",
 					"includeCRLDistributionPoints": true,
-					"lintConfig": "test/config-next/zlint.toml",
-					"ignoredLints": [
-						"w_subject_common_name_included",
-						"w_ext_subject_key_identifier_not_recommended_subscriber"
-					]
-				},
-				"legacyRenamedPriorToDeletion": {
-					"allowMustStaple": true,
 					"maxValidityPeriod": "7776000s",
 					"maxValidityBackdate": "1h5m",
 					"lintConfig": "test/config-next/zlint.toml",
@ -88,19 +78,6 @@
 						"w_ext_subject_key_identifier_missing_sub_cert"
 					]
 				},
-				"modernRenamedPriorToDeletion": {
-					"allowMustStaple": true,
-					"omitCommonName": true,
-					"omitKeyEncipherment": true,
-					"omitClientAuth": true,
-					"omitSKID": true,
-					"maxValidityPeriod": "583200s",
-					"maxValidityBackdate": "1h5m",
-					"lintConfig": "test/config-next/zlint.toml",
-					"ignoredLints": [
-						"w_ext_subject_key_identifier_missing_sub_cert"
-					]
-				},
 				"shortlived": {
 					"allowMustStaple": true,
 					"omitCommonName": true,
--- a/test/config/ca.json
+++ b/test/config/ca.json
@ -58,6 +58,7 @@
 			"certProfiles": {
 				"legacy": {
 					"allowMustStaple": true,
+					"includeCRLDistributionPoints": true,
 					"maxValidityPeriod": "7776000s",
 					"maxValidityBackdate": "1h5m",
 					"lintConfig": "test/config-next/zlint.toml",
@ -72,6 +73,7 @@
 					"omitKeyEncipherment": true,
 					"omitClientAuth": true,
 					"omitSKID": true,
+					"includeCRLDistributionPoints": true,
 					"maxValidityPeriod": "583200s",
 					"maxValidityBackdate": "1h5m",
 					"lintConfig": "test/config-next/zlint.toml",
@ -85,6 +87,7 @@
 					"omitKeyEncipherment": true,
 					"omitClientAuth": true,
 					"omitSKID": true,
+					"includeCRLDistributionPoints": true,
 					"maxValidityPeriod": "160h",
 					"maxValidityBackdate": "1h5m",
 					"lintConfig": "test/config-next/zlint.toml",
@ -101,6 +104,7 @@
 			"issuers": [
 				{
 					"active": true,
+					"crlShards": 10,
 					"issuerURL": "http://ca.example.org:4502/int-ecdsa-a",
 					"ocspURL": "http://ca.example.org:4002/",
 					"crlURLBase": "http://ca.example.org:4501/lets-encrypt-crls/43104258997432926/",
@ -112,6 +116,7 @@
 				},
 				{
 					"active": true,
+					"crlShards": 10,
 					"issuerURL": "http://ca.example.org:4502/int-ecdsa-b",
 					"ocspURL": "http://ca.example.org:4002/",
 					"crlURLBase": "http://ca.example.org:4501/lets-encrypt-crls/17302365692836921/",
@ -123,6 +128,7 @@
 				},
 				{
 					"active": false,
+					"crlShards": 10,
 					"issuerURL": "http://ca.example.org:4502/int-ecdsa-c",
 					"ocspURL": "http://ca.example.org:4002/",
 					"crlURLBase": "http://ca.example.org:4501/lets-encrypt-crls/56560759852043581/",
@ -134,6 +140,7 @@
 				},
 				{
 					"active": true,
+					"crlShards": 10,
 					"issuerURL": "http://ca.example.org:4502/int-rsa-a",
 					"ocspURL": "http://ca.example.org:4002/",
 					"crlURLBase": "http://ca.example.org:4501/lets-encrypt-crls/29947985078257530/",
@ -145,6 +152,7 @@
 				},
 				{
 					"active": true,
+					"crlShards": 10,
 					"issuerURL": "http://ca.example.org:4502/int-rsa-b",
 					"ocspURL": "http://ca.example.org:4002/",
 					"crlURLBase": "http://ca.example.org:4501/lets-encrypt-crls/6762885421992935/",
@ -156,6 +164,7 @@
 				},
 				{
 					"active": false,
+					"crlShards": 10,
 					"issuerURL": "http://ca.example.org:4502/int-rsa-c",
 					"ocspURL": "http://ca.example.org:4002/",
 					"crlURLBase": "http://ca.example.org:4501/lets-encrypt-crls/56183656833365902/",
@ -167,7 +176,7 @@
 				}
 			]
 		},
-		"serialPrefix": 127,
+		"serialPrefixHex": "6e",
 		"maxNames": 100,
 		"lifespanOCSP": "96h",
 		"goodkey": {
--- a/test/config/crl-updater.json
+++ b/test/config/crl-updater.json
@ -46,9 +46,15 @@
 		"numShards": 10,
 		"shardWidth": "240h",
 		"lookbackPeriod": "24h",
-		"updatePeriod": "6h",
+		"updatePeriod": "10m",
+		"updateTimeout": "1m",
+		"expiresMargin": "5m",
+		"cacheControl": "stale-if-error=60",
+		"temporallyShardedSerialPrefixes": [
+			"7f"
+		],
 		"maxParallelism": 10,
-		"maxAttempts": 5,
+		"maxAttempts": 2,
 		"features": {}
 	},
 	"syslog": {
--- a/test/config/ocsp-responder.json
+++ b/test/config/ocsp-responder.json
@ -62,7 +62,8 @@
 		"maxInflightSignings": 20,
 		"debugAddr": ":8005",
 		"requiredSerialPrefixes": [
-			"7f"
+			"7f",
+			"6e"
 		],
 		"features": {}
 	},