From 3fa3184f7a72e8dc5c335ae91618443362f2e356 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Tue, 25 Mar 2025 21:37:57 -0700 Subject: [PATCH] Simplify profile config hashing --- ca/ca.go | 4 +- ca/ca_test.go | 24 ++++---- cmd/boulder-ca/main.go | 2 +- issuance/cert.go | 102 ++------------------------------ issuance/cert_test.go | 29 +-------- issuance/issuer_test.go | 4 +- test/config-next/ca.json | 23 ------- test/config/ca.json | 11 +++- test/config/crl-updater.json | 10 +++- test/config/ocsp-responder.json | 3 +- 10 files changed, 46 insertions(+), 166 deletions(-) diff --git a/ca/ca.go b/ca/ca.go index a598fc5cd09..067e298a2be 100644 --- 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, diff --git a/ca/ca_test.go b/ca/ca_test.go index f6b4cc50121..6d535ea3125 100644 --- 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}, }, }, }, diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index d853a62595c..e579a2ba68c 100644 --- 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:"-"` diff --git a/issuance/cert.go b/issuance/cert.go index 1502f3c7bcf..84e17c7d90f 100644 --- 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 } diff --git a/issuance/cert_test.go b/issuance/cert_test.go index 90e227e46bc..b4ebf3cde51 100644 --- 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) } diff --git a/issuance/issuer_test.go b/issuance/issuer_test.go index 46f23d38579..39e409fa059 100644 --- 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}, diff --git a/test/config-next/ca.json b/test/config-next/ca.json index 6692c568f3f..020b974dcf2 100644 --- 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, diff --git a/test/config/ca.json b/test/config/ca.json index a64ec7ac255..47dd4ed74bb 100644 --- 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": { diff --git a/test/config/crl-updater.json b/test/config/crl-updater.json index eb5ba23e0da..adb2b01e5f4 100644 --- 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": { diff --git a/test/config/ocsp-responder.json b/test/config/ocsp-responder.json index 2e4ba1461a5..aedd971c73a 100644 --- 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": {} },