amadan
Fix round 1: propagate purposeKey's HKDF read error instead of discarding it
purposeKey's io.ReadFull error was discarded, and aeadFor (used by the production Seal/Open path) called it without a way to see a failure. Unreachable today — a fixed 32-byte read is always inside HKDF-SHA256's 8160-byte expand limit — but this is the app's only encryption surface, and a silently discarded read here would degrade into a short or zero-padded key if the key length ever changed: ciphertext that looks fine while being trivially breakable. purposeKey now returns ([]byte, error); aeadFor and the golden-vector test both check it. Keeps the de-duplication (aeadFor still calls the shared purposeKey rather than deriving inline).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APtNStXDZ5H2fAc1ZuGqtS
2 files changed,
+22
−6
internal/sheets/seal/seal.go+17 −5internal/sheets/seal/vectors_test.go+5 −1
Intent
diff --git a/internal/sheets/seal/seal.go b/internal/sheets/seal/seal.go| index e59ecfa..0b15d04 100644 |
| --- a/internal/sheets/seal/seal.go |
| +++ b/internal/sheets/seal/seal.go |
| @@ -37,17 +37,29 @@ func New(dek []byte) (*Sealer, error) { |
| return &Sealer{dek: c}, nil |
| } |
| -// purposeKey derives the AEAD key for purpose via HKDF over the DEK. |
| -func (s *Sealer) purposeKey(purpose string) []byte { |
| +// purposeKey derives the AEAD key for purpose via HKDF over the DEK. A |
| +// fixed 32-byte read is always inside HKDF-SHA256's 8160-byte expand |
| +// limit, so this cannot fail today — but the error is still checked and |
| +// propagated: this is the app's only encryption surface, and a silently |
| +// discarded read here would degrade into a short or zero-padded key if |
| +// the key length ever changed, which is the kind of bug that leaves |
| +// ciphertext looking perfectly fine while being trivially breakable. |
| +func (s *Sealer) purposeKey(purpose string) ([]byte, error) { |
| key := make([]byte, 32) |
| r := hkdf.New(sha256New, s.dek, nil, []byte("oficina/sheets:"+purpose)) |
| - io.ReadFull(r, key) |
| - return key |
| + if _, err := io.ReadFull(r, key); err != nil { |
| + return nil, err |
| + } |
| + return key, nil |
| } |
| // aeadFor derives the purpose key and returns its AEAD. |
| func (s *Sealer) aeadFor(purpose string) (cipher.AEAD, error) { |
| - block, err := aes.NewCipher(s.purposeKey(purpose)) |
| + key, err := s.purposeKey(purpose) |
| + if err != nil { |
| + return nil, err |
| + } |
| + block, err := aes.NewCipher(key) |
| if err != nil { |
| return nil, err |
| } |
diff --git a/internal/sheets/seal/vectors_test.go b/internal/sheets/seal/vectors_test.go| index a81f695..956c114 100644 |
| --- a/internal/sheets/seal/vectors_test.go |
| +++ b/internal/sheets/seal/vectors_test.go |
| @@ -14,7 +14,11 @@ func TestPurposeKeyVectors(t *testing.T) { |
| {"cell.input", "cbc5cfc98184cea8e11127274e61a8973520c1d8cc81014678a7e9160ce68b63"}, |
| {"cell.cached", "6c94c298a000b2fd440767b542a77b97366e653d7412e4b1fab9b7545cee34b5"}, |
| } { |
| - got := hex.EncodeToString(s.purposeKey(tc.purpose)) |
| + key, err := s.purposeKey(tc.purpose) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + got := hex.EncodeToString(key) |
| if tc.want == "" { |
| t.Logf("RECORD %s = %s", tc.purpose, got) |
| continue |