Skip to content
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

raise a better error when decryption of a message with an uncompressed EC point is attempted #21

Open
mattsb42-aws opened this issue Oct 14, 2017 · 2 comments

Comments

@mattsb42-aws
Copy link
Member

Problem

In testing to verify what happens if a caller attempts to decrypt a file created by mrcrypt, I realized that if you attempt to decrypt a message that was written with an uncompressed EC point (note: this is not supported under the AWS Encryption SDK spec), you get a very unhelpful error.

$ mrcrypt encrypt alias/exampleKey test_plaintext -r us-west-2 
$ python
>>> import aws_encryption_sdk
>>> mkp = aws_encryption_sdk.KMSMasterKeyProvider()
>>> with open('test_plaintext.encrypted', 'rb') as f:
...     ct = f.read()
... 
>>> aws_encryption_sdk.decrypt(source=ct, key_provider=mkp)
No handlers could be found for logger "aws_encryption_sdk.streaming_client"
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/__init__.py", line 121, in decrypt
    plaintext = decryptor.read()
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/streaming_client.py", line 201, in read
    self._prep_message()
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/streaming_client.py", line 687, in _prep_message
    self._header, self.header_auth = self._read_header()
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/streaming_client.py", line 719, in _read_header
    decryption_materials = self.config.materials_manager.decrypt_materials(request=decrypt_materials_request)
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/materials_managers/default.py", line 149, in decrypt_materials
    encryption_context=request.encryption_context
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/materials_managers/default.py", line 129, in _load_verification_key_from_encryption_context
    encoded_point=encoded_verification_key
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/internal/crypto/authentication.py", line 159, in from_encoded_point
    compressed_point=base64.b64decode(encoded_point)
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/internal/crypto/elliptic_curve.py", line 182, in _ecc_public_numbers_from_compressed_point
    x, y = _ecc_decode_compressed_point(curve, compressed_point)
  File "/Users/bullocm/tmp/mrt/lib/python2.7/site-packages/aws_encryption_sdk/internal/crypto/elliptic_curve.py", line 140, in _ecc_decode_compressed_point
    y_order = y_order_map[raw_y]
KeyError: '\x04'

Solution

Catch appropriate errors in internal.crypto.elliptic_curve._ecc_decode_compressed_point and raise NotSupportedError('Uncompressed points are not supported')

@austinmoore-
Copy link

Would you consider throwing a subtype of NotSupportedError/AWSEncryptionSDKClientError for this exception? Maybe something like UncompressedKeyError? Instead of checking for the string "Uncompressed points are not supported" I could handle that specific exception, and just let the other exceptions bubble up, keeping things a little neater:

try:
    return super(MrcryptLegacyCompatibilityCryptoMaterialsManager, self).decrypt_materials(request)
except UncompressedKeyError:
    # Uncompressed key handling logic (all other exceptions will just bubble up)
    # ...

@mattsb42-aws
Copy link
Member Author

That sounds reasonable, yes. Likely it'll be raising an error that "we couldn't load a compressed key", as that's really the only thing we can reliably state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants