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

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

Merged
merged 5 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 18 additions & 2 deletions builtin/logical/pki/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,16 +590,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
}
Expand Down
10 changes: 9 additions & 1 deletion builtin/logical/pki/cert_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
113 changes: 110 additions & 3 deletions builtin/logical/pki/crl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)

Expand Down Expand Up @@ -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)
}
Loading