From 32568227914812414853ae3ba9c75556fbf41b92 Mon Sep 17 00:00:00 2001 From: Adriano Hernandez Date: Wed, 7 Aug 2019 15:42:26 -0700 Subject: [PATCH] Fixed issue #135. Took longer than I expected because was trying to get the value of the key in the encryption context dictionary to be the same (as well as name of the key), but then learned this wasn't what we wanted. Wrote a test that I confirm works only after the change is added. --- .../materials_managers/default.py | 12 ++++++++- test/unit/test_deserialize.py | 8 ++++++ test/unit/test_material_managers_default.py | 25 ++++++++++++++++++- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/aws_encryption_sdk/materials_managers/default.py b/src/aws_encryption_sdk/materials_managers/default.py index 402a853ce..c2cc0525b 100644 --- a/src/aws_encryption_sdk/materials_managers/default.py +++ b/src/aws_encryption_sdk/materials_managers/default.py @@ -49,13 +49,21 @@ def _generate_signing_key_and_update_encryption_context(self, algorithm, encrypt :param dict encryption_context: Encryption context from request :returns: Signing key bytes :rtype: bytes or None + :raises ValueError if a signer key that is already present in the encryption + context is added """ _LOGGER.debug("Generating signing key") if algorithm.signing_algorithm_info is None: return None signer = Signer(algorithm=algorithm, key=generate_ecc_signing_key(algorithm=algorithm)) - encryption_context[ENCODED_SIGNER_KEY] = to_str(signer.encoded_public_key()) + signer_key = signer.encoded_public_key() + # raise error if key already present, even if different value + # we don't care about the value, but we do care about the key (in the DICT) + # remember this is dict(key -> value) = dict(key_name -> key) + if ENCODED_SIGNER_KEY in encryption_context: + raise ValueError("Tried to add key that was already present in Encryption Context") + encryption_context[ENCODED_SIGNER_KEY] = to_str(signer_key) return signer.key_bytes() def get_encryption_materials(self, request): @@ -68,6 +76,8 @@ def get_encryption_materials(self, request): :raises MasterKeyProviderError: if no master keys are available from the underlying master key provider :raises MasterKeyProviderError: if the primary master key provided by the underlying master key provider is not included in the full set of master keys provided by that provider + :raises ValueError if in calling _generate_signing_key_and_update_encryption_context() + a key is attempted to be added to the encryption context, when it already has that key. """ algorithm = request.algorithm if request.algorithm is not None else self.algorithm encryption_context = request.encryption_context.copy() diff --git a/test/unit/test_deserialize.py b/test/unit/test_deserialize.py index 8a96ea4ca..552cd49ef 100644 --- a/test/unit/test_deserialize.py +++ b/test/unit/test_deserialize.py @@ -27,6 +27,14 @@ pytestmark = [pytest.mark.unit, pytest.mark.local] +def test_deserialize_malformed_encryption_context(): + """ + If the client deserialization receives a malformed ciphertext + that defines the AAD length as 0 and then also defines a AAD fields + as 0, the deserialization logic SHOULD raise an error. + """ + bad_ciphertext = b'AYAAFJwN8IgQ9+0sxyy7+90cCCgAAgAAAAEAE1dFQi1DUllQVE8tUlNBLU9BRVAAKDhDRUQyRkQyMEZDODhBOUMwNkVGREIwNzM3MDdFQjFFRjE2NTU3ODABAFbIi+gmSrvejfOCjbE08rTYHym2uLWsiizQHnTy3z8/VeR+7MKvNv7ZfPf5LX7i9amYwxCMISvY+BCcndLakH/RlDUdgz5/Q0KAxrE5LX7DHxO/wMviJCi+qXWMb+5u0mhwepRihO/dk+3kGqyaLhnGuA6xqYmThUlCZR5BwfyEddSango7umEWw1YQ8vokjqUzCKRyk3VpXwQTXQLLrBz7ZmZ7Anzn0SoaLYk8D0rPWhKHvUXQDJYDYdQ7vpedxpsE5vliLI98CAcIWllkst964DIBwKgAX6Ic8Nj+8T7VurdK2SFuTH4LIvkebmEGCxngdRpfopEU/Rd0LYXZik4CAAAAAAwAAAAGAAAAAAAAAAAAAAAAK9vNRvymDkoxO6dy67pDuf////8AAAABAAAAAAAAAAAAAAABAAAABTAqmilQragTFTYdPz23w1NMR+c8Uw==' + pass def test_deserialize_non_framed_values(): iv = b"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x10\x11" diff --git a/test/unit/test_material_managers_default.py b/test/unit/test_material_managers_default.py index 9d6bd949f..7144b0b0f 100644 --- a/test/unit/test_material_managers_default.py +++ b/test/unit/test_material_managers_default.py @@ -12,6 +12,7 @@ # language governing permissions and limitations under the License. """Test suite for aws_encryption_sdk.materials_managers.default""" import pytest +import os from mock import MagicMock, sentinel from pytest_mock import mocker # noqa pylint: disable=unused-import @@ -20,10 +21,14 @@ from aws_encryption_sdk.identifiers import Algorithm from aws_encryption_sdk.internal.defaults import ALGORITHM, ENCODED_SIGNER_KEY from aws_encryption_sdk.key_providers.base import MasterKeyProvider -from aws_encryption_sdk.materials_managers import EncryptionMaterials +from aws_encryption_sdk.materials_managers import EncryptionMaterials, EncryptionMaterialsRequest from aws_encryption_sdk.materials_managers.default import DefaultCryptoMaterialsManager from aws_encryption_sdk.structures import DataKey +from aws_encryption_sdk.internal.crypto.authentication import Signer +from aws_encryption_sdk.internal.crypto.elliptic_curve import generate_ecc_signing_key +from aws_encryption_sdk.internal.str_ops import to_str + pytestmark = [pytest.mark.unit, pytest.mark.local] @@ -234,3 +239,21 @@ def test_decrypt_materials(mocker, patch_for_dcmm_decrypt): ) assert test.data_key is cmm.master_key_provider.decrypt_data_key_from_list.return_value assert test.verification_key == patch_for_dcmm_decrypt + +# tests that we correctly through an error when we try to add a key +# to the encryption context but it is already present +def test_signer_key_in_encryption_context(): + cmm = build_cmm() + algo = ALGORITHM # default + + # key value does not matter + key = to_str(Signer(algorithm=algo, key=generate_ecc_signing_key(algorithm=algo)).encoded_public_key()) + context = {ENCODED_SIGNER_KEY: key} + request = EncryptionMaterialsRequest( + algorithm=algo, + encryption_context=context, + frame_length=4096 + ) + with pytest.raises(ValueError) as excinfo: + cmm.get_encryption_materials(request) + excinfo.match(r"Tried to add key that was already present in Encryption Context") \ No newline at end of file