issuance: add CRLDistributionPoints to certs

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())
 	}

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)
 	}

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())

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)

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

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/",

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)
 		}
 	}
 }