Support RFC 3394 (AESKeyWrap with **no** padding)#658
Conversation
Manuthor
left a comment
There was a problem hiding this comment.
Good job. Just a few comments
c634e92 to
8cbd024
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for RFC 3394 (AES Key Wrap without padding) to comply with Azure EKM specifications. The major change is a remapping of BlockCipherMode values: NISTKeyWrap now refers to RFC 3394 (no padding), while AESKeyWrapPadding refers to RFC 5649 (with padding). The PR includes backward compatibility migration for legacy databases that stored the old vendor-extension value 0x8000_000D.
Key changes:
- Implemented RFC 3394 key wrap/unwrap operations using OpenSSL
- Refactored RFC 5649 implementation to use OpenSSL directly instead of manual implementation
- Added migration logic to convert legacy
BlockCipherMode::LegacyNISTKeyWrap(0x8000_000D) toAESKeyWrapPadding(0x0000_000C) - Updated default wrapping algorithm from
NISTKeyWraptoAESKeyWrapPadding(RFC 5649) - Added support for 24-byte KEKs in addition to 16 and 32-byte KEKs for both RFCs
- Added deprecation warnings for RFC 3394 usage, recommending RFC 5649 instead
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
ui/src/KeysExport.tsx |
Added aes-key-wrap-padding as a wrapping algorithm option and updated labels to clarify RFC 3394 vs RFC 5649 |
crate/server_database/src/tests/mod.rs |
Added calls to new migration test for all database backends |
crate/server_database/src/tests/database_tests.rs |
Added test to verify legacy BlockCipherMode migration from LegacyNISTKeyWrap to AESKeyWrapPadding |
crate/server_database/src/stores/sql/sqlite.rs |
Applied migration function after deserializing objects from SQLite |
crate/server_database/src/stores/sql/pgsql.rs |
Applied migration function after deserializing objects from PostgreSQL |
crate/server_database/src/stores/sql/mysql.rs |
Applied migration function after deserializing objects from MySQL |
crate/server_database/src/stores/redis/objects_db.rs |
Applied migration function after deserializing objects from Redis |
crate/server_database/src/lib.rs |
Added migrate_block_cipher_mode_if_needed function to convert legacy BlockCipherMode values |
crate/server_database/README.md |
Fixed GitHub link to use https protocol |
crate/kmip/src/ttlv/xml/deserializer.rs |
Added FIXME comment for legacy NISTKeyWrap XML deserialization |
crate/kmip/src/kmip_0/kmip_types.rs |
Remapped BlockCipherMode values: NISTKeyWrap now 0x0000_000D (RFC 3394), added deprecated LegacyNISTKeyWrap as 0x8000_000D |
crate/crypto/src/crypto/wrap/wrap_key.rs |
Changed default BlockCipherMode from NISTKeyWrap to AESKeyWrapPadding |
crate/crypto/src/crypto/symmetric/symmetric_ciphers.rs |
Added RFC 3394 cipher variants, routing logic, and deprecation warnings; refactored constants |
crate/crypto/src/crypto/symmetric/rfc5649.rs |
Complete rewrite to use OpenSSL's native AES Key Wrap with Padding implementation |
crate/crypto/src/crypto/symmetric/rfc3394.rs |
New file implementing RFC 3394 (AES Key Wrap without padding) using OpenSSL |
crate/crypto/src/crypto/symmetric/mod.rs |
Added rfc3394 module export |
crate/crypto/src/crypto/rsa/ckm_rsa_aes_key_wrap.rs |
Updated comment to reference AESKeyWrapPadding instead of NistKeyWrap |
crate/client_utils/src/symmetric_utils.rs |
Added RFC 3394 constants and separated them from RFC 5649 constants |
crate/client_utils/src/export_utils.rs |
Added AESKeyWrapPadding as a wrapping algorithm option |
crate/cli/src/tests/kms/shared/export_import.rs |
Added AESKeyWrapPadding to wrapping algorithm tests |
crate/cli/src/tests/kms/shared/export.rs |
Removed explicit wrapping_algorithm parameter to test default behavior |
crate/cli/src/tests/kms/secret_data/create_secret.rs |
Added AESKeyWrapPadding to wrapping algorithm tests |
crate/cli/src/actions/kms/symmetric/mod.rs |
Added RFC3394 and RFC5649 as separate key encryption algorithms with correct BlockCipherMode mappings |
crate/cli/src/actions/kms/symmetric/decrypt.rs |
Removed unnecessary blank lines (formatting) |
.vscode/settings.json |
Added trailing comma for JSON formatting |
8faf49b to
01acb19
Compare
There was a problem hiding this comment.
I think work here is probably over, I will rebase azure EKM on this
The last modifications are squashed on this commit to ease review 01acb19
Possible to read the commit message to see what's new (reviews fixes + ui fixes + migrate fixes + a test)
fix: major bugé feat: add a lot of things fis: add back provider for ci tests fix:clip fix: a lot more stuff fix: a lot more stuff2 fix: some fixes fix: finish up
feat: missing file fix: ui fix: review fixes fix: grammar fixes
01acb19 to
2d39ef8
Compare
Manuthor
left a comment
There was a problem hiding this comment.
LGTM, thanks for the fixes.
This PR is mandatory for Azure EKM specs : #601
Due to the nature of this update, it has been separated from the #601 in order to focus on keeping the code consistent and avoid dept
Overview :
New BlockCipherMode mapping :
According to available specifications, from now on, when choosing :
BlockCipherMode::NISTKeyWrap -> The RFC 3394 Algorithm will be used
BlockCipherMode::AESKeyWrapPadding -> RFC 5694
In any case, it is preffered to avoid using 3394 as it's deprecated in favor of the 5694
Previously saved keys in a database will still be compatible and nothing has to be done, the code will fix their blockCipherMode to the new mapping if set
However , any previously manually exported key in the JSON format should be manually fixed. This can be trivially done using the following command :
sed -i 's/NISTKeyWrap/AESKeyWrapPadding/g' your_exported_key.json