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.

Sign up for GitHub

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 a max_crl_size parameter to CRL config #28654

Merged
merged 12 commits into from
Oct 9, 2024
Merged

Add a max_crl_size parameter to CRL config #28654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c5369cb
wip
sgmiller Oct 8, 2024
4a97042
Unit test the CRL limit, wire up config
sgmiller Oct 9, 2024
3fd3d5d
Bigger error
sgmiller Oct 9, 2024
1eded62
API docs
sgmiller Oct 9, 2024
e96810f
wording
sgmiller Oct 9, 2024
637305e
max_crl_entries, + ignore 0 or < -1 values to the config endpoint
sgmiller Oct 9, 2024
10e0671
changelog
sgmiller Oct 9, 2024
b14405e
rename field in docs
sgmiller Oct 9, 2024
52eb7eb
Update website/content/api-docs/secret/pki/index.mdx
sgmiller Oct 9, 2024
bae2874
Update website/content/api-docs/secret/pki/index.mdx
sgmiller Oct 9, 2024
3587b6c
Merge branch 'sgm/vault-22664/max-crl-size' of github.com:/hashicorp/…
sgmiller Oct 9, 2024
ae2d8d1
Merge branch 'main' into sgm/vault-22664/max-crl-size
sgmiller Oct 9, 2024
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
1 change: 1 addition & 0 deletions builtin/logical/pki/backend_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6103,6 +6103,7 @@ func TestPKI_EmptyCRLConfigUpgraded(t *testing.T) {
require.Equal(t, resp.Data["auto_rebuild_grace_period"], pki_backend.DefaultCrlConfig.AutoRebuildGracePeriod)
require.Equal(t, resp.Data["enable_delta"], pki_backend.DefaultCrlConfig.EnableDelta)
require.Equal(t, resp.Data["delta_rebuild_interval"], pki_backend.DefaultCrlConfig.DeltaRebuildInterval)
require.Equal(t, resp.Data["max_crl_entries"], pki_backend.DefaultCrlConfig.MaxCRLEntries)
}

