setup(mlkem specifications): initial specifications for mlkem kms key…#319
setup(mlkem specifications): initial specifications for mlkem kms key…#319ElvinLit wants to merge 12 commits into
Conversation
lucasmcdonald3
left a comment
There was a problem hiding this comment.
Still leaving comments but this is in a good enough spot to write a POC and start co-iterating on both the spec and code
| # AWS KMS ML-KEM Keyring | ||
|
|
||
| ## Version | ||
|
|
||
| 0.1.0-preview | ||
|
|
||
| ### Changelog | ||
|
|
||
| - 0.1.0-preview | ||
|
|
||
| - Initial record. | ||
|
|
||
| ## Implementations | ||
|
|
||
| | Language | Confirmed Compatible with Spec Version | Minimum Version Confirmed | Implementation | | ||
| | -------- | -------------------------------------- | ------------------------- | -------------- | |
There was a problem hiding this comment.
Let's move away from this type of versioning, the git history serves as a sufficient versioning/changelog mechanism
| Add an AWS KMS ML-KEM Keyring that protects data keys with a NIST-standardized | ||
| post-quantum Key Encapsulation Mechanism (ML-KEM, FIPS 203). | ||
| The keyring uses an ML-KEM KMS key to establish a per-message shared secret, | ||
| derives a wrapping key from it, and AES-GCM-wraps the data key. |
There was a problem hiding this comment.
Do we need to say AES 256 to signify wrapping key size?
There was a problem hiding this comment.
I can add to clarify. I also mentioned the 256 size in lines 347 and 362 using RFC keywords.
| ML-KEM is the NIST-standardized post-quantum KEM and is now exposed by AWS KMS | ||
| via `Encapsulate` / `Decapsulate`. | ||
| This keyring lets customers migrate envelope encryption to a quantum-resistant | ||
| primitive without changing the message format, the algorithm suites, or any |
There was a problem hiding this comment.
I am confused with this statement. What do you mean by migrating to quantum-resistant without changing any other key ring?
There was a problem hiding this comment.
Moreso if the ML-KEM keyring wasn't available, then customers would have to create a custom keyring for PQ.
| The following table describes the fields that form the key provider information. | ||
| The bytes are appended in the order shown. | ||
|
|
||
| | Field | Length (bytes) | Interpreted as | | ||
| | -------------- | -------------- | -------------- | | ||
| | Version | 1 | `0x01` | | ||
| | Key ARN Length | 2 | UInt16 | | ||
| | Key ARN | Variable | UTF-8 Bytes | |
There was a problem hiding this comment.
Generally for tables I'd suggest making Duvet-able requirements out of them ("The Key Provider information MUST consist of, in order, Version, Key ARN Length, and Key ARN"), and Duvet-able requirements about each field ("The length of the serialized Version field MUST be 0x01").
Then a note above the table saying "The following table is a non-normative representation of the normative requirements in this section."
| The `FixedInfo` field MUST be serialized in the following order, | ||
| with single `0x00` separator bytes between fields: | ||
|
|
||
| ``` | ||
| UTF8("AWS-KMS-ML-KEM-KEY-DERIVATION") || 0x00 || | ||
| UTF8(parameter_set) || 0x00 || | ||
| UTF8("HMAC_SHA384") || 0x00 || | ||
| UTF8(kms_key_arn) || 0x00 || | ||
| keyring_version_byte || 0x00 || | ||
| canonicalized(encryption_context) | ||
| ``` |
There was a problem hiding this comment.
What if one of these fields has a 0x00 byte?
Why not have it as pairs of fields: first a fixed-byte length field describing the length of the next field, then the value of the field?
There was a problem hiding this comment.
That works.
Loop: parse length from the byte-length field, extract the following bytes.
2 bytes should be enough as a maximum length as well (2^15).
|
|
||
| The Key Derivation Function Configuration is defined as: | ||
|
|
||
| - Key Derivation Function: |
There was a problem hiding this comment.
Is there already spec written for KDF and are we using the same one? If yes, should be use the reference rather than writing it over here again?
There was a problem hiding this comment.
No. Other keyrings e.g. ECDH write a separate file for their own KDF
| Encapsulation source is configurable (KMS or local); | ||
| decapsulation is always performed by AWS KMS. |
There was a problem hiding this comment.
how will you ensure that the one of the corresponding keys in encapsulation is a KMS key? Or will we not and just error?
There was a problem hiding this comment.
Caller is responsible for supplying the correct KMS public key, keyring does not do a verification.
| - **`LocalEncapsulation`** — | ||
| `OnEncrypt` MUST perform ML-KEM `Encapsulate` locally against the configured | ||
| [ML-KEM public key](#ml-kem-public-key). | ||
| If the public key is not supplied at configuration time, | ||
| the keyring MUST obtain it once via AWS KMS `GetPublicKey` and cache it. |
There was a problem hiding this comment.
if we ever provide a raw ML-KEM keyring, this configuration will get confusing. I suggest you always require a public key and fail to construct the keyring if they dont
There was a problem hiding this comment.
Agreed, will require public key for local encapsulate
| - For `KmsEncapsulation`, the keyring MUST call AWS KMS `Encapsulate` | ||
| with a request constructed as follows: | ||
| - `KeyId` MUST be the configured AWS KMS key identifier. | ||
| - `EncapsulationAlgorithm` MUST be `ML_KEM`. | ||
| - `GrantTokens` MUST be this keyring's grant tokens. |
There was a problem hiding this comment.
do you not need to supply an ML-KEM public key to KMS encapsulate? or is the public key too large to send over the wire?
There was a problem hiding this comment.
The KMS API for ML-KEM encapsulate does not accept the public key, only way is to refer to it using the key ARN, key ID, alias name, or alias ARN.
| ML-KEM's implicit rejection ensures that decapsulating an invalid KEM ciphertext | ||
| yields a pseudo-random shared secret rather than an error, | ||
| which deterministically derives a wrong wrapping key. |
There was a problem hiding this comment.
why is the AES-GCM aad tag failure the right place to error here? Why shouldn't we use a key commitment value?
There was a problem hiding this comment.
ML-KEM has implicit rejection by design and is mirrored here. The original KMS ML-KEM specs (https://amazon.awsapps.com/workdocs-amazon/index.html#/document/6a4acf0de7b43a8937e6333a449d9c58d3f11ab2c52b6811205cffca451d0097) in FAQ 24 cite it to be an intentional security feature by always returning a (possibly garbage) shared secret without any alerts
| identifying the ML-KEM KMS key | ||
| that produced the [KEM Ciphertext](#kem-ciphertext). | ||
|
|
||
| ### Ciphertext |
There was a problem hiding this comment.
Why are we not including the public keys used in the encapsulation process in the ciphertext?
There was a problem hiding this comment.
ML-KEM doesnt need any party's public key for decapsulation
| | [Test Vectors — Key Description](../../framework/test-vectors/key-description.md) | | ||
| | [Test Vectors — Keys Manifest](../../framework/test-vectors/keys-manifest.md) | | ||
| | [Test Vectors — MPL Enumeration](../../framework/test-vectors/mpl-test-vector-enumeration.md) | | ||
| | [Test Vectors — AWS KMS ML-KEM Keyring](../../framework/test-vectors/complete-vectors/ml-kem.md) (new) | |
There was a problem hiding this comment.
We need to make sure we run TV only for Java.
|
|
||
| | Language | Version Introduced | Version Removed | Implementation | | ||
| | -------- | ------------------ | --------------- | -------------- | | ||
| | Java | TBD | n/a | TBD | |
There was a problem hiding this comment.
I will add this to the backlog for after the ml-kem feature is merged. Current major version is 3.X, I assume 4.X marks PQ for the ESDK?
| ## Out of Scope | ||
|
|
||
| - A Raw ML-KEM Keyring that performs decapsulation locally. | ||
| - Cached shared-secret reuse across messages. |
There was a problem hiding this comment.
Q: Does CachingCMM not work with keyrings?
There was a problem hiding this comment.
It does, I will remove this line to avoid confusion
| A test MUST attempt to encrypt and decrypt | ||
| with every [algorithm suite](../../algorithm-suites.md#algorithm-suite-id). |
There was a problem hiding this comment.
What's the diff between this and what stated on line 19?
|
|
||
| For each ML-KEM parameter set, | ||
| a test MUST encrypt with `encapsulation` set to `kms` | ||
| and decrypt with `encapsulation` set to `local`, |
There was a problem hiding this comment.
Decrypt with encapsulation?
There was a problem hiding this comment.
Meant to say: decryption should work interchangeably with either KMS or local encryption.
| A test MUST verify that the deterministic portion of the keyring's wrapping | ||
| produces byte-exact output for fixed inputs. | ||
| The deterministic portion is the | ||
| [key derivation](../../aws-kms/aws-kms-ml-kem-keyring.md#key-derivation) |
There was a problem hiding this comment.
How is it related to ML-KEM? Isn't this testing KDF (and wrapping)?
There was a problem hiding this comment.
Will remove KDF tests as theyre generalizable and not specific to ML-KEM
| If the public key is not supplied at configuration time, | ||
| the keyring MUST obtain it once via AWS KMS `GetPublicKey` and cache it. |
There was a problem hiding this comment.
If the source is KMS, why do we need to call GetPublicKey? Isn't kms:encapsulate enough?
There was a problem hiding this comment.
This is for local encapsulate where we wouldn't want to use kms:encapsulate. I wanted to minimize kms calls for this local config, with the fallback being GetPublicKey if the user doesn't provide a key. For consistency I'll stick with Jose's suggestion and require a public key as input
| [encryption context serialization specification](../structures.md#serialization) | ||
| to the materials' encryption context. | ||
|
|
||
| This keyring does NOT use a key commitment construction. |
There was a problem hiding this comment.
It kind does by adding the EDK doesn't it?
|
|
||
| - The keyring MUST use the derived wrapping key as the AES-GCM cipher key. | ||
| - The keyring MUST use the plaintext data key as the AES-GCM message. | ||
| - The keyring MUST use a zeroed-out 12-byte IV. |
There was a problem hiding this comment.
I'll keep zeros for consistency
| Add an AWS KMS ML-KEM Keyring that protects data keys with a NIST-standardized | ||
| post-quantum Key Encapsulation Mechanism (ML-KEM, FIPS 203). | ||
| The keyring uses an ML-KEM KMS key to establish a per-message shared secret, | ||
| derives a wrapping key from it, and AES-GCM-wraps the data key. |
There was a problem hiding this comment.
nit: AES-GCM encrypts the data key, GCM doesn't have a Wrap mode
| ## Out of Scope | ||
|
|
||
| - A Raw ML-KEM Keyring that performs decapsulation locally. | ||
| - Cached shared-secret reuse across messages. |
There was a problem hiding this comment.
Is this MLKEM specific caching, or compatibility with existing data key caching?
There was a problem hiding this comment.
Referring to ML-KEM specific caching. Existing data key caching should still be compatible
| ## Version | ||
|
|
||
| 0.1.0-preview | ||
|
|
||
| ### Changelog | ||
|
|
||
| - 0.1.0-preview | ||
|
|
||
| - Initial record. |
There was a problem hiding this comment.
nit: I think we decided at some point to stop including this in each spec
| A keyring that uses an AWS KMS ML-KEM key | ||
| to establish a per-message shared secret, | ||
| derives a wrapping key from it, | ||
| and AES-GCM-wraps the data key. |
|
|
||
| For local encapsulation, an ML-KEM public key | ||
| MUST be a DER-encoded ASN.1 `SubjectPublicKeyInfo` as defined for ML-KEM in | ||
| [NIST FIPS 203](https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.203.pdf). |
There was a problem hiding this comment.
I will modify to say that the keyring must fail elsewise
|
|
||
| - The keyring MUST use the derived wrapping key as the AES-GCM cipher key. | ||
| - The keyring MUST use the plaintext data key as the AES-GCM message. | ||
| - The keyring MUST use a zeroed-out 12-byte IV. |
There was a problem hiding this comment.
editorial: we could use all ones instead of zeros given the validation issue with one of the Ruby interpreters but since the ESDK/MPL already use zeroed out IVs in many other places it's better to just be consistent.
There was a problem hiding this comment.
I'll keep zeros for consistency
| - The keyring MUST use the derived wrapping key as the AES-GCM cipher key. | ||
| - The keyring MUST use the plaintext data key as the AES-GCM message. | ||
| - The keyring MUST use a zeroed-out 12-byte IV. | ||
| - The keyring MUST use an authentication tag of length 16 bytes. |
There was a problem hiding this comment.
nit: maybe something more like "The keyring MUST encrypt the plaintext data key using an authentication tag..." to be more specific about when/where rather than just "the keyring"
| **Encryption context binding.** | ||
| The canonicalized encryption context is bound into the wrapping key via the KDF's | ||
| `FixedInfo` and into the data key wrap via the AES-GCM AAD; | ||
| both bindings MUST agree on decrypt. |
There was a problem hiding this comment.
nit: maybe avoid RFC2119 keyword here since you probably don't want these security considerations to be annotated. you could also organize them into a separate file and exclude that file from the Duvet report.
| [Data Key Wrapping](#data-key-wrapping) | ||
| does not violate AES-GCM nonce uniqueness across messages. | ||
|
|
||
| ## Multi-Keyring Compatibility |
There was a problem hiding this comment.
imo put this above security considerations since it describes requirements
| - The serialized [Ciphertext](../../aws-kms/aws-kms-ml-kem-keyring.md#ciphertext) | ||
| structure, given the fixed KEM ciphertext, salt, plaintext data key, and IV. | ||
|
|
||
| ### Negative tests |
There was a problem hiding this comment.
i suspect there are probably more failure modes involving invalid inputs or malformed ciphertexts
| encrypted data key. | ||
| Local encapsulation removes the per-message KMS call and the `kms:Encapsulate` | ||
| permission requirement on encrypt, | ||
| at the cost of trusting the local ML-KEM provider for `Encapsulate` correctness. |
There was a problem hiding this comment.
I am not sure if we should say this. We can definitely use the same crypto provider as KMS is using.
There was a problem hiding this comment.
I'll remove the line. Current plan and reviewed with crypto BR is to use ACCP
initial specs draft Co-authored-by: Lucas McDonald <lucasmcdonald3@gmail.com> Co-authored-by: José Corella <39066999+josecorella@users.noreply.github.com> Co-authored-by: Kess Plasmeier <76071473+kessplas@users.noreply.github.com>
…separated enumerations into multiple Duvet requirements for clarity.
…d serializing ciphertext in elvinli/mlkem-serialization
…ngle source of truth for encrypt/decrypt paths
…ovider info / ciphertext
…ferences for accuracy
Description of changes:
Added initial specifications for ML-KEM including keyring and key derivation.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.