Skip to content

Conversation

@mboorstin-circle
Copy link

@mboorstin-circle mboorstin-circle commented Nov 6, 2025

Motivation

Add support for the KMS Decrypt Recipient field. This is used by Nitro Enclaves to decrypt sensitive without their host being able to read it, by KMS re-encrypting the decrypted data to the provided RSA key before it is returned to the caller. This flow is described here.

Changes

I've added basic support to the KMS Decrypt command for the Recipient field by looking for the attestation, parsing it to extract the recipient public key, and returning CiphertextForRecipient instead of Plaintext. It's not clear to me how to simulate verifying the attestation (which involves a signature from AWS that the PCR values are correct) without a significant effort to add proper support for Nitro to Localstack, and I think this PR as-is provides valuable functionality by allowing systems that use Nitro and KMS to be tested against Localstack without having to special-case their calls to Localstack's KMS implementation. So, I don't attempt to do any validation on the provided attestation document beyond what is required to extract the recipient public key.

I've unfortunately had to do a significant amount of manual crypto operations here with asn1crypto and PyCryptoDome because cryptography.hazmat doesn't appear to support the exact algorithms that AWS uses; I apologize for the verbosity and am happy to change it to cryptography.hazmat if there is a way to use it.

I've taken a compromise approach to ensuring that this change is backwards-compatible; if callers pass in a nonsense value to Recipient it will be logged and ignored and Plaintext will be returned, but if an attestation with a valid public key is passed in then CiphertextForRecipient instead of Plaintext will be returned. It is more to spec to error if a nonsense value is passed in for Recipient but that feels a bit more breaking. I'm happy to change to erroring if the maintainers would prefer that now, or it could be done as part of the next major release or similar.

Tests

I've added basic tests for the functionality. I've had to mark them as only_localstack because it is quite complex to call AWS to set up an actual Nitro Enclave, and until Localstack has broader Nitro support I think this will suffice to test. They do test that the flow works end to end. In addition, I have manually tested that this implementation produces a PKCS7 envelope similar to what actual AWS produces for an actual Nitro Enclave, and I have manually tested that my application that works on actual Nitro and KMS works against this PR.

Related

Addresses #12901.

@localstack-bot
Copy link
Contributor

localstack-bot commented Nov 6, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link
Contributor

@localstack-bot localstack-bot left a comment

Choose a reason for hiding this comment

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

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

@mboorstin-circle
Copy link
Author

(I know that I need to sign the CLA; I just need to consult with my employer's legal department before doing so).

@mboorstin-circle
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@mboorstin-circle
Copy link
Author

recheck

localstack-bot added a commit that referenced this pull request Nov 6, 2025
@mboorstin-circle
Copy link
Author

recheck

@mboorstin-circle
Copy link
Author

@sannya-singal @alexrashed @bentsku (tagging some of the automatically selected reviewers) I believe this should be ready for review. Thank you in advance!

@alexrashed alexrashed added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases docs: skip Pull request does not require documentation changes notes: needed Pull request should be mentioned in the release notes labels Nov 7, 2025
@alexrashed alexrashed added this to the 4.11 milestone Nov 7, 2025
Copy link
Contributor

@sannya-singal sannya-singal left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for adding this functionality 🚀 The implementation looks good and the approach of gracefully handling invalid attestations is really great. I just have a few suggestions to improve it, please let me know if you require some help or have questions.

import logging
import os

from cbor2._decoder import loads as cbor2_loads
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than using an internal method _decoder here we should use the following from cbor2 import loads.

Copy link
Author

Choose a reason for hiding this comment

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

Done, thanks.

f" constraint: [Member must satisfy enum value set: {VALID_OPERATIONS}]"
)

def _extract_attestation_pubkey(self, attestation_document: bytes):
Copy link
Contributor

Choose a reason for hiding this comment

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

A return type hint needs to be added to this function.

Copy link
Author

Choose a reason for hiding this comment

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

done, thanks.

Comment on lines 9 to 11
from Crypto.Cipher import AES, PKCS1_OAEP
from Crypto.Hash import SHA256
from Crypto.PublicKey import RSA as CryptoRSA
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion we could eliminate the use of Crypto here and use dependencies from cryptography.hazmat.primitives.

