feat: remove wrapping_algorithm input parameter from RawAESKeyring #247
Conversation
| # | ||
| # 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) |
There was a problem hiding this comment.
The java version of this example has a comment here "256 bits" to draw the connection to AES-256
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
Yeah that sounds good, or maybe "a 256-bit (32 byte) AES key"
| self._wrapping_algorithm = key_size_to_wrapping_algorithm[len(self._wrapping_key)] | ||
| except KeyError: | ||
| raise ValueError( | ||
| "Invalid wrapping key length. Must be one of {}".format(key_size_to_wrapping_algorithm.keys()) |
There was a problem hiding this comment.
what does this error message look like, would it print out "Must be one of AES_128_GCM_IV12_TAG16_NO_PADDING, AES_192_GCM_IV12_TAG16_NO_PADDING, etc" or just the key sizes?
There was a problem hiding this comment.
It would print out the key sizes.
There was a problem hiding this comment.
I updated this to be deterministic (it already was for 3.6+ but not for 3.5 or 2.7) and updated the test to catch any changes to the set.
…tic and check for exact allowed options
Issue #, if available:
resolves: #246
Description of changes:
This implements proposed solution number 2 detailed in #246, removing wrapping algorithm as an input parameter, restricting wrapping key material length to supported lengths, and deriving the wrapping algorithm suite from the wrapping key material length.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: