Skip to content

feat: remove wrapping_algorithm input parameter from RawAESKeyring #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions examples/src/keyring/raw_aes/raw_aes.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import os

import aws_encryption_sdk
from aws_encryption_sdk.identifiers import WrappingAlgorithm
from aws_encryption_sdk.keyrings.raw import RawAESKeyring


Expand All @@ -30,14 +29,10 @@ def run(source_plaintext):
"the data you are handling": "is what you think it is",
}

# Choose the wrapping algorithm for the keyring to use.
wrapping_algorithm = WrappingAlgorithm.AES_256_GCM_IV12_TAG16_NO_PADDING

# Generate an AES key to use with your keyring.
# The key size depends on the wrapping algorithm.
# Generate a 256-bit (32 byte) AES key to use with your keyring.
#
# In practice, you should get this key from a secure key management system such as an HSM.
key = os.urandom(wrapping_algorithm.algorithm.kdf_input_len)
key = os.urandom(32)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The java version of this example has a comment here "256 bits" to draw the connection to AES-256

Copy link
Member Author

@mattsb42-aws mattsb42-aws Apr 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 32:

# Generate an AES-256 key to use with your keyring.

Do you think it would be clearer to say "a 256-bit AES key"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that sounds good, or maybe "a 256-bit (32 byte) AES key"


# Create the keyring that determines how your data keys are protected.
keyring = RawAESKeyring(
Expand All @@ -50,7 +45,6 @@ def run(source_plaintext):
key_namespace="some managed raw keys",
key_name=b"my AES wrapping key",
wrapping_key=key,
wrapping_algorithm=wrapping_algorithm,
)

# Encrypt your plaintext data.
Expand Down
35 changes: 31 additions & 4 deletions src/aws_encryption_sdk/keyrings/raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import attr
import six
from attr.validators import instance_of, optional
from attr.validators import in_, instance_of, optional
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import serialization
from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey, RSAPublicKey
Expand Down Expand Up @@ -76,7 +76,6 @@ class RawAESKeyring(Keyring):
:param str key_namespace: String defining the keyring.
:param bytes key_name: Key ID
:param bytes wrapping_key: Encryption key with which to wrap plaintext data key.
:param WrappingAlgorithm wrapping_algorithm: Wrapping Algorithm with which to wrap plaintext data key.

.. note::

Expand All @@ -86,11 +85,28 @@ class RawAESKeyring(Keyring):
key_namespace = attr.ib(validator=instance_of(six.string_types))
key_name = attr.ib(validator=instance_of(six.binary_type))
_wrapping_key = attr.ib(repr=False, validator=instance_of(six.binary_type))
_wrapping_algorithm = attr.ib(repr=False, validator=instance_of(WrappingAlgorithm))

def __attrs_post_init__(self):
# type: () -> None
"""Prepares initial values not handled by attrs."""
key_size_to_wrapping_algorithm = {
wrapper.algorithm.kdf_input_len: wrapper
for wrapper in (
WrappingAlgorithm.AES_128_GCM_IV12_TAG16_NO_PADDING,
WrappingAlgorithm.AES_192_GCM_IV12_TAG16_NO_PADDING,
WrappingAlgorithm.AES_256_GCM_IV12_TAG16_NO_PADDING,
)
}

try:
self._wrapping_algorithm = key_size_to_wrapping_algorithm[len(self._wrapping_key)]
except KeyError:
raise ValueError(
"Invalid wrapping key length. Must be one of {} bytes.".format(
sorted(key_size_to_wrapping_algorithm.keys())
)
)

self._key_provider = MasterKeyInfo(provider_id=self.key_namespace, key_info=self.key_name)

