Skip to content

Commit 5aa2e93

Browse files
committed
fix(kek): guard FileWrapper Wrap/Unwrap against nil/zero value (PR719 codex P2)
Wrap and Unwrap previously dereferenced w.aead unconditionally, so a caller bypassing NewFileWrapper — var w kek.FileWrapper{} or a nil *FileWrapper — would nil-deref panic on the first call. A wiring mistake during bootstrap or rotation paths (where KEK loading is mandatory) crashed the process instead of bubbling up a recoverable error. Add an ErrNilFileWrapper sentinel and check for nil receiver / nil embedded AEAD at the top of both methods. Mirrors the encryption.Cipher (0256bad) and encryption.Keystore (d922b09) zero-value contracts.
1 parent 4230d7a commit 5aa2e93

2 files changed

Lines changed: 60 additions & 0 deletions

File tree

internal/encryption/kek/file.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ import (
1919
// (0o400 / 0o600) are accepted; anything with bits in 0o077 is not.
2020
var ErrInsecureKEKFile = errors.New("kek: file is group/world-accessible; require owner-only mode")
2121

22+
// ErrNilFileWrapper is returned by Wrap/Unwrap when called on a nil
23+
// receiver or a zero-value FileWrapper (i.e. one whose internal AEAD
24+
// was never initialised by NewFileWrapper). Surfaced as a typed error
25+
// rather than a nil-deref panic so a wiring mistake during bootstrap
26+
// or rotation surfaces as a recoverable failure instead of a process
27+
// crash. Mirrors the encryption.Cipher / encryption.Keystore
28+
// zero-value contract.
29+
var ErrNilFileWrapper = errors.New("kek: FileWrapper is nil or uninitialised; construct with NewFileWrapper")
30+
2231
const (
2332
// fileKEKSize is the on-disk KEK length: 32 bytes for AES-256.
2433
fileKEKSize = 32
@@ -39,6 +48,10 @@ const (
3948
// the KEK on a sealed tmpfs volume. Production deployments should
4049
// prefer a KMS-backed Wrapper (Stage 9: aws_kms.go, gcp_kms.go,
4150
// vault.go); see §5.1 for the recommended provider ordering.
51+
//
52+
// The zero value is NOT safe to use: Wrap/Unwrap return
53+
// ErrNilFileWrapper for a nil pointer or a FileWrapper whose internal
54+
// AEAD was never initialised. Always construct via NewFileWrapper.
4255
type FileWrapper struct {
4356
aead cipher.AEAD
4457
path string
@@ -130,7 +143,15 @@ func checkSecureKEKModeFD(f *os.File, path string) error {
130143
// [nonce 12 bytes] [ciphertext 32 bytes] [tag 16 bytes]
131144
//
132145
// Total wrapped size: 60 bytes for a 32-byte DEK.
146+
//
147+
// Returns ErrNilFileWrapper if w is nil or the embedded AEAD was
148+
// never initialised by NewFileWrapper. A wiring/configuration
149+
// mistake during bootstrap or rotation surfaces as a typed error
150+
// rather than a nil-deref panic.
133151
func (w *FileWrapper) Wrap(dek []byte) ([]byte, error) {
152+
if w == nil || w.aead == nil {
153+
return nil, errors.WithStack(ErrNilFileWrapper)
154+
}
134155
if len(dek) != fileKEKSize {
135156
return nil, errors.Errorf("kek: dek is %d bytes, want %d", len(dek), fileKEKSize)
136157
}
@@ -157,7 +178,13 @@ func (w *FileWrapper) Wrap(dek []byte) ([]byte, error) {
157178
// The post-Open length check that earlier drafts had was unreachable —
158179
// the strict-length input check above guarantees Open returns exactly
159180
// fileKEKSize bytes on success — and has been removed.
181+
//
182+
// Returns ErrNilFileWrapper for a nil receiver or zero-value
183+
// FileWrapper, symmetric with Wrap.
160184
func (w *FileWrapper) Unwrap(wrapped []byte) ([]byte, error) {
185+
if w == nil || w.aead == nil {
186+
return nil, errors.WithStack(ErrNilFileWrapper)
187+
}
161188
if len(wrapped) != fileNonceSize+fileKEKSize+fileTagSize {
162189
return nil, errors.Errorf("kek: wrapped DEK is %d bytes, want %d",
163190
len(wrapped), fileNonceSize+fileKEKSize+fileTagSize)

internal/encryption/kek/file_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,3 +307,36 @@ func TestFileWrapper_RejectsDirectory(t *testing.T) {
307307
t.Fatalf("expected regular-file error, got: %v", err)
308308
}
309309
}
310+
311+
// TestFileWrapper_NilOrZeroValueRejected covers the PR #719 codex P2
312+
// finding: a wiring/configuration mistake in a bootstrap or rotation
313+
// path could hand callers a nil *FileWrapper or a zero-value
314+
// FileWrapper{}, and Wrap/Unwrap dereference w.aead unconditionally.
315+
// They now return ErrNilFileWrapper symmetrically with the
316+
// encryption.Cipher / encryption.Keystore zero-value contract.
317+
func TestFileWrapper_NilOrZeroValueRejected(t *testing.T) {
318+
t.Parallel()
319+
dek := make([]byte, 32)
320+
wrapped := make([]byte, 60)
321+
cases := []struct {
322+
name string
323+
w *kek.FileWrapper
324+
}{
325+
{"nil receiver", (*kek.FileWrapper)(nil)},
326+
{"zero-value", &kek.FileWrapper{}},
327+
}
328+
for _, tc := range cases {
329+
t.Run(tc.name+"/Wrap", func(t *testing.T) {
330+
_, err := tc.w.Wrap(dek)
331+
if !errors.Is(err, kek.ErrNilFileWrapper) {
332+
t.Fatalf("expected ErrNilFileWrapper, got %v", err)
333+
}
334+
})
335+
t.Run(tc.name+"/Unwrap", func(t *testing.T) {
336+
_, err := tc.w.Unwrap(wrapped)
337+
if !errors.Is(err, kek.ErrNilFileWrapper) {
338+
t.Fatalf("expected ErrNilFileWrapper, got %v", err)
339+
}
340+
})
341+
}
342+
}

0 commit comments

Comments
 (0)