Skip to content

Commit

Permalink
Use forked crl x509 (#6204)
Browse files Browse the repository at this point in the history
Use the new //crl/x509 library in the CA, to make handling the
ReasonCode of each CRL entry significantly easier. This also
allows us to log the reason code along with each serial in the
CRL.

Also, make a couple tiny tweaks to the //crl/x509 package that
were discovered to be useful while writing this change. These
include moving it to a //crl/crl_x509 directory so that it doesn't
have to be aliased at import time.

Fixes #6199
  • Loading branch information
aarongable authored Jul 1, 2022
1 parent 67e3aa9 commit 63f7936
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 59 deletions.
48 changes: 17 additions & 31 deletions ca/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ package ca
import (
"crypto/rand"
"crypto/sha256"
"crypto/x509"
"crypto/x509/pkix"
"encoding/asn1"
"errors"
"fmt"
"io"
Expand All @@ -16,6 +13,7 @@ import (
capb "github.com/letsencrypt/boulder/ca/proto"
"github.com/letsencrypt/boulder/core"
corepb "github.com/letsencrypt/boulder/core/proto"
"github.com/letsencrypt/boulder/crl/crl_x509"
"github.com/letsencrypt/boulder/issuance"
blog "github.com/letsencrypt/boulder/log"
)
Expand Down Expand Up @@ -53,9 +51,9 @@ func NewCRLImpl(issuers []*issuance.Issuer, lifetime time.Duration, maxLogLen in

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

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

// TODO: Figure out how best to include the reason code here, since it's
// slow/difficult to extract it from the already-encoded entry extension.
fmt.Fprintf(&builder, "%x,", rcs[i].SerialNumber.Bytes())
reason := 0
if rcs[i].ReasonCode != nil {
reason = *rcs[i].ReasonCode
}
fmt.Fprintf(&builder, "%x:%d,", rcs[i].SerialNumber.Bytes(), reason)

if builder.Len() != ci.maxLogLen {
ci.log.AuditInfof("%s", builder)
Expand All @@ -127,7 +127,7 @@ func (ci *crlImpl) GenerateCRL(stream capb.CRLGenerator_GenerateCRLServer) error
}

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

func (ci *crlImpl) metadataToTemplate(meta *capb.CRLMetadata) (*x509.RevocationList, error) {
func (ci *crlImpl) metadataToTemplate(meta *capb.CRLMetadata) (*crl_x509.RevocationList, error) {
if meta.IssuerNameID == 0 || meta.ThisUpdate == 0 {
return nil, errors.New("got incomplete metadata message")
}
Expand All @@ -174,15 +174,15 @@ func (ci *crlImpl) metadataToTemplate(meta *capb.CRLMetadata) (*x509.RevocationL
number := big.NewInt(meta.ThisUpdate)
thisUpdate := time.Unix(0, meta.ThisUpdate)

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

}

func (ci *crlImpl) entryToRevokedCertificate(entry *corepb.CRLEntry) (*pkix.RevokedCertificate, error) {
func (ci *crlImpl) entryToRevokedCertificate(entry *corepb.CRLEntry) (*crl_x509.RevokedCertificate, error) {
serial, err := core.StringToSerial(entry.Serial)
if err != nil {
return nil, err
Expand All @@ -193,29 +193,15 @@ func (ci *crlImpl) entryToRevokedCertificate(entry *corepb.CRLEntry) (*pkix.Revo
}
revokedAt := time.Unix(0, entry.RevokedAt)

// RFC 5280 Section 5.3.1 says "the reason code CRL entry extension SHOULD be
// absent instead of using the unspecified (0) reasonCode value.", so we make
// sure we only add this extension if we have a non-zero revocation reason.
var extensions []pkix.Extension
var reason *int
if entry.Reason != 0 {
reasonBytes, err := asn1.Marshal(asn1.Enumerated(entry.Reason))
if err != nil {
return nil, err
}

extensions = []pkix.Extension{
// The Reason Code extension, as defined in RFC 5280 Section 5.3.1:
// https://datatracker.ietf.org/doc/html/rfc5280#section-5.3.1
{
Id: asn1.ObjectIdentifier{2, 5, 29, 21}, // id-ce-reasonCode
Value: reasonBytes,
},
}
reason = new(int)
*reason = int(entry.Reason)
}

return &pkix.RevokedCertificate{
return &crl_x509.RevokedCertificate{
SerialNumber: serial,
RevocationTime: revokedAt,
Extensions: extensions,
ReasonCode: reason,
}, nil
}
File renamed without changes.
File renamed without changes.
43 changes: 21 additions & 22 deletions crl/x509/crl.go → crl/crl_x509/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Package x509 parses X.509-encoded keys and certificates.
package x509
// Package crl_x509 parses X.509-encoded certificate revocation lists.
package crl_x509

import (
"bytes"
Expand All @@ -26,9 +26,8 @@ var (
oidExtensionReasonCode = []int{2, 5, 29, 21}
)

// RevokedCertificate contains the fields used to create an entry in the revokedCertificates
// list of a CRL.
// revoked certificates.
// RevokedCertificate represents an entry in the revokedCertificates sequence of
// a CRL.
// NOTE: This type does not exist in upstream.
type RevokedCertificate struct {
// SerialNumber represents the serial number of a revoked certificate. It is
Expand All @@ -46,15 +45,16 @@ type RevokedCertificate struct {
// value of 0 represents a reasonCode extension containing enum value 0 (this
// SHOULD NOT happen, but can and does).
ReasonCode *int
// Extensions contains all raw extensions parsed from the CRL entry, or all
// extra extensions to add to the CRL entry. When creating a CRL, if
// Extensions contains a reasonCode extension, it will be ignored in favor of
// the ReasonCode field above.
Extensions []pkix.Extension
// When creating a CRL, ExtraExtensions should contain all extra extensions to
// add to the CRL entry. If ExtraExtensions contains a reasonCode extension,
// it will be ignored in favor of the ReasonCode field above. When parsing a
// CRL, ExtraExtensions contains all raw extensions parsed from the CRL entry,
// except for reasonCode which is represented by the ReasonCode field above.
ExtraExtensions []pkix.Extension
}

// RevocationList contains the fields used to create an X.509 v2 Certificate
// Revocation list with CreateRevocationList.
// RevocationList represents a Certificate Revocation List (CRL) as specified
// by RFC 5280.
type RevocationList struct {
// Raw, RawTBSRevocationList, and RawIssuer contain the raw bytes of the whole
// CRL, the tbsCertList field, and the issuer field, respectively. They are
Expand Down Expand Up @@ -91,16 +91,15 @@ type RevocationList struct {
// revokedCertificates sequence will be omitted from the CRL entirely.
RevokedCertificates []RevokedCertificate

// Number represents the X.509 v2 CRLNumber extension, which should be a
// monotonically increasing sequence number for a given CRL scope and CRL
// issuer. It is both used when creating a CRL and populated when parsing a
// CRL. When creating a CRL, it MUST NOT be nil, and MUST NOT be longer than
// 20 bytes.
// Number represents the CRLNumber extension, which should be a monotonically
// increasing sequence number for a given CRL scope and CRL issuer. It is both
// used when creating a CRL and populated when parsing a CRL. When creating a
// CRL, it MUST NOT be nil, and MUST NOT be longer than 20 bytes.
Number *big.Int

// AuthorityKeyId is populated from the X.509 v2 authorityKeyIdentifier
// extension in the CRL. It is ignored when creating a CRL: the extension is
// populated from the issuer information instead.
// AuthorityKeyId is populated from the authorityKeyIdentifier extension in
// the CRL. It is ignored when creating a CRL: the extension is populated from
// the issuer information instead.
AuthorityKeyId []byte

// Extensions contains raw X.509 extensions. When creating a CRL,
Expand Down Expand Up @@ -243,7 +242,7 @@ func ParseRevocationList(der []byte) (*RevocationList, error) {
}
continue
}
rc.Extensions = append(rc.Extensions, ext)
rc.ExtraExtensions = append(rc.ExtraExtensions, ext)
}
}

Expand Down Expand Up @@ -337,7 +336,7 @@ func CreateRevocationList(rand io.Reader, template *RevocationList, issuer *x509
// Copy over any extra extensions, except for a Reason Code extension,
// because we'll synthesize that ourselves to ensure it is correct.
exts := make([]pkix.Extension, 0)
for _, ext := range rc.Extensions {
for _, ext := range rc.ExtraExtensions {
if ext.Id.Equal(oidExtensionReasonCode) {
continue
}
Expand Down
4 changes: 2 additions & 2 deletions crl/x509/crl_test.go → crl/crl_x509/crl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package x509
package crl_x509

import (
"crypto"
Expand Down Expand Up @@ -223,7 +223,7 @@ func TestCreateRevocationList(t *testing.T) {
SerialNumber: big.NewInt(2),
RevocationTime: time.Time{}.Add(time.Hour),
ReasonCode: &reasonKeyCompromise,
Extensions: []pkix.Extension{
ExtraExtensions: []pkix.Extension{
{
Id: []int{1, 1},
Value: []byte{5, 0},
Expand Down
2 changes: 1 addition & 1 deletion crl/x509/parser.go → crl/crl_x509/parser.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2021 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package x509
package crl_x509

import (
"crypto/x509/pkix"
Expand Down
2 changes: 1 addition & 1 deletion crl/x509/parser_test.go → crl/crl_x509/parser_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2021 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package x509
package crl_x509

import (
"encoding/asn1"
Expand Down
2 changes: 1 addition & 1 deletion crl/x509/x509.go → crl/crl_x509/x509.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// license that can be found in the LICENSE file.

// Package x509 parses X.509-encoded keys and certificates.
package x509
package crl_x509

import (
"bytes"
Expand Down
2 changes: 1 addition & 1 deletion crl/x509/x509_test.go → crl/crl_x509/x509_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package x509
package crl_x509

import (
_ "crypto/sha256"
Expand Down

0 comments on commit 63f7936

Please sign in to comment.