mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-06-13 23:36:45 +03:00
e1d5e3899f
* 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.
106 lines
3.9 KiB
Go
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),
|
|
}
|
|
}
|