Conversation
7d16d1b to
9b2cd4c
Compare
9b2cd4c to
982234f
Compare
|
@copilot search this PR for "TODO" comments, useless comments, or stuff I have "forgot" around |
|
Documentation isn't finished yet but it's just a matter of redaction The CLI, the UI, and the integration tests are all finished tho For now the only problem here is that, for some reason, when using an algorithm other that RSAES_OAEP_SHA_256 to wrap the AWS KMS returns an "invalid cyphertext error"... Until this is fixed I can safely say that RSAES_OAEP_SHA_256 wrapping is correctly implemented if an AWS BYOK use case is needed by someone, but please don't use the other ones before this is finished |
d3d79e0 to
8748da3
Compare
1ac2e52 to
1f2f99d
Compare
There was a problem hiding this comment.
Pull request overview
Adds AWS KMS BYOK support across the UI, CLI, docs, and CLI integration tests, including utilities to simulate AWS import/unwrapping locally via OpenSSL.
Changes:
- Adds AWS BYOK UI screens (import KEK + export key material) and navigation/routes.
- Introduces CLI
kms aws byok {import,export}commands and OpenSSL-based integration tests. - Updates documentation navigation and adds AWS BYOK docs + downloadable helper scripts.
Reviewed changes
Copilot reviewed 34 out of 38 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/menuItems.tsx | Adds AWS section entries in the UI menu. |
| ui/src/components/ExternalLink.tsx | Introduces a reusable external link component. |
| ui/src/CseInfo.tsx | Replaces raw <a> with ExternalLink. |
| ui/src/AzureImportKek.tsx | Replaces raw <a> with ExternalLink. |
| ui/src/AwsImportKek.tsx | Adds UI flow to import AWS KEK (file/base64) into KMS. |
| ui/src/AwsExportKeyMaterial.tsx | Adds UI flow to export wrapped key material for AWS import. |
| ui/src/App.tsx | Registers AWS routes and components. |
| documentation/mkdocs.yml | Adds AWS BYOK page and adjusts AWS doc paths. |
| documentation/docs/index.md | Adds AWS BYOK link + minor punctuation/wording tweaks. |
| documentation/docs/azure/byok/byok.md | Switches TOC to [TOC] macro. |
| documentation/docs/aws/byok.md | New AWS BYOK documentation page + script download links. |
| documentation/docs/aws/byok_scripts/*.sh | Adds scripts to automate AWS BYOK flows (RSA/ECC/AES). |
| crate/server/src/main.rs | Removes aws_xks_kek_user from embedded sample config. |
| crate/cli/src/tests/kms/shared/openssl_utils.rs | Adds OpenSSL helpers to simulate unwrap/import steps in tests. |
| crate/cli/src/tests/kms/shared/mod.rs | Exposes openssl_utils in test shared module. |
| crate/cli/src/tests/kms/mod.rs | Adds AWS test module. |
| crate/cli/src/tests/kms/azure/mod.rs | Reuses OpenSSL helpers & adds unwrap verification for Azure BYOK export. |
| crate/cli/src/tests/kms/aws/* | Adds AWS BYOK integration tests. |
| crate/cli/src/lib.rs | Allows additional clippy lint in tests. |
| crate/cli/src/actions/kms/shared/import_key.rs | Improves stdout success message punctuation. |
| crate/cli/src/actions/kms/console.rs | Adjusts stdout formatting/spacing and adds trailing blank line. |
| crate/cli/src/actions/kms/azure/byok/export_byok.rs | Fixes unwrap flag so wrapping key is applied. |
| crate/cli/src/actions/kms/aws/* | Adds AWS command modules + BYOK import/export implementation. |
| crate/cli/src/actions/kms/actions.rs | Registers kms aws commands. |
| crate/cli/src/actions/kms/aws/README.ms | Adds (temporary) AWS notes file. |
| .vscode/settings.json | Adds “Byok” to cSpell dictionary. |
Comments suppressed due to low confidence (3)
ui/src/AwsImportKek.tsx:1
- The
RsaOaepSha256andRsaAesKeyWrapSha256string values don’t encode the SHA-256 suffix while the SHA-1 variants do. Given the comment that these MUST match a kebab-case serialization, this asymmetry is a strong indicator of a mismatch that could select the wrong wrapping algorithm at export time. Align the string literals so SHA-256 values are explicitly represented (or source them from a single shared mapping that matches the Rust/wasm side).
ui/src/AwsExportKeyMaterial.tsx:1 - Using
String.fromCharCode(...wrappedKeyBytes)spreads the entire byte array into function arguments, which can throw (or be very slow) for larger buffers due to argument/stack limits. Prefer a chunked conversion or a dedicated bytes→base64 utility to avoid crashes when exporting larger wrapped key blobs.
ui/src/AwsExportKeyMaterial.tsx:1 - The sentence is grammatically incomplete (“imported into” is missing its object). Consider rewording to explicitly mention AWS KMS, e.g. “...to be imported into AWS KMS.”
| // Test constants from AWS KMS GetParametersForImport response | ||
| const TEST_KEY_ARN: &str = | ||
| "arn:aws:kms:eu-west-3:447182645454:key/e8518bca-e1d0-4519-a915-d80da8e8f38a"; | ||
|
|
||
| const TEST_PUBLIC_KEY_BASE64: &str = "MIIBojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEApujv1m1gfctmaIaWD4ns9b5MWrr2JwYJYo82Ri3AoQZkOq0BQKkBazO61Scn/+buRE57x5tYTfUTZdnwUe4OuGgTRmH/2SPbcILbpulLP31YnqEP5IxLnn7Z9NR6VODn0QiUyv/uaHE/uBD7mt1+KHKEOBn+rL53/ht3yrboGgqxKj84FITNPaiOZ7yTccB0yCqvlKWYpcrIPeTBdGlpXni10GyBxRqGfkmKuX9/rxwDlBbzdAXn9nHOmhhZlzBUHDzidXZvYrfWEqxfnYAuTbb0Dwj/7eTiFUKseV7NXU/KpAyIG3OghDjNF7PnKT7Zlf7CvSYE+9DOqadBzjQjbOu10lLdoo2nWfCtkvE5XrZkqJHHk+9DUBnkQX3I6MdCWlfTp8QWHiwbo8rFLC4ZSLCB/QqhTh8XnHwdVkmrDKhpYQH6m1pJcsG4sIICDwIkdMSkw/CHOk+bl76TIsVqCu/7QyvFLtsvIDG3Ia0qwshYpUuIoKxXfgwUuZiwSN2RAgMBAAE="; |
There was a problem hiding this comment.
TEST_PUBLIC_KEY_BASE64 is declared but not used in the test module, which can cause warnings (and potentially fail CI if warnings are denied). Remove it or use it in a test case.
documentation/docs/aws/byok.md
Outdated
|
|
||
| For this example, we will create an 2048 bits RSA key material, wrapped using a 4096 kek with RSAES_OAEP_SHA_256. | ||
|
|
||
| First, Sign in to the AWS Management Console and open the AWS Key Management Service (AWS KMS) console at [https://console.aws.amazon.com/kms](https://console.aws.amazon.com/kms) and complete the neccessary steps to [create a KMS key with external key material](). Be mindful to provide the correct [key spec](https://docs.aws.amazon.com/kms/latest/developerguide/symm-asymm-choose-key-spec.html) - otherwise the console will expect a symmetric key by default. For this example, we will use `RSA_2048`. |
There was a problem hiding this comment.
Correct the typo 'neccessary' to 'necessary'.
| First, Sign in to the AWS Management Console and open the AWS Key Management Service (AWS KMS) console at [https://console.aws.amazon.com/kms](https://console.aws.amazon.com/kms) and complete the neccessary steps to [create a KMS key with external key material](). Be mindful to provide the correct [key spec](https://docs.aws.amazon.com/kms/latest/developerguide/symm-asymm-choose-key-spec.html) - otherwise the console will expect a symmetric key by default. For this example, we will use `RSA_2048`. | |
| First, Sign in to the AWS Management Console and open the AWS Key Management Service (AWS KMS) console at [https://console.aws.amazon.com/kms](https://console.aws.amazon.com/kms) and complete the necessary steps to [create a KMS key with external key material](). Be mindful to provide the correct [key spec](https://docs.aws.amazon.com/kms/latest/developerguide/symm-asymm-choose-key-spec.html) - otherwise the console will expect a symmetric key by default. For this example, we will use `RSA_2048`. |
documentation/docs/aws/byok.md
Outdated
|
|
||
| First, Sign in to the AWS Management Console and open the AWS Key Management Service (AWS KMS) console at [https://console.aws.amazon.com/kms](https://console.aws.amazon.com/kms) and complete the neccessary steps to [create a KMS key with external key material](). Be mindful to provide the correct [key spec](https://docs.aws.amazon.com/kms/latest/developerguide/symm-asymm-choose-key-spec.html) - otherwise the console will expect a symmetric key by default. For this example, we will use `RSA_2048`. | ||
|
|
||
| The next step is to [download the wrapping public key and import token](https://docs.aws.amazon.com/kms/latest/developerguide/importing-keys-get-public-key-and-token.html#importing-keys-get-public-key-and-token-console). **Be mindful that an RSA_AES_KEY_WRAP_SHA_* wrapping algorithm is required for wrapping RSA private key material (except in China Regions).** Chosing `RSAES_OAEP_SHA_256` will work for this example. |
There was a problem hiding this comment.
Correct the typo 'Chosing' to 'Choosing'.
| The next step is to [download the wrapping public key and import token](https://docs.aws.amazon.com/kms/latest/developerguide/importing-keys-get-public-key-and-token.html#importing-keys-get-public-key-and-token-console). **Be mindful that an RSA_AES_KEY_WRAP_SHA_* wrapping algorithm is required for wrapping RSA private key material (except in China Regions).** Chosing `RSAES_OAEP_SHA_256` will work for this example. | |
| The next step is to [download the wrapping public key and import token](https://docs.aws.amazon.com/kms/latest/developerguide/importing-keys-get-public-key-and-token.html#importing-keys-get-public-key-and-token-console). **Be mindful that an RSA_AES_KEY_WRAP_SHA_* wrapping algorithm is required for wrapping RSA private key material (except in China Regions).** Choosing `RSAES_OAEP_SHA_256` will work for this example. |
| key_format: ImportKeyFormat::Aes, // Indicates this is an AES symmetric key | ||
| ..Default::default() | ||
| }; | ||
|
|
| std::fs::remove_file(&kek_file_path)?; | ||
| std::fs::remove_file(&output_file_path)?; |
There was a problem hiding this comment.
I would have created those files in temp directories (to avoid clean up)
HatemMn
left a comment
There was a problem hiding this comment.
Thanks for the review
You will notice some unexpected changes :
- Commented code in files unrealated to AWS because I deleted a "deadcode" clippy directive and it flagged dead code that's not mone so I just commented it
- new copilot instructions to avoid some false negatives
| key_format: ImportKeyFormat::Aes, // Indicates this is an AES symmetric key | ||
| ..Default::default() | ||
| }; | ||
|
|
|
Opened this issue : #757 Since those tests are not urgent, might be considered to merge this is everything is ok |
feat: work feat: work advance feat: first test feat: work
fix: last fixes before ui update + rebase feat: WIP on ui code feat: ui is good feat: some last fixes feat: more last fixes feat: ui fixes fix: more work fix: more work2 fix: restore cli commands(I have no idea where that disappeared) fix: fix some docs fix: stuff
feat: push the final E2E test script, document and reference them
Introduce 2 cli actions to permit BYOK flows with AWS KMS (docs below)
Create full flow integration tests - to avoid static mocks, utility functions that mimic (a correct) AWS KMS were created to run locally using openssl
Fix stdout console formatting when an export action happens
Checklist
Bonus :
added some scripts that automate the aws byok flow, they are placed with the docs and avalable to download
References :
Creating keys on amazon kms : https://docs.aws.amazon.com/kms/latest/developerguide/importing-keys-conceptual.html
Importing keys https://docs.aws.amazon.com/kms/latest/developerguide/importing-keys.html
Requirements for key material :https://docs.aws.amazon.com/kms/latest/developerguide/importing-keys-conceptual.html#importing-keys-material-requirements
Closes #685
Closes #650