Skip to content

Commit

Permalink
Add ceremony tool support for CRL OnlyContainsCACerts
Browse files Browse the repository at this point in the history
  • Loading branch information
pgporada committed Aug 31, 2023
1 parent bd8558d commit 1964e28
Show file tree
Hide file tree
Showing 6 changed files with 369 additions and 17 deletions.
78 changes: 72 additions & 6 deletions cmd/ceremony/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package main
import (
"crypto"
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"encoding/pem"
"errors"
"fmt"
Expand All @@ -13,7 +15,9 @@ import (
"github.com/letsencrypt/boulder/linter"
)

func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nextUpdate time.Time, number int64, revokedCertificates []crl_x509.RevokedCertificate) ([]byte, error) {
type issuerNameID int64

func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nextUpdate time.Time, number int64, idpBase string, revokedCertificates []crl_x509.RevokedCertificate) ([]byte, error) {
template := &crl_x509.RevocationList{
RevokedCertificates: revokedCertificates,
Number: big.NewInt(number),
Expand All @@ -35,16 +39,19 @@ func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nex
if nextUpdate.Sub(thisUpdate) > time.Hour*24*365 {
return nil, errors.New("nextUpdate must be less than 12 months after thisUpdate")
}
// Add the Issuing Distribution Point extension.
issuerNameID := truncatedHash(issuer.RawSubject)
idp, err := makeIDPExt(idpBase, issuerNameID)
if err != nil {
return nil, fmt.Errorf("creating IDP extension: %w", err)
}
template.ExtraExtensions = append(template.ExtraExtensions, *idp)

err := linter.CheckCRL(template, issuer, signer, []string{
err = linter.CheckCRL(template, issuer, signer, []string{
// We skip this lint because our ceremony tooling issues CRLs with validity
// periods up to 12 months, but the lint only allows up to 10 days (which
// is the limit for CRLs containing Subscriber Certificates).
"e_crl_validity_period",
// We skip this lint because it is only applicable for sharded/partitioned
// CRLs, which our Subscriber CRLs are, but our higher-level CRLs issued by
// this tool are not.
"e_crl_has_idp",
})
if err != nil {
return nil, fmt.Errorf("crl failed pre-issuance lint: %w", err)
Expand All @@ -62,3 +69,62 @@ func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nex

return pem.EncodeToMemory(&pem.Block{Type: "X509 CRL", Bytes: crlBytes}), nil
}

// truncatedHash computes a truncated SHA1 hash across arbitrary bytes. Uses
// SHA1 because that is the algorithm most commonly used in OCSP requests.
// PURPOSEFULLY NOT EXPORTED. Exists only to ensure that the implementations of
// Certificate.NameID() and GetIssuerNameID() never diverge. Use those instead.
func truncatedHash(name []byte) issuerNameID {
h := crypto.SHA1.New()
h.Write(name)
s := h.Sum(nil)
return issuerNameID(big.NewInt(0).SetBytes(s[:7]).Int64())
}

// 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"`
OnlyContainsCACerts bool `asn1:"optional,tag:2"`
}

// makeIDPExt returns a critical IssuingDistributionPoint extension containing a
// URI built from the base url and the issuer's NameID. It also sets the
// OnlyContainsCACerts boolean to true.
func makeIDPExt(base string, issuer issuerNameID) (*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.crl", base, issuer)),
},
},
},
OnlyContainsCACerts: 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
}
15 changes: 8 additions & 7 deletions cmd/ceremony/crl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,28 @@ import (
)

func TestGenerateCRLTimeBounds(t *testing.T) {
_, err := generateCRL(nil, nil, time.Now().Add(time.Hour), time.Now(), 1, nil)
_, err := generateCRL(nil, nil, time.Now().Add(time.Hour), time.Now(), 1, "idp", nil)
test.AssertError(t, err, "generateCRL did not fail")
test.AssertEquals(t, err.Error(), "thisUpdate must be before nextUpdate")

_, err = generateCRL(nil, &x509.Certificate{
NotBefore: time.Now().Add(time.Hour),
NotAfter: time.Now(),
}, time.Now(), time.Now(), 1, nil)
}, time.Now(), time.Now(), 1, "idp", nil)
test.AssertError(t, err, "generateCRL did not fail")
test.AssertEquals(t, err.Error(), "thisUpdate is before issuing certificate's notBefore")

