Skip to content

Commit 7ca15e8

Browse files
authored
chore(tests): add security tests (#174)
* chore: add tests around downgrade and EC tampering
1 parent 515aa4b commit 7ca15e8

8 files changed

Lines changed: 781 additions & 4 deletions

File tree

src/s3_encryption/decryptor.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,11 @@ def finalize(self, data: bytes) -> bytes:
7272
plaintext = self._decryptor.update(data) if data else b""
7373
plaintext += self._decryptor.finalize()
7474
return self._unpadder.update(plaintext) + self._unpadder.finalize()
75-
except Exception as e:
76-
raise S3EncryptionClientSecurityError(f"Failed to decrypt CBC content: {e}") from e
75+
except Exception:
76+
# Use a fixed message for all CBC failures to prevent padding oracle attacks.
77+
# Different failure modes (bad padding, truncated ciphertext, wrong key) MUST
78+
# produce identical error responses so an attacker cannot distinguish them.
79+
raise S3EncryptionClientSecurityError("Failed to decrypt CBC content.") from None
7780

7881

7982
@define

src/s3_encryption/materials/kms_keyring.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,16 @@ def on_decrypt(self, dec_materials, encrypted_data_keys=None):
200200
f"Enable legacy wrapping algorithms to use legacy key wrapping "
201201
f"algorithm: {edk.key_provider_info}"
202202
)
203+
# The KmsV1 wrapping algorithm does not support caller-provided
204+
# encryption context. If the caller provided encryption context,
205+
# the client MUST reject the request. This prevents a downgrade
206+
# from kms+context to kms from bypassing context validation.
207+
if dec_materials.encryption_context_from_request:
208+
raise S3EncryptionClientError(
209+
"Encryption context is not supported with the KmsV1 (kms) "
210+
"wrapping algorithm. Use kms+context wrapping algorithm to "
211+
"use encryption context."
212+
)
203213
else:
204214
raise S3EncryptionClientError(
205215
f"{edk.key_provider_info} is not a valid key wrapping algorithm!"

src/s3_encryption/pipelines.py

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,30 @@ def decrypt(
316316
# Determine the algorithm suite from the metadata
317317
algorithm_suite = self._determine_algorithm_suite(metadata)
318318

319+
# Reject metadata that contains keys from multiple format versions.
320+
# This prevents format confusion attacks where an attacker injects
321+
# V2 keys via an instruction file to bypass V3 key-commitment verification.
322+
if metadata.has_exclusive_key_collision():
323+
raise S3EncryptionClientError(
324+
"Object metadata contains keys from multiple format versions. "
325+
"The object or its instruction file may have been tampered with."
326+
)
327+
328+
# Also reject V2 format metadata that contains V3 content keys.
329+
# In the instruction file injection scenario, the attacker replaces
330+
# V3 EDK keys with V2 keys, but V3 content keys (x-amz-c, x-amz-d,
331+
# x-amz-i) remain from the object metadata. This combination is
332+
# never produced by legitimate encryption.
333+
if metadata.is_v2_format() and (
334+
metadata.content_cipher_v3 is not None
335+
or metadata.key_commitment_v3 is not None
336+
or metadata.message_id_v3 is not None
337+
):
338+
raise S3EncryptionClientError( # pragma: no cover — only reachable via instruction file merge; covered by TestInstructionFileFormatConfusion
339+
"Object metadata contains V2 format keys alongside V3 content keys. "
340+
"The object or its instruction file may have been tampered with."
341+
)
342+
319343
# Determine which format we're dealing with and get decryption materials
320344
if metadata.is_v1_format():
321345
dec_materials = self._decrypt_v1(metadata, encryption_context)
@@ -590,7 +614,13 @@ def _decrypt_v3(self, metadata, encryption_context) -> DecryptionMaterials:
590614

591615
# Map V3 compressed wrapping algorithm to canonical key_provider_info
592616
raw_wrap_alg = metadata.encrypted_data_key_algorithm_v3 or "12"
593-
wrap_alg = self._V3_WRAP_ALG_MAP.get(raw_wrap_alg, raw_wrap_alg)
617+
wrap_alg = self._V3_WRAP_ALG_MAP.get(raw_wrap_alg)
618+
if wrap_alg is None:
619+
raise S3EncryptionClientError(
620+
f"Unknown V3 wrapping algorithm: '{raw_wrap_alg}'. "
621+
f"Valid values are: {list(self._V3_WRAP_ALG_MAP.keys())}. "
622+
f"The object metadata may have been tampered with."
623+
)
594624

595625
encrypted_data_key = EncryptedDataKey(
596626
key_provider_id=b"S3Keyring",
@@ -607,8 +637,13 @@ def _decrypt_v3(self, metadata, encryption_context) -> DecryptionMaterials:
607637
stored_context = {}
608638
if wrap_alg == "kms+context":
609639
raw_ctx = metadata.encryption_context_v3
610-
else:
640+
elif wrap_alg in ("AES/GCM", "RSA-OAEP-SHA1"):
611641
raw_ctx = metadata.mat_desc_v3
642+
else:
643+
raise S3EncryptionClientError( # pragma: no cover — defense in depth, unreachable
644+
f"Unexpected V3 wrapping algorithm for context selection: '{wrap_alg}'. "
645+
f"The object metadata may have been tampered with."
646+
)
612647

613648
if raw_ctx is not None:
614649
if isinstance(raw_ctx, dict):

test-server/java-tests/src/it/java/software/amazon/encryption/s3/RoundTripTests.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,10 @@ public void kmsV1Legacy(TestUtils.LanguageServerTarget language) {
332332
@ParameterizedTest(name = "{displayName} for Encrypt: Java, Decrypt: {0}")
333333
@MethodSource("software.amazon.encryption.s3.TestUtils#clientsForTest")
334334
public void kmsV1LegacyWithEncCtx(TestUtils.LanguageServerTarget language) {
335+
if (KMSV1_ENCRYPTION_CONTEXT_UNSUPPORTED.contains(language.getLanguageName())) {
336+
throw new TestAbortedException(
337+
"KmsV1 with encryption context not supported for: " + language.getLanguageName());
338+
}
335339
S3ECTestServerClient client = testServerClientFor(language);
336340
final String objectKey = appendTestSuffix("test-key-kms-v1-with-enc-ctx-" + language);
337341
final String input = "simple-test-input";

test-server/java-tests/src/it/java/software/amazon/encryption/s3/TestUtils.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ public class TestUtils {
9898
public static final Set<String> ENCRYPTION_CONTEXT_ON_ENCRYPT_UNSUPPORTED =
9999
Set.of(NET_V3_TRANSITION, NET_V4);
100100

101+
// Languages that reject caller-provided encryption context when the
102+
// wrapping algorithm is KmsV1 ("kms").
103+
public static final Set<String> KMSV1_ENCRYPTION_CONTEXT_UNSUPPORTED =
104+
Set.of(PYTHON_V3);
105+
101106
public static final Set<String> RE_ENCRYPT_SUPPORTED =
102107
Set.of(JAVA_V3_TRANSITION, JAVA_V4);
103108

0 commit comments

Comments
 (0)