-
Notifications
You must be signed in to change notification settings - Fork 30
Support RFC 3394 (AESKeyWrap with **no** padding) #658
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
Conversation
Manuthor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job. Just a few comments
HatemMn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will resume by trying to solve this : #662
c634e92 to
8cbd024
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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