Simplify profile config hashing

Author: aarongable

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,

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,26 +576,26 @@ 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,
 			},
 			expectedErrSubstr: "duplicate certificate profile hash",
 		},
 		{
 			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},
 				},
 			},
 		},

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:"-"`

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
 	}

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)
 	}

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},

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,