Skip to content

Commit dd87e8d

Browse files
committed
transit: warn when irreversible config flags cannot be disabled
keys/<name>/config documents that exportable and allow_plaintext_backup cannot be disabled once set, but a write that supplies false still returns success with no warning while leaving the flags enabled. An operator can believe they revoked export or plaintext backup when nothing changed. Emit warnings on ignored disable attempts, matching the existing config-write warning pattern for forced min-version adjustments.
1 parent eb70a82 commit dd87e8d

2 files changed

Lines changed: 62 additions & 4 deletions

File tree

builtin/logical/transit/path_keys_config.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
100100
}
101101
defer p.Unlock()
102102

103-
var warning string
103+
var warnings []string
104104

105105
originalMinDecryptionVersion := p.MinDecryptionVersion
106106
originalMinEncryptionVersion := p.MinEncryptionVersion
@@ -130,7 +130,7 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
130130

131131
if minDecryptionVersion == 0 {
132132
minDecryptionVersion = 1
133-
warning = "since Vault 0.3, transit key numbering starts at 1; forcing minimum to 1"
133+
warnings = append(warnings, "since Vault 0.3, transit key numbering starts at 1; forcing minimum to 1")
134134
}
135135

136136
if minDecryptionVersion != p.MinDecryptionVersion {
@@ -193,6 +193,8 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
193193
if exportable && !p.Exportable {
194194
p.Exportable = exportable
195195
persistNeeded = true
196+
} else if !exportable && p.Exportable {
197+
warnings = append(warnings, "exportable cannot be disabled once set; the supplied value was ignored")
196198
}
197199
}
198200

@@ -203,6 +205,8 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
203205
if allowPlaintextBackup && !p.AllowPlaintextBackup {
204206
p.AllowPlaintextBackup = allowPlaintextBackup
205207
persistNeeded = true
208+
} else if !allowPlaintextBackup && p.AllowPlaintextBackup {
209+
warnings = append(warnings, "allow_plaintext_backup cannot be disabled once set; the supplied value was ignored")
206210
}
207211
}
208212

@@ -233,7 +237,7 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
233237
return nil, err
234238
}
235239
resp.Data["key_usages"] = p.Type.KeyUsages()
236-
if warning != "" {
240+
for _, warning := range warnings {
237241
resp.AddWarning(warning)
238242
}
239243
return resp, nil
@@ -255,7 +259,7 @@ func (b *backend) pathKeysConfigWrite(ctx context.Context, req *logical.Request,
255259
return nil, err
256260
}
257261
resp.Data["key_usages"] = p.Type.KeyUsages()
258-
if warning != "" {
262+
for _, warning := range warnings {
259263
resp.AddWarning(warning)
260264
}
261265
return resp, nil

builtin/logical/transit/path_keys_config_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,3 +477,57 @@ func TestTransit_KeyUsagesInConfigResponse(t *testing.T) {
477477
})
478478
}
479479
}
480+
481+
482+
func TestTransit_ConfigWarnsOnIrreversibleFlagDisable(t *testing.T) {
483+
b, storage := createBackendWithSysView(t)
484+
485+
doReq := func(op logical.Operation, path string, data map[string]interface{}) *logical.Response {
486+
t.Helper()
487+
resp, err := b.HandleRequest(context.Background(), &logical.Request{
488+
Storage: storage,
489+
Operation: op,
490+
Path: path,
491+
Data: data,
492+
})
493+
if err != nil || (resp != nil && resp.IsError()) {
494+
t.Fatalf("unexpected error for %s %s: err=%v resp=%v", op, path, err, resp)
495+
}
496+
return resp
497+
}
498+
499+
hasWarning := func(resp *logical.Response, substr string) bool {
500+
t.Helper()
501+
for _, w := range resp.Warnings {
502+
if strings.Contains(w, substr) {
503+
return true
504+
}
505+
}
506+
return false
507+
}
508+
509+
// Create an exportable key with plaintext backup enabled.
510+
doReq(logical.UpdateOperation, "keys/irrev", map[string]interface{}{
511+
"exportable": true,
512+
"allow_plaintext_backup": true,
513+
})
514+
515+
// Attempt to disable both irreversible flags. Policy must stay enabled,
516+
// and the response must warn that the supplied values were ignored.
517+
resp := doReq(logical.UpdateOperation, "keys/irrev/config", map[string]interface{}{
518+
"exportable": false,
519+
"allow_plaintext_backup": false,
520+
})
521+
if resp.Data["exportable"] != true {
522+
t.Fatalf("exportable should remain true after disable attempt, got %#v", resp.Data["exportable"])
523+
}
524+
if resp.Data["allow_plaintext_backup"] != true {
525+
t.Fatalf("allow_plaintext_backup should remain true after disable attempt, got %#v", resp.Data["allow_plaintext_backup"])
526+
}
527+
if !hasWarning(resp, "exportable cannot be disabled once set") {
528+
t.Fatalf("expected exportable disable warning, got %#v", resp.Warnings)
529+
}
530+
if !hasWarning(resp, "allow_plaintext_backup cannot be disabled once set") {
531+
t.Fatalf("expected allow_plaintext_backup disable warning, got %#v", resp.Warnings)
532+
}
533+
}

0 commit comments

Comments
 (0)