self._wrapping_key_structure = WrappingKey(
Expand Down Expand Up @@ -244,7 +260,18 @@ class RawRSAKeyring(Keyring):

key_namespace = attr.ib(validator=instance_of(six.string_types))
key_name = attr.ib(validator=instance_of(six.binary_type))
_wrapping_algorithm = attr.ib(repr=False, validator=instance_of(WrappingAlgorithm))
_wrapping_algorithm = attr.ib(
repr=False,
validator=in_(
(
WrappingAlgorithm.RSA_PKCS1,
WrappingAlgorithm.RSA_OAEP_SHA1_MGF1,
WrappingAlgorithm.RSA_OAEP_SHA256_MGF1,
WrappingAlgorithm.RSA_OAEP_SHA384_MGF1,
WrappingAlgorithm.RSA_OAEP_SHA512_MGF1,
)
),
)
_private_wrapping_key = attr.ib(default=None, repr=False, validator=optional(instance_of(RSAPrivateKey)))
_public_wrapping_key = attr.ib(default=None, repr=False, validator=optional(instance_of(RSAPublicKey)))

Expand Down
33 changes: 6 additions & 27 deletions test/functional/keyrings/raw/test_raw_aes.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,14 @@ def sample_encryption_materials():


@pytest.mark.parametrize("encryption_materials_samples", sample_encryption_materials())
@pytest.mark.parametrize("wrapping_algorithm_samples", _WRAPPING_ALGORITHM)
def test_raw_aes_encryption_decryption(encryption_materials_samples, wrapping_algorithm_samples):
def test_raw_aes_encryption_decryption(encryption_materials_samples):

# Initializing attributes
key_namespace = _PROVIDER_ID
key_name = _KEY_ID
_wrapping_algorithm = wrapping_algorithm_samples

# Creating an instance of a raw AES keyring
test_raw_aes_keyring = RawAESKeyring(
key_namespace=key_namespace,
key_name=key_name,
wrapping_key=_WRAPPING_KEY,
wrapping_algorithm=_wrapping_algorithm,
)
test_raw_aes_keyring = RawAESKeyring(key_namespace=key_namespace, key_name=key_name, wrapping_key=_WRAPPING_KEY,)

# Call on_encrypt function for the keyring
encryption_materials = test_raw_aes_keyring.on_encrypt(encryption_materials=encryption_materials_samples)
Expand All @@ -101,21 +94,14 @@ def test_raw_aes_encryption_decryption(encryption_materials_samples, wrapping_al


@pytest.mark.parametrize("encryption_materials_samples", sample_encryption_materials())
@pytest.mark.parametrize("wrapping_algorithm_samples", _WRAPPING_ALGORITHM)
def test_raw_master_key_decrypts_what_raw_keyring_encrypts(encryption_materials_samples, wrapping_algorithm_samples):
def test_raw_master_key_decrypts_what_raw_keyring_encrypts(encryption_materials_samples):

# Initializing attributes
key_namespace = _PROVIDER_ID
key_name = _KEY_ID
_wrapping_algorithm = wrapping_algorithm_samples

# Creating an instance of a raw AES keyring
test_raw_aes_keyring = RawAESKeyring(
key_namespace=key_namespace,
key_name=key_name,
wrapping_key=_WRAPPING_KEY,
wrapping_algorithm=_wrapping_algorithm,
)
test_raw_aes_keyring = RawAESKeyring(key_namespace=key_namespace, key_name=key_name, wrapping_key=_WRAPPING_KEY,)

# Creating an instance of a raw master key
test_raw_master_key = RawMasterKey(
Expand All @@ -139,21 +125,14 @@ def test_raw_master_key_decrypts_what_raw_keyring_encrypts(encryption_materials_


@pytest.mark.parametrize("encryption_materials_samples", sample_encryption_materials())
@pytest.mark.parametrize("wrapping_algorithm_samples", _WRAPPING_ALGORITHM)
def test_raw_keyring_decrypts_what_raw_master_key_encrypts(encryption_materials_samples, wrapping_algorithm_samples):
def test_raw_keyring_decrypts_what_raw_master_key_encrypts(encryption_materials_samples):

# Initializing attributes
key_namespace = _PROVIDER_ID
key_name = _KEY_ID
_wrapping_algorithm = wrapping_algorithm_samples

# Creating an instance of a raw AES keyring
test_raw_aes_keyring = RawAESKeyring(
key_namespace=key_namespace,
key_name=key_name,
wrapping_key=_WRAPPING_KEY,
wrapping_algorithm=_wrapping_algorithm,
)
test_raw_aes_keyring = RawAESKeyring(key_namespace=key_namespace, key_name=key_name, wrapping_key=_WRAPPING_KEY,)

# Creating an instance of a raw master key
test_raw_master_key = RawMasterKey(
Expand Down
14 changes: 2 additions & 12 deletions test/functional/keyrings/test_multi.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,7 @@
)

_MULTI_KEYRING_WITH_GENERATOR_AND_CHILDREN = MultiKeyring(
generator=RawAESKeyring(
key_namespace=_PROVIDER_ID,
key_name=_KEY_ID,
wrapping_algorithm=WrappingAlgorithm.AES_256_GCM_IV12_TAG16_NO_PADDING,
wrapping_key=_WRAPPING_KEY_AES,
),
generator=RawAESKeyring(key_namespace=_PROVIDER_ID, key_name=_KEY_ID, wrapping_key=_WRAPPING_KEY_AES,),
children=[
RawRSAKeyring(
key_namespace=_PROVIDER_ID,
Expand Down Expand Up @@ -95,12 +90,7 @@
public_exponent=65537, key_size=2048, backend=default_backend()
),
),
RawAESKeyring(
key_namespace=_PROVIDER_ID,
key_name=_KEY_ID,
wrapping_algorithm=WrappingAlgorithm.AES_128_GCM_IV12_TAG16_NO_PADDING,
wrapping_key=_WRAPPING_KEY_AES,
),
RawAESKeyring(key_namespace=_PROVIDER_ID, key_name=_KEY_ID, wrapping_key=_WRAPPING_KEY_AES,),
]
)

Expand Down
21 changes: 11 additions & 10 deletions test/unit/keyrings/raw/test_raw_aes.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,7 @@

@pytest.fixture
def raw_aes_keyring():
return RawAESKeyring(
key_namespace=_PROVIDER_ID,
key_name=_KEY_ID,
wrapping_algorithm=WrappingAlgorithm.AES_256_GCM_IV12_TAG16_NO_PADDING,
wrapping_key=_WRAPPING_KEY,
)
return RawAESKeyring(key_namespace=_PROVIDER_ID, key_name=_KEY_ID, wrapping_key=_WRAPPING_KEY,)


@pytest.fixture
Expand Down Expand Up @@ -113,12 +108,18 @@ def test_valid_parameters(raw_aes_keyring):
def test_invalid_parameters(key_namespace, key_name, wrapping_algorithm, wrapping_key):
with pytest.raises(TypeError):
RawAESKeyring(
key_namespace=key_namespace,
key_name=key_name,
wrapping_algorithm=wrapping_algorithm,
wrapping_key=wrapping_key,
key_namespace=key_namespace, key_name=key_name, wrapping_key=wrapping_key,
)


def test_invalid_key_length():
with pytest.raises(ValueError) as excinfo:
RawAESKeyring(
key_namespace=_PROVIDER_ID, key_name=_KEY_ID, wrapping_key=b"012345",
)

excinfo.match(r"Invalid wrapping key length. Must be one of \[16, 24, 32\] bytes.")


def test_on_encrypt_when_data_encryption_key_given(raw_aes_keyring, patch_generate_data_key):
test_raw_aes_keyring = raw_aes_keyring
Expand Down
18 changes: 18 additions & 0 deletions test/unit/keyrings/raw/test_raw_rsa.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,24 @@ def test_invalid_parameters(key_namespace, key_name, wrapping_algorithm, private
)


@pytest.mark.parametrize(
"wrapping_algorithm",
(
WrappingAlgorithm.AES_128_GCM_IV12_TAG16_NO_PADDING,
WrappingAlgorithm.AES_192_GCM_IV12_TAG16_NO_PADDING,
WrappingAlgorithm.AES_256_GCM_IV12_TAG16_NO_PADDING,
),
)
def test_invalid_wrapping_algorithm_suite(wrapping_algorithm):
with pytest.raises(ValueError):
RawRSAKeyring(
key_namespace=_PROVIDER_ID,
key_name=_KEY_ID,
wrapping_algorithm=wrapping_algorithm,
private_wrapping_key=raw_rsa_private_key(),
)


def test_public_and_private_key_not_provided():
with pytest.raises(TypeError) as exc_info:
RawRSAKeyring(
Expand Down
16 changes: 2 additions & 14 deletions test/unit/keyrings/test_multi.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,26 +96,14 @@ def test_parent():


def test_keyring_with_generator_but_no_children():
generator_keyring = RawAESKeyring(
key_namespace=_PROVIDER_ID,
key_name=_KEY_ID,
wrapping_key=_WRAPPING_KEY_AES,
wrapping_algorithm=WrappingAlgorithm.AES_256_GCM_IV12_TAG16_NO_PADDING,
)
generator_keyring = RawAESKeyring(key_namespace=_PROVIDER_ID, key_name=_KEY_ID, wrapping_key=_WRAPPING_KEY_AES,)
test_multi_keyring = MultiKeyring(generator=generator_keyring)
assert test_multi_keyring.generator is generator_keyring
assert not test_multi_keyring.children


def test_keyring_with_children_but_no_generator():
children_keyring = [
RawAESKeyring(
key_namespace=_PROVIDER_ID,
key_name=_KEY_ID,
wrapping_key=_WRAPPING_KEY_AES,
wrapping_algorithm=WrappingAlgorithm.AES_256_GCM_IV12_TAG16_NO_PADDING,
)
]
children_keyring = [RawAESKeyring(key_namespace=_PROVIDER_ID, key_name=_KEY_ID, wrapping_key=_WRAPPING_KEY_AES,)]
test_multi_keyring = MultiKeyring(children=children_keyring)
assert test_multi_keyring.children is children_keyring
assert test_multi_keyring.generator is None
Expand Down
19 changes: 3 additions & 16 deletions test/unit/unit_test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,7 @@ def get_decryption_materials_without_data_key():

def get_multi_keyring_with_generator_and_children():
return MultiKeyring(
generator=RawAESKeyring(
key_namespace=_PROVIDER_ID,
key_name=_KEY_ID,
wrapping_algorithm=WrappingAlgorithm.AES_256_GCM_IV12_TAG16_NO_PADDING,
wrapping_key=_WRAPPING_KEY_AES,
),
generator=RawAESKeyring(key_namespace=_PROVIDER_ID, key_name=_KEY_ID, wrapping_key=_WRAPPING_KEY_AES,),
children=[
RawRSAKeyring(
key_namespace=_PROVIDER_ID,
Expand Down Expand Up @@ -307,12 +302,7 @@ def get_multi_keyring_with_no_generator():
public_exponent=65537, key_size=2048, backend=default_backend()
),
),
RawAESKeyring(
key_namespace=_PROVIDER_ID,
key_name=_KEY_ID,
wrapping_algorithm=WrappingAlgorithm.AES_128_GCM_IV12_TAG16_NO_PADDING,
wrapping_key=_WRAPPING_KEY_AES,
),
RawAESKeyring(key_namespace=_PROVIDER_ID, key_name=_KEY_ID, wrapping_key=_WRAPPING_KEY_AES,),
]
)

Expand Down Expand Up @@ -482,10 +472,7 @@ def ephemeral_raw_aes_keyring(wrapping_algorithm=WrappingAlgorithm.AES_256_GCM_I
if key is None:
key = os.urandom(key_length)
return RawAESKeyring(
key_namespace="fake",
key_name="aes-{}".format(key_length * 8).encode("utf-8"),
wrapping_algorithm=wrapping_algorithm,
wrapping_key=key,
key_namespace="fake", key_name="aes-{}".format(key_length * 8).encode("utf-8"), wrapping_key=key,
)


Expand Down