implementation of CKM_RSA_AES_KEY_WRAP mechanism by keldonin · Pull Request #480 · latchset/kryoptic

keldonin · 2026-07-12T21:45:44Z

Description

This PR adds support for CKM_RSA_AES_KEY_WRAP wrapping mechanism, that combines in one go RSA wrapping of a temporary AES key, then AES wrapping of any extractable key.

Checklist

Test suite updated
Rustdoc string were added or updated
CHANGELOG and/or other documentation added or updated
This is not a code change

Reviewer's checklist:

Any issues marked for closing are fully addressed
There is a test suite reasonably covering new functionality or modifications
This feature/change has adequate documentation added
A changelog entry is added if the change is significant
Code conform to coding style that today cannot yet be enforced via the check style test
Commits have short titles and sensible text
Doc string are properly updated

Copilot

Pull request overview

This PR adds PKCS#11 v3.1 support for the CKM_RSA_AES_KEY_WRAP composite wrapping mechanism (RSA-OAEP wrap of a temporary AES key + AES-KWP wrap of the target key), along with test vectors, warnings for weak wrapping strength, and FIPS indicators/changelog updates.

Changes:

Implemented CKM_RSA_AES_KEY_WRAP in the RSA mechanism layer and registered it in the mechanism table.
Added unit tests plus an OpenSSL-generated interoperability test vector (JSON) for unwrap validation.
Added weak-wrap warning utilities and a FIPS indicators table entry; updated the changelog.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
testdata/rsa_aes_key_wrap.json	Adds an OpenSSL-generated interop vector for RSA-AES key wrap framing and unwrap validation.
src/tests/rsa.rs	Adds wrap/unwrap round-trip tests and an interoperability unwrap test using the JSON vector.
src/rsa.rs	Implements and registers the `CKM_RSA_AES_KEY_WRAP` mechanism, including ephemeral AES key generation and composite wrap/unwrap logic.
src/misc.rs	Adds helper logic to detect/log weak key wrap strength relative to AES key size.
src/fips/indicators.rs	Extends the FIPS mechanism indicators table to include `CKM_RSA_AES_KEY_WRAP`.
CHANGELOG.md	Notes the new mechanism support under Unreleased.

Comments suppressed due to low confidence (1)

CHANGELOG.md:11

The Unreleased entry is currently listed before the "### What Changed" section, leaving that section empty. This is inconsistent with the formatting used in the rest of the changelog (bullets are under "### What Changed").

## [Unreleased]

* Added support for CKM_RSA_AES_KEY_WRAP mechanism

### What Changed

Jakuje · 2026-07-13T12:42:46Z

+            return Err(CKR_WRAPPING_KEY_TYPE_INCONSISTENT)?;
+        }
+
+        let params = mech.get_parameters::<CK_RSA_AES_KEY_WRAP_PARAMS>()?;


I think this is a fix for the get_parameters() function, rather than this place.

+            return Err(CKR_UNWRAPPING_KEY_TYPE_INCONSISTENT)?;
+        }
+
+        let params = mech.get_parameters::<CK_RSA_AES_KEY_WRAP_PARAMS>()?;


+
+        /* AES-KWP output length (C2), per NIST SP800-38F: the input padded to
+         * an 8-byte boundary plus an 8-byte integrity check block */
+        let c2_len = ((keydata.len() + AES_BLOCK_SIZE - 1) / 8) * 8;


keldonin · 2026-07-13T11:47:51Z

@simo5 I didn't realize there was a similar PR in the queue, although it seems to be hanging for a while now. I'm fine either way, provided that support for this mechanism is eventually merged into the code base.

Let me know of your pref,

Regards

Jakuje · 2026-07-13T12:59:34Z

The #448 did not get update for months so we would better get one active merged than none.

Jakuje · 2026-07-13T12:40:47Z

+
+    /// Implements the RSA AES key wrap operation (Wrap)
+    ///
+    /// [RSA AES KEY WRAP](https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/os/pkcs11-spec-v3.1-os.html#_Toc111203524)


I think the 3.2 is already out so I would reference the latest issue here:

https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.2/pkcs11-spec-v3.2.html#_Toc234407237

Jakuje · 2026-07-13T12:42:46Z

+            return Err(CKR_WRAPPING_KEY_TYPE_INCONSISTENT)?;
+        }
+
+        let params = mech.get_parameters::<CK_RSA_AES_KEY_WRAP_PARAMS>()?;


I think this is a fix for the get_parameters() function, rather than this place.

Jakuje · 2026-07-13T13:09:41Z

