CRLs: include IssuingDistributionPoint extension (#6412)

Add the Issuing Distribution Point extension to all of our end-entity
CRLs. The extension contains the Distribution Point, the URL from
which this CRL is meant to be downloaded. Because our CRLs are
sharded, this URL prevents an on-path attacker from substituting a
different shard than the client expected in order to hide a revocation.
The extension also contains the OnlyContainsUserCerts boolean,
because our CRLs only contain end-entity certificates.

The Distribution Point url is constructed from a configurable base URI,
the issuer's NameID, the shard index, and the suffix ".crl". The base
URI must use the "http://" scheme and must not end with a slash.

openssl displays the IDP extension as:
```
X509v3 Issuing Distribution Point: critical
  Full Name:
    URI:http://c.boulder.test/66283756913588288/0.crl                Only User Certificates
```

Fixes #6410

Author: aarongable

ca/ca_test.go

@@ -263,6 +263,7 @@ func setup(t *testing.T) *testCtx {
 	crl, err := NewCRLImpl(
 		boulderIssuers,
 		time.Hour,
+		"http://c.boulder.test",
 		100,
 		blog.NewMock(),
 	)

ca/crl.go

@@ -3,6 +3,8 @@ package ca
 import (
 	"crypto/rand"
 	"crypto/sha256"
+	"crypto/x509/pkix"
+	"encoding/asn1"
 	"errors"
 	"fmt"
 	"io"
@@ -22,11 +24,17 @@ type crlImpl struct {
 	capb.UnimplementedCRLGeneratorServer
 	issuers   map[issuance.IssuerNameID]*issuance.Issuer
 	lifetime  time.Duration
+	idpBase   string
 	maxLogLen int
 	log       blog.Logger
 }
 
-func NewCRLImpl(issuers []*issuance.Issuer, lifetime time.Duration, maxLogLen int, logger blog.Logger) (*crlImpl, error) {
+// NewCRLImpt returns a new object which fulfils the ca.proto CRLGenerator
+// interface. It uses the list of issuers to determine what issuers it can
+// issue CRLs from. lifetime sets the validity period (inclusive) of the
+// resulting CRLs. idpBase is the base URL from which IssuingDistributionPoint
+// URIs will constructed; it must use the http:// scheme.
+func NewCRLImpl(issuers []*issuance.Issuer, lifetime time.Duration, idpBase string, maxLogLen int, logger blog.Logger) (*crlImpl, error) {
 	issuersByNameID := make(map[issuance.IssuerNameID]*issuance.Issuer, len(issuers))
 	for _, issuer := range issuers {
 		issuersByNameID[issuer.Cert.NameID()] = issuer
@@ -41,9 +49,17 @@ func NewCRLImpl(issuers []*issuance.Issuer, lifetime time.Duration, maxLogLen in
 		return nil, fmt.Errorf("crl lifetime must be positive, got %q", lifetime)
 	}
 
+	if !strings.HasPrefix(idpBase, "http://") {
+		return nil, fmt.Errorf("issuingDistributionPoint base URI must use http:// scheme, got %q", idpBase)
+	}
+	if strings.HasSuffix(idpBase, "/") {
+		return nil, fmt.Errorf("issuingDistributionPoint base URI must not end with a slash, got %q", idpBase)
+	}
+
 	return &crlImpl{
 		issuers:   issuersByNameID,
 		lifetime:  lifetime,
+		idpBase:   idpBase,
 		maxLogLen: maxLogLen,
 		log:       logger,
 	}, nil
@@ -100,6 +116,13 @@ func (ci *crlImpl) GenerateCRL(stream capb.CRLGenerator_GenerateCRLServer) error
 		return errors.New("no crl metadata received")
 	}
 
+	// Add the Issuing Distribution Point extension.
+	idp, err := makeIDPExt(ci.idpBase, issuer.Cert.NameID(), shard)
+	if err != nil {
+		return fmt.Errorf("creating IDP extension: %w", err)
+	}
+	template.ExtraExtensions = append(template.ExtraExtensions, *idp)
+
 	// Compute a unique ID for this issuer-number-shard combo, to tie together all
 	// the audit log lines related to its issuance.
 	logID := blog.LogLineChecksum(fmt.Sprintf("%d", issuer.Cert.NameID()) + template.Number.String() + fmt.Sprintf("%d", shard))
@@ -133,7 +156,7 @@ func (ci *crlImpl) GenerateCRL(stream capb.CRLGenerator_GenerateCRLServer) error
 
 	template.RevokedCertificates = rcs
 
-	err := issuer.Linter.CheckCRL(template)
+	err = issuer.Linter.CheckCRL(template)
 	if err != nil {
 		return err
 	}
@@ -185,7 +208,6 @@ func (ci *crlImpl) metadataToTemplate(meta *capb.CRLMetadata) (*crl_x509.Revocat
 		ThisUpdate: thisUpdate,
 		NextUpdate: thisUpdate.Add(-time.Second).Add(ci.lifetime),
 	}, nil
-
 }
 
 func (ci *crlImpl) entryToRevokedCertificate(entry *corepb.CRLEntry) (*crl_x509.RevokedCertificate, error) {
@@ -211,3 +233,51 @@ func (ci *crlImpl) entryToRevokedCertificate(entry *corepb.CRLEntry) (*crl_x509.
 		ReasonCode:     reason,
 	}, nil
 }
+
+// distributionPointName represents the ASN.1 DistributionPointName CHOICE as
+// defined in RFC 5280 Section 4.2.1.13. We only use one of the fields, so the
+// others are omitted.
+type distributionPointName struct {
+	// Technically, FullName is of type GeneralNames, which is of type SEQUENCE OF
+	// GeneralName. But GeneralName itself is of type CHOICE, and the ans1.Marhsal
+	// function doesn't support marshalling structs to CHOICEs, so we have to use
+	// asn1.RawValue and encode the GeneralName ourselves.
+	FullName []asn1.RawValue `asn1:"optional,tag:0"`
+}
+
+// issuingDistributionPoint represents the ASN.1 IssuingDistributionPoint
+// SEQUENCE as defined in RFC 5280 Section 5.2.5. We only use two of the fields,
+// so the others are omitted.
+type issuingDistributionPoint struct {
+	DistributionPoint     distributionPointName `asn1:"optional,tag:0"`
+	OnlyContainsUserCerts bool                  `asn1:"optional,tag:1"`
+}
+
+// makeIDPExt returns a critical IssuingDistributionPoint extension containing a
+// URI built from the base url, the issuer's NameID, and the shard number. It
+// also sets the OnlyContainsUserCerts boolean to true.
+func makeIDPExt(base string, issuer issuance.IssuerNameID, shardIdx int64) (*pkix.Extension, error) {
+	val := issuingDistributionPoint{
+		DistributionPoint: distributionPointName{
+			[]asn1.RawValue{ // GeneralNames
+				{ // GeneralName
+					Class: 2, // context-specific
+					Tag:   6, // uniformResourceIdentifier, IA5String
+					Bytes: []byte(fmt.Sprintf("%s/%d/%d.crl", base, issuer, shardIdx)),
+				},
+			},
+		},
+		OnlyContainsUserCerts: true,
+	}
+
+	valBytes, err := asn1.Marshal(val)
+	if err != nil {
+		return nil, err
+	}
+
+	return &pkix.Extension{
+		Id:       asn1.ObjectIdentifier{2, 5, 29, 28}, // id-ce-issuingDistributionPoint
+		Value:    valBytes,
+		Critical: true,
+	}, nil
+}

cmd/boulder-ca/main.go

@@ -294,6 +294,7 @@ func main() {
 	crli, err := ca.NewCRLImpl(
 		boulderIssuers,
 		c.CA.LifespanCRL.Duration,
+		c.CA.CRLDPBase,
 		c.CA.OCSPLogMaxLength,
 		logger,
 	)

linter/lints/crl/lints.go

@@ -5,6 +5,7 @@ import (
 	"encoding/asn1"
 	"errors"
 	"fmt"
+	"net/url"
 	"time"
 
 	"github.com/zmap/zlint/v3"
@@ -36,7 +37,7 @@ func init() {
 		"hasAKI":                         hasAKI,
 		"hasNumber":                      hasNumber,
 		"isNotDelta":                     isNotDelta,
-		"hasNoIDP":                       hasNoIDP,
+		"checkIDP":                       checkIDP,
 		"hasNoFreshest":                  hasNoFreshest,
 		"hasNoAIA":                       hasNoAIA,
 		"noZeroReasonCodes":              noZeroReasonCodes,
@@ -219,17 +220,125 @@ func isNotDelta(crl *crl_x509.RevocationList) *lint.LintResult {
 	return &lint.LintResult{Status: lint.Pass}
 }
 
-// hasNoIDP checks that the CRL does not have an Issuing Distribution Point
-// extension (RFC 5280, Section 5.2.5). There's no requirement against this, but
-// IDPs come with extra requirements we don't want to deal with.
-func hasNoIDP(crl *crl_x509.RevocationList) *lint.LintResult {
+// checkIDP checks that the CRL does have an Issuing Distribution Point, that it
+// is critical, that it contains a single http distributionPointName, that it
+// asserts the onlyContainsUserCerts boolean, and that it does not contain any
+// of the other fields. (RFC 5280, Section 5.2.5).
+func checkIDP(crl *crl_x509.RevocationList) *lint.LintResult {
 	idpOID := asn1.ObjectIdentifier{2, 5, 29, 28} // id-ce-issuingDistributionPoint
-	if getExtWithOID(crl.Extensions, idpOID) != nil {
+	idpe := getExtWithOID(crl.Extensions, idpOID)
+	if idpe == nil {
 		return &lint.LintResult{
-			Status:  lint.Notice,
-			Details: "CRL has an Issuing Distribution Point url",
+			Status:  lint.Warn,
+			Details: "CRL missing IDP",
+		}
+	}
+	if !idpe.Critical {
+		return &lint.LintResult{
+			Status:  lint.Error,
+			Details: "IDP MUST be critical",
+		}
+	}
+
+	// Step inside the outer issuingDistributionPoint sequence to get access to
+	// its constituent fields, DistributionPoint and OnlyContainsUserCerts.
+	idpv := cryptobyte.String(idpe.Value)
+	if !idpv.ReadASN1(&idpv, cryptobyte_asn1.SEQUENCE) {
+		return &lint.LintResult{
+			Status:  lint.Warn,
+			Details: "Failed to read issuingDistributionPoint",
+		}
+	}
+
+	// Ensure that the DistributionPoint is a reasonable URI. To get to the URI,
+	// we have to step inside the DistributionPointName, then step inside that's
+	// FullName, and finally read the singular SEQUENCE OF GeneralName element.
+	if !idpv.PeekASN1Tag(cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) {
+		return &lint.LintResult{
+			Status:  lint.Warn,
+			Details: "IDP should contain distributionPoint",
+		}
+	}
+
+	var dpName cryptobyte.String
+	if !idpv.ReadASN1(&dpName, cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) {
+		return &lint.LintResult{
+			Status:  lint.Warn,
+			Details: "Failed to read IDP distributionPoint",
+		}
+	}
+
+	if !dpName.ReadASN1(&dpName, cryptobyte_asn1.Tag(0).ContextSpecific().Constructed()) {
+		return &lint.LintResult{
+			Status:  lint.Warn,
+			Details: "Failed to read IDP distributionPoint fullName",
 		}
 	}
+
+	fmt.Printf("%x\n", dpName)
+	uriBytes := make([]byte, 0)
+	if !dpName.ReadASN1Bytes(&uriBytes, cryptobyte_asn1.Tag(6).ContextSpecific()) {
+		return &lint.LintResult{
+			Status:  lint.Warn,
+			Details: "Failed to read IDP URI",
+		}
+	}
+
+	uri, err := url.Parse(string(uriBytes))
+	if err != nil {
+		return &lint.LintResult{
+			Status:  lint.Error,
+			Details: "Failed to parse IDP URI",
+		}
+	}
+
+	if uri.Scheme != "http" {
+		return &lint.LintResult{
+			Status:  lint.Error,
+			Details: "IDP URI MUST use http scheme",
+		}
+	}
+
+	if !dpName.Empty() {
+		return &lint.LintResult{
+			Status:  lint.Warn,
+			Details: "IDP should contain only one distributionPoint",
+		}
+	}
+
+	// Ensure that OnlyContainsUserCerts is True. We have to read this boolean as
+	// a byte and ensure its value is 0xFF because cryptobyte.ReadASN1Boolean
+	// can't handle custom encoding rules like this field's [1] tag.
+	if !idpv.PeekASN1Tag(cryptobyte_asn1.Tag(1).ContextSpecific()) {
+		return &lint.LintResult{
+			Status:  lint.Warn,
+			Details: "IDP should contain onlyContainsUserCerts",
+		}
+	}
+
+	onlyContainsUserCerts := make([]byte, 0)
+	if !idpv.ReadASN1Bytes(&onlyContainsUserCerts, cryptobyte_asn1.Tag(1).ContextSpecific()) {
+		return &lint.LintResult{
+			Status:  lint.Error,
+			Details: "Failed to read IDP onlyContainsUserCerts",
+		}
+	}
+
+	if len(onlyContainsUserCerts) != 1 || onlyContainsUserCerts[0] != 0xFF {
+		return &lint.LintResult{
+			Status:  lint.Error,
+			Details: "IDP should set onlyContainsUserCerts: TRUE",
+		}
+	}
+
+	// Ensure that no other fields are set.
+	if !idpv.Empty() {
+		return &lint.LintResult{
+			Status:  lint.Warn,
+			Details: "IDP should not contain fields other than distributionPoint and onlyContainsUserCerts",
+		}
+	}
+
 	return &lint.LintResult{Status: lint.Pass}
 }
 

linter/lints/crl/lints_test.go

@@ -107,15 +107,35 @@ func TestIsNotDelta(t *testing.T) {
 	test.AssertContains(t, res.Details, "Delta")
 }
 
-func TestHasNoIDP(t *testing.T) {
+func TestCheckIDP(t *testing.T) {
 	crl := loadPEMCRL(t, "testdata/good.pem")
-	res := hasNoIDP(crl)
+	res := checkIDP(crl)
 	test.AssertEquals(t, res.Status, lint.Pass)
 
-	crl = loadPEMCRL(t, "testdata/idp.pem")
-	res = hasNoIDP(crl)
-	test.AssertEquals(t, res.Status, lint.Notice)
-	test.AssertContains(t, res.Details, "Issuing Distribution Point")
+	crl = loadPEMCRL(t, "testdata/no_idp.pem")
+	res = checkIDP(crl)
+	test.AssertEquals(t, res.Status, lint.Warn)
+	test.AssertContains(t, res.Details, "missing IDP")
+
+	crl = loadPEMCRL(t, "testdata/idp_no_uri.pem")
+	res = checkIDP(crl)
+	test.AssertEquals(t, res.Status, lint.Warn)
+	test.AssertContains(t, res.Details, "should contain distributionPoint")
+
+	crl = loadPEMCRL(t, "testdata/idp_two_uris.pem")
+	res = checkIDP(crl)
+	test.AssertEquals(t, res.Status, lint.Warn)
+	test.AssertContains(t, res.Details, "only one distributionPoint")
+
+	crl = loadPEMCRL(t, "testdata/idp_no_usercerts.pem")
+	res = checkIDP(crl)
+	test.AssertEquals(t, res.Status, lint.Warn)
+	test.AssertContains(t, res.Details, "should contain onlyContainsUserCerts")
+
+	crl = loadPEMCRL(t, "testdata/idp_some_reasons.pem")
+	res = checkIDP(crl)
+	test.AssertEquals(t, res.Status, lint.Warn)
+	test.AssertContains(t, res.Details, "should not contain fields other than")
 }
 
 func TestHasNoFreshest(t *testing.T) {

linter/lints/crl/testdata/good.pem

@@ -1,9 +1,11 @@
 -----BEGIN X509 CRL-----
-MIIBRTCBzQIBATAKBggqhkjOPQQDAzBJMQswCQYDVQQGEwJYWDEVMBMGA1UEChMM
-Qm91bGRlciBUZXN0MSMwIQYDVQQDExooVEVTVCkgRWxlZ2FudCBFbGVwaGFudCBF
-MRcNMjIwNzA2MTY0MzM4WhcNMjIwNzE1MTY0MzM4WjAbMBkCCAOuUdtRFVo8Fw0y
-MjA3MDYxNTQzMzhaoDYwNDAfBgNVHSMEGDAWgBQB2rt6yyUgjl551vmWQi8CQSkH
-vjARBgNVHRQECgIIFv9LJt+yGA8wCgYIKoZIzj0EAwMDZwAwZAIwVrITRYutGjFp
-fNht08CLsAQSvnc4i6UM0Pi8+U3T8DRHImIiuB9cQ+qxULB6pKhBAjBbuGCwTop7
-vCfGO7Fz6N0ruITInFtt6BDR5izWUMfXXa7mXhSQ6ig9hOHOWRxR00I=
+MIIBmDCCAR8CAQEwCgYIKoZIzj0EAwMwSTELMAkGA1UEBhMCWFgxFTATBgNVBAoT
+DEJvdWxkZXIgVGVzdDEjMCEGA1UEAxMaKFRFU1QpIEVsZWdhbnQgRWxlcGhhbnQg
+RTEXDTIyMTAxMDIwMTIwN1oXDTIyMTAxOTIwMTIwNlowKTAnAggDrlHbURVaPBcN
+MjIxMDEwMTkxMjA3WjAMMAoGA1UdFQQDCgEBoHoweDAfBgNVHSMEGDAWgBQB2rt6
+yyUgjl551vmWQi8CQSkHvjARBgNVHRQECgIIFxzOPeSCumEwQgYDVR0cAQH/BDgw
+NqAxoC+GLWh0dHA6Ly9jLmJvdWxkZXIudGVzdC82NjI4Mzc1NjkxMzU4ODI4OC8w
+LmNybIEB/zAKBggqhkjOPQQDAwNnADBkAjAvDkIUnTYavJ6h8606MDyFh2uw/cF+
+OVnM4sE8nUdGy0XYg0hGfbR4MY+kRxRQayICMFeQPpcpIr0zgXpP6lUXU0rcLSva
+tuaeQSVr24nGjZ7Py0vc94w0n7idZ8wje5+/Mw==
 -----END X509 CRL-----

linter/lints/crl/testdata/idp_no_uri.pem

@@ -0,0 +1,10 @@
+-----BEGIN X509 CRL-----
+MIIBZDCB7AIBATAKBggqhkjOPQQDAzBJMQswCQYDVQQGEwJYWDEVMBMGA1UEChMM
+Qm91bGRlciBUZXN0MSMwIQYDVQQDExooVEVTVCkgRWxlZ2FudCBFbGVwaGFudCBF
+MRcNMjIxMDEwMjAxMjA3WhcNMjIxMDE5MjAxMjA2WjApMCcCCAOuUdtRFVo8Fw0y
+MjEwMTAxOTEyMDdaMAwwCgYDVR0VBAMKAQGgRzBFMB8GA1UdIwQYMBaAFAHau3rL
+JSCOXnnW+ZZCLwJBKQe+MBEGA1UdFAQKAggXHM495IK6YTAPBgNVHRwBAf8EBTAD
+gQH/MAoGCCqGSM49BAMDA2cAMGQCMC8OQhSdNhq8nqHzrTowPIWHa7D9wX45Wczi
+wTydR0bLRdiDSEZ9tHgxj6RHFFBrIgIwV5A+lykivTOBek/qVRdTStwtK9q25p5B
+JWvbicaNns/LS9z3jDSfuJ1nzCN7n78z
+-----END X509 CRL-----

linter/lints/crl/testdata/idp_no_usercerts.pem

@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBlTCCARwCAQEwCgYIKoZIzj0EAwMwSTELMAkGA1UEBhMCWFgxFTATBgNVBAoT
+DEJvdWxkZXIgVGVzdDEjMCEGA1UEAxMaKFRFU1QpIEVsZWdhbnQgRWxlcGhhbnQg
+RTEXDTIyMTAxMDIwMTIwN1oXDTIyMTAxOTIwMTIwNlowKTAnAggDrlHbURVaPBcN
+MjIxMDEwMTkxMjA3WjAMMAoGA1UdFQQDCgEBoHcwdTAfBgNVHSMEGDAWgBQB2rt6
+yyUgjl551vmWQi8CQSkHvjARBgNVHRQECgIIFxzOPeSCumEwPwYDVR0cAQH/BDUw
+M6AxoC+GLWh0dHA6Ly9jLmJvdWxkZXIudGVzdC82NjI4Mzc1NjkxMzU4ODI4OC8w
+LmNybDAKBggqhkjOPQQDAwNnADBkAjAvDkIUnTYavJ6h8606MDyFh2uw/cF+OVnM
+4sE8nUdGy0XYg0hGfbR4MY+kRxRQayICMFeQPpcpIr0zgXpP6lUXU0rcLSvatuae
+QSVr24nGjZ7Py0vc94w0n7idZ8wje5+/Mw==
+-----END X509 CRL-----

linter/lints/crl/testdata/idp_some_reasons.pem

@@ -0,0 +1,11 @@
+-----BEGIN X509 CRL-----
+MIIBnDCCASMCAQEwCgYIKoZIzj0EAwMwSTELMAkGA1UEBhMCWFgxFTATBgNVBAoT
+DEJvdWxkZXIgVGVzdDEjMCEGA1UEAxMaKFRFU1QpIEVsZWdhbnQgRWxlcGhhbnQg
+RTEXDTIyMTAxMDIwMTIwN1oXDTIyMTAxOTIwMTIwNlowKTAnAggDrlHbURVaPBcN
+MjIxMDEwMTkxMjA3WjAMMAoGA1UdFQQDCgEBoH4wfDAfBgNVHSMEGDAWgBQB2rt6
+yyUgjl551vmWQi8CQSkHvjARBgNVHRQECgIIFxzOPeSCumEwRgYDVR0cAQH/BDww
+OqAxoC+GLWh0dHA6Ly9jLmJvdWxkZXIudGVzdC82NjI4Mzc1NjkxMzU4ODI4OC8w
+LmNybIEB/6MCBkAwCgYIKoZIzj0EAwMDZwAwZAIwLw5CFJ02GryeofOtOjA8hYdr
+sP3BfjlZzOLBPJ1HRstF2INIRn20eDGPpEcUUGsiAjBXkD6XKSK9M4F6T+pVF1NK
+3C0r2rbmnkEla9uJxo2ez8tL3PeMNJ+4nWfMI3ufvzM=
+-----END X509 CRL-----