Files
seaweedfs/weed/s3api/s3_sse_kms_utils.go
T
Chris Lu e1d5e3899f fix(s3): add HMAC-SHA256 key commitment to SSE-S3 and SSE-KMS (#8879)
* fix(s3): add HMAC-SHA256 key commitment to SSE encryption modes

* fix(s3): validate SSE-S3 IV length before cipher.NewCTR

CreateSSES3DecryptedReader took the IV directly from object metadata
and passed it to cipher.NewCTR. The standard library panics when the
IV length differs from the block size, so a tampered metadata field
turned what should be a decryption error into a crash. Validate via
the existing ValidateIV helper first.

Addresses coderabbit review on PR #8879.

* fix(s3): centralize SSE-KMS key commitment in createSSEKMSKey

KeyCommitment used to be set explicitly in only one of the four
encryption entry points; the multipart-aware variants and the bucket-
bound CreateSSEKMSEncryptedReaderForBucket left it empty, so writes
through those paths landed with metadata that VerifyKeyCommitment
treats as legacy and accepts unconditionally — exactly the downgrade
gap the gemini review flagged.

Move the HMAC computation into createSSEKMSKey so every helper-driven
path picks it up automatically, and add the same call to the bucket-
scoped path that builds its own struct literal.

Addresses gemini review on PR #8879.

* fix(s3): store base IV (not derived IV) for offset-aware SSE-KMS chunks

CreateSSEKMSEncryptedReaderWithBaseIVAndOffset used to store the
already-offset-derived IV plus the chunk offset in metadata. The
decrypt path then applied calculateIVWithOffset a second time to
that stored IV, producing the wrong CTR keystream and a decryption
mismatch. Centralizing the key commitment made the bug visible as a
commitment failure, but the underlying issue predates that change.

Pass the base IV through to createSSEKMSKey so sseKey.IV is the
unmodified base on disk; the decrypt path's offset application then
recovers the correct chunk-level IV. The HMAC commitment binds the
base IV — same value the verify call at decrypt time hashes — so the
new commitment path stays consistent.

Addresses gemini security-high review on PR #8879.

* fix(s3): opt-in strict-commitment mode to close the downgrade vector

WEED_S3_REQUIRE_KEY_COMMITMENT=true flips VerifyKeyCommitment from
accept-when-missing (the AWS-compatible default needed for objects
written before commitments shipped) to reject-when-missing. With
the env var set, an attacker who strips the commitment field from
object metadata can no longer bypass integrity verification — every
object must carry a commitment that hashes to the right value.

Default stays false so existing legacy objects keep decrypting; the
warning the gemini review raised about the silent-downgrade vector
is closed for operators who explicitly opt in once their bucket is
fully migrated. SetRequireKeyCommitment exposes a runtime seam for
tests and future config-reload paths.

Addresses the security-medium review on PR #8879.

* test(s3): fix mislabelled IV literal in strict-commitment test

The "strict-mode-iv-16" literal was actually 17 bytes — the trailing
"16" was meant as a comment but counted as content. The IV is fed
into HMAC, not AES, so the length didn't matter for behaviour, but
the discrepancy was confusing. Tighten to a real 16-byte literal and
explain the choice in a comment.

Addresses coderabbit minor review on PR #8879.

* fix(s3): store KMS-resolved KeyID in SSE-KMS metadata, not the request

CreateSSEKMSEncryptedReaderForBucket built its SSEKMSKey with the
caller-supplied keyID, but CreateSSEKMSDecryptedReader later compares
decryptResp.KeyID against sseKey.KeyID. A request that used an alias
would resolve to a different ARN in the response; storing the request
form would then trip the mismatch check at decrypt time and surface
as a "KMS key ID mismatch" error against the operator's own object.
The helper-driven encryption paths already do the right thing via
createSSEKMSKey; this is the bucket-bound path catching up.

Addresses coderabbit review on PR #8879.

* test(s3): cover key commitment rejection paths under tampering

Adds the negative-path tests coderabbit flagged as missing: a
tampered key, IV, algorithm, or commitment field must fail
VerifyKeyCommitment, otherwise a regression in the rejection logic
could land silently. The HMAC binds all three inputs plus the
commitment itself, so any single mutation is enough.

Addresses coderabbit nitpick on PR #8879.
2026-05-04 19:14:41 -07:00

106 lines
3.9 KiB
Go

package s3api
import (
"context"
"crypto/aes"
"crypto/cipher"
"fmt"
"strings"
"github.com/seaweedfs/seaweedfs/weed/kms"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
// KMSDataKeyResult holds the result of data key generation
type KMSDataKeyResult struct {
Response *kms.GenerateDataKeyResponse
Block cipher.Block
}
// generateKMSDataKey generates a new data encryption key using KMS
// This function encapsulates the common pattern used across all SSE-KMS functions
func generateKMSDataKey(keyID string, encryptionContext map[string]string) (*KMSDataKeyResult, error) {
// Validate keyID to prevent injection attacks and malformed requests to KMS service
if !isValidKMSKeyID(keyID) {
return nil, fmt.Errorf("invalid KMS key ID format: key ID must be non-empty, without spaces or control characters")
}
// Validate encryption context to prevent malformed requests to KMS service
if encryptionContext != nil {
for key, value := range encryptionContext {
// Validate context keys and values for basic security
if strings.TrimSpace(key) == "" {
return nil, fmt.Errorf("invalid encryption context: keys cannot be empty or whitespace-only")
}
if strings.ContainsAny(key, "\x00\n\r\t") || strings.ContainsAny(value, "\x00\n\r\t") {
return nil, fmt.Errorf("invalid encryption context: keys and values cannot contain control characters")
}
// AWS KMS has limits on key/value lengths
if len(key) > 2048 || len(value) > 2048 {
return nil, fmt.Errorf("invalid encryption context: keys and values must be ≤ 2048 characters (key=%d, value=%d)", len(key), len(value))
}
}
// AWS KMS has a limit on the total number of context pairs
if len(encryptionContext) > s3_constants.MaxKMSEncryptionContextPairs {
return nil, fmt.Errorf("invalid encryption context: cannot exceed %d key-value pairs, got %d", s3_constants.MaxKMSEncryptionContextPairs, len(encryptionContext))
}
}
// Get KMS provider
kmsProvider := kms.GetGlobalKMS()
if kmsProvider == nil {
return nil, fmt.Errorf("KMS is not configured")
}
// Create data key request
generateDataKeyReq := &kms.GenerateDataKeyRequest{
KeyID: keyID,
KeySpec: kms.KeySpecAES256,
EncryptionContext: encryptionContext,
}
// Generate the data key
dataKeyResp, err := kmsProvider.GenerateDataKey(context.Background(), generateDataKeyReq)
if err != nil {
return nil, fmt.Errorf("failed to generate KMS data key: %v", err)
}
// Create AES cipher with the plaintext data key
block, err := aes.NewCipher(dataKeyResp.Plaintext)
if err != nil {
// Clear sensitive data before returning error
kms.ClearSensitiveData(dataKeyResp.Plaintext)
return nil, fmt.Errorf("failed to create AES cipher: %v", err)
}
return &KMSDataKeyResult{
Response: dataKeyResp,
Block: block,
}, nil
}
// clearKMSDataKey safely clears sensitive data from a KMSDataKeyResult
func clearKMSDataKey(result *KMSDataKeyResult) {
if result != nil && result.Response != nil {
kms.ClearSensitiveData(result.Response.Plaintext)
}
}
// createSSEKMSKey creates an SSEKMSKey struct from data key result and parameters.
// The HMAC key commitment is computed here (rather than at each call site) so
// every SSE-KMS encryption path produces metadata that can later be verified
// against tampering — a missing commitment was an attacker-controlled silent
// downgrade vector. plaintext must still be live; deferred clearKMSDataKey
// runs after this function returns.
func createSSEKMSKey(result *KMSDataKeyResult, encryptionContext map[string]string, bucketKeyEnabled bool, iv []byte, chunkOffset int64) *SSEKMSKey {
return &SSEKMSKey{
KeyID: result.Response.KeyID,
EncryptedDataKey: result.Response.CiphertextBlob,
EncryptionContext: encryptionContext,
BucketKeyEnabled: bucketKeyEnabled,
IV: iv,
ChunkOffset: chunkOffset,
KeyCommitment: ComputeKeyCommitment(result.Response.Plaintext, iv, s3_constants.SSEAlgorithmKMS),
}
}