+                | CKF_DECRYPT
+                | CKF_WRAP
+                | CKF_UNWRAP
+                | CKF_DERIVE,


We have the CKF_DERIVE on other wrapping mechanisms, but not on the output AES key itself, see the AES key definition above in FipsKeyType.
If I see right, the only AES Derive operation that exists in specs are CKM_AES_CBC_ENCRYPT_DATA and CKM_AES_ECB_ENCRYPT_DATA, which are not FIPS methods so I think the genflags should not contain the DERIVE. Otherwise the derve should be also on the AES key to be consistent. This is more to @simo5 to clarify FIPS considerations here.

Jakuje · 2026-07-13T13:13:22Z

+            (CKA_SENSITIVE, true),
+            (CKA_UNWRAP, true),
+            (CKA_DECRYPT, true),
+            (CKA_EXTRACTABLE, true),


does the wrapping key need to be extractable for this test?

Jakuje · 2026-07-13T13:19:20Z

+            (CKA_EXPONENT_2, exponent_2.as_slice()),
+            (CKA_COEFFICIENT, coefficient.as_slice()),
+        ],
+        &[(CKA_UNWRAP, true), (CKA_DECRYPT, true)],


Is the explicit DECRYPT flag on the private key needed for the unwrapping operation? I guess this is due to the reuse of the decrypt operation, which does the flag check. I wonder if we should not bypass that in this case (and do the checks only externally to low-level encryption/decryption functions).

I know the usage could be limited with the ALLOWED_MECHANISMS, but I would consider limiting the usage also on the attributes level to either wrapping or encryption without the need for the other one.

Jakuje · 2026-07-13T13:24:33Z

+        assert!(!is_weak_key_wrap(112, 128));
+        assert!(!is_weak_key_wrap(128, 128));
+        assert!(!is_weak_key_wrap(80, 128));


nit: could these be sorted by size?

Jakuje · 2026-07-13T13:36:16Z

+/// Mechanism object implementing the CKM_RSA_AES_KEY_WRAP composite key
+/// wrapping mechanism.
+static RSA_AES_KW_MECH: LazyLock<Box<dyn Mechanism>> = LazyLock::new(|| {
+    Box::new(RsaAesKeyWrapMechanism {
+        info: CK_MECHANISM_INFO {
+            ulMinKeySize: CK_ULONG::try_from(MIN_RSA_SIZE_BITS).unwrap(),
+            ulMaxKeySize: CK_ULONG::try_from(MAX_RSA_SIZE_BITS).unwrap(),
+            flags: CKF_WRAP | CKF_UNWRAP,
+        },
+    })
+});
+


nit: why not include this mechanism into the RSA_MECHS table? I do not have a strong opinion either way, but we have all the others. Not sure if there is some performance difference in having one or more LazyLock, boxes.

Jakuje · 2026-07-13T13:46:11Z

+            ulParameterLen: CK_ULONG::try_from(std::mem::size_of::<
+                CK_RSA_PKCS_OAEP_PARAMS,


could we use the convenience macro sizeof! here from misc.rs?

Suggested change

ulParameterLen: CK_ULONG::try_from(std::mem::size_of::<

CK_RSA_PKCS_OAEP_PARAMS,

ulParameterLen: sizeof!(CK_RSA_PKCS_OAEP_PARAMS),

implementation of CKM_RSA_AES_KEY_WRAP mechanism

f6e04d8

Copilot AI review requested due to automatic review settings July 12, 2026 21:45

Copilot started reviewing on behalf of keldonin July 12, 2026 21:46 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

Jakuje reviewed Jul 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

implementation of CKM_RSA_AES_KEY_WRAP mechanism#480

implementation of CKM_RSA_AES_KEY_WRAP mechanism#480
keldonin wants to merge 1 commit into
latchset:mainfrom
keldonin:implement_ckm_rsa_aes_key_wrap

keldonin commented Jul 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

keldonin commented Jul 13, 2026

Uh oh!

Jakuje commented Jul 13, 2026

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

Jakuje Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		ulParameterLen: CK_ULONG::try_from(std::mem::size_of::<
		CK_RSA_PKCS_OAEP_PARAMS,

	ulParameterLen: CK_ULONG::try_from(std::mem::size_of::<
	CK_RSA_PKCS_OAEP_PARAMS,
	ulParameterLen: sizeof!(CK_RSA_PKCS_OAEP_PARAMS),

Uh oh!

Conversation

keldonin commented Jul 12, 2026

Description

Checklist

Reviewer's checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keldonin commented Jul 13, 2026

Uh oh!

Jakuje commented Jul 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants