issuance: add new IncludeCRLDistributionPoints bool

To achieve this without breaking hashes of deployed configs,
create a ProfileConfigNew containing the new field (and removing
some deprecated fields).

Move the CA's profile-hashing logic into the `issuance` package,
and gate it on the presence of IncludeCRLDistributionPoints. If
that field is false (the default), create an instance of the old
`ProfileConfig` with the appropriate values and encode/hash that
instead.

Author: jsha

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,10 @@ 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)
+		hash, err := profileConfig.Hash()
 		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())
 
 		withID := certProfileWithID{
 			name:    name,
@@ -256,7 +245,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,

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,21 +571,21 @@ 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",
 		},
 		{
 			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{

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

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.
+//
+// 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,78 @@ 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.
+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"`
+
+	MaxValidityPeriod   config.Duration `asn1:"tag:6,optional"`
+	MaxValidityBackdate config.Duration `asn1:"tag:7,optional"`
+
+	IncludeCRLDistributionPoints bool `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"`
@@ -92,7 +185,7 @@ type Profile struct {
 }
 
 // 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 {

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 = "5939ea199deb3327d1b529da08d6fb07eb8de4c4cca9f6bf499d76da4b67d1e8"
+	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() *ProfileConfig {
-	return &ProfileConfig{
+func defaultProfileConfig() *ProfileConfigNew {
+	return &ProfileConfigNew{
 		AllowMustStaple:     true,
 		MaxValidityPeriod:   config.Duration{Duration: time.Hour},
 		MaxValidityBackdate: config.Duration{Duration: time.Hour},