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

Support for generating Delta CRLs #16773

Merged
merged 6 commits into from
Aug 29, 2022
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
22 changes: 21 additions & 1 deletion builtin/logical/pki/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,16 @@ func Backend(conf *logical.BackendConfig) *backend {
"ca/pem",
"ca_chain",
"ca",
"crl/delta",
"crl/delta/pem",
"crl/pem",
"crl",
"issuer/+/crl/der",
"issuer/+/crl/pem",
"issuer/+/crl",
"issuer/+/crl/delta/der",
"issuer/+/crl/delta/pem",
"issuer/+/crl/delta",
"issuer/+/pem",
"issuer/+/der",
"issuer/+/json",
Expand All @@ -89,7 +94,8 @@ func Backend(conf *logical.BackendConfig) *backend {
},

LocalStorage: []string{
"revoked/",
revokedPath,
deltaWALPath,
legacyCRLPath,
"crls/",
"certs/",
Expand Down Expand Up @@ -398,6 +404,13 @@ func (b *backend) periodicFunc(ctx context.Context, request *logical.Request) er
return err
}

// As we're (below) modifying the backing storage, we need to ensure
// we're not on a standby/secondary node.
if b.System().ReplicationState().HasState(consts.ReplicationPerformanceStandby) ||
b.System().ReplicationState().HasState(consts.ReplicationDRSecondary) {
return nil
}

// Check if we're set to auto rebuild and a CRL is set to expire.
if err := b.crlBuilder.checkForAutoRebuild(sc); err != nil {
return err
Expand All @@ -408,6 +421,13 @@ func (b *backend) periodicFunc(ctx context.Context, request *logical.Request) er
return err
}

// If a delta CRL was rebuilt above as part of the complete CRL rebuild,
cipherboy marked this conversation as resolved.
Show resolved Hide resolved
// 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); err != nil {
return err
}

