Skip to content

Commit

Permalink
issuance: add new IncludeCRLDistributionPoints bool (#7985)
Browse files Browse the repository at this point in the history 
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.

Note: the IncludeCRLDistributionPoints field does not yet control any
behavior. That will be part of #7974.

Part of #7094
  • Loading branch information
@jsha
jsha authored Jan 30, 2025
1 parent c7da120 commit d93f0c3
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 34 deletions.
21 changes: 3 additions & 18 deletions ca/ca.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"encoding/gob"
"encoding/hex"
"errors"
"fmt"
Expand Down Expand Up @@ -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")
}
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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,
Expand Down
20 changes: 10 additions & 10 deletions ca/ca_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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},
Expand All @@ -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
}{
Expand All @@ -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,
},
Expand All @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion cmd/boulder-ca/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
Expand Down
113 changes: 111 additions & 2 deletions issuance/cert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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"`
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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) {
Expand Down
52 changes: 51 additions & 1 deletion issuance/cert_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
}
4 changes: 2 additions & 2 deletions issuance/issuer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down

0 comments on commit d93f0c3

Please sign in to comment.