func TestPKI_ListRevokedCerts(t *testing.T) {
Expand Down
28 changes: 25 additions & 3 deletions builtin/logical/pki/crl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func crlEnableDisableTestForBackend(t *testing.T, b *backend, s logical.Storage,
}

serials := make(map[int]string)
for i := 0; i < 6; i++ {
for i := 0; i < 7; i++ {
resp, err := CBWrite(b, s, "issue/test", map[string]interface{}{
"common_name": "test.foobar.com",
})
Expand Down Expand Up @@ -323,11 +323,15 @@ func crlEnableDisableTestForBackend(t *testing.T, b *backend, s logical.Storage,
}
}

revoke := func(serialIndex int) {
revoke := func(serialIndex int, errorText ...string) {
_, err = CBWrite(b, s, "revoke", map[string]interface{}{
"serial_number": serials[serialIndex],
})
if err != nil {
if err != nil && len(errorText) == 1 {
if strings.Contains(err.Error(), errorText[0]) {
err = nil
return
}
t.Fatal(err)
}

Expand Down Expand Up @@ -377,6 +381,24 @@ func crlEnableDisableTestForBackend(t *testing.T, b *backend, s logical.Storage,

crlCreationTime2 := getParsedCrlFromBackend(t, b, s, "crl").TBSCertList.ThisUpdate
require.NotEqual(t, crlCreationTime1, crlCreationTime2)

// Set a limit, and test that it blocks building an over-large CRL
CBWrite(b, s, "config/crl", map[string]interface{}{
"max_crl_entries": 6,
})
revoke(6, "revocation list size (7) exceeds configured maximum (6)")
test(6)

_, err = CBRead(b, s, "crl/rotate")
require.Error(t, err)
require.True(t, strings.Contains(err.Error(), "revocation list size (7) exceeds configured maximum (6)"))

// Set unlimited, and try again
CBWrite(b, s, "config/crl", map[string]interface{}{
"max_crl_entries": -1,
})
_, err = CBRead(b, s, "crl/rotate")
require.NoError(t, err)
}

func TestBackend_Secondary_CRL_Rebuilding(t *testing.T) {
Expand Down
14 changes: 14 additions & 0 deletions builtin/logical/pki/crl_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1768,6 +1768,20 @@ func buildAnyCRLsWithCerts(
internalCRLConfig.LastModified = time.Now().UTC()
}

// Enforce the max CRL size guard before building the actual CRL
if globalCRLConfig.MaxCRLEntries > -1 {
limit := maxCRLEntriesOrDefault(globalCRLConfig.MaxCRLEntries)
revokedCount := len(revokedCerts)
if revokedCount > limit {
// Also log a nasty error to get the operator's attention
sc.Logger().Error("CRL was not updated, as it exceeds the configured max size. The CRL now does not contain all revoked certificates! This may be indicative of a runaway issuance/revocation pattern.", "limit", limit)
return nil, fmt.Errorf("error building CRL: revocation list size (%d) exceeds configured maximum (%d)", revokedCount, limit)
}
if revokedCount > int(float32(limit)*0.90) {
sc.Logger().Warn("warning, revoked certificate count is within 10% of the configured maximum CRL size", "revoked_certs", revokedCount, "limit", limit)
}
}

// Lastly, build the CRL.
nextUpdate, err := buildCRL(sc, globalCRLConfig, forceNew, representative, revokedCerts, crlIdentifier, crlNumber, isUnified, isDelta, lastCompleteNumber)
if err != nil {
Expand Down
263 changes: 82 additions & 181 deletions builtin/logical/pki/path_config_crl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,73 +16,79 @@ import (
"github.com/hashicorp/vault/sdk/logical"
)

func pathConfigCRL(b *backend) *framework.Path {
return &framework.Path{
Pattern: "config/crl",

DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: operationPrefixPKI,
},

Fields: map[string]*framework.FieldSchema{
"expiry": {
Type: framework.TypeString,
Description: `The amount of time the generated CRL should be
var configCRLFields = map[string]*framework.FieldSchema{
"expiry": {
Type: framework.TypeString,
Description: `The amount of time the generated CRL should be
valid; defaults to 72 hours`,
Default: "72h",
},
"disable": {
Type: framework.TypeBool,
Description: `If set to true, disables generating the CRL entirely.`,
},
"ocsp_disable": {
Type: framework.TypeBool,
Description: `If set to true, ocsp unauthorized responses will be returned.`,
},
"ocsp_expiry": {
Type: framework.TypeString,
Description: `The amount of time an OCSP response will be valid (controls
Default: "72h",
},
"disable": {
Type: framework.TypeBool,
Description: `If set to true, disables generating the CRL entirely.`,
},
"ocsp_disable": {
Type: framework.TypeBool,
Description: `If set to true, ocsp unauthorized responses will be returned.`,
},
"ocsp_expiry": {
Type: framework.TypeString,
Description: `The amount of time an OCSP response will be valid (controls
the NextUpdate field); defaults to 12 hours`,
Default: "1h",
},
"auto_rebuild": {
Type: framework.TypeBool,
Description: `If set to true, enables automatic rebuilding of the CRL`,
},
"auto_rebuild_grace_period": {
Type: framework.TypeString,
Description: `The time before the CRL expires to automatically rebuild it, when enabled. Must be shorter than the CRL expiry. Defaults to 12h.`,
Default: "12h",
},
"enable_delta": {
Type: framework.TypeBool,
Description: `Whether to enable delta CRLs between authoritative CRL rebuilds`,
},
"delta_rebuild_interval": {
Type: framework.TypeString,
Description: `The time between delta CRL rebuilds if a new revocation has occurred. Must be shorter than the CRL expiry. Defaults to 15m.`,
Default: "15m",
},
"cross_cluster_revocation": {
Type: framework.TypeBool,
Description: `Whether to enable a global, cross-cluster revocation queue.
Default: "1h",
},
"auto_rebuild": {
Type: framework.TypeBool,
Description: `If set to true, enables automatic rebuilding of the CRL`,
},
"auto_rebuild_grace_period": {
Type: framework.TypeString,
Description: `The time before the CRL expires to automatically rebuild it, when enabled. Must be shorter than the CRL expiry. Defaults to 12h.`,
Default: "12h",
},
"enable_delta": {
Type: framework.TypeBool,
Description: `Whether to enable delta CRLs between authoritative CRL rebuilds`,
},
"delta_rebuild_interval": {
Type: framework.TypeString,
Description: `The time between delta CRL rebuilds if a new revocation has occurred. Must be shorter than the CRL expiry. Defaults to 15m.`,
Default: "15m",
},
"cross_cluster_revocation": {
Type: framework.TypeBool,
Description: `Whether to enable a global, cross-cluster revocation queue.
Must be used with auto_rebuild=true.`,
},
"unified_crl": {
Type: framework.TypeBool,
Description: `If set to true enables global replication of revocation entries,
},
"unified_crl": {
Type: framework.TypeBool,
Description: `If set to true enables global replication of revocation entries,
also enabling unified versions of OCSP and CRLs if their respective features are enabled.
disable for CRLs and ocsp_disable for OCSP.`,
Default: "false",
},
"unified_crl_on_existing_paths": {
Type: framework.TypeBool,
Description: `If set to true,
Default: "false",
},
"unified_crl_on_existing_paths": {
Type: framework.TypeBool,
Description: `If set to true,
existing CRL and OCSP paths will return the unified CRL instead of a response based on cluster-local data`,
Default: "false",
},
Default: "false",
},
"max_crl_entries": {
Type: framework.TypeInt,
Description: `The maximum number of entries the CRL can contain. This is meant as a guard against accidental runaway revocations overloading Vault storage. If this limit is exceeded writing the CRL will fail. If set to -1 this limit is disabled.`,
Default: pki_backend.DefaultCrlConfig.MaxCRLEntries,
},
}

func pathConfigCRL(b *backend) *framework.Path {
return &framework.Path{
Pattern: "config/crl",

DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: operationPrefixPKI,
},

Fields: configCRLFields,
Operations: map[logical.Operation]framework.OperationHandler{
logical.ReadOperation: &framework.PathOperation{
DisplayAttrs: &framework.DisplayAttributes{
Expand All @@ -92,69 +98,7 @@ existing CRL and OCSP paths will return the unified CRL instead of a response ba
Responses: map[int][]framework.Response{
http.StatusOK: {{
Description: "OK",
Fields: map[string]*framework.FieldSchema{
"expiry": {
Type: framework.TypeString,
Description: `The amount of time the generated CRL should be
valid; defaults to 72 hours`,
Required: true,
},
"disable": {
Type: framework.TypeBool,
Description: `If set to true, disables generating the CRL entirely.`,
Required: true,
},
"ocsp_disable": {
Type: framework.TypeBool,
Description: `If set to true, ocsp unauthorized responses will be returned.`,
Required: true,
},
"ocsp_expiry": {
Type: framework.TypeString,
Description: `The amount of time an OCSP response will be valid (controls
the NextUpdate field); defaults to 12 hours`,
Required: true,
},
"auto_rebuild": {
Type: framework.TypeBool,
Description: `If set to true, enables automatic rebuilding of the CRL`,
Required: true,
},
"auto_rebuild_grace_period": {
Type: framework.TypeString,
Description: `The time before the CRL expires to automatically rebuild it, when enabled. Must be shorter than the CRL expiry. Defaults to 12h.`,
Required: true,
},
"enable_delta": {
Type: framework.TypeBool,
Description: `Whether to enable delta CRLs between authoritative CRL rebuilds`,
Required: true,
},
"delta_rebuild_interval": {
Type: framework.TypeString,
Description: `The time between delta CRL rebuilds if a new revocation has occurred. Must be shorter than the CRL expiry. Defaults to 15m.`,
Required: true,
},
"cross_cluster_revocation": {
Type: framework.TypeBool,
Description: `Whether to enable a global, cross-cluster revocation queue.
Must be used with auto_rebuild=true.`,
Required: true,
},
"unified_crl": {
Type: framework.TypeBool,
Description: `If set to true enables global replication of revocation entries,
also enabling unified versions of OCSP and CRLs if their respective features are enabled.
disable for CRLs and ocsp_disable for OCSP.`,
Required: true,
},
"unified_crl_on_existing_paths": {
Type: framework.TypeBool,
Description: `If set to true,
existing CRL and OCSP paths will return the unified CRL instead of a response based on cluster-local data`,
Required: true,
},
},
Fields: configCRLFields,
}},
},
},
Expand All @@ -167,65 +111,7 @@ existing CRL and OCSP paths will return the unified CRL instead of a response ba
Responses: map[int][]framework.Response{
http.StatusOK: {{
Description: "OK",
Fields: map[string]*framework.FieldSchema{
"expiry": {
Type: framework.TypeString,
Description: `The amount of time the generated CRL should be
valid; defaults to 72 hours`,
Default: "72h",
},
"disable": {
Type: framework.TypeBool,
Description: `If set to true, disables generating the CRL entirely.`,
},
"ocsp_disable": {
Type: framework.TypeBool,
Description: `If set to true, ocsp unauthorized responses will be returned.`,
},
"ocsp_expiry": {
Type: framework.TypeString,
Description: `The amount of time an OCSP response will be valid (controls
the NextUpdate field); defaults to 12 hours`,
Default: "1h",
},
"auto_rebuild": {
Type: framework.TypeBool,
Description: `If set to true, enables automatic rebuilding of the CRL`,
},
"auto_rebuild_grace_period": {
Type: framework.TypeString,
Description: `The time before the CRL expires to automatically rebuild it, when enabled. Must be shorter than the CRL expiry. Defaults to 12h.`,
Default: "12h",
},
"enable_delta": {
Type: framework.TypeBool,
Description: `Whether to enable delta CRLs between authoritative CRL rebuilds`,
},
"delta_rebuild_interval": {
Type: framework.TypeString,
Description: `The time between delta CRL rebuilds if a new revocation has occurred. Must be shorter than the CRL expiry. Defaults to 15m.`,
Default: "15m",
},
"cross_cluster_revocation": {
Type: framework.TypeBool,
Description: `Whether to enable a global, cross-cluster revocation queue.
Must be used with auto_rebuild=true.`,
Required: false,
},
"unified_crl": {
Type: framework.TypeBool,
Description: `If set to true enables global replication of revocation entries,
also enabling unified versions of OCSP and CRLs if their respective features are enabled.
disable for CRLs and ocsp_disable for OCSP.`,
Required: false,
},
"unified_crl_on_existing_paths": {
Type: framework.TypeBool,
Description: `If set to true,
existing CRL and OCSP paths will return the unified CRL instead of a response based on cluster-local data`,
Required: false,
},
},
Fields: configCRLFields,
}},
},
// Read more about why these flags are set in backend.go.
Expand Down Expand Up @@ -326,6 +212,13 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
config.UnifiedCRLOnExistingPaths = unifiedCrlOnExistingPathsRaw.(bool)
}

if maxCRLEntriesRaw, ok := d.GetOk("max_crl_entries"); ok {
v := maxCRLEntriesRaw.(int)
if v == -1 || v > 0 {
config.MaxCRLEntries = v
}
}

if config.UnifiedCRLOnExistingPaths && !config.UnifiedCRL {
return logical.ErrorResponse("unified_crl_on_existing_paths cannot be enabled if unified_crl is disabled"), nil
}
Expand Down Expand Up @@ -408,6 +301,13 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
return resp, nil
}

func maxCRLEntriesOrDefault(size int) int {
if size == 0 {
return pki_backend.DefaultCrlConfig.MaxCRLEntries
}
return size
}

func genResponseFromCrlConfig(config *pki_backend.CrlConfig) *logical.Response {
return &logical.Response{
Data: map[string]interface{}{
Expand All @@ -422,6 +322,7 @@ func genResponseFromCrlConfig(config *pki_backend.CrlConfig) *logical.Response {
"cross_cluster_revocation": config.UseGlobalQueue,
"unified_crl": config.UnifiedCRL,
"unified_crl_on_existing_paths": config.UnifiedCRLOnExistingPaths,
"max_crl_entries": maxCRLEntriesOrDefault(config.MaxCRLEntries),
},
}
}
Expand Down
Loading
Loading