// All good!
return nil
}
10 changes: 9 additions & 1 deletion builtin/logical/pki/cert_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func fetchCertBySerial(ctx context.Context, b *backend, req *logical.Request, pr
case strings.HasPrefix(prefix, "revoked/"):
legacyPath = "revoked/" + colonSerial
path = "revoked/" + hyphenSerial
case serial == legacyCRLPath:
case serial == legacyCRLPath || serial == deltaCRLPath:
if err = b.crlBuilder.rebuildIfForced(ctx, b, req); err != nil {
return nil, err
}
Expand All @@ -183,6 +183,14 @@ func fetchCertBySerial(ctx context.Context, b *backend, req *logical.Request, pr
if err != nil {
return nil, err
}

if serial == deltaCRLPath {
if sc.Backend.useLegacyBundleCaStorage() {
return nil, fmt.Errorf("refusing to serve delta CRL with legacy CA bundle")
}

path += deltaCRLPathSuffix
}
default:
legacyPath = "certs/" + colonSerial
path = "certs/" + hyphenSerial
Expand Down
111 changes: 99 additions & 12 deletions builtin/logical/pki/crl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,14 @@ func crlEnableDisableTestForBackend(t *testing.T, b *backend, s logical.Storage,
for _, serialNum := range expectedSerials {
requireSerialNumberInCRL(t, certList, serialNum)
}

// Since this test assumes a complete CRL was rebuilt, we can grab
// the delta CRL and ensure it is empty.
deltaList := getParsedCrlFromBackend(t, b, s, "crl/delta").TBSCertList
lenDeltaList := len(deltaList.RevokedCertificates)
if lenDeltaList != 0 {
t.Fatalf("expected zero revoked certificates on the delta CRL due to complete CRL rebuild, found %d", lenDeltaList)
}
}

revoke := func(serialIndex int) {
Expand Down Expand Up @@ -378,7 +386,7 @@ func TestCrlRebuilder(t *testing.T) {
require.NoError(t, err)

req := &logical.Request{Storage: s}
cb := crlBuilder{}
cb := newCRLBuilder()

// Force an initial build
err = cb.rebuild(ctx, b, req, true)
Expand Down Expand Up @@ -848,9 +856,14 @@ func TestAutoRebuild(t *testing.T) {
// the rollback manager timer ticks. With the new helper, we can
// modify the rollback manager timer period directly, allowing us
// to shorten the total test time significantly.
newPeriod := 6 * time.Second
gracePeriod := (newPeriod + 1*time.Second).String()
crlTime := (newPeriod + 2*time.Second).String()
//
// We set the delta CRL time to ensure it executes prior to the
// main CRL rebuild, and the new CRL doesn't rebuild until after
// we're done.
newPeriod := 1 * time.Second
deltaPeriod := (newPeriod + 1*time.Second).String()
crlTime := (6*newPeriod + 2*time.Second).String()
gracePeriod := (3 * newPeriod).String()
delta := 2 * newPeriod

// This test requires the periodicFunc to trigger, which requires we stand
Expand Down Expand Up @@ -913,9 +926,10 @@ func TestAutoRebuild(t *testing.T) {
require.Equal(t, resp.Data["ocsp_disable"], defaultCrlConfig.OcspDisable)
require.Equal(t, resp.Data["auto_rebuild"], defaultCrlConfig.AutoRebuild)
require.Equal(t, resp.Data["auto_rebuild_grace_period"], defaultCrlConfig.AutoRebuildGracePeriod)
require.Equal(t, resp.Data["enable_delta"], defaultCrlConfig.EnableDelta)
require.Equal(t, resp.Data["delta_rebuild_interval"], defaultCrlConfig.DeltaRebuildInterval)

// Safety guard: we play with rebuild timing below. We don't expect
// this entire test to take longer than 80s.
// Safety guard: we play with rebuild timing below.
_, err = client.Logical().Write("pki/config/crl", map[string]interface{}{
"expiry": crlTime,
})
Expand Down Expand Up @@ -946,6 +960,8 @@ func TestAutoRebuild(t *testing.T) {
"expiry": crlTime,
"auto_rebuild": true,
"auto_rebuild_grace_period": gracePeriod,
"enable_delta": true,
"delta_rebuild_interval": deltaPeriod,
})
require.NoError(t, err)

Expand Down Expand Up @@ -976,7 +992,7 @@ func TestAutoRebuild(t *testing.T) {
require.NotEmpty(t, resp.Data["uuid"])
pkiMount := resp.Data["uuid"].(string)
require.NotEmpty(t, pkiMount)
revEntryPath := "logical/" + pkiMount + "/" + revokedPath + strings.ReplaceAll(newLeafSerial, ":", "-")
revEntryPath := "logical/" + pkiMount + "/" + revokedPath + normalizeSerial(newLeafSerial)

// storage from cluster.Core[0] is a physical storage copy, not a logical
// storage. This difference means, if we were to do a storage.Get(...)
Expand All @@ -989,17 +1005,15 @@ func TestAutoRebuild(t *testing.T) {
require.NotNil(t, resp.Data)
require.NotEmpty(t, resp.Data["value"])
revEntryValue := resp.Data["value"].(string)
fmt.Println(resp)
var revInfo revocationInfo
err = json.Unmarshal([]byte(revEntryValue), &revInfo)
require.NoError(t, err)
require.Equal(t, revInfo.CertificateIssuer, issuerID(rootIssuer))

// Serial should not appear on CRL.
// New serial should not appear on CRL.
crl = getCrlCertificateList(t, client, "pki")
thisCRLNumber := crl.Version
requireSerialNumberInCRL(t, crl, leafSerial)

requireSerialNumberInCRL(t, crl, leafSerial) // But the old one should.
now := time.Now()
graceInterval, _ := time.ParseDuration(gracePeriod)
expectedUpdate := lastCRLExpiry.Add(-1 * graceInterval)
Expand All @@ -1018,6 +1032,79 @@ func TestAutoRebuild(t *testing.T) {
t.Fatalf("shouldn't be here")
}

// This serial should exist in the delta WAL section for the mount...
resp, err = client.Logical().List("sys/raw/logical/" + pkiMount + "/" + deltaWALPath)
require.NoError(t, err)
require.NotNil(t, resp)
require.NotEmpty(t, resp.Data)
require.NotEmpty(t, resp.Data["keys"])
require.Contains(t, resp.Data["keys"], normalizeSerial(newLeafSerial))

haveUpdatedDeltaCRL := false
interruptChan := time.After(4*newPeriod + delta)
for {
if haveUpdatedDeltaCRL {
break
}

select {
case <-interruptChan:
t.Fatalf("expected to regenerate delta CRL within a couple of periodicFunc invocations (plus %v grace period)", delta)
default:
// Check and see if there's a storage entry for the last rebuild
// serial. If so, validate the delta CRL contains this entry.
resp, err = client.Logical().List("sys/raw/logical/" + pkiMount + "/" + deltaWALPath)
require.NoError(t, err)
require.NotNil(t, resp)
require.NotEmpty(t, resp.Data)
require.NotEmpty(t, resp.Data["keys"])

haveRebuildMarker := false
for _, rawEntry := range resp.Data["keys"].([]interface{}) {
entry := rawEntry.(string)
if entry == deltaWALLastRevokedSerialName {
haveRebuildMarker = true
break
}
}

if !haveRebuildMarker {
time.Sleep(1 * time.Second)
continue
}

// Read the marker and see if its correct.
resp, err = client.Logical().Read("sys/raw/logical/" + pkiMount + "/" + deltaWALLastBuildSerial)
require.NoError(t, err)
if resp == nil {
time.Sleep(1 * time.Second)
continue
}

require.NotNil(t, resp)
require.NotEmpty(t, resp.Data)
require.NotEmpty(t, resp.Data["value"])

// Easy than JSON decoding...
if !strings.Contains(resp.Data["value"].(string), newLeafSerial) {
time.Sleep(1 * time.Second)
continue
}

haveUpdatedDeltaCRL = true

// Ensure it has what we want.
deltaCrl := getParsedCrlAtPath(t, client, "/v1/pki/crl/delta").TBSCertList
if !requireSerialNumberInCRL(nil, deltaCrl, newLeafSerial) {
// Check if it is on the main CRL because its already regenerated.
mainCRL := getParsedCrlAtPath(t, client, "/v1/pki/crl").TBSCertList
requireSerialNumberInCRL(t, mainCRL, newLeafSerial)
}

break
}
}

// Now, wait until we're within the grace period... Then start prompting
// for regeneration.
if expectedUpdate.After(now) {
Expand All @@ -1026,7 +1113,7 @@ func TestAutoRebuild(t *testing.T) {

// Otherwise, the absolute latest we're willing to wait is some delta
// after CRL expiry (to let stuff regenerate &c).
interruptChan := time.After(lastCRLExpiry.Sub(now) + delta)
interruptChan = time.After(lastCRLExpiry.Sub(now) + delta)
for {
select {
case <-interruptChan:
Expand Down
Loading