So something like:

encrypted_session_key = recipient_pubkey.encrypt(
        session_key,
        padding.OAEP(
            mgf=padding.MGF1(algorithm=hashes.SHA256()),
            algorithm=hashes.SHA256(),
            label=None
        )
    )
cipher = Cipher(algorithms.AES(session_key), modes.CBC(iv), backend=default_backend())
encryptor = cipher.encryptor()

padder = sym_padding.PKCS7(algorithms.AES.block_size).padder()
padded_plaintext = padder.update(plaintext) + padder.finalize()
encrypted_content = encryptor.update(padded_plaintext) + encryptor.finalize()

Please let me know if you would like to know more context here.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, thank you! This all makes sense. I went down this path because Hazmat's one-shot PKCS7 only supports RSA-PKCS1.5 rather than the RSA-OAEP that AWS provides, but yes, I can still do the individual encryption operations with Hazmat and then just use the asn1crypto library to build them into a PKCS7 envelope. So I've updated things to follow your example and remove the PyCryptoDome usage here and in the test file. Let me know if things now look better. Thank you!

Comment on lines 9 to 12
from Crypto.Cipher import AES, PKCS1_OAEP
from Crypto.Hash import SHA256
from Crypto.PublicKey import RSA as CryptoRSA
from Crypto.Random import get_random_bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use os.urandom here instead?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, thank you. Updated.

from Crypto.Hash import SHA256
from Crypto.PublicKey import RSA as CryptoRSA
from Crypto.Random import get_random_bytes
from Crypto.Util.Padding import pad as crypto_pad
Copy link
Contributor

Choose a reason for hiding this comment

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

using from cryptography.hazmat.primitives.asymmetric import padding should suffice the purpose here as well.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, updated to use it as per your example. Thank you!

return decrypted


def pkcs7_envelope_encrypt(plaintext: bytes, recipient_pubkey) -> bytes:
Copy link
Contributor

Choose a reason for hiding this comment

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

Parameter recipient_pubkey lacks type hint here, can you please add it?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, added.

Comment on lines 14 to 17
from Crypto.Cipher import AES, PKCS1_OAEP
from Crypto.Hash import SHA256
from Crypto.PublicKey import RSA as CryptoRSA
from Crypto.Util.Padding import unpad as crypto_unpad
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as suggested in the crypto.py most of this can be done with cryptography library.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, thank you! I've updated it in the same way of still using as1ncrypto to deserialize the envelope, but then using the normal Hazmat methods to do the actual decryption. Thanks!

assert base64.b64decode(plaintext) == message

@markers.aws.only_localstack
def test_decrypt_recipient_invalid_attestation(self, kms_create_key, snapshot, aws_client):
Copy link
Contributor

Choose a reason for hiding this comment

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

We can safely remove snapshot fixture as its not being used in this test.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed, thanks.

snapshot.match("decrypt_response_with_invalid_ciphertext", e.value.response)

@markers.aws.only_localstack
def test_decrypt_recipient(self, kms_create_key, snapshot, aws_client):
Copy link
Contributor

Choose a reason for hiding this comment

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

We can safely remove snapshot fixture as its not being used in this test.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed, thanks.

@mboorstin-circle
Copy link
Author

Thanks @sannya-singal ! All of your comments make sense and I have updated the diff appropriately. As you requested, I changed back to using cryptography.hazmat for the individual encryption / decryption operations (I think I still need asn1crypto to do the envelope serialization / deserialization, as like I said Hazmat's one-stop PKCS7 doesn't support RSA-OAEP). Let me know if things now look better. Thanks!

Copy link
Contributor

@sannya-singal sannya-singal left a comment

Choose a reason for hiding this comment

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

Thanks @mboorstin-circle for addressing all the comments so quickly 🚀 🎉 Changes LGTM, tagging @k-a-il for a quick sanity check on the dependency file changes.

@mboorstin-circle
Copy link
Author

Sounds great. Thanks @sannya-singal and (in advance) @k-a-il !

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

Labels

docs: skip Pull request does not require documentation changes notes: needed Pull request should be mentioned in the release notes semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants