Skip to content

Commit 004dfc4

Browse files
sgmillerstevendpclark
sgmiller
and
stevendpclark
authored
Add a max_crl_size parameter to CRL config (#28654)
* wip * Unit test the CRL limit, wire up config * Bigger error * API docs * wording * max_crl_entries, + ignore 0 or < -1 values to the config endpoint * changelog * rename field in docs * Update website/content/api-docs/secret/pki/index.mdx Co-authored-by: Steven Clark <steven.clark@hashicorp.com> * Update website/content/api-docs/secret/pki/index.mdx Co-authored-by: Steven Clark <steven.clark@hashicorp.com> --------- Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
1 parent 3b0614a commit 004dfc4

File tree

7 files changed

+135
-185
lines changed

7 files changed

+135
-185
lines changed

‎builtin/logical/pki/backend_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6103,6 +6103,7 @@ func TestPKI_EmptyCRLConfigUpgraded(t *testing.T) {
61036103
require.Equal(t, resp.Data["auto_rebuild_grace_period"], pki_backend.DefaultCrlConfig.AutoRebuildGracePeriod)
61046104
require.Equal(t, resp.Data["enable_delta"], pki_backend.DefaultCrlConfig.EnableDelta)
61056105
require.Equal(t, resp.Data["delta_rebuild_interval"], pki_backend.DefaultCrlConfig.DeltaRebuildInterval)
6106+
require.Equal(t, resp.Data["max_crl_entries"], pki_backend.DefaultCrlConfig.MaxCRLEntries)
61066107
}
61076108

61086109
func TestPKI_ListRevokedCerts(t *testing.T) {

‎builtin/logical/pki/crl_test.go‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ func crlEnableDisableTestForBackend(t *testing.T, b *backend, s logical.Storage,
285285
}
286286

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

326-
revoke := func(serialIndex int) {
326+
revoke := func(serialIndex int, errorText ...string) {
327327
_, err = CBWrite(b, s, "revoke", map[string]interface{}{
328328
"serial_number": serials[serialIndex],
329329
})
330-
if err != nil {
330+
if err != nil && len(errorText) == 1 {
331+
if strings.Contains(err.Error(), errorText[0]) {
332+
err = nil
333+
return
334+
}
331335
t.Fatal(err)
332336
}
333337

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

378382
crlCreationTime2 := getParsedCrlFromBackend(t, b, s, "crl").TBSCertList.ThisUpdate
379383
require.NotEqual(t, crlCreationTime1, crlCreationTime2)
384+
385+
// Set a limit, and test that it blocks building an over-large CRL
386+
CBWrite(b, s, "config/crl", map[string]interface{}{
387+
"max_crl_entries": 6,
388+
})
389+
revoke(6, "revocation list size (7) exceeds configured maximum (6)")
390+
test(6)
391+
392+
_, err = CBRead(b, s, "crl/rotate")
393+
require.Error(t, err)
394+
require.True(t, strings.Contains(err.Error(), "revocation list size (7) exceeds configured maximum (6)"))
395+
396+
// Set unlimited, and try again
397+
CBWrite(b, s, "config/crl", map[string]interface{}{
398+
"max_crl_entries": -1,
399+
})
400+
_, err = CBRead(b, s, "crl/rotate")
401+
require.NoError(t, err)
380402
}
381403

382404
func TestBackend_Secondary_CRL_Rebuilding(t *testing.T) {

‎builtin/logical/pki/crl_util.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1768,6 +1768,20 @@ func buildAnyCRLsWithCerts(
17681768
internalCRLConfig.LastModified = time.Now().UTC()
17691769
}
17701770

1771+
// Enforce the max CRL size guard before building the actual CRL
1772+
if globalCRLConfig.MaxCRLEntries > -1 {
1773+
limit := maxCRLEntriesOrDefault(globalCRLConfig.MaxCRLEntries)
1774+
revokedCount := len(revokedCerts)
1775+
if revokedCount > limit {
1776+
// Also log a nasty error to get the operator's attention
1777+
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)
1778+
return nil, fmt.Errorf("error building CRL: revocation list size (%d) exceeds configured maximum (%d)", revokedCount, limit)
1779+
}
1780+
if revokedCount > int(float32(limit)*0.90) {
1781+
sc.Logger().Warn("warning, revoked certificate count is within 10% of the configured maximum CRL size", "revoked_certs", revokedCount, "limit", limit)
1782+
}
1783+
}
1784+
17711785
// Lastly, build the CRL.
17721786
nextUpdate, err := buildCRL(sc, globalCRLConfig, forceNew, representative, revokedCerts, crlIdentifier, crlNumber, isUnified, isDelta, lastCompleteNumber)
17731787
if err != nil {

‎builtin/logical/pki/path_config_crl.go‎

Lines changed: 82 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -16,73 +16,79 @@ import (
1616
"github.com/hashicorp/vault/sdk/logical"
1717
)
1818

19-
func pathConfigCRL(b *backend) *framework.Path {
20-
return &framework.Path{
21-
Pattern: "config/crl",
22-
23-
DisplayAttrs: &framework.DisplayAttributes{
24-
OperationPrefix: operationPrefixPKI,
25-
},
26-
27-
Fields: map[string]*framework.FieldSchema{
28-
"expiry": {
29-
Type: framework.TypeString,
30-
Description: `The amount of time the generated CRL should be
19+
var configCRLFields = map[string]*framework.FieldSchema{
20+
"expiry": {
21+
Type: framework.TypeString,
22+
Description: `The amount of time the generated CRL should be
3123
valid; defaults to 72 hours`,
32-
Default: "72h",
33-
},
34-
"disable": {
35-
Type: framework.TypeBool,
36-
Description: `If set to true, disables generating the CRL entirely.`,
37-
},
38-
"ocsp_disable": {
39-
Type: framework.TypeBool,
40-
Description: `If set to true, ocsp unauthorized responses will be returned.`,
41-
},
42-
"ocsp_expiry": {
43-
Type: framework.TypeString,
44-
Description: `The amount of time an OCSP response will be valid (controls
24+
Default: "72h",
25+
},
26+
"disable": {
27+
Type: framework.TypeBool,
28+
Description: `If set to true, disables generating the CRL entirely.`,
29+
},
30+
"ocsp_disable": {
31+
Type: framework.TypeBool,
32+
Description: `If set to true, ocsp unauthorized responses will be returned.`,
33+
},
34+
"ocsp_expiry": {
35+
Type: framework.TypeString,
36+
Description: `The amount of time an OCSP response will be valid (controls
4537
the NextUpdate field); defaults to 12 hours`,
46-
Default: "1h",
47-
},
48-
"auto_rebuild": {
49-
Type: framework.TypeBool,
50-
Description: `If set to true, enables automatic rebuilding of the CRL`,
51-
},
52-
"auto_rebuild_grace_period": {
53-
Type: framework.TypeString,
54-
Description: `The time before the CRL expires to automatically rebuild it, when enabled. Must be shorter than the CRL expiry. Defaults to 12h.`,
55-
Default: "12h",
56-
},
57-
"enable_delta": {
58-
Type: framework.TypeBool,
59-
Description: `Whether to enable delta CRLs between authoritative CRL rebuilds`,
60-
},
61-
"delta_rebuild_interval": {
62-
Type: framework.TypeString,
63-
Description: `The time between delta CRL rebuilds if a new revocation has occurred. Must be shorter than the CRL expiry. Defaults to 15m.`,
64-
Default: "15m",
65-
},
66-
"cross_cluster_revocation": {
67-
Type: framework.TypeBool,
68-
Description: `Whether to enable a global, cross-cluster revocation queue.
38+
Default: "1h",
39+
},
40+
"auto_rebuild": {
41+
Type: framework.TypeBool,
42+
Description: `If set to true, enables automatic rebuilding of the CRL`,
43+
},
44+
"auto_rebuild_grace_period": {
45+
Type: framework.TypeString,
46+
Description: `The time before the CRL expires to automatically rebuild it, when enabled. Must be shorter than the CRL expiry. Defaults to 12h.`,
47+
Default: "12h",
48+
},
49+
"enable_delta": {
50+
Type: framework.TypeBool,
51+
Description: `Whether to enable delta CRLs between authoritative CRL rebuilds`,
52+
},
53+
"delta_rebuild_interval": {
54+
Type: framework.TypeString,
55+
Description: `The time between delta CRL rebuilds if a new revocation has occurred. Must be shorter than the CRL expiry. Defaults to 15m.`,
56+
Default: "15m",
57+
},
58+
"cross_cluster_revocation": {
59+
Type: framework.TypeBool,
60+
Description: `Whether to enable a global, cross-cluster revocation queue.
6961
Must be used with auto_rebuild=true.`,
70-
},
71-
"unified_crl": {
72-
Type: framework.TypeBool,
73-
Description: `If set to true enables global replication of revocation entries,
62+
},
63+
"unified_crl": {
64+
Type: framework.TypeBool,
65+
Description: `If set to true enables global replication of revocation entries,
7466
also enabling unified versions of OCSP and CRLs if their respective features are enabled.
7567
disable for CRLs and ocsp_disable for OCSP.`,
76-
Default: "false",
77-
},
78-
"unified_crl_on_existing_paths": {
79-
Type: framework.TypeBool,
80-
Description: `If set to true,
68+
Default: "false",
69+
},
70+
"unified_crl_on_existing_paths": {
71+
Type: framework.TypeBool,
72+
Description: `If set to true,
8173
existing CRL and OCSP paths will return the unified CRL instead of a response based on cluster-local data`,
82-
Default: "false",
83-
},
74+
Default: "false",
75+
},
76+
"max_crl_entries": {
77+
Type: framework.TypeInt,
78+
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.`,
79+
Default: pki_backend.DefaultCrlConfig.MaxCRLEntries,
80+
},
81+
}
82+
83+
func pathConfigCRL(b *backend) *framework.Path {
84+
return &framework.Path{
85+
Pattern: "config/crl",
86+
87+
DisplayAttrs: &framework.DisplayAttributes{
88+
OperationPrefix: operationPrefixPKI,
8489
},
8590

91+
Fields: configCRLFields,
8692
Operations: map[logical.Operation]framework.OperationHandler{
8793
logical.ReadOperation: &framework.PathOperation{
8894
DisplayAttrs: &framework.DisplayAttributes{
@@ -92,69 +98,7 @@ existing CRL and OCSP paths will return the unified CRL instead of a response ba
9298
Responses: map[int][]framework.Response{
9399
http.StatusOK: {{
94100
Description: "OK",
95-
Fields: map[string]*framework.FieldSchema{
96-
"expiry": {
97-
Type: framework.TypeString,
98-
Description: `The amount of time the generated CRL should be
99-
valid; defaults to 72 hours`,
100-
Required: true,
101-
},
102-
"disable": {
103-
Type: framework.TypeBool,
104-
Description: `If set to true, disables generating the CRL entirely.`,
105-
Required: true,
106-
},
107-
"ocsp_disable": {
108-
Type: framework.TypeBool,
109-
Description: `If set to true, ocsp unauthorized responses will be returned.`,
110-
Required: true,
111-
},
112-
"ocsp_expiry": {
113-
Type: framework.TypeString,
114-
Description: `The amount of time an OCSP response will be valid (controls
115-
the NextUpdate field); defaults to 12 hours`,
116-
Required: true,
117-
},
118-
"auto_rebuild": {
119-
Type: framework.TypeBool,
120-
Description: `If set to true, enables automatic rebuilding of the CRL`,
121-
Required: true,
122-
},
123-
"auto_rebuild_grace_period": {
124-
Type: framework.TypeString,
125-
Description: `The time before the CRL expires to automatically rebuild it, when enabled. Must be shorter than the CRL expiry. Defaults to 12h.`,
126-
Required: true,
127-
},
128-
"enable_delta": {
129-
Type: framework.TypeBool,
130-
Description: `Whether to enable delta CRLs between authoritative CRL rebuilds`,
131-
Required: true,
132-
},
133-
"delta_rebuild_interval": {
134-
Type: framework.TypeString,
135-
Description: `The time between delta CRL rebuilds if a new revocation has occurred. Must be shorter than the CRL expiry. Defaults to 15m.`,
136-
Required: true,
137-
},
138-
"cross_cluster_revocation": {
139-
Type: framework.TypeBool,
140-
Description: `Whether to enable a global, cross-cluster revocation queue.
141-
Must be used with auto_rebuild=true.`,
142-
Required: true,
143-
},
144-
"unified_crl": {
145-
Type: framework.TypeBool,
146-
Description: `If set to true enables global replication of revocation entries,
147-
also enabling unified versions of OCSP and CRLs if their respective features are enabled.
148-
disable for CRLs and ocsp_disable for OCSP.`,
149-
Required: true,
150-
},
151-
"unified_crl_on_existing_paths": {
152-
Type: framework.TypeBool,
153-
Description: `If set to true,
154-
existing CRL and OCSP paths will return the unified CRL instead of a response based on cluster-local data`,
155-
Required: true,
156-
},
157-
},
101+
Fields: configCRLFields,
158102
}},
159103
},
160104
},
@@ -167,65 +111,7 @@ existing CRL and OCSP paths will return the unified CRL instead of a response ba
167111
Responses: map[int][]framework.Response{
168112
http.StatusOK: {{
169113
Description: "OK",
170-
Fields: map[string]*framework.FieldSchema{
171-
"expiry": {
172-
Type: framework.TypeString,
173-
Description: `The amount of time the generated CRL should be
174-
valid; defaults to 72 hours`,
175-
Default: "72h",
176-
},
177-
"disable": {
178-
Type: framework.TypeBool,
179-
Description: `If set to true, disables generating the CRL entirely.`,
180-
},
181-
"ocsp_disable": {
182-
Type: framework.TypeBool,
183-
Description: `If set to true, ocsp unauthorized responses will be returned.`,
184-
},
185-
"ocsp_expiry": {
186-
Type: framework.TypeString,
187-
Description: `The amount of time an OCSP response will be valid (controls
188-
the NextUpdate field); defaults to 12 hours`,
189-
Default: "1h",
190-
},
191-
"auto_rebuild": {
192-
Type: framework.TypeBool,
193-
Description: `If set to true, enables automatic rebuilding of the CRL`,
194-
},
195-
"auto_rebuild_grace_period": {
196-
Type: framework.TypeString,
197-
Description: `The time before the CRL expires to automatically rebuild it, when enabled. Must be shorter than the CRL expiry. Defaults to 12h.`,
198-
Default: "12h",
199-
},
200-
"enable_delta": {
201-
Type: framework.TypeBool,
202-
Description: `Whether to enable delta CRLs between authoritative CRL rebuilds`,
203-
},
204-
"delta_rebuild_interval": {
205-
Type: framework.TypeString,
206-
Description: `The time between delta CRL rebuilds if a new revocation has occurred. Must be shorter than the CRL expiry. Defaults to 15m.`,
207-
Default: "15m",
208-
},
209-
"cross_cluster_revocation": {
210-
Type: framework.TypeBool,
211-
Description: `Whether to enable a global, cross-cluster revocation queue.
212-
Must be used with auto_rebuild=true.`,
213-
Required: false,
214-
},
215-
"unified_crl": {
216-
Type: framework.TypeBool,
217-
Description: `If set to true enables global replication of revocation entries,
218-
also enabling unified versions of OCSP and CRLs if their respective features are enabled.
219-
disable for CRLs and ocsp_disable for OCSP.`,
220-
Required: false,
221-
},
222-
"unified_crl_on_existing_paths": {
223-
Type: framework.TypeBool,
224-
Description: `If set to true,
225-
existing CRL and OCSP paths will return the unified CRL instead of a response based on cluster-local data`,
226-
Required: false,
227-
},
228-
},
114+
Fields: configCRLFields,
229115
}},
230116
},
231117
// Read more about why these flags are set in backend.go.
@@ -326,6 +212,13 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
326212
config.UnifiedCRLOnExistingPaths = unifiedCrlOnExistingPathsRaw.(bool)
327213
}
328214

215+
if maxCRLEntriesRaw, ok := d.GetOk("max_crl_entries"); ok {
216+
v := maxCRLEntriesRaw.(int)
217+
if v == -1 || v > 0 {
218+
config.MaxCRLEntries = v
219+
}
220+
}
221+
329222
if config.UnifiedCRLOnExistingPaths && !config.UnifiedCRL {
330223
return logical.ErrorResponse("unified_crl_on_existing_paths cannot be enabled if unified_crl is disabled"), nil
331224
}
@@ -408,6 +301,13 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
408301
return resp, nil
409302
}
410303

304+
func maxCRLEntriesOrDefault(size int) int {
305+
if size == 0 {
306+
return pki_backend.DefaultCrlConfig.MaxCRLEntries
307+
}
308+
return size
309+
}
310+
411311
func genResponseFromCrlConfig(config *pki_backend.CrlConfig) *logical.Response {
412312
return &logical.Response{
413313
Data: map[string]interface{}{
@@ -422,6 +322,7 @@ func genResponseFromCrlConfig(config *pki_backend.CrlConfig) *logical.Response {
422322
"cross_cluster_revocation": config.UseGlobalQueue,
423323
"unified_crl": config.UnifiedCRL,
424324
"unified_crl_on_existing_paths": config.UnifiedCRLOnExistingPaths,
325+
"max_crl_entries": maxCRLEntriesOrDefault(config.MaxCRLEntries),
425326
},
426327
}
427328
}

0 commit comments

Comments
 (0)