Skip to content

Conversation

@bgrieder
Copy link
Contributor

@bgrieder bgrieder commented Dec 16, 2025

Check PR review below for a description

@bgrieder bgrieder requested a review from Copilot December 16, 2025 18:53
Copy link
Contributor

Copilot AI left a 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 AWS XKS (External Key Store) v2, enabling the KMS server to act as an XKS proxy that integrates with AWS KMS. The implementation includes AWS SigV4 authentication middleware, health status monitoring, key metadata retrieval, and encrypt/decrypt operations.

Key changes:

  • Added AWS XKS configuration options and parameter structures
  • Implemented SigV4 authentication middleware for AWS requests
  • Added XKS API endpoints (health status, key metadata, encrypt, decrypt)
  • Updated Rust toolchain from 1.90.0 to 1.91.0

Reviewed changes

Copilot reviewed 30 out of 31 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
rust-toolchain.toml Updated Rust version to 1.91.0
crate/server/src/routes/aws_xks/* New AWS XKS implementation including middleware, endpoints, and error handling
crate/server/src/config/* Added AWS XKS configuration parameters
crate/server/src/core/operations/{encrypt,decrypt}.rs Improved error handling to distinguish between authorization and not-found errors
crate/server/src/start_kms_server.rs Integrated AWS XKS service endpoints into server
Cargo.toml Added new dependencies for AWS signature verification and HTTP handling

request.requestMetadata.kmsRequestId,
request.requestMetadata.awsPrincipalArn
);
debug!("encrypt request: {:?}", request.requestMetadata);
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug message incorrectly says 'encrypt request' when this is the decrypt endpoint. Should say 'decrypt request'.

Suggested change
debug!("encrypt request: {:?}", request.requestMetadata);
debug!("decrypt request: {:?}", request.requestMetadata);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 38 changed files in this pull request and generated 7 comments.

#[allow(clippy::struct_field_names)]
pub struct AwsXksConfig {
/// This setting turns on endpoints handling the AWS XKS feature
#[clap(long, env = "KMS_AWX_XKS_ENABLE", default_value = "false")]
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of environment variable prefix from 'AWX' to 'AWS' to match AWS naming conventions.

Copilot uses AI. Check for mistakes.
/// The AWS XKS region to use for signing requests (sigv4)
#[clap(
long,
env = "KMS_AWX_XKS_REGION",
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of environment variable prefix from 'AWX' to 'AWS' to match AWS naming conventions.

Copilot uses AI. Check for mistakes.
/// The AWS XKS service name to use for signing requests (sigv4)
#[clap(
long,
env = "KMS_AWX_XKS_SERVICE",
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of environment variable prefix from 'AWX' to 'AWS' to match AWS naming conventions.

Copilot uses AI. Check for mistakes.

#[clap(
long,
env = "KMS_AWX_XKS_SIGV4_ACCESS_KEY_ID",
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of environment variable prefix from 'AWX' to 'AWS' to match AWS naming conventions.

Copilot uses AI. Check for mistakes.

#[clap(
long,
env = "KMS_AWX_XKS_SIGV4_SECRET_ACCESS_KEY",
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of environment variable prefix from 'AWX' to 'AWS' to match AWS naming conventions.

Copilot uses AI. Check for mistakes.

#[clap(
long,
env = "KMS_AWX_XKS_KEK_USER",
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of environment variable prefix from 'AWX' to 'AWS' to match AWS naming conventions.

Copilot uses AI. Check for mistakes.
- Start the server with XKS enabled:

```bash
cargo run --bin cosmian_kms_server -- --enable-xks-service
Copy link

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command line flag --enable-xks-service does not match the actual configuration structure which uses --aws-xks-enable. Update the documentation to use the correct flag.

Suggested change
cargo run --bin cosmian_kms_server -- --enable-xks-service
cargo run --bin cosmian_kms_server -- --aws-xks-enable

Copilot uses AI. Check for mistakes.
@bgrieder bgrieder marked this pull request as draft December 29, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants