Skip to content

Commit

Permalink
Unfork crl x509 (letsencrypt#7078)
Browse files Browse the repository at this point in the history
Delete our forked version of the x509 library, and update all call-sites
to use the version that we upstreamed and got released in go1.21. This
requires making a few changes to calling code:
- replace crl_x509.RevokedCertificate with x509.RevocationListEntry
- replace RevocationList.RevokedCertificates with
RevocationList.RevokedCertificateEntries
- make RevocationListEntry.ReasonCode a non-pointer integer

Our lints cannot yet be updated to use the new types and fields, because
those improvements have not yet been adopted by the zcrypto/x509 package
used by the linting framework.

Fixes letsencrypt#6741
  • Loading branch information
aarongable authored Sep 16, 2023
1 parent 034316e commit cb28a00
Show file tree
Hide file tree
Showing 22 changed files with 79 additions and 1,868 deletions.
32 changes: 11 additions & 21 deletions ca/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ca
import (
"crypto/rand"
"crypto/sha256"
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"errors"
Expand All @@ -15,7 +16,6 @@ import (
"github.com/letsencrypt/boulder/core"
corepb "github.com/letsencrypt/boulder/core/proto"
bcrl "github.com/letsencrypt/boulder/crl"
"github.com/letsencrypt/boulder/crl/crl_x509"
"github.com/letsencrypt/boulder/issuance"
blog "github.com/letsencrypt/boulder/log"
)
Expand Down Expand Up @@ -67,9 +67,9 @@ func NewCRLImpl(issuers []*issuance.Issuer, lifetime time.Duration, idpBase stri

func (ci *crlImpl) GenerateCRL(stream capb.CRLGenerator_GenerateCRLServer) error {
var issuer *issuance.Issuer
var template *crl_x509.RevocationList
var template *x509.RevocationList
var shard int64
rcs := make([]crl_x509.RevokedCertificate, 0)
rcs := make([]x509.RevocationListEntry, 0)

for {
in, err := stream.Recv()
Expand Down Expand Up @@ -138,11 +138,7 @@ func (ci *crlImpl) GenerateCRL(stream capb.CRLGenerator_GenerateCRLServer) error
fmt.Fprintf(&builder, "Signing CRL: logID=[%s] entries=[", logID)
}

reason := 0
if rcs[i].ReasonCode != nil {
reason = *rcs[i].ReasonCode
}
fmt.Fprintf(&builder, "%x:%d,", rcs[i].SerialNumber.Bytes(), reason)
fmt.Fprintf(&builder, "%x:%d,", rcs[i].SerialNumber.Bytes(), rcs[i].ReasonCode)

if builder.Len() >= ci.maxLogLen {
fmt.Fprint(&builder, "]")
Expand All @@ -154,14 +150,14 @@ func (ci *crlImpl) GenerateCRL(stream capb.CRLGenerator_GenerateCRLServer) error
ci.log.AuditInfo(builder.String())
}

template.RevokedCertificates = rcs
template.RevokedCertificateEntries = rcs

err = issuer.Linter.CheckCRL(template)
if err != nil {
return err
}

crlBytes, err := crl_x509.CreateRevocationList(
crlBytes, err := x509.CreateRevocationList(
rand.Reader,
template,
issuer.Cert.Certificate,
Expand Down Expand Up @@ -196,21 +192,21 @@ func (ci *crlImpl) GenerateCRL(stream capb.CRLGenerator_GenerateCRLServer) error
return nil
}

func (ci *crlImpl) metadataToTemplate(meta *capb.CRLMetadata) (*crl_x509.RevocationList, error) {
func (ci *crlImpl) metadataToTemplate(meta *capb.CRLMetadata) (*x509.RevocationList, error) {
if meta.IssuerNameID == 0 || meta.ThisUpdateNS == 0 {
return nil, errors.New("got incomplete metadata message")
}
thisUpdate := time.Unix(0, meta.ThisUpdateNS)
number := bcrl.Number(thisUpdate)

return &crl_x509.RevocationList{
return &x509.RevocationList{
Number: number,
ThisUpdate: thisUpdate,
NextUpdate: thisUpdate.Add(-time.Second).Add(ci.lifetime),
}, nil
}

func (ci *crlImpl) entryToRevokedCertificate(entry *corepb.CRLEntry) (*crl_x509.RevokedCertificate, error) {
func (ci *crlImpl) entryToRevokedCertificate(entry *corepb.CRLEntry) (*x509.RevocationListEntry, error) {
serial, err := core.StringToSerial(entry.Serial)
if err != nil {
return nil, err
Expand All @@ -221,16 +217,10 @@ func (ci *crlImpl) entryToRevokedCertificate(entry *corepb.CRLEntry) (*crl_x509.
}
revokedAt := time.Unix(0, entry.RevokedAtNS)

var reason *int
if entry.Reason != 0 {
reason = new(int)
*reason = int(entry.Reason)
}

return &crl_x509.RevokedCertificate{
return &x509.RevocationListEntry{
SerialNumber: serial,
RevocationTime: revokedAt,
ReasonCode: reason,
ReasonCode: int(entry.Reason),
}, nil
}

Expand Down
12 changes: 6 additions & 6 deletions ca/crl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,10 @@ func TestGenerateCRL(t *testing.T) {
<-done
test.AssertNotError(t, err, "generating empty CRL should work")
test.Assert(t, len(crlBytes) > 0, "should have gotten some CRL bytes")
crl, err := x509.ParseCRL(crlBytes)
crl, err := x509.ParseRevocationList(crlBytes)
test.AssertNotError(t, err, "should be able to parse empty CRL")
test.AssertEquals(t, len(crl.TBSCertList.RevokedCertificates), 0)
err = testCtx.boulderIssuers[0].Cert.CheckCRLSignature(crl)
test.AssertEquals(t, len(crl.RevokedCertificateEntries), 0)
err = crl.CheckSignatureFrom(testCtx.boulderIssuers[0].Cert.Certificate)
test.AssertNotError(t, err, "CRL signature should validate")

// Test that generating a CRL with some entries works.
Expand Down Expand Up @@ -255,9 +255,9 @@ func TestGenerateCRL(t *testing.T) {
<-done
test.AssertNotError(t, err, "generating empty CRL should work")
test.Assert(t, len(crlBytes) > 0, "should have gotten some CRL bytes")
crl, err = x509.ParseCRL(crlBytes)
crl, err = x509.ParseRevocationList(crlBytes)
test.AssertNotError(t, err, "should be able to parse empty CRL")
test.AssertEquals(t, len(crl.TBSCertList.RevokedCertificates), 5)
err = testCtx.boulderIssuers[0].Cert.CheckCRLSignature(crl)
test.AssertEquals(t, len(crl.RevokedCertificateEntries), 5)
err = crl.CheckSignatureFrom(testCtx.boulderIssuers[0].Cert.Certificate)
test.AssertNotError(t, err, "CRL signature should validate")
}
15 changes: 7 additions & 8 deletions cmd/ceremony/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,15 @@ import (
"math/big"
"time"

"github.com/letsencrypt/boulder/crl/crl_x509"
"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) {
template := &crl_x509.RevocationList{
RevokedCertificates: revokedCertificates,
Number: big.NewInt(number),
ThisUpdate: thisUpdate,
NextUpdate: nextUpdate,
func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nextUpdate time.Time, number int64, revokedCertificates []x509.RevocationListEntry) ([]byte, error) {
template := &x509.RevocationList{
RevokedCertificateEntries: revokedCertificates,
Number: big.NewInt(number),
ThisUpdate: thisUpdate,
NextUpdate: nextUpdate,
}

if nextUpdate.Before(thisUpdate) {
Expand Down Expand Up @@ -55,7 +54,7 @@ func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nex
// at the HSM we don't need to pass a real reader. Instead of passing a nil reader
// we use one that always returns errors in case the internal usage of this reader
// changes.
crlBytes, err := crl_x509.CreateRevocationList(&failReader{}, template, issuer, signer)
crlBytes, err := x509.CreateRevocationList(&failReader{}, template, issuer, signer)
if err != nil {
return nil, err
}
Expand Down
11 changes: 5 additions & 6 deletions cmd/ceremony/crl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"testing"
"time"

"github.com/letsencrypt/boulder/crl/crl_x509"
"github.com/letsencrypt/boulder/test"
)

Expand Down Expand Up @@ -87,11 +86,11 @@ func TestGenerateCRLLints(t *testing.T) {
// However, only the last of those should show up in the error message,
// 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, []x509.RevocationListEntry{
{
SerialNumber: big.NewInt(12345),
ReasonCode: &six,
SerialNumber: big.NewInt(12345),
RevocationTime: time.Now().Add(time.Hour),
ReasonCode: 6,
},
})
test.AssertError(t, err, "generateCRL did not fail")
Expand Down Expand Up @@ -131,7 +130,7 @@ func TestGenerateCRL(t *testing.T) {
test.AssertNotError(t, err, "failed to parse CRL")
err = goCRL.CheckSignatureFrom(cert)
test.AssertNotError(t, err, "CRL signature check failed")
test.AssertEquals(t, len(goCRL.RevokedCertificates), 0)
test.AssertEquals(t, len(goCRL.RevokedCertificateEntries), 0)

// fully parse the CRL to check that the version is correct, and that
// it contains the CRL number extension containing the number we expect
Expand Down
9 changes: 4 additions & 5 deletions cmd/ceremony/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@ import (
"slices"
"time"

"github.com/letsencrypt/boulder/crl/crl_x509"
"golang.org/x/crypto/ocsp"
"gopkg.in/yaml.v3"

"github.com/letsencrypt/boulder/goodkey"
"github.com/letsencrypt/boulder/linter"
"github.com/letsencrypt/boulder/pkcs11helpers"
"github.com/letsencrypt/boulder/strictyaml"
"golang.org/x/crypto/ocsp"
"gopkg.in/yaml.v3"
)

var kp goodkey.KeyPolicy
Expand Down Expand Up @@ -933,7 +932,7 @@ func crlCeremony(configBytes []byte) error {
return fmt.Errorf("unable to parse crl-profile.next-update: %s", err)
}

var revokedCertificates []crl_x509.RevokedCertificate
var revokedCertificates []x509.RevocationListEntry
for _, rc := range config.CRLProfile.RevokedCertificates {
cert, err := loadCert(rc.CertificatePath)
if err != nil {
Expand All @@ -943,7 +942,7 @@ func crlCeremony(configBytes []byte) error {
if err != nil {
return fmt.Errorf("unable to parse crl-profile.revoked-certificates.revocation-date")
}
revokedCert := crl_x509.RevokedCertificate{
revokedCert := x509.RevocationListEntry{
SerialNumber: cert.SerialNumber,
RevocationTime: revokedAt,
}
Expand Down
9 changes: 4 additions & 5 deletions cmd/crl-checker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ import (
"github.com/letsencrypt/boulder/cmd"
"github.com/letsencrypt/boulder/core"
"github.com/letsencrypt/boulder/crl/checker"
"github.com/letsencrypt/boulder/crl/crl_x509"
)

func downloadShard(url string) (*crl_x509.RevocationList, error) {
func downloadShard(url string) (*x509.RevocationList, error) {
resp, err := http.Get(url)
if err != nil {
return nil, fmt.Errorf("downloading crl: %w", err)
Expand All @@ -32,7 +31,7 @@ func downloadShard(url string) (*crl_x509.RevocationList, error) {
return nil, fmt.Errorf("reading CRL bytes: %w", err)
}

crl, err := crl_x509.ParseRevocationList(crlBytes)
crl, err := x509.ParseRevocationList(crlBytes)
if err != nil {
return nil, fmt.Errorf("parsing CRL: %w", err)
}
Expand Down Expand Up @@ -100,7 +99,7 @@ func main() {

totalBytes += len(crl.Raw)

zcrl, err := crl_x509.ParseRevocationList(crl.Raw)
zcrl, err := x509.ParseRevocationList(crl.Raw)
if err != nil {
errCount += 1
logger.Errf("parsing CRL %q failed: %s", u, err)
Expand All @@ -118,7 +117,7 @@ func main() {
oldestTimestamp = crl.ThisUpdate
}

for _, c := range crl.RevokedCertificates {
for _, c := range crl.RevokedCertificateEntries {
serial := core.SerialToString(c.SerialNumber)
if _, seen := seenSerials[serial]; seen {
errCount += 1
Expand Down
13 changes: 6 additions & 7 deletions crl/checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@ import (
zlint_x509 "github.com/zmap/zcrypto/x509"
"github.com/zmap/zlint/v3"

"github.com/letsencrypt/boulder/crl/crl_x509"
"github.com/letsencrypt/boulder/linter"
)

// Validate runs the given CRL through our set of lints, ensures its signature
// validates (if supplied with a non-nil issuer), and checks that the CRL is
// less than ageLimit old. It returns an error if any of these conditions are
// not met.
func Validate(crl *crl_x509.RevocationList, issuer *x509.Certificate, ageLimit time.Duration) error {
func Validate(crl *x509.RevocationList, issuer *x509.Certificate, ageLimit time.Duration) error {
zcrl, err := zlint_x509.ParseRevocationList(crl.Raw)
if err != nil {
return fmt.Errorf("parsing CRL: %w", err)
Expand Down Expand Up @@ -55,7 +54,7 @@ type diffResult struct {
// CRLs. In order to be comparable, the CRLs must come from the same issuer, and
// be given in the correct order (the "old" CRL's Number and ThisUpdate must
// both precede the "new" CRL's).
func Diff(old, new *crl_x509.RevocationList) (*diffResult, error) {
func Diff(old, new *x509.RevocationList) (*diffResult, error) {
if !bytes.Equal(old.AuthorityKeyId, new.AuthorityKeyId) {
return nil, fmt.Errorf("CRLs were not issued by same issuer")
}
Expand All @@ -69,16 +68,16 @@ func Diff(old, new *crl_x509.RevocationList) (*diffResult, error) {
}

// Sort both sets of serials so we can march through them in order.
oldSerials := make([]*big.Int, len(old.RevokedCertificates))
for i, rc := range old.RevokedCertificates {
oldSerials := make([]*big.Int, len(old.RevokedCertificateEntries))
for i, rc := range old.RevokedCertificateEntries {
oldSerials[i] = rc.SerialNumber
}
sort.Slice(oldSerials, func(i, j int) bool {
return oldSerials[i].Cmp(oldSerials[j]) < 0
})

newSerials := make([]*big.Int, len(new.RevokedCertificates))
for j, rc := range new.RevokedCertificates {
newSerials := make([]*big.Int, len(new.RevokedCertificateEntries))
for j, rc := range new.RevokedCertificateEntries {
newSerials[j] = rc.SerialNumber
}
sort.Slice(newSerials, func(i, j int) bool {
Expand Down
22 changes: 11 additions & 11 deletions crl/checker/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package checker

import (
"crypto/rand"
"crypto/x509"
"encoding/pem"
"io"
"math/big"
Expand All @@ -10,7 +11,6 @@ import (
"time"

"github.com/letsencrypt/boulder/core"
"github.com/letsencrypt/boulder/crl/crl_x509"
"github.com/letsencrypt/boulder/issuance"
"github.com/letsencrypt/boulder/test"
)
Expand All @@ -21,7 +21,7 @@ func TestValidate(t *testing.T) {
crlPEM, err := io.ReadAll(crlFile)
test.AssertNotError(t, err, "reading test crl file")
crlDER, _ := pem.Decode(crlPEM)
crl, err := crl_x509.ParseRevocationList(crlDER.Bytes)
crl, err := x509.ParseRevocationList(crlDER.Bytes)
test.AssertNotError(t, err, "parsing test crl")
issuer, err := core.LoadCert("../../test/hierarchy/int-e1.cert.pem")
test.AssertNotError(t, err, "loading test issuer")
Expand All @@ -44,7 +44,7 @@ func TestValidate(t *testing.T) {
crlPEM, err = io.ReadAll(crlFile)
test.AssertNotError(t, err, "reading test crl file")
crlDER, _ = pem.Decode(crlPEM)
crl, err = crl_x509.ParseRevocationList(crlDER.Bytes)
crl, err = x509.ParseRevocationList(crlDER.Bytes)
test.AssertNotError(t, err, "parsing test crl")
err = Validate(crl, issuer, 100*365*24*time.Hour)
test.AssertError(t, err, "validating crl with lint error")
Expand All @@ -59,11 +59,11 @@ func TestDiff(t *testing.T) {
test.AssertNotError(t, err, "loading test issuer")

now := time.Now()
template := crl_x509.RevocationList{
template := x509.RevocationList{
ThisUpdate: now,
NextUpdate: now.Add(24 * time.Hour),
Number: big.NewInt(1),
RevokedCertificates: []crl_x509.RevokedCertificate{
RevokedCertificateEntries: []x509.RevocationListEntry{
{
SerialNumber: big.NewInt(1),
RevocationTime: now.Add(-time.Hour),
Expand All @@ -75,17 +75,17 @@ func TestDiff(t *testing.T) {
},
}

oldCRLDER, err := crl_x509.CreateRevocationList(rand.Reader, &template, issuer.Certificate, signer)
oldCRLDER, err := x509.CreateRevocationList(rand.Reader, &template, issuer.Certificate, signer)
test.AssertNotError(t, err, "creating old crl")
oldCRL, err := crl_x509.ParseRevocationList(oldCRLDER)
oldCRL, err := x509.ParseRevocationList(oldCRLDER)
test.AssertNotError(t, err, "parsing old crl")

now = now.Add(time.Hour)
template = crl_x509.RevocationList{
template = x509.RevocationList{
ThisUpdate: now,
NextUpdate: now.Add(24 * time.Hour),
Number: big.NewInt(2),
RevokedCertificates: []crl_x509.RevokedCertificate{
RevokedCertificateEntries: []x509.RevocationListEntry{
{
SerialNumber: big.NewInt(1),
RevocationTime: now.Add(-2 * time.Hour),
Expand All @@ -97,9 +97,9 @@ func TestDiff(t *testing.T) {
},
}

newCRLDER, err := crl_x509.CreateRevocationList(rand.Reader, &template, issuer.Certificate, signer)
newCRLDER, err := x509.CreateRevocationList(rand.Reader, &template, issuer.Certificate, signer)
test.AssertNotError(t, err, "creating old crl")
newCRL, err := crl_x509.ParseRevocationList(newCRLDER)
newCRL, err := x509.ParseRevocationList(newCRLDER)
test.AssertNotError(t, err, "parsing old crl")

res, err := Diff(oldCRL, newCRL)
Expand Down
Loading

0 comments on commit cb28a00

Please sign in to comment.