_, err = generateCRL(nil, &x509.Certificate{
NotBefore: time.Now(),
NotAfter: time.Now().Add(time.Hour * 2),
}, time.Now().Add(time.Hour), time.Now().Add(time.Hour*3), 1, nil)
}, time.Now().Add(time.Hour), time.Now().Add(time.Hour*3), 1, "idp", nil)
test.AssertError(t, err, "generateCRL did not fail")
test.AssertEquals(t, err.Error(), "nextUpdate is after issuing certificate's notAfter")

_, err = generateCRL(nil, &x509.Certificate{
NotBefore: time.Now(),
NotAfter: time.Now().Add(time.Hour * 24 * 370),
}, time.Now(), time.Now().Add(time.Hour*24*366), 1, nil)
}, time.Now(), time.Now().Add(time.Hour*24*366), 1, "idp", nil)
test.AssertError(t, err, "generateCRL did not fail")
test.AssertEquals(t, err.Error(), "nextUpdate must be less than 12 months after thisUpdate")
}
Expand Down Expand Up @@ -88,7 +88,7 @@ func TestGenerateCRLLints(t *testing.T) {
// because the first two should be explicitly removed from the lint registry
// by the ceremony tool.
six := 6
_, err = generateCRL(&wrappedSigner{k}, cert, time.Now().Add(time.Hour), time.Now().Add(100*24*time.Hour), 1, []crl_x509.RevokedCertificate{
_, err = generateCRL(&wrappedSigner{k}, cert, time.Now().Add(time.Hour), time.Now().Add(100*24*time.Hour), 1, "idp", []crl_x509.RevokedCertificate{
{
SerialNumber: big.NewInt(12345),
ReasonCode: &six,
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestGenerateCRL(t *testing.T) {
cert, err := x509.ParseCertificate(certBytes)
test.AssertNotError(t, err, "failed to parse test cert")

crlPEM, err := generateCRL(&wrappedSigner{k}, cert, time.Now().Add(time.Hour), time.Now().Add(time.Hour*2), 1, nil)
crlPEM, err := generateCRL(&wrappedSigner{k}, cert, time.Now().Add(time.Hour), time.Now().Add(time.Hour*2), 1, "idp", nil)
test.AssertNotError(t, err, "generateCRL failed with valid profile")

pemBlock, _ := pem.Decode(crlPEM)
Expand All @@ -139,8 +139,9 @@ func TestGenerateCRL(t *testing.T) {
_, err = asn1.Unmarshal(crlDER, &crl)
test.AssertNotError(t, err, "failed to parse CRL")
test.AssertEquals(t, crl.TBS.Version, 1) // x509v2 == 1
test.AssertEquals(t, len(crl.TBS.Extensions), 2) // AKID, CRL number
test.AssertEquals(t, len(crl.TBS.Extensions), 3) // AKID, CRL number, IssuingDistributionPOint
test.Assert(t, crl.TBS.Extensions[1].Id.Equal(asn1.ObjectIdentifier{2, 5, 29, 20}), "unexpected OID in extension")
test.Assert(t, crl.TBS.Extensions[2].Id.Equal(asn1.ObjectIdentifier{2, 5, 29, 28}), "unexpected OID in extension")
var number int
_, err = asn1.Unmarshal(crl.TBS.Extensions[1].Value, &number)
test.AssertNotError(t, err, "failed to parse CRL number extension")
Expand Down
6 changes: 5 additions & 1 deletion cmd/ceremony/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ type crlConfig struct {
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
} `yaml:"revoked-certificates"`
IssuingDistributionPoint string `yaml:"issuing-distribution-point"`
} `yaml:"crl-profile"`
}

Expand Down Expand Up @@ -446,6 +447,9 @@ func (cc crlConfig) validate() error {
if cc.CRLProfile.Number == 0 {
return errors.New("crl-profile.number must be non-zero")
}
if cc.CRLProfile.IssuingDistributionPoint == "" {
return errors.New("crl-profile.issuing-distribution-point is required")
}
for _, rc := range cc.CRLProfile.RevokedCertificates {
if rc.CertificatePath == "" {
return errors.New("crl-profile.revoked-certificates.certificate-path is required")
Expand Down Expand Up @@ -959,7 +963,7 @@ func crlCeremony(configBytes []byte) error {
revokedCertificates = append(revokedCertificates, revokedCert)
}

crlBytes, err := generateCRL(signer, issuer, thisUpdate, nextUpdate, config.CRLProfile.Number, revokedCertificates)
crlBytes, err := generateCRL(signer, issuer, thisUpdate, nextUpdate, config.CRLProfile.Number, config.CRLProfile.IssuingDistributionPoint, revokedCertificates)
if err != nil {
return err
}
Expand Down
61 changes: 58 additions & 3 deletions cmd/ceremony/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1171,8 +1171,10 @@ func TestCRLConfig(t *testing.T) {
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
} `yaml:"revoked-certificates"`
IssuingDistributionPoint string `yaml:"issuing-distribution-point"`
}{
ThisUpdate: "this-update",
ThisUpdate: "this-update",
IssuingDistributionPoint: "http://example.com",
},
},
expectedError: "crl-profile.next-update is required",
Expand Down Expand Up @@ -1203,9 +1205,11 @@ func TestCRLConfig(t *testing.T) {
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
} `yaml:"revoked-certificates"`
IssuingDistributionPoint string `yaml:"issuing-distribution-point"`
}{
ThisUpdate: "this-update",
NextUpdate: "next-update",
ThisUpdate: "this-update",
NextUpdate: "next-update",
IssuingDistributionPoint: "http://example.com",
},
},
expectedError: "crl-profile.number must be non-zero",
Expand Down Expand Up @@ -1236,6 +1240,7 @@ func TestCRLConfig(t *testing.T) {
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
} `yaml:"revoked-certificates"`
IssuingDistributionPoint string `yaml:"issuing-distribution-point"`
}{
ThisUpdate: "this-update",
NextUpdate: "next-update",
Expand All @@ -1245,6 +1250,7 @@ func TestCRLConfig(t *testing.T) {
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
}{{}},
IssuingDistributionPoint: "http://example.com",
},
},
expectedError: "crl-profile.revoked-certificates.certificate-path is required",
Expand Down Expand Up @@ -1275,6 +1281,7 @@ func TestCRLConfig(t *testing.T) {
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
} `yaml:"revoked-certificates"`
IssuingDistributionPoint string `yaml:"issuing-distribution-point"`
}{
ThisUpdate: "this-update",
NextUpdate: "next-update",
Expand All @@ -1286,6 +1293,7 @@ func TestCRLConfig(t *testing.T) {
}{{
CertificatePath: "path",
}},
IssuingDistributionPoint: "http://example.com",
},
},
expectedError: "crl-profile.revoked-certificates.revocation-date is required",
Expand Down Expand Up @@ -1316,6 +1324,7 @@ func TestCRLConfig(t *testing.T) {
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
} `yaml:"revoked-certificates"`
IssuingDistributionPoint string `yaml:"issuing-distribution-point"`
}{
ThisUpdate: "this-update",
NextUpdate: "next-update",
Expand All @@ -1328,10 +1337,54 @@ func TestCRLConfig(t *testing.T) {
CertificatePath: "path",
RevocationDate: "date",
}},
IssuingDistributionPoint: "http://example.com",
},
},
expectedError: "crl-profile.revoked-certificates.revocation-reason is required",
},
{
name: "no issuing distribution point path provided",
config: crlConfig{
PKCS11: PKCS11SigningConfig{
Module: "module",
SigningLabel: "label",
},
Inputs: struct {
IssuerCertificatePath string `yaml:"issuer-certificate-path"`
}{
IssuerCertificatePath: "path",
},
Outputs: struct {
CRLPath string `yaml:"crl-path"`
}{
CRLPath: "path",
},
CRLProfile: struct {
ThisUpdate string `yaml:"this-update"`
NextUpdate string `yaml:"next-update"`
Number int64 `yaml:"number"`
RevokedCertificates []struct {
CertificatePath string `yaml:"certificate-path"`
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
} `yaml:"revoked-certificates"`
IssuingDistributionPoint string `yaml:"issuing-distribution-point"`
}{
ThisUpdate: "this-update",
NextUpdate: "next-update",
Number: 1,
RevokedCertificates: []struct {
CertificatePath string `yaml:"certificate-path"`
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
}{{
CertificatePath: "path",
RevocationDate: "date",
}},
},
},
expectedError: "crl-profile.issuing-distribution-point is required",
},
{
name: "good",
config: crlConfig{
Expand All @@ -1358,6 +1411,7 @@ func TestCRLConfig(t *testing.T) {
RevocationDate string `yaml:"revocation-date"`
RevocationReason int `yaml:"revocation-reason"`
} `yaml:"revoked-certificates"`
IssuingDistributionPoint string `yaml:"issuing-distribution-point"`
}{
ThisUpdate: "this-update",
NextUpdate: "next-update",
Expand All @@ -1371,6 +1425,7 @@ func TestCRLConfig(t *testing.T) {
RevocationDate: "date",
RevocationReason: 1,
}},
IssuingDistributionPoint: "http://example.com",
},
},
},
Expand Down
Loading

0 comments on commit 1964e28

Please sign in to comment.