mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-05-14 05:41:29 +00:00
* 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.
164 lines
5.9 KiB
Go
164 lines
5.9 KiB
Go
package s3api
|
|
|
|
import (
|
|
"crypto/hmac"
|
|
"crypto/sha256"
|
|
"fmt"
|
|
"os"
|
|
"strings"
|
|
"sync/atomic"
|
|
|
|
"github.com/seaweedfs/seaweedfs/weed/glog"
|
|
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
|
)
|
|
|
|
// RequireKeyCommitmentEnv is the environment variable that flips the
|
|
// commitment check from "skip when missing" (the AWS-compatible default,
|
|
// needed for objects written before commitments shipped) to "reject when
|
|
// missing". Operators who have either re-encrypted all legacy objects or
|
|
// who never wrote any objects under the pre-commitment code path can opt
|
|
// in via this env var to close the silent downgrade vector that an
|
|
// attacker with write access to object metadata could otherwise exploit
|
|
// by stripping the commitment field.
|
|
const RequireKeyCommitmentEnv = "WEED_S3_REQUIRE_KEY_COMMITMENT"
|
|
|
|
// requireKeyCommitment is the runtime mirror of the env var, kept as an
|
|
// atomic so config-reload paths can flip it without a global mutex.
|
|
var requireKeyCommitment atomic.Bool
|
|
|
|
func init() {
|
|
if v := os.Getenv(RequireKeyCommitmentEnv); v == "1" || strings.EqualFold(v, "true") {
|
|
requireKeyCommitment.Store(true)
|
|
glog.V(1).Infof("SSE: %s=true; SSE objects without a key commitment will be rejected", RequireKeyCommitmentEnv)
|
|
}
|
|
}
|
|
|
|
// SetRequireKeyCommitment toggles strict-commitment enforcement at runtime.
|
|
// Used by tests and by future config-reload code paths.
|
|
func SetRequireKeyCommitment(require bool) {
|
|
requireKeyCommitment.Store(require)
|
|
}
|
|
|
|
// ComputeKeyCommitment computes an HMAC-SHA256 key commitment over the
|
|
// encryption parameters (IV + algorithm). This binds the ciphertext to the
|
|
// exact key material and IV that were used, preventing key-confusion and
|
|
// IV-manipulation attacks against unauthenticated AES-CTR.
|
|
//
|
|
// The commitment is stored alongside the IV in object metadata. On decrypt
|
|
// the commitment is re-derived and compared; a mismatch means the key or IV
|
|
// was tampered with.
|
|
func ComputeKeyCommitment(key []byte, iv []byte, algorithm string) []byte {
|
|
mac := hmac.New(sha256.New, key)
|
|
mac.Write(iv)
|
|
mac.Write([]byte(algorithm))
|
|
return mac.Sum(nil)
|
|
}
|
|
|
|
// VerifyKeyCommitment checks a previously stored commitment against the
|
|
// current key, IV, and algorithm. Returns nil on success.
|
|
//
|
|
// When the commitment is empty (legacy object written before commitments
|
|
// shipped), the default behaviour is to accept the object — this is the
|
|
// AWS-compatible path. Setting WEED_S3_REQUIRE_KEY_COMMITMENT=true (via
|
|
// env at startup or via SetRequireKeyCommitment at runtime) flips that
|
|
// to reject, closing the silent-downgrade vector at the cost of locking
|
|
// out un-migrated legacy objects.
|
|
func VerifyKeyCommitment(key []byte, iv []byte, algorithm string, commitment []byte) error {
|
|
if len(commitment) == 0 {
|
|
if requireKeyCommitment.Load() {
|
|
return fmt.Errorf("key commitment is required but missing from object metadata: %s set; legacy objects must be re-encrypted before this flag can be enabled", RequireKeyCommitmentEnv)
|
|
}
|
|
// Legacy data written before key commitments were added; skip.
|
|
return nil
|
|
}
|
|
expected := ComputeKeyCommitment(key, iv, algorithm)
|
|
if !hmac.Equal(expected, commitment) {
|
|
return fmt.Errorf("key commitment verification failed: encryption parameters may have been tampered with")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// isValidKMSKeyID performs basic validation of KMS key identifiers.
|
|
// Following Minio's approach: be permissive and accept any reasonable key format.
|
|
// Only reject keys with leading/trailing spaces or other obvious issues.
|
|
//
|
|
// This function is used across multiple S3 API handlers to ensure consistent
|
|
// validation of KMS key IDs in various contexts (bucket encryption, object operations, etc.).
|
|
func isValidKMSKeyID(keyID string) bool {
|
|
// Reject empty keys
|
|
if keyID == "" {
|
|
return false
|
|
}
|
|
|
|
// Following Minio's validation: reject keys with leading/trailing spaces
|
|
if strings.HasPrefix(keyID, " ") || strings.HasSuffix(keyID, " ") {
|
|
return false
|
|
}
|
|
|
|
// Also reject keys with internal spaces (common sense validation)
|
|
if strings.Contains(keyID, " ") {
|
|
return false
|
|
}
|
|
|
|
// Reject keys with control characters or newlines
|
|
if strings.ContainsAny(keyID, "\t\n\r\x00") {
|
|
return false
|
|
}
|
|
|
|
// Accept any reasonable length key (be permissive for various KMS providers)
|
|
if len(keyID) > 0 && len(keyID) <= s3_constants.MaxKMSKeyIDLength {
|
|
return true
|
|
}
|
|
|
|
return false
|
|
}
|
|
|
|
// ValidateIV validates that an initialization vector has the correct length for AES encryption
|
|
func ValidateIV(iv []byte, name string) error {
|
|
if len(iv) != s3_constants.AESBlockSize {
|
|
return fmt.Errorf("invalid %s length: expected %d bytes, got %d", name, s3_constants.AESBlockSize, len(iv))
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// ValidateSSEKMSKey validates that an SSE-KMS key is not nil and has required fields
|
|
func ValidateSSEKMSKey(sseKey *SSEKMSKey) error {
|
|
if sseKey == nil {
|
|
return fmt.Errorf("SSE-KMS key cannot be nil")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// ValidateSSES3Key validates that an SSE-S3 key has valid structure and contents
|
|
func ValidateSSES3Key(sseKey *SSES3Key) error {
|
|
if sseKey == nil {
|
|
return fmt.Errorf("SSE-S3 key cannot be nil")
|
|
}
|
|
|
|
// Validate key bytes
|
|
if sseKey.Key == nil {
|
|
return fmt.Errorf("SSE-S3 key bytes cannot be nil")
|
|
}
|
|
if len(sseKey.Key) != SSES3KeySize {
|
|
return fmt.Errorf("invalid SSE-S3 key size: expected %d bytes, got %d", SSES3KeySize, len(sseKey.Key))
|
|
}
|
|
|
|
// Validate algorithm
|
|
if sseKey.Algorithm != SSES3Algorithm {
|
|
return fmt.Errorf("invalid SSE-S3 algorithm: expected %q, got %q", SSES3Algorithm, sseKey.Algorithm)
|
|
}
|
|
|
|
// Validate key ID (should not be empty)
|
|
if sseKey.KeyID == "" {
|
|
return fmt.Errorf("SSE-S3 key ID cannot be empty")
|
|
}
|
|
|
|
// IV validation is optional during key creation - it will be set during encryption
|
|
// If IV is set, validate its length
|
|
if len(sseKey.IV) > 0 && len(sseKey.IV) != s3_constants.AESBlockSize {
|
|
return fmt.Errorf("invalid SSE-S3 IV length: expected %d bytes, got %d", s3_constants.AESBlockSize, len(sseKey.IV))
|
|
}
|
|
|
|
return nil
|
|
}
|