Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forward PKI revocation requests received by standby nodes to active node #19624

Merged
merged 4 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions builtin/logical/pki/crl_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ func (cb *crlBuilder) reloadConfigIfRequired(sc *storageContext) error {
}

previousConfig := cb.config

// Set the default config if none was returned to us.
if config != nil {
cb.config = *config
Expand Down Expand Up @@ -354,7 +353,7 @@ func (cb *crlBuilder) _getPresentDeltaWALForClearing(sc *storageContext, path st
// Clearing of the delta WAL occurs after a new complete CRL has been built.
walSerials, err := sc.Storage.List(sc.Context, path)
if err != nil {
return nil, fmt.Errorf("error fetching list of delta WAL certificates to clear: %s", err)
return nil, fmt.Errorf("error fetching list of delta WAL certificates to clear: %w", err)
}

// We _should_ remove the special WAL entries here, but we don't really
Expand All @@ -380,7 +379,7 @@ func (cb *crlBuilder) _clearDeltaWAL(sc *storageContext, walSerials []string, pa
}

if err := sc.Storage.Delete(sc.Context, path+serial); err != nil {
return fmt.Errorf("error clearing delta WAL certificate: %s", err)
return fmt.Errorf("error clearing delta WAL certificate: %w", err)
}
}

Expand Down Expand Up @@ -658,7 +657,7 @@ func (cb *crlBuilder) processRevocationQueue(sc *storageContext) error {
(!sc.Backend.System().LocalMount() && sc.Backend.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary))

if err := cb.maybeGatherQueueForFirstProcess(sc, isNotPerfPrimary); err != nil {
return fmt.Errorf("failed to gather first queue: %v", err)
return fmt.Errorf("failed to gather first queue: %w", err)
}

revQueue := cb.revQueue.Iterate()
Expand Down Expand Up @@ -973,13 +972,13 @@ func revokeCert(sc *storageContext, config *crlConfig, cert *x509.Certificate) (

revEntry, err := logical.StorageEntryJSON(revokedPath+hyphenSerial, revInfo)
if err != nil {
return nil, fmt.Errorf("error creating revocation entry")
return nil, fmt.Errorf("error creating revocation entry: %w", err)
}

certsCounted := sc.Backend.certsCounted.Load()
err = sc.Storage.Put(sc.Context, revEntry)
if err != nil {
return nil, fmt.Errorf("error saving revoked certificate to new location")
return nil, fmt.Errorf("error saving revoked certificate to new location: %w", err)
}
sc.Backend.ifCountEnabledIncrementTotalRevokedCertificatesCount(certsCounted, revEntry.Key)

Expand Down Expand Up @@ -1085,7 +1084,7 @@ func writeSpecificRevocationDeltaWALs(sc *storageContext, hyphenSerial string, c
}

if err = sc.Storage.Put(sc.Context, walEntry); err != nil {
return fmt.Errorf("error saving delta CRL WAL entry")
return fmt.Errorf("error saving delta CRL WAL entry: %w", err)
}

// In order for periodic delta rebuild to be mildly efficient, we
Expand All @@ -1097,7 +1096,7 @@ func writeSpecificRevocationDeltaWALs(sc *storageContext, hyphenSerial string, c
return fmt.Errorf("unable to create last delta CRL WAL entry")
}
if err = sc.Storage.Put(sc.Context, lastWALEntry); err != nil {
return fmt.Errorf("error saving last delta CRL WAL entry")
return fmt.Errorf("error saving last delta CRL WAL entry: %w", err)
}

return nil
Expand Down Expand Up @@ -1844,12 +1843,12 @@ func getLocalRevokedCertEntries(sc *storageContext, issuerIDCertMap map[issuerID
// we should update the entry to make future CRL builds faster.
revokedEntry, err = logical.StorageEntryJSON(revokedPath+serial, revInfo)
if err != nil {
return nil, nil, fmt.Errorf("error creating revocation entry for existing cert: %v", serial)
return nil, nil, fmt.Errorf("error creating revocation entry for existing cert: %v: %w", serial, err)
}

err = sc.Storage.Put(sc.Context, revokedEntry)
if err != nil {
return nil, nil, fmt.Errorf("error updating revoked certificate at existing location: %v", serial)
return nil, nil, fmt.Errorf("error updating revoked certificate at existing location: %v: %w", serial, err)
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions builtin/logical/pki/path_revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"strings"
"time"

"github.com/hashicorp/vault/sdk/helper/consts"

"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/certutil"
"github.com/hashicorp/vault/sdk/helper/errutil"
Expand Down Expand Up @@ -591,6 +593,13 @@ func (b *backend) pathRevokeWrite(ctx context.Context, req *logical.Request, dat
}
}

// Assumption: this check is cheap. Call this twice, in the cert-import
// case, to allow cert verification to get rejected on the standby node,
// but we still need it to protect the serial number case.
if b.System().ReplicationState().HasState(consts.ReplicationPerformanceStandby) {
return nil, logical.ErrReadOnly
}

b.revokeStorageLock.Lock()
defer b.revokeStorageLock.Unlock()

Expand Down
3 changes: 3 additions & 0 deletions changelog/19624.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
secrets/pki: Fix PKI revocation request forwarding from standby nodes due to an error wrapping bug
```