diff --git a/cmd/boulder-ca/main.go b/cmd/boulder-ca/main.go index a4e60fced4f..0248cf62b90 100644 --- a/cmd/boulder-ca/main.go +++ b/cmd/boulder-ca/main.go @@ -3,6 +3,7 @@ package notmain import ( "context" "flag" + "fmt" "os" "strconv" "time" @@ -176,10 +177,19 @@ func main() { } clk := cmd.Clock() + var crlShards int issuers := make([]*issuance.Issuer, 0, len(c.CA.Issuance.Issuers)) - for _, issuerConfig := range c.CA.Issuance.Issuers { + for i, issuerConfig := range c.CA.Issuance.Issuers { issuer, err := issuance.LoadIssuer(issuerConfig, clk) cmd.FailOnError(err, "Loading issuer") + // All issuers should have the same number of CRL shards, because + // crl-updater assumes they all have the same number. + if issuerConfig.CRLShards != 0 && crlShards == 0 { + crlShards = issuerConfig.CRLShards + } + if issuerConfig.CRLShards != crlShards { + cmd.Fail(fmt.Sprintf("issuer %d has %d shards, want %d", i, issuerConfig.CRLShards, crlShards)) + } issuers = append(issuers, issuer) logger.Infof("Loaded issuer: name=[%s] keytype=[%s] nameID=[%v] isActive=[%t]", issuer.Name(), issuer.KeyType(), issuer.NameID(), issuer.IsActive()) } diff --git a/issuance/cert.go b/issuance/cert.go index 43064c25495..8964b8b814a 100644 --- a/issuance/cert.go +++ b/issuance/cert.go @@ -182,6 +182,8 @@ type Profile struct { omitClientAuth bool omitSKID bool + includeCRLDistributionPoints bool + maxBackdate time.Duration maxValidity time.Duration @@ -218,15 +220,16 @@ func NewProfile(profileConfig *ProfileConfigNew) (*Profile, error) { } sp := &Profile{ - allowMustStaple: profileConfig.AllowMustStaple, - omitCommonName: profileConfig.OmitCommonName, - omitKeyEncipherment: profileConfig.OmitKeyEncipherment, - omitClientAuth: profileConfig.OmitClientAuth, - omitSKID: profileConfig.OmitSKID, - maxBackdate: profileConfig.MaxValidityBackdate.Duration, - maxValidity: profileConfig.MaxValidityPeriod.Duration, - lints: lints, - hash: hash, + allowMustStaple: profileConfig.AllowMustStaple, + omitCommonName: profileConfig.OmitCommonName, + omitKeyEncipherment: profileConfig.OmitKeyEncipherment, + omitClientAuth: profileConfig.OmitClientAuth, + omitSKID: profileConfig.OmitSKID, + includeCRLDistributionPoints: profileConfig.IncludeCRLDistributionPoints, + maxBackdate: profileConfig.MaxValidityBackdate.Duration, + maxValidity: profileConfig.MaxValidityPeriod.Duration, + lints: lints, + hash: hash, } return sp, nil @@ -311,9 +314,6 @@ func (i *Issuer) generateTemplate() *x509.Certificate { PolicyIdentifiers: []asn1.ObjectIdentifier{{2, 23, 140, 1, 2, 1}}, } - // TODO(#7294): Use i.crlURLBase and a shard calculation to create a - // crlDistributionPoint. - return template } @@ -488,6 +488,19 @@ func (i *Issuer) Prepare(prof *Profile, req *IssuanceRequest) ([]byte, *issuance return nil, nil, errors.New("invalid request contains neither sctList nor precertDER") } + // If explicit CRL sharding is enabled, pick a shard based on the serial number + // modulus the number of shards. This gives us random distribution that is + // nonetheless consistent between precert and cert. + if prof.includeCRLDistributionPoints { + if i.crlShards <= 0 { + return nil, nil, errors.New("IncludeCRLDistributionPoints was set but CRLShards was not set") + } + shardZeroBased := big.NewInt(0).Mod(template.SerialNumber, big.NewInt(int64(i.crlShards))) + shard := int(shardZeroBased.Int64()) + 1 + url := i.crlURL(shard) + template.CRLDistributionPoints = []string{url} + } + if req.IncludeMustStaple { template.ExtraExtensions = append(template.ExtraExtensions, mustStapleExt) } diff --git a/issuance/cert_test.go b/issuance/cert_test.go index ca8386146fc..b8292dde760 100644 --- a/issuance/cert_test.go +++ b/issuance/cert_test.go @@ -12,6 +12,7 @@ import ( "encoding/asn1" "encoding/base64" "fmt" + "reflect" "testing" "time" @@ -70,6 +71,18 @@ func TestGenerateValidity(t *testing.T) { } } +func TestCRLURL(t *testing.T) { + issuer, err := newIssuer(defaultIssuerConfig(), issuerCert, issuerSigner, clock.NewFake()) + if err != nil { + t.Fatalf("newIssuer: %s", err) + } + url := issuer.crlURL(4928) + want := "http://crl-url.example.org/4928.crl" + if url != want { + t.Errorf("crlURL(4928)=%s, want %s", url, want) + } +} + func TestRequestValid(t *testing.T) { fc := clock.NewFake() fc.Add(time.Hour * 24) @@ -380,10 +393,55 @@ func TestIssue(t *testing.T) { test.AssertDeepEquals(t, cert.PublicKey, pk.Public()) test.AssertEquals(t, len(cert.Extensions), 9) // Constraints, KU, EKU, SKID, AKID, AIA, SAN, Policies, Poison test.AssertEquals(t, cert.KeyUsage, tc.ku) + if len(cert.CRLDistributionPoints) > 0 { + t.Errorf("want CRLDistributionPoints=[], got %v", cert.CRLDistributionPoints) + } }) } } +func TestIssueWithCRLDP(t *testing.T) { + fc := clock.NewFake() + issuerConfig := defaultIssuerConfig() + issuerConfig.CRLURLBase = "http://crls.example.net/" + issuerConfig.CRLShards = 999 + signer, err := newIssuer(issuerConfig, issuerCert, issuerSigner, fc) + if err != nil { + t.Fatalf("newIssuer: %s", err) + } + pk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa.GenerateKey: %s", err) + } + profile := defaultProfile() + profile.includeCRLDistributionPoints = true + _, issuanceToken, err := signer.Prepare(profile, &IssuanceRequest{ + PublicKey: MarshalablePublicKey{pk.Public()}, + SubjectKeyId: goodSKID, + Serial: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9}, + DNSNames: []string{"example.com"}, + NotBefore: fc.Now(), + NotAfter: fc.Now().Add(time.Hour - time.Second), + IncludeCTPoison: true, + }) + if err != nil { + t.Fatalf("signer.Prepare: %s", err) + } + certBytes, err := signer.Issue(issuanceToken) + if err != nil { + t.Fatalf("signer.Issue: %s", err) + } + cert, err := x509.ParseCertificate(certBytes) + if err != nil { + t.Fatalf("x509.ParseCertificate: %s", err) + } + // Because CRL shard is calculated deterministically from serial, we know which shard will be chosen. + expectedCRLDP := []string{"http://crls.example.net/919.crl"} + if !reflect.DeepEqual(cert.CRLDistributionPoints, expectedCRLDP) { + t.Errorf("CRLDP=%+v, want %+v", cert.CRLDistributionPoints, expectedCRLDP) + } +} + func TestIssueCommonName(t *testing.T) { fc := clock.NewFake() fc.Set(time.Now()) diff --git a/issuance/crl.go b/issuance/crl.go index 48fc54e3f57..9e2de44a624 100644 --- a/issuance/crl.go +++ b/issuance/crl.go @@ -59,6 +59,11 @@ type CRLRequest struct { Entries []x509.RevocationListEntry } +// crlURL combines the CRL URL base with a shard, and adds a suffix. +func (i *Issuer) crlURL(shard int) string { + return fmt.Sprintf("%s%d.crl", i.crlURLBase, shard) +} + func (i *Issuer) IssueCRL(prof *CRLProfile, req *CRLRequest) ([]byte, error) { backdatedBy := i.clk.Now().Sub(req.ThisUpdate) if backdatedBy > prof.maxBackdate { @@ -82,7 +87,7 @@ func (i *Issuer) IssueCRL(prof *CRLProfile, req *CRLRequest) ([]byte, error) { // Concat the base with the shard directly, since we require that the base // end with a single trailing slash. idp, err := idp.MakeUserCertsExt([]string{ - fmt.Sprintf("%s%d.crl", i.crlURLBase, req.Shard), + i.crlURL(int(req.Shard)), }) if err != nil { return nil, fmt.Errorf("creating IDP extension: %w", err) diff --git a/issuance/issuer.go b/issuance/issuer.go index 6917f0728c5..950ce44cec0 100644 --- a/issuance/issuer.go +++ b/issuance/issuer.go @@ -162,7 +162,12 @@ type IssuerConfig struct { IssuerURL string `validate:"required,url"` OCSPURL string `validate:"required,url"` - CRLURLBase string `validate:"omitempty,url,startswith=http://,endswith=/"` + CRLURLBase string `validate:"required,url,startswith=http://,endswith=/"` + + // Number of CRL shards. + // This must be nonzero if adding CRLDistributionPoints to certificates + // (that is, if profile.IncludeCRLDistributionPoints is true). + CRLShards int Location IssuerLoc } @@ -204,9 +209,11 @@ type Issuer struct { // certificates. ocspURL string // Used to set the Issuing Distribution Point extension in issued CRLs - // *and* (eventually) the CRL Distribution Point extension in issued certs. + // and the CRL Distribution Point extension in issued certs. crlURLBase string + crlShards int + clk clock.Clock } @@ -276,6 +283,7 @@ func newIssuer(config IssuerConfig, cert *Certificate, signer crypto.Signer, clk issuerURL: config.IssuerURL, ocspURL: config.OCSPURL, crlURLBase: config.CRLURLBase, + crlShards: config.CRLShards, clk: clk, } return i, nil diff --git a/test/config-next/ca.json b/test/config-next/ca.json index 3c1accbf7f5..cdaa7783375 100644 --- a/test/config-next/ca.json +++ b/test/config-next/ca.json @@ -45,6 +45,17 @@ "defaultCertificateProfileName": "legacy", "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", @@ -55,6 +66,20 @@ ] }, "modern": { + "allowMustStaple": true, + "omitCommonName": true, + "omitKeyEncipherment": true, + "omitClientAuth": true, + "omitSKID": true, + "includeCRLDistributionPoints": true, + "maxValidityPeriod": "583200s", + "maxValidityBackdate": "1h5m", + "lintConfig": "test/config-next/zlint.toml", + "ignoredLints": [ + "w_ext_subject_key_identifier_missing_sub_cert" + ] + }, + "modernRenamedPriorToDeletion": { "allowMustStaple": true, "omitCommonName": true, "omitKeyEncipherment": true, @@ -75,6 +100,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/", @@ -86,6 +112,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/", @@ -97,6 +124,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/", @@ -108,6 +136,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/", @@ -119,6 +148,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/", @@ -130,6 +160,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/", diff --git a/test/integration/revocation_test.go b/test/integration/revocation_test.go index aa105dddde4..8ed980617ee 100644 --- a/test/integration/revocation_test.go +++ b/test/integration/revocation_test.go @@ -127,28 +127,45 @@ func getAllCRLs(t *testing.T) map[string][]*x509.RevocationList { return ret } -func checkRevoked(t *testing.T, revocations map[string][]*x509.RevocationList, cert *x509.Certificate, reason int) { +func checkRevoked(t *testing.T, revocations map[string][]*x509.RevocationList, cert *x509.Certificate, expectedReason int) { t.Helper() akid := hex.EncodeToString(cert.AuthorityKeyId) if len(revocations[akid]) == 0 { t.Errorf("no CRLs found for authorityKeyID %s", akid) } - var matches []x509.RevocationListEntry + var matchingCRLs []string var count int for _, list := range revocations[akid] { for _, entry := range list.RevokedCertificateEntries { count++ if entry.SerialNumber.Cmp(cert.SerialNumber) == 0 { - matches = append(matches, entry) + idpURIs, err := idp.GetIDPURIs(list.Extensions) + if err != nil { + t.Errorf("getting IDP URIs: %s", err) + } + idpURI := idpURIs[0] + if entry.ReasonCode != expectedReason { + t.Errorf("revoked certificate %x in CRL %s: revocation reason %d, want %d", cert.SerialNumber, idpURI, entry.ReasonCode, expectedReason) + } + matchingCRLs = append(matchingCRLs, idpURI) } } } - if len(matches) == 0 { + if len(matchingCRLs) == 0 { t.Errorf("searching for %x in CRLs: no entry on combined CRLs of length %d", cert.SerialNumber, count) } - for _, match := range matches { - if match.ReasonCode != reason { - t.Errorf("revoked certificate %x: got reason %d, want %d", cert.SerialNumber, match.ReasonCode, reason) + + // If the cert has a CRLDP, it must be listed on the CRL served at that URL. + if len(cert.CRLDistributionPoints) > 0 { + expectedCRLDP := cert.CRLDistributionPoints[0] + found := false + for _, crl := range matchingCRLs { + if crl == expectedCRLDP { + found = true + } + } + if !found { + t.Errorf("revoked certificate %x: seen on CRLs %s, want to see on CRL %s", cert.SerialNumber, matchingCRLs, expectedCRLDP) } } }