Skip to content

Commit

Permalink
PKI: Do not read revoked certificates if CRL is disabled (#17384)
Browse files Browse the repository at this point in the history
* PKI: Do not read revoked certificates if CRL is disabled

 - Restore the prior behavior of not reading in all revoked certificates
   if the CRL has been disabled.

* Remove incorrect comment text copied from 1.12

* Add cl
  • Loading branch information
stevendpclark authored Oct 3, 2022
1 parent 4624c4d commit 0c2f920
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 11 deletions.
36 changes: 25 additions & 11 deletions builtin/logical/pki/crl_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,20 @@ func buildCRLs(ctx context.Context, b *backend, req *logical.Request, forceNew b
var err error
var issuers []issuerID
var wasLegacy bool

crlInfo, err := b.CRL(ctx, req.Storage)
if err != nil {
return errutil.InternalError{Err: fmt.Sprintf("error fetching CRL config information: %s", err)}
}

if crlInfo.Disable && !forceNew {
// We build a single long-lived empty CRL in the event that we disable
// the CRL, but we don't keep updating it with newer, more-valid empty
// CRLs in the event that we later re-enable it. This is a historical
// behavior.
return nil
}

if !b.useLegacyBundleCaStorage() {
issuers, err = listIssuers(ctx, req.Storage)
if err != nil {
Expand Down Expand Up @@ -356,9 +370,15 @@ func buildCRLs(ctx context.Context, b *backend, req *logical.Request, forceNew b
// these certificates to an issuer. Some certificates will not be
// assignable (if they were issued by a since-deleted issuer), so we need
// a separate pool for those.
unassignedCerts, revokedCertsMap, err := getRevokedCertEntries(ctx, req, issuerIDCertMap)
if err != nil {
return fmt.Errorf("error building CRLs: unable to get revoked certificate entries: %v", err)
var unassignedCerts []pkix.RevokedCertificate
var revokedCertsMap map[issuerID][]pkix.RevokedCertificate

// If the CRL is disabled do not bother reading in all the revoked certificates.
if !crlInfo.Disable {
unassignedCerts, revokedCertsMap, err = getRevokedCertEntries(ctx, req, issuerIDCertMap)
if err != nil {
return fmt.Errorf("error building CRLs: unable to get revoked certificate entries: %v", err)
}
}

// Now we can call buildCRL once, on an arbitrary/representative issuer
Expand Down Expand Up @@ -413,7 +433,7 @@ func buildCRLs(ctx context.Context, b *backend, req *logical.Request, forceNew b
crlConfig.CRLNumberMap[crlIdentifier] += 1

// Lastly, build the CRL.
if err := buildCRL(ctx, b, req, forceNew, representative, revokedCerts, crlIdentifier, crlNumber); err != nil {
if err := buildCRL(ctx, b, req, forceNew, representative, revokedCerts, crlIdentifier, crlNumber, crlInfo); err != nil {
return fmt.Errorf("error building CRLs: unable to build CRL for issuer (%v): %v", representative, err)
}
}
Expand Down Expand Up @@ -474,7 +494,6 @@ func buildCRLs(ctx context.Context, b *backend, req *logical.Request, forceNew b
func getRevokedCertEntries(ctx context.Context, req *logical.Request, issuerIDCertMap map[issuerID]*x509.Certificate) ([]pkix.RevokedCertificate, map[issuerID][]pkix.RevokedCertificate, error) {
var unassignedCerts []pkix.RevokedCertificate
revokedCertsMap := make(map[issuerID][]pkix.RevokedCertificate)

revokedSerials, err := req.Storage.List(ctx, revokedPath)
if err != nil {
return nil, nil, errutil.InternalError{Err: fmt.Sprintf("error fetching list of revoked certs: %s", err)}
Expand Down Expand Up @@ -569,12 +588,7 @@ func getRevokedCertEntries(ctx context.Context, req *logical.Request, issuerIDCe

// Builds a CRL by going through the list of revoked certificates and building
// a new CRL with the stored revocation times and serial numbers.
func buildCRL(ctx context.Context, b *backend, req *logical.Request, forceNew bool, thisIssuerId issuerID, revoked []pkix.RevokedCertificate, identifier crlID, crlNumber int64) error {
crlInfo, err := b.CRL(ctx, req.Storage)
if err != nil {
return errutil.InternalError{Err: fmt.Sprintf("error fetching CRL config information: %s", err)}
}

func buildCRL(ctx context.Context, b *backend, req *logical.Request, forceNew bool, thisIssuerId issuerID, revoked []pkix.RevokedCertificate, identifier crlID, crlNumber int64, crlInfo *crlConfig) error {
crlLifetime := b.crlLifetime
var revokedCerts []pkix.RevokedCertificate

Expand Down
3 changes: 3 additions & 0 deletions changelog/17384.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
secrets/pki: Do not read revoked certificates from backend when CRL is disabled
```

0 comments on commit 0c2f920

Please sign in to comment.