diff --git a/ca/ca_test.go b/ca/ca_test.go index 9e81ef08c..0ec950db4 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -159,26 +159,21 @@ func setup(t *testing.T) *testCtx { test.AssertNotError(t, err, "Couldn't set hostname policy") certProfiles := make(map[string]issuance.ProfileConfig, 0) - certProfiles["defaultBoulderCertificateProfile"] = issuance.ProfileConfig{ + certProfiles["legacy"] = issuance.ProfileConfig{ AllowMustStaple: true, - AllowCTPoison: true, - AllowSCTList: true, - AllowCommonName: true, Policies: []issuance.PolicyConfig{ {OID: "2.23.140.1.2.1"}, }, - MaxValidityPeriod: config.Duration{Duration: time.Hour * 8760}, + MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 365}, MaxValidityBackdate: config.Duration{Duration: time.Hour}, } - certProfiles["longerLived"] = issuance.ProfileConfig{ + certProfiles["modern"] = issuance.ProfileConfig{ AllowMustStaple: true, - AllowCTPoison: true, - AllowSCTList: true, - AllowCommonName: true, + OmitCommonName: true, Policies: []issuance.PolicyConfig{ {OID: "2.23.140.1.2.1"}, }, - MaxValidityPeriod: config.Duration{Duration: time.Hour * 8761}, + MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 90}, MaxValidityBackdate: config.Duration{Duration: time.Hour}, } test.AssertEquals(t, len(certProfiles), 2) @@ -249,7 +244,7 @@ func setup(t *testing.T) *testCtx { pa: pa, ocsp: ocsp, crl: crl, - defaultCertProfileName: "defaultBoulderCertificateProfile", + defaultCertProfileName: "legacy", lints: lints, certProfiles: certProfiles, certExpiry: 8760 * time.Hour, @@ -590,46 +585,37 @@ func TestProfiles(t *testing.T) { sa := &mockSA{} - duplicateProfiles := make(map[string]issuance.ProfileConfig, 0) // These profiles contain the same data which will produce an identical // hash, even though the names are different. - duplicateProfiles["defaultBoulderCertificateProfile"] = issuance.ProfileConfig{ + duplicateProfiles := make(map[string]issuance.ProfileConfig, 0) + duplicateProfiles["legacy"] = issuance.ProfileConfig{ AllowMustStaple: false, - AllowCTPoison: false, - AllowSCTList: false, - AllowCommonName: false, Policies: []issuance.PolicyConfig{ {OID: "2.23.140.1.2.1"}, }, - MaxValidityPeriod: config.Duration{Duration: time.Hour * 8760}, + MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 90}, MaxValidityBackdate: config.Duration{Duration: time.Hour}, } - duplicateProfiles["uhoh_ohno"] = issuance.ProfileConfig{ + duplicateProfiles["modern"] = issuance.ProfileConfig{ AllowMustStaple: false, - AllowCTPoison: false, - AllowSCTList: false, - AllowCommonName: false, Policies: []issuance.PolicyConfig{ {OID: "2.23.140.1.2.1"}, }, - MaxValidityPeriod: config.Duration{Duration: time.Hour * 8760}, + MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 90}, MaxValidityBackdate: config.Duration{Duration: time.Hour}, } test.AssertEquals(t, len(duplicateProfiles), 2) - jackedProfiles := make(map[string]issuance.ProfileConfig, 0) - jackedProfiles["ruhroh"] = issuance.ProfileConfig{ + badProfiles := make(map[string]issuance.ProfileConfig, 0) + badProfiles["ruhroh"] = issuance.ProfileConfig{ AllowMustStaple: false, - AllowCTPoison: false, - AllowSCTList: false, - AllowCommonName: false, Policies: []issuance.PolicyConfig{ {OID: "2.23.140.1.2.1"}, }, - MaxValidityPeriod: config.Duration{Duration: time.Hour * 9000}, + MaxValidityPeriod: config.Duration{Duration: time.Hour * 24 * 399}, MaxValidityBackdate: config.Duration{Duration: time.Hour}, } - test.AssertEquals(t, len(jackedProfiles), 1) + test.AssertEquals(t, len(badProfiles), 1) type nameToHash struct { name string @@ -664,30 +650,30 @@ func TestProfiles(t *testing.T) { profileConfigs: testCtx.certProfiles, expectedProfiles: []nameToHash{ { - name: testCtx.defaultCertProfileName, - hash: [32]byte{205, 182, 88, 236, 32, 18, 154, 120, 148, 194, 42, 215, 117, 140, 13, 169, 127, 196, 219, 67, 82, 36, 147, 67, 254, 117, 65, 112, 202, 60, 185, 9}, + name: "legacy", + hash: [32]byte{0xf7, 0x53, 0xef, 0xcd, 0x5a, 0x17, 0x22, 0x2, 0x79, 0xc8, 0xcb, 0x7a, 0x3d, 0x60, 0x46, 0x8b, 0xa0, 0x3b, 0x8b, 0x1a, 0x81, 0x19, 0x7e, 0x84, 0x19, 0x7f, 0x16, 0xd7, 0x95, 0x39, 0xef, 0x8e}, }, { - name: "longerLived", - hash: [32]byte{80, 228, 198, 83, 7, 184, 187, 236, 113, 17, 103, 213, 226, 245, 172, 212, 135, 241, 125, 92, 122, 200, 34, 159, 139, 72, 191, 41, 1, 244, 86, 62}, + name: "modern", + hash: [32]byte{0x44, 0x1d, 0x9d, 0xbc, 0x4c, 0xfc, 0xf, 0x95, 0xa4, 0x32, 0xc0, 0xcb, 0x76, 0xd1, 0x1b, 0xcb, 0xbe, 0xf1, 0x14, 0xe7, 0xa3, 0xf6, 0xfd, 0x7, 0x14, 0xb5, 0xfa, 0x2f, 0xb6, 0x1, 0x22, 0xc}, }, }, }, { name: "no profile matching default name", - profileConfigs: jackedProfiles, + profileConfigs: badProfiles, expectedErrSubstr: "profile object was not found for that name", }, { name: "certificate profile hash changed mid-issuance", - profileConfigs: jackedProfiles, + profileConfigs: badProfiles, defaultName: "ruhroh", expectedProfiles: []nameToHash{ { // We'll change the mapped hash key under the hood during // the test. name: "ruhroh", - hash: [32]byte{84, 131, 8, 59, 3, 244, 7, 36, 151, 161, 118, 68, 117, 183, 197, 177, 179, 232, 215, 10, 188, 48, 159, 195, 195, 140, 19, 204, 201, 182, 239, 235}, + hash: [32]byte{0x4f, 0x23, 0x87, 0xe0, 0xc3, 0x1, 0xac, 0x3d, 0x40, 0x94, 0xa8, 0x5c, 0x71, 0xef, 0x40, 0xb5, 0xa, 0xfd, 0xb0, 0xb2, 0xb0, 0x6b, 0x2c, 0x2a, 0xf9, 0xb1, 0x5, 0xf, 0x37, 0x42, 0xf4, 0x23}, }, }, }, @@ -725,13 +711,13 @@ func TestProfiles(t *testing.T) { } if tc.expectedProfiles != nil { - test.AssertEquals(t, len(tc.expectedProfiles), len(tCA.certProfiles.profileByName)) + test.AssertEquals(t, len(tCA.certProfiles.profileByName), len(tc.expectedProfiles)) } for _, expected := range tc.expectedProfiles { cpwid, ok := tCA.certProfiles.profileByName[expected.name] test.Assert(t, ok, "Profile name was not found, but should have been") - test.AssertEquals(t, expected.hash, cpwid.hash) + test.AssertEquals(t, cpwid.hash, expected.hash) if tc.name == "certificate profile hash changed mid-issuance" { // This is an attempt to simulate the hash changing, but the @@ -1029,7 +1015,7 @@ func TestIssueCertificateForPrecertificateWithSpecificCertificateProfile(t *test testCtx.fc) test.AssertNotError(t, err, "Failed to create CA") - selectedProfile := "longerLived" + selectedProfile := "legacy" certProfile, ok := ca.certProfiles.profileByName[selectedProfile] test.Assert(t, ok, "Certificate profile was expected to exist") diff --git a/issuance/cert.go b/issuance/cert.go index 7e6c38549..b14adaf24 100644 --- a/issuance/cert.go +++ b/issuance/cert.go @@ -38,10 +38,15 @@ type ProfileConfig struct { // Deprecated: We intend to include SCTs in all final Certificates for the // foreseeable future. AllowSCTList bool - // AllowCommonName, when false, causes all IssuanceRequests which specify a CN - // to be rejected. + // 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 + MaxValidityPeriod config.Duration MaxValidityBackdate config.Duration @@ -57,7 +62,7 @@ type PolicyConfig struct { // Profile is the validated structure created by reading in ProfileConfigs and IssuerConfigs type Profile struct { allowMustStaple bool - allowCommonName bool + omitCommonName bool maxBackdate time.Duration maxValidity time.Duration @@ -69,7 +74,7 @@ type Profile struct { func NewProfile(profileConfig ProfileConfig, lints lint.Registry) (*Profile, error) { sp := &Profile{ allowMustStaple: profileConfig.AllowMustStaple, - allowCommonName: profileConfig.AllowCommonName, + omitCommonName: profileConfig.OmitCommonName, maxBackdate: profileConfig.MaxValidityBackdate.Duration, maxValidity: profileConfig.MaxValidityPeriod.Duration, lints: lints, @@ -103,10 +108,6 @@ func (i *Issuer) requestValid(clk clock.Clock, prof *Profile, req *IssuanceReque return errors.New("cannot include both ct poison and sct list extensions") } - if !prof.allowCommonName && req.CommonName != "" { - return errors.New("common name cannot be included") - } - // The validity period is calculated inclusive of the whole second represented // by the notAfter timestamp. validity := req.NotAfter.Add(time.Second).Sub(req.NotBefore) @@ -255,7 +256,7 @@ func (i *Issuer) Prepare(prof *Profile, req *IssuanceRequest) ([]byte, *issuance // populate template from the issuance request template.NotBefore, template.NotAfter = req.NotBefore, req.NotAfter template.SerialNumber = big.NewInt(0).SetBytes(req.Serial) - if req.CommonName != "" { + if req.CommonName != "" && !prof.omitCommonName { template.Subject.CommonName = req.CommonName } template.DNSNames = req.DNSNames diff --git a/issuance/cert_test.go b/issuance/cert_test.go index f612895e8..28cde1f58 100644 --- a/issuance/cert_test.go +++ b/issuance/cert_test.go @@ -106,19 +106,6 @@ func TestRequestValid(t *testing.T) { }, expectedError: "cannot include both ct poison and sct list extensions", }, - { - name: "common name not allowed", - issuer: &Issuer{ - active: true, - }, - profile: &Profile{}, - request: &IssuanceRequest{ - PublicKey: &ecdsa.PublicKey{}, - SubjectKeyId: goodSKID, - CommonName: "cn", - }, - expectedError: "common name cannot be included", - }, { name: "negative validity", issuer: &Issuer{ @@ -386,13 +373,14 @@ func TestIssueCommonName(t *testing.T) { PublicKey: pk.Public(), SubjectKeyId: goodSKID, Serial: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}, - CommonName: "example.com", DNSNames: []string{"example.com", "www.example.com"}, NotBefore: fc.Now(), NotAfter: fc.Now().Add(time.Hour - time.Second), IncludeCTPoison: true, } + // In the default profile, the common name is allowed if requested. + ir.CommonName = "example.com" _, issuanceToken, err := signer.Prepare(cnProfile, ir) test.AssertNotError(t, err, "Prepare failed") certBytes, err := signer.Issue(issuanceToken) @@ -401,10 +389,7 @@ func TestIssueCommonName(t *testing.T) { test.AssertNotError(t, err, "failed to parse certificate") test.AssertEquals(t, cert.Subject.CommonName, "example.com") - cnProfile.allowCommonName = false - _, _, err = signer.Prepare(cnProfile, ir) - test.AssertError(t, err, "Prepare should have failed") - + // But not including the common name should be acceptable as well. ir.CommonName = "" _, issuanceToken, err = signer.Prepare(cnProfile, ir) test.AssertNotError(t, err, "Prepare failed") @@ -413,7 +398,17 @@ func TestIssueCommonName(t *testing.T) { cert, err = x509.ParseCertificate(certBytes) test.AssertNotError(t, err, "failed to parse certificate") test.AssertEquals(t, cert.Subject.CommonName, "") - test.AssertDeepEquals(t, cert.DNSNames, []string{"example.com", "www.example.com"}) + + // And the common name should be omitted if the profile is so configured. + ir.CommonName = "example.com" + cnProfile.omitCommonName = true + _, issuanceToken, err = signer.Prepare(cnProfile, ir) + test.AssertNotError(t, err, "Prepare failed") + certBytes, err = signer.Issue(issuanceToken) + test.AssertNotError(t, err, "Issue failed") + cert, err = x509.ParseCertificate(certBytes) + test.AssertNotError(t, err, "failed to parse certificate") + test.AssertEquals(t, cert.Subject.CommonName, "") } func TestIssueCTPoison(t *testing.T) { diff --git a/issuance/issuer_test.go b/issuance/issuer_test.go index fe3b0a90f..4e31d6e08 100644 --- a/issuance/issuer_test.go +++ b/issuance/issuer_test.go @@ -24,9 +24,6 @@ import ( func defaultProfileConfig() ProfileConfig { return ProfileConfig{ - AllowCommonName: true, - AllowCTPoison: true, - AllowSCTList: true, 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 4659fe4ca..42399ecbe 100644 --- a/test/config-next/ca.json +++ b/test/config-next/ca.json @@ -42,11 +42,21 @@ "hostOverride": "sa.boulder" }, "issuance": { - "defaultCertificateProfileName": "defaultBoulderCertificateProfile", + "defaultCertificateProfileName": "legacy", "certProfiles": { - "defaultBoulderCertificateProfile": { + "legacy": { "allowMustStaple": true, - "allowCommonName": true, + "policies": [ + { + "oid": "2.23.140.1.2.1" + } + ], + "maxValidityPeriod": "7776000s", + "maxValidityBackdate": "1h5m" + }, + "modern": { + "allowMustStaple": true, + "omitCommonName": true, "policies": [ { "oid": "2.23.140.1.2.1"