Add warnings to crl rebuilds, allowing notifying operator of empty issuer equivalency sets (#20253)

* Add infrastructure for warnings on CRL rebuilds

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add warning on issuer missing KU for CRL Signing

When an entire issuer equivalency class is missing CRL signing usage
(but otherwise has key material present), we should add a warning so
operators can either correct this issuer or create an equivalent version
with KU specified.

Resolves: https://github.com/hashicorp/vault/issues/20137

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add tests for issuer warnings

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Fix return order of CRL builders

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2023-04-19 12:55:37 -04:00 committed by GitHub
parent dae5489787
commit 189a776307
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 314 additions and 86 deletions

View File

@ -600,16 +600,32 @@ func (b *backend) periodicFunc(ctx context.Context, request *logical.Request) er
}
// Then attempt to rebuild the CRLs if required.
if err := b.crlBuilder.rebuildIfForced(sc); err != nil {
warnings, err := b.crlBuilder.rebuildIfForced(sc)
if err != nil {
return err
}
if len(warnings) > 0 {
msg := "During rebuild of complete CRL, got the following warnings:"
for index, warning := range warnings {
msg = fmt.Sprintf("%v\n %d. %v", msg, index+1, warning)
}
b.Logger().Warn(msg)
}
// If a delta CRL was rebuilt above as part of the complete CRL rebuild,
// this will be a no-op. However, if we do need to rebuild delta CRLs,
// this would cause us to do so.
if err := b.crlBuilder.rebuildDeltaCRLsIfForced(sc, false); err != nil {
warnings, err = b.crlBuilder.rebuildDeltaCRLsIfForced(sc, false)
if err != nil {
return err
}
if len(warnings) > 0 {
msg := "During rebuild of delta CRL, got the following warnings:"
for index, warning := range warnings {
msg = fmt.Sprintf("%v\n %d. %v", msg, index+1, warning)
}
b.Logger().Warn(msg)
}
return nil
}

View File

@ -177,9 +177,17 @@ func fetchCertBySerial(sc *storageContext, prefix, serial string) (*logical.Stor
legacyPath = "revoked/" + colonSerial
path = "revoked/" + hyphenSerial
case serial == legacyCRLPath || serial == deltaCRLPath || serial == unifiedCRLPath || serial == unifiedDeltaCRLPath:
if err = sc.Backend.crlBuilder.rebuildIfForced(sc); err != nil {
warnings, err := sc.Backend.crlBuilder.rebuildIfForced(sc)
if err != nil {
return nil, err
}
if len(warnings) > 0 {
msg := "During rebuild of CRL for cert fetch, got the following warnings:"
for index, warning := range warnings {
msg = fmt.Sprintf("%v\n %d. %v", msg, index+1, warning)
}
sc.Backend.Logger().Warn(msg)
}
unified := serial == unifiedCRLPath || serial == unifiedDeltaCRLPath
path, err = sc.resolveIssuerCRLPath(defaultRef, unified)

View File

@ -415,15 +415,18 @@ func TestCrlRebuilder(t *testing.T) {
cb := newCRLBuilder(true /* can rebuild and write CRLs */)
// Force an initial build
err = cb.rebuild(sc, true)
warnings, err := cb.rebuild(sc, true)
require.NoError(t, err, "Failed to rebuild CRL")
require.Empty(t, warnings, "unexpectedly got warnings rebuilding CRL")
resp := requestCrlFromBackend(t, s, b)
crl1 := parseCrlPemBytes(t, resp.Data["http_raw_body"].([]byte))
// We shouldn't rebuild within this call.
err = cb.rebuildIfForced(sc)
warnings, err = cb.rebuildIfForced(sc)
require.NoError(t, err, "Failed to rebuild if forced CRL")
require.Empty(t, warnings, "unexpectedly got warnings rebuilding CRL")
resp = requestCrlFromBackend(t, s, b)
crl2 := parseCrlPemBytes(t, resp.Data["http_raw_body"].([]byte))
require.Equal(t, crl1.ThisUpdate, crl2.ThisUpdate, "According to the update field, we rebuilt the CRL")
@ -439,8 +442,9 @@ func TestCrlRebuilder(t *testing.T) {
// This should rebuild the CRL
cb.requestRebuildIfActiveNode(b)
err = cb.rebuildIfForced(sc)
warnings, err = cb.rebuildIfForced(sc)
require.NoError(t, err, "Failed to rebuild if forced CRL")
require.Empty(t, warnings, "unexpectedly got warnings rebuilding CRL")
resp = requestCrlFromBackend(t, s, b)
schema.ValidateResponse(t, schema.GetResponseSchema(t, b.Route("crl/pem"), logical.ReadOperation), resp, true)
@ -1325,3 +1329,106 @@ func requestCrlFromBackend(t *testing.T, s logical.Storage, b *backend) *logical
require.False(t, resp.IsError(), "crl error response: %v", resp)
return resp
}
func TestCRLWarningsEmptyKeyUsage(t *testing.T) {
t.Parallel()
b, s := CreateBackendWithStorage(t)
// Generated using OpenSSL with a configuration lacking KeyUsage on
// the CA certificate.
cert := `-----BEGIN CERTIFICATE-----
MIIDBjCCAe6gAwIBAgIBATANBgkqhkiG9w0BAQsFADATMREwDwYDVQQDDAhyb290
LW9sZDAeFw0yMDAxMDEwMTAxMDFaFw0yMTAxMDEwMTAxMDFaMBMxETAPBgNVBAMM
CHJvb3Qtb2xkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzqhSZxAL
PwFhCIPL1jFPq6jxp1wFgo6YNSfVI13gfaGIjfErxsQUbosmlEuTeOc50zXXN3kb
SDufy5Yi1OeSkFZRdJ78zdKzsEDIVR1ukUngVsSrt05gdNMJlh8XOPbcrJo78jYG
lRgtkkFSc/wCu+ue6JqkfKrbUY/G9WK0UM8ppHm1Ux67ZGoypyEgaqqxKHBRC4Yl
D+lAs1vP4C6cavqdUMKgAPTKmMBzlbpCuYPLHSzWh9Com3WQSqCbrlo3uH5RT3V9
5Gjuk3mMUhY1l6fRL7wG3f+4x+DS+ICQNT0o4lnMxpIsiTh0cEHUFgY7G0iHWYPj
CIN8UDhpZIpoCQIDAQABo2UwYzAdBgNVHQ4EFgQUJlHk3PN7pfC22FGxAb0rWgQt
L4cwHwYDVR0jBBgwFoAUJlHk3PN7pfC22FGxAb0rWgQtL4cwDAYDVR0TBAUwAwEB
/zATBgNVHSUEDDAKBggrBgEFBQcDATANBgkqhkiG9w0BAQsFAAOCAQEAcaU0FbXb
FfXluBrjKfOzVKz+kvQ1CVv3xe3MBkS6wvqybBjJCFChnqCPxEe57BdSbBXNU5LZ
zCR/OqYas4Csv9+msSn9BI2FSMAmfMDTsp5/6iIQJqlJx9L8a7bjzVMGX6QJm/3x
S/EgGsMETAgewQXeu4jhI6StgJ2V/4Ofe498hYw4LAiBapJmkU/nHezWodNBZJ7h
LcLOzVj0Hu5MZplGBgJFgRqBCVVkqXA0q7tORuhNzYtNdJFpv3pZIhvVFFu3HUPf
wYQPhLye5WNtosz5xKe8X0Q9qp8g6azMTk+5Qe7u1d8MYAA2AIlGuKUvPHRruOmN
NC+gQnS7AK1lCw==
-----END CERTIFICATE-----`
privKey := `-----BEGIN PRIVATE KEY-----
MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDOqFJnEAs/AWEI
g8vWMU+rqPGnXAWCjpg1J9UjXeB9oYiN8SvGxBRuiyaUS5N45znTNdc3eRtIO5/L
liLU55KQVlF0nvzN0rOwQMhVHW6RSeBWxKu3TmB00wmWHxc49tysmjvyNgaVGC2S
QVJz/AK7657omqR8qttRj8b1YrRQzymkebVTHrtkajKnISBqqrEocFELhiUP6UCz
W8/gLpxq+p1QwqAA9MqYwHOVukK5g8sdLNaH0KibdZBKoJuuWje4flFPdX3kaO6T
eYxSFjWXp9EvvAbd/7jH4NL4gJA1PSjiWczGkiyJOHRwQdQWBjsbSIdZg+MIg3xQ
OGlkimgJAgMBAAECggEABKmCdmXDwy+eR0ll41aoc/hzPzHRxADAiU51Pf+DrYHj
6UPcF3db+KR2Adl0ocEhqlSoHs3CIk6KC9c+wOvagBwaaVWe4WvT9vF3M4he8rMm
dv6n2xJPFcOfDz5zUSssjk5KdOvoGRv7BzYnDIvOafvmUVwPwuo92Wizddy8saf4
Xuea0Cupz1PELPKkbXcAqb+TzbAZrwdPj1Y7vTe/KGE4+aoDqCW/sFB1E0UsMGlt
/yfGwFP48b7kdkqSpcEQW5H8+WL3TfqRcolCD9To4vo2J+1Po0S/8qPNRvkNQDDX
AypHtrXFBOWHpJgXT4rKyH+ZGJchrCRDblt9s/sNQwKBgQD7NytvYET3pWemYiX+
MB9uc6cPuMFONvlzjA9T6dbOSi/HLaeDoW027aMUZqb7QeaQCoWcUwh13dI2SZq0
5+l9hei4JkWjoDhbWmPe7zDuQr3UMl0CSk3egz3BSHkjAhRAuUxK0QLKGB23zWxz
k8mUWYZaZRA39C6aqMt/jbJjDwKBgQDSl+eO+DjpwPzrjPSphpF4xYo4XDje9ovK
9q4KTHye7Flc3cMCX3WZBmzdt0zbqu6gWZjJH0XbWX/+SkJBGh77XWD0FeQnU7Vk
ipoeb8zTsCVxD9EytQuXti3cqBgClcCMvLKgLOJIcNYTnygojwg3t+jboQqbtV7p
VpQfAC6jZwKBgQCxJ46x1CnOmg4l/0DbqAQCV/yP0bI//fSbz0Ff459fimF3DHL9
GHF0MtC2Kk3HEgoNud3PB58Hv43mSrGWsZSuuCgM9LBXWz1i7rNPG05eNyK26W09
mDihmduK2hjS3zx5CDMM76gP7EHIxEyelLGqtBdS18JAMypKVo5rPPl3cQKBgQCG
ueXLImQOr4dfDntLpSqV0BLAQcekZKhEPZJURmCHr37wGXNzpixurJyjL2w9MFqf
PRKwwJAJZ3Wp8kn2qkZd23x2Szb+LeBjJQS6Kh4o44zgixTz0r1K3qLygptxs+pO
Xz4LmQte+skKHo0rfW3tb3vKXnmR6fOBZgE23//2SwKBgHck44hoE1Ex2gDEfIq1
04OBoS1cpuc9ge4uHEmv+8uANjzwlsYf8hY1qae513MGixRBOkxcI5xX/fYPQV9F
t3Jfh8QX85JjnGntuXuraYZJMUjpwXr3QHPx0jpvAM3Au5j6qD3biC9Vrwq9Chkg
hbiiPARizZA/Tsna/9ox1qDT
-----END PRIVATE KEY-----`
resp, err := CBWrite(b, s, "issuers/import/bundle", map[string]interface{}{
"pem_bundle": cert + "\n" + privKey,
})
require.NoError(t, err)
require.NotNil(t, resp)
require.NotEmpty(t, resp.Warnings)
originalWarnings := resp.Warnings
resp, err = CBRead(b, s, "crl/rotate")
require.NoError(t, err)
require.NotNil(t, resp)
require.NotEmpty(t, resp.Warnings)
// All CRL-specific warnings should've already occurred earlier on the
// import's CRL rebuild.
for _, warning := range resp.Warnings {
require.Contains(t, originalWarnings, warning)
}
// Deleting the issuer and key should remove the warning.
_, err = CBDelete(b, s, "root")
require.NoError(t, err)
resp, err = CBRead(b, s, "crl/rotate")
require.NoError(t, err)
require.NotNil(t, resp)
require.Empty(t, resp.Warnings)
// Adding back just the cert shouldn't cause CRL rebuild warnings.
resp, err = CBWrite(b, s, "issuers/import/bundle", map[string]interface{}{
"pem_bundle": cert,
})
require.NoError(t, err)
require.NotNil(t, resp)
require.NotNil(t, resp.Data)
require.NotNil(t, resp.Data["mapping"])
require.NotEmpty(t, resp.Data["mapping"])
require.Equal(t, len(resp.Data["mapping"].(map[string]string)), 1)
for key, value := range resp.Data["mapping"].(map[string]string) {
require.NotEmpty(t, key)
require.Empty(t, value)
}
resp, err = CBRead(b, s, "crl/rotate")
require.NoError(t, err)
require.NotNil(t, resp)
require.Empty(t, resp.Warnings)
}

View File

@ -301,16 +301,16 @@ func (cb *crlBuilder) flushCRLBuildTimeInvalidation(sc *storageContext) error {
// rebuildIfForced is to be called by readers or periodic functions that might need to trigger
// a refresh of the CRL before the read occurs.
func (cb *crlBuilder) rebuildIfForced(sc *storageContext) error {
func (cb *crlBuilder) rebuildIfForced(sc *storageContext) ([]string, error) {
if cb.forceRebuild.Load() {
return cb._doRebuild(sc, true, _enforceForceFlag)
}
return nil
return nil, nil
}
// rebuild is to be called by various write apis that know the CRL is to be updated and can be now.
func (cb *crlBuilder) rebuild(sc *storageContext, forceNew bool) error {
func (cb *crlBuilder) rebuild(sc *storageContext, forceNew bool) ([]string, error) {
return cb._doRebuild(sc, forceNew, _ignoreForceFlag)
}
@ -329,7 +329,7 @@ func (cb *crlBuilder) requestRebuildIfActiveNode(b *backend) {
cb.forceRebuild.Store(true)
}
func (cb *crlBuilder) _doRebuild(sc *storageContext, forceNew bool, ignoreForceFlag bool) error {
func (cb *crlBuilder) _doRebuild(sc *storageContext, forceNew bool, ignoreForceFlag bool) ([]string, error) {
cb._builder.Lock()
defer cb._builder.Unlock()
// Re-read the lock in case someone beat us to the punch between the previous load op.
@ -346,7 +346,7 @@ func (cb *crlBuilder) _doRebuild(sc *storageContext, forceNew bool, ignoreForceF
return buildCRLs(sc, myForceNew)
}
return nil
return nil, nil
}
func (cb *crlBuilder) _getPresentDeltaWALForClearing(sc *storageContext, path string) ([]string, error) {
@ -415,7 +415,7 @@ func (cb *crlBuilder) clearUnifiedDeltaWAL(sc *storageContext, walSerials []stri
return cb._clearDeltaWAL(sc, walSerials, unifiedDeltaWALPrefix)
}
func (cb *crlBuilder) rebuildDeltaCRLsIfForced(sc *storageContext, override bool) error {
func (cb *crlBuilder) rebuildDeltaCRLsIfForced(sc *storageContext, override bool) ([]string, error) {
// Delta CRLs use the same expiry duration as the complete CRL. Because
// we always rebuild the complete CRL and then the delta CRL, we can
// be assured that the delta CRL always expires after a complete CRL,
@ -427,18 +427,18 @@ func (cb *crlBuilder) rebuildDeltaCRLsIfForced(sc *storageContext, override bool
// within our time window for updating it.
cfg, err := cb.getConfigWithUpdate(sc)
if err != nil {
return err
return nil, err
}
if !cfg.EnableDelta {
// We explicitly do not update the last check time here, as we
// want to persist the last rebuild window if it hasn't been set.
return nil
return nil, nil
}
deltaRebuildDuration, err := time.ParseDuration(cfg.DeltaRebuildInterval)
if err != nil {
return err
return nil, err
}
// Acquire CRL building locks before we get too much further.
@ -454,7 +454,7 @@ func (cb *crlBuilder) rebuildDeltaCRLsIfForced(sc *storageContext, override bool
// If we're still before the time of our next rebuild check, we can
// safely return here even if we have certs. We'll wait for a bit,
// retrigger this check, and then do the rebuild.
return nil
return nil, nil
}
// Update our check time. If we bail out below (due to storage errors
@ -465,16 +465,16 @@ func (cb *crlBuilder) rebuildDeltaCRLsIfForced(sc *storageContext, override bool
rebuildLocal, err := cb._shouldRebuildLocalCRLs(sc, override)
if err != nil {
return err
return nil, fmt.Errorf("error determining if local CRLs should be rebuilt: %w", err)
}
rebuildUnified, err := cb._shouldRebuildUnifiedCRLs(sc, override)
if err != nil {
return err
return nil, fmt.Errorf("error determining if unified CRLs should be rebuilt: %w", err)
}
if !rebuildLocal && !rebuildUnified {
return nil
return nil, nil
}
// Finally, we must've needed to do the rebuild. Execute!
@ -601,14 +601,14 @@ func (cb *crlBuilder) _shouldRebuildUnifiedCRLs(sc *storageContext, override boo
return shouldRebuild, nil
}
func (cb *crlBuilder) rebuildDeltaCRLs(sc *storageContext, forceNew bool) error {
func (cb *crlBuilder) rebuildDeltaCRLs(sc *storageContext, forceNew bool) ([]string, error) {
cb._builder.Lock()
defer cb._builder.Unlock()
return cb.rebuildDeltaCRLsHoldingLock(sc, forceNew)
}
func (cb *crlBuilder) rebuildDeltaCRLsHoldingLock(sc *storageContext, forceNew bool) error {
func (cb *crlBuilder) rebuildDeltaCRLsHoldingLock(sc *storageContext, forceNew bool) ([]string, error) {
return buildAnyCRLs(sc, forceNew, true /* building delta */)
}
@ -1064,7 +1064,7 @@ func revokeCert(sc *storageContext, config *crlConfig, cert *x509.Certificate) (
// already rebuilt the full CRL so the Delta WAL will be cleared
// afterwards. Writing an entry only to immediately remove it
// isn't necessary.
crlErr := sc.Backend.crlBuilder.rebuild(sc, false)
warnings, crlErr := sc.Backend.crlBuilder.rebuild(sc, false)
if crlErr != nil {
switch crlErr.(type) {
case errutil.UserError:
@ -1073,6 +1073,9 @@ func revokeCert(sc *storageContext, config *crlConfig, cert *x509.Certificate) (
return nil, fmt.Errorf("error encountered during CRL building: %w", crlErr)
}
}
for index, warning := range warnings {
resp.AddWarning(fmt.Sprintf("Warning %d during CRL rebuild: %v", index+1, warning))
}
} else if config.EnableDelta {
if err := writeRevocationDeltaWALs(sc, config, resp, failedWritingUnifiedCRL, hyphenSerial, colonSerial); err != nil {
return nil, fmt.Errorf("failed to write WAL entries for Delta CRLs: %w", err)
@ -1165,11 +1168,11 @@ func writeSpecificRevocationDeltaWALs(sc *storageContext, hyphenSerial string, c
return nil
}
func buildCRLs(sc *storageContext, forceNew bool) error {
func buildCRLs(sc *storageContext, forceNew bool) ([]string, error) {
return buildAnyCRLs(sc, forceNew, false)
}
func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error {
func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) ([]string, error) {
// In order to build all CRLs, we need knowledge of all issuers. Any two
// issuers with the same keys _and_ subject should have the same CRL since
// they're functionally equivalent.
@ -1204,7 +1207,7 @@ func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error {
// buildCRL.
globalCRLConfig, err := sc.Backend.crlBuilder.getConfigWithUpdate(sc)
if err != nil {
return fmt.Errorf("error building CRL: while updating config: %w", err)
return nil, fmt.Errorf("error building CRL: while updating config: %w", err)
}
if globalCRLConfig.Disable && !forceNew {
@ -1216,13 +1219,13 @@ func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error {
// So, since tidy can now associate issuers on revocation entries, we
// can skip the rest of this function and exit early without updating
// anything.
return nil
return nil, nil
}
if !sc.Backend.useLegacyBundleCaStorage() {
issuers, err = sc.listIssuers()
if err != nil {
return fmt.Errorf("error building CRL: while listing issuers: %w", err)
return nil, fmt.Errorf("error building CRL: while listing issuers: %w", err)
}
} else {
// Here, we hard-code the legacy issuer entry instead of using the
@ -1236,13 +1239,13 @@ func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error {
// Users should upgrade symmetrically, rather than attempting
// backward compatibility for new features across disparate versions.
if isDelta {
return nil
return []string{"refusing to rebuild delta CRL with legacy bundle; finish migrating to newer issuer storage layout"}, nil
}
}
issuersConfig, err := sc.getIssuersConfig()
if err != nil {
return fmt.Errorf("error building CRLs: while getting the default config: %w", err)
return nil, fmt.Errorf("error building CRLs: while getting the default config: %w", err)
}
// We map issuerID->entry for fast lookup and also issuerID->Cert for
@ -1259,7 +1262,7 @@ func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error {
// legacy path is automatically ignored.
thisEntry, _, err := sc.fetchCertBundleByIssuerId(issuer, false)
if err != nil {
return fmt.Errorf("error building CRLs: unable to fetch specified issuer (%v): %w", issuer, err)
return nil, fmt.Errorf("error building CRLs: unable to fetch specified issuer (%v): %w", issuer, err)
}
if len(thisEntry.KeyID) == 0 {
@ -1284,7 +1287,7 @@ func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error {
thisCert, err := thisEntry.GetCertificate()
if err != nil {
return fmt.Errorf("error building CRLs: unable to parse issuer (%v)'s certificate: %w", issuer, err)
return nil, fmt.Errorf("error building CRLs: unable to parse issuer (%v)'s certificate: %w", issuer, err)
}
issuerIDCertMap[issuer] = thisCert
@ -1299,19 +1302,27 @@ func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error {
// Now we do two calls: building the cluster-local CRL, and potentially
// building the global CRL if we're on the active node of the performance
// primary.
currLocalDeltaSerials, err := buildAnyLocalCRLs(sc, issuersConfig, globalCRLConfig,
currLocalDeltaSerials, localWarnings, err := buildAnyLocalCRLs(sc, issuersConfig, globalCRLConfig,
issuers, issuerIDEntryMap,
issuerIDCertMap, keySubjectIssuersMap,
wasLegacy, forceNew, isDelta)
if err != nil {
return err
return nil, err
}
currUnifiedDeltaSerials, err := buildAnyUnifiedCRLs(sc, issuersConfig, globalCRLConfig,
currUnifiedDeltaSerials, unifiedWarnings, err := buildAnyUnifiedCRLs(sc, issuersConfig, globalCRLConfig,
issuers, issuerIDEntryMap,
issuerIDCertMap, keySubjectIssuersMap,
wasLegacy, forceNew, isDelta)
if err != nil {
return err
return nil, err
}
var warnings []string
for _, warning := range localWarnings {
warnings = append(warnings, fmt.Sprintf("warning from local CRL rebuild: %v", warning))
}
for _, warning := range unifiedWarnings {
warnings = append(warnings, fmt.Sprintf("warning from unified CRL rebuild: %v", warning))
}
// Finally, we decide if we need to rebuild the Delta CRLs again, for both
@ -1320,17 +1331,21 @@ func buildAnyCRLs(sc *storageContext, forceNew bool, isDelta bool) error {
// After we've confirmed the primary CRLs have built OK, go ahead and
// clear the delta CRL WAL and rebuild it.
if err := sc.Backend.crlBuilder.clearLocalDeltaWAL(sc, currLocalDeltaSerials); err != nil {
return fmt.Errorf("error building CRLs: unable to clear Delta WAL: %w", err)
return nil, fmt.Errorf("error building CRLs: unable to clear Delta WAL: %w", err)
}
if err := sc.Backend.crlBuilder.clearUnifiedDeltaWAL(sc, currUnifiedDeltaSerials); err != nil {
return fmt.Errorf("error building CRLs: unable to clear Delta WAL: %w", err)
return nil, fmt.Errorf("error building CRLs: unable to clear Delta WAL: %w", err)
}
if err := sc.Backend.crlBuilder.rebuildDeltaCRLsHoldingLock(sc, forceNew); err != nil {
return fmt.Errorf("error building CRLs: unable to rebuild empty Delta WAL: %w", err)
deltaWarnings, err := sc.Backend.crlBuilder.rebuildDeltaCRLsHoldingLock(sc, forceNew)
if err != nil {
return nil, fmt.Errorf("error building CRLs: unable to rebuild empty Delta WAL: %w", err)
}
for _, warning := range deltaWarnings {
warnings = append(warnings, fmt.Sprintf("warning from delta CRL rebuild: %v", warning))
}
}
return nil
return warnings, nil
}
func getLastWALSerial(sc *storageContext, path string) (string, error) {
@ -1363,8 +1378,9 @@ func buildAnyLocalCRLs(
wasLegacy bool,
forceNew bool,
isDelta bool,
) ([]string, error) {
) ([]string, []string, error) {
var err error
var warnings []string
// Before we load cert entries, we want to store the last seen delta WAL
// serial number. The subsequent List will have at LEAST that certificate
@ -1375,7 +1391,7 @@ func buildAnyLocalCRLs(
if isDelta {
lastDeltaSerial, err = getLastWALSerial(sc, localDeltaWALLastRevokedSerial)
if err != nil {
return nil, err
return nil, nil, err
}
}
@ -1386,7 +1402,7 @@ func buildAnyLocalCRLs(
if !isDelta {
currDeltaCerts, err = sc.Backend.crlBuilder.getPresentLocalDeltaWALForClearing(sc)
if err != nil {
return nil, fmt.Errorf("error building CRLs: unable to get present delta WAL entries for removal: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to get present delta WAL entries for removal: %w", err)
}
}
@ -1401,7 +1417,7 @@ func buildAnyLocalCRLs(
// a separate pool for those.
unassignedCerts, revokedCertsMap, err = getLocalRevokedCertEntries(sc, issuerIDCertMap, isDelta)
if err != nil {
return nil, fmt.Errorf("error building CRLs: unable to get revoked certificate entries: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to get revoked certificate entries: %w", err)
}
if !isDelta {
@ -1414,7 +1430,7 @@ func buildAnyLocalCRLs(
// duplicate this serial number on the delta, hence the above
// guard for isDelta.
if err := augmentWithRevokedIssuers(issuerIDEntryMap, issuerIDCertMap, revokedCertsMap); err != nil {
return nil, fmt.Errorf("error building CRLs: unable to parse revoked issuers: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to parse revoked issuers: %w", err)
}
}
}
@ -1423,21 +1439,25 @@ func buildAnyLocalCRLs(
// CRLs.
internalCRLConfig, err := sc.getLocalCRLConfig()
if err != nil {
return nil, fmt.Errorf("error building CRLs: unable to fetch cluster-local CRL configuration: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to fetch cluster-local CRL configuration: %w", err)
}
if err := buildAnyCRLsWithCerts(sc, issuersConfig, globalCRLConfig, internalCRLConfig,
rebuildWarnings, err := buildAnyCRLsWithCerts(sc, issuersConfig, globalCRLConfig, internalCRLConfig,
issuers, issuerIDEntryMap, keySubjectIssuersMap,
unassignedCerts, revokedCertsMap,
forceNew, false /* isUnified */, isDelta); err != nil {
return nil, fmt.Errorf("error building CRLs: %w", err)
forceNew, false /* isUnified */, isDelta)
if err != nil {
return nil, nil, fmt.Errorf("error building CRLs: %w", err)
}
if len(rebuildWarnings) > 0 {
warnings = append(warnings, rebuildWarnings...)
}
// Finally, persist our potentially updated local CRL config. Only do this
// if we didn't have a legacy CRL bundle.
if !wasLegacy {
if err := sc.setLocalCRLConfig(internalCRLConfig); err != nil {
return nil, fmt.Errorf("error building CRLs: unable to persist updated cluster-local CRL config: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to persist updated cluster-local CRL config: %w", err)
}
}
@ -1453,17 +1473,17 @@ func buildAnyLocalCRLs(
lastDeltaBuildEntry, err := logical.StorageEntryJSON(localDeltaWALLastBuildSerial, deltaInfo)
if err != nil {
return nil, fmt.Errorf("error creating last delta CRL rebuild serial entry: %w", err)
return nil, nil, fmt.Errorf("error creating last delta CRL rebuild serial entry: %w", err)
}
err = sc.Storage.Put(sc.Context, lastDeltaBuildEntry)
if err != nil {
return nil, fmt.Errorf("error persisting last delta CRL rebuild info: %w", err)
return nil, nil, fmt.Errorf("error persisting last delta CRL rebuild info: %w", err)
}
}
}
return currDeltaCerts, nil
return currDeltaCerts, warnings, nil
}
func buildAnyUnifiedCRLs(
@ -1477,19 +1497,20 @@ func buildAnyUnifiedCRLs(
wasLegacy bool,
forceNew bool,
isDelta bool,
) ([]string, error) {
) ([]string, []string, error) {
var err error
var warnings []string
// Unified CRL can only be built by the main cluster.
b := sc.Backend
if b.System().ReplicationState().HasState(consts.ReplicationDRSecondary|consts.ReplicationPerformanceStandby) ||
(!b.System().LocalMount() && b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) {
return nil, nil
return nil, nil, nil
}
// Unified CRL should only be built if enabled.
if !globalCRLConfig.UnifiedCRL && !forceNew {
return nil, nil
return nil, nil, nil
}
// Before we load cert entries, we want to store the last seen delta WAL
@ -1503,14 +1524,14 @@ func buildAnyUnifiedCRLs(
if isDelta {
clusters, err := sc.Storage.List(sc.Context, unifiedDeltaWALPrefix)
if err != nil {
return nil, fmt.Errorf("error listing clusters for unified delta WAL building: %w", err)
return nil, nil, fmt.Errorf("error listing clusters for unified delta WAL building: %w", err)
}
for index, cluster := range clusters {
path := unifiedDeltaWALPrefix + cluster + deltaWALLastRevokedSerialName
serial, err := getLastWALSerial(sc, path)
if err != nil {
return nil, fmt.Errorf("error getting last written Delta WAL serial for cluster (%v / %v): %w", index, cluster, err)
return nil, nil, fmt.Errorf("error getting last written Delta WAL serial for cluster (%v / %v): %w", index, cluster, err)
}
lastDeltaSerial[cluster] = serial
@ -1524,7 +1545,7 @@ func buildAnyUnifiedCRLs(
if !isDelta {
currDeltaCerts, err = sc.Backend.crlBuilder.getPresentUnifiedDeltaWALForClearing(sc)
if err != nil {
return nil, fmt.Errorf("error building CRLs: unable to get present delta WAL entries for removal: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to get present delta WAL entries for removal: %w", err)
}
}
@ -1539,7 +1560,7 @@ func buildAnyUnifiedCRLs(
// a separate pool for those.
unassignedCerts, revokedCertsMap, err = getUnifiedRevokedCertEntries(sc, issuerIDCertMap, isDelta)
if err != nil {
return nil, fmt.Errorf("error building CRLs: unable to get revoked certificate entries: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to get revoked certificate entries: %w", err)
}
if !isDelta {
@ -1552,7 +1573,7 @@ func buildAnyUnifiedCRLs(
// duplicate this serial number on the delta, hence the above
// guard for isDelta.
if err := augmentWithRevokedIssuers(issuerIDEntryMap, issuerIDCertMap, revokedCertsMap); err != nil {
return nil, fmt.Errorf("error building CRLs: unable to parse revoked issuers: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to parse revoked issuers: %w", err)
}
}
}
@ -1561,21 +1582,25 @@ func buildAnyUnifiedCRLs(
// CRLs.
internalCRLConfig, err := sc.getUnifiedCRLConfig()
if err != nil {
return nil, fmt.Errorf("error building CRLs: unable to fetch cluster-local CRL configuration: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to fetch cluster-local CRL configuration: %w", err)
}
if err := buildAnyCRLsWithCerts(sc, issuersConfig, globalCRLConfig, internalCRLConfig,
rebuildWarnings, err := buildAnyCRLsWithCerts(sc, issuersConfig, globalCRLConfig, internalCRLConfig,
issuers, issuerIDEntryMap, keySubjectIssuersMap,
unassignedCerts, revokedCertsMap,
forceNew, true /* isUnified */, isDelta); err != nil {
return nil, fmt.Errorf("error building CRLs: %w", err)
forceNew, true /* isUnified */, isDelta)
if err != nil {
return nil, nil, fmt.Errorf("error building CRLs: %w", err)
}
if len(rebuildWarnings) > 0 {
warnings = append(warnings, rebuildWarnings...)
}
// Finally, persist our potentially updated local CRL config. Only do this
// if we didn't have a legacy CRL bundle.
if !wasLegacy {
if err := sc.setUnifiedCRLConfig(internalCRLConfig); err != nil {
return nil, fmt.Errorf("error building CRLs: unable to persist updated cluster-local CRL config: %w", err)
return nil, nil, fmt.Errorf("error building CRLs: unable to persist updated cluster-local CRL config: %w", err)
}
}
@ -1599,17 +1624,17 @@ func buildAnyUnifiedCRLs(
deltaInfo := lastDeltaInfo{Serial: serial}
lastDeltaBuildEntry, err := logical.StorageEntryJSON(path, deltaInfo)
if err != nil {
return nil, fmt.Errorf("error creating last delta CRL rebuild serial entry: %w", err)
return nil, nil, fmt.Errorf("error creating last delta CRL rebuild serial entry: %w", err)
}
err = sc.Storage.Put(sc.Context, lastDeltaBuildEntry)
if err != nil {
return nil, fmt.Errorf("error persisting last delta CRL rebuild info: %w", err)
return nil, nil, fmt.Errorf("error persisting last delta CRL rebuild info: %w", err)
}
}
}
return currDeltaCerts, nil
return currDeltaCerts, warnings, nil
}
func buildAnyCRLsWithCerts(
@ -1625,9 +1650,10 @@ func buildAnyCRLsWithCerts(
forceNew bool,
isUnified bool,
isDelta bool,
) error {
) ([]string, error) {
// Now we can call buildCRL once, on an arbitrary/representative issuer
// from each of these (keyID, subject) sets.
var warnings []string
for _, subjectIssuersMap := range keySubjectIssuersMap {
for _, issuersSet := range subjectIssuersMap {
if len(issuersSet) == 0 {
@ -1675,7 +1701,7 @@ func buildAnyCRLsWithCerts(
// Finally, check our crlIdentifier.
if thisCRLId, ok := internalCRLConfig.IssuerIDCRLMap[issuerId]; ok && len(thisCRLId) > 0 {
if len(crlIdentifier) > 0 && crlIdentifier != thisCRLId {
return fmt.Errorf("error building CRLs: two issuers with same keys/subjects (%v vs %v) have different internal CRL IDs: %v vs %v", issuerId, crlIdIssuer, thisCRLId, crlIdentifier)
return nil, fmt.Errorf("error building CRLs: two issuers with same keys/subjects (%v vs %v) have different internal CRL IDs: %v vs %v", issuerId, crlIdIssuer, thisCRLId, crlIdentifier)
}
crlIdentifier = thisCRLId
@ -1687,6 +1713,24 @@ func buildAnyCRLsWithCerts(
// Skip this set for the time being; while we have valid
// issuers and associated keys, this occurred because we lack
// crl-signing usage on all issuers in this set.
//
// But, tell the user about this, so they can either correct
// this by reissuing the CA certificate or adding an equivalent
// version with KU bits if the CA cert lacks KU altogether.
//
// See also: https://github.com/hashicorp/vault/issues/20137
warning := "Issuer equivalency set with associated keys lacked an issuer with CRL Signing KeyUsage; refusing to rebuild CRL for this group of issuers: "
var issuers []string
for _, issuerId := range issuersSet {
issuers = append(issuers, issuerId.String())
}
warning += strings.Join(issuers, ",")
// We only need this warning once. :-)
if !isUnified && !isDelta {
warnings = append(warnings, warning)
}
continue
}
@ -1727,7 +1771,7 @@ func buildAnyCRLsWithCerts(
// Lastly, build the CRL.
nextUpdate, err := buildCRL(sc, globalCRLConfig, forceNew, representative, revokedCerts, crlIdentifier, crlNumber, isUnified, isDelta, lastCompleteNumber)
if err != nil {
return fmt.Errorf("error building CRLs: unable to build CRL for issuer (%v): %w", representative, err)
return nil, fmt.Errorf("error building CRLs: unable to build CRL for issuer (%v): %w", representative, err)
}
internalCRLConfig.CRLExpirationMap[crlIdentifier] = *nextUpdate
@ -1776,13 +1820,13 @@ func buildAnyCRLsWithCerts(
if !stillHaveIssuerForID {
if err := sc.Storage.Delete(sc.Context, "crls/"+crlId.String()); err != nil {
return fmt.Errorf("error building CRLs: unable to clean up deleted issuers' CRL: %w", err)
return nil, fmt.Errorf("error building CRLs: unable to clean up deleted issuers' CRL: %w", err)
}
}
}
// All good :-)
return nil
return warnings, nil
}
func isRevInfoIssuerValid(revInfo *revocationInfo, issuerIDCertMap map[issuerID]*x509.Certificate) bool {

View File

@ -421,6 +421,8 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
b.crlBuilder.markConfigDirty()
b.crlBuilder.reloadConfigIfRequired(sc)
resp := genResponseFromCrlConfig(config)
// Note this only affects/happens on the main cluster node, if you need to
// notify something based on a configuration change on all server types
// have a look at crlBuilder::reloadConfigIfRequired
@ -429,7 +431,7 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
// auto-rebuild and we aren't now or equivalently, we changed our
// mind about delta CRLs and need a new complete one or equivalently,
// we changed our mind about unified CRLs), rotate the CRLs.
crlErr := b.crlBuilder.rebuild(sc, true)
warnings, crlErr := b.crlBuilder.rebuild(sc, true)
if crlErr != nil {
switch crlErr.(type) {
case errutil.UserError:
@ -438,9 +440,12 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
return nil, fmt.Errorf("error encountered during CRL building: %w", crlErr)
}
}
for index, warning := range warnings {
resp.AddWarning(fmt.Sprintf("Warning %d during CRL rebuild: %v", index+1, warning))
}
}
return genResponseFromCrlConfig(config), nil
return resp, nil
}
func genResponseFromCrlConfig(config *crlConfig) *logical.Response {

View File

@ -1192,9 +1192,20 @@ func (b *backend) pathGetIssuerCRL(ctx context.Context, req *logical.Request, da
}
sc := b.makeStorageContext(ctx, req.Storage)
if err := b.crlBuilder.rebuildIfForced(sc); err != nil {
warnings, err := b.crlBuilder.rebuildIfForced(sc)
if err != nil {
return nil, err
}
if len(warnings) > 0 {
// Since this is a fetch of a specific CRL, this most likely comes
// from an automated system of some sort; these warnings would be
// ignored and likely meaningless. Log them instead.
msg := "During rebuild of CRL on issuer CRL fetch, got the following warnings:"
for index, warning := range warnings {
msg = fmt.Sprintf("%v\n %d. %v", msg, index+1, warning)
}
b.Logger().Warn(msg)
}
var certificate []byte
var contentType string

View File

@ -388,7 +388,7 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d
}
if len(createdIssuers) > 0 {
err := b.crlBuilder.rebuild(sc, true)
warnings, err := b.crlBuilder.rebuild(sc, true)
if err != nil {
// Before returning, check if the error message includes the
// string "PSS". If so, it indicates we might've wanted to modify
@ -403,6 +403,9 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d
return nil, err
}
for index, warning := range warnings {
response.AddWarning(fmt.Sprintf("Warning %d during CRL rebuild: %v", index+1, warning))
}
var issuersWithKeys []string
for _, issuer := range createdIssuers {
@ -709,7 +712,7 @@ func (b *backend) pathRevokeIssuer(ctx context.Context, req *logical.Request, da
}
// Rebuild the CRL to include the newly revoked issuer.
crlErr := b.crlBuilder.rebuild(sc, false)
warnings, crlErr := b.crlBuilder.rebuild(sc, false)
if crlErr != nil {
switch crlErr.(type) {
case errutil.UserError:
@ -725,6 +728,9 @@ func (b *backend) pathRevokeIssuer(ctx context.Context, req *logical.Request, da
// Impossible.
return nil, err
}
for index, warning := range warnings {
response.AddWarning(fmt.Sprintf("Warning %d during CRL rebuild: %v", index+1, warning))
}
// For sanity, we'll add a warning message here if there's no other
// issuer which verifies this issuer.

View File

@ -651,7 +651,7 @@ func (b *backend) pathRotateCRLRead(ctx context.Context, req *logical.Request, _
defer b.revokeStorageLock.RUnlock()
sc := b.makeStorageContext(ctx, req.Storage)
crlErr := b.crlBuilder.rebuild(sc, false)
warnings, crlErr := b.crlBuilder.rebuild(sc, false)
if crlErr != nil {
switch crlErr.(type) {
case errutil.UserError:
@ -661,11 +661,17 @@ func (b *backend) pathRotateCRLRead(ctx context.Context, req *logical.Request, _
}
}
return &logical.Response{
resp := &logical.Response{
Data: map[string]interface{}{
"success": true,
},
}, nil
}
for index, warning := range warnings {
resp.AddWarning(fmt.Sprintf("Warning %d during CRL rebuild: %v", index+1, warning))
}
return resp, nil
}
func (b *backend) pathRotateDeltaCRLRead(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) {
@ -678,7 +684,7 @@ func (b *backend) pathRotateDeltaCRLRead(ctx context.Context, req *logical.Reque
isEnabled := cfg.EnableDelta
crlErr := b.crlBuilder.rebuildDeltaCRLsIfForced(sc, true)
warnings, crlErr := b.crlBuilder.rebuildDeltaCRLsIfForced(sc, true)
if crlErr != nil {
switch crlErr.(type) {
case errutil.UserError:
@ -697,6 +703,9 @@ func (b *backend) pathRotateDeltaCRLRead(ctx context.Context, req *logical.Reque
if !isEnabled {
resp.AddWarning("requested rebuild of delta CRL when delta CRL is not enabled; this is a no-op")
}
for index, warning := range warnings {
resp.AddWarning(fmt.Sprintf("Warning %d during CRL rebuild: %v", index+1, warning))
}
return resp, nil
}

View File

@ -297,10 +297,13 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
b.ifCountEnabledIncrementTotalCertificatesCount(certsCounted, key)
// Build a fresh CRL
err = b.crlBuilder.rebuild(sc, true)
warnings, err = b.crlBuilder.rebuild(sc, true)
if err != nil {
return nil, err
}
for index, warning := range warnings {
resp.AddWarning(fmt.Sprintf("Warning %d during CRL rebuild: %v", index+1, warning))
}
if parsedBundle.Certificate.MaxPathLen == 0 {
resp.AddWarning("Max path length of the generated certificate is zero. This certificate cannot be used to issue intermediate CA certificates.")

View File

@ -977,9 +977,17 @@ func (b *backend) doTidyRevocationStore(ctx context.Context, req *logical.Reques
}
if !config.AutoRebuild {
if err := b.crlBuilder.rebuild(sc, false); err != nil {
warnings, err := b.crlBuilder.rebuild(sc, false)
if err != nil {
return err
}
if len(warnings) > 0 {
msg := "During rebuild of CRL for tidy, got the following warnings:"
for index, warning := range warnings {
msg = fmt.Sprintf("%v\n %d. %v", msg, index+1, warning)
}
b.Logger().Warn(msg)
}
}
}
@ -1081,9 +1089,17 @@ func (b *backend) doTidyExpiredIssuers(ctx context.Context, req *logical.Request
b.revokeStorageLock.Lock()
defer b.revokeStorageLock.Unlock()
if err := b.crlBuilder.rebuild(sc, false); err != nil {
warnings, err := b.crlBuilder.rebuild(sc, false)
if err != nil {
return err
}
if len(warnings) > 0 {
msg := "During rebuild of CRL for tidy, got the following warnings:"
for index, warning := range warnings {
msg = fmt.Sprintf("%v\n %d. %v", msg, index+1, warning)
}
b.Logger().Warn(msg)
}
}
return nil

3
changelog/20253.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
secrets/pki: Add warning when issuer lacks KeyUsage during CRL rebuilds; expose in logs and on rotation.
```