Skip to content
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

feat: support FIPS endpoints for AWS KMS #245

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sbstjn
Copy link

@sbstjn sbstjn commented Sep 22, 2024

AWS services offer FIPS endpoints for some services and regions. For AWS KMS, all regions support FIPS endpoints. When using the CLI, you can normally use --endpoint-url https://kms-fips.us-east-1.amazonaws.com and when using any AWS SDK, you can configure the endpoint "mode" by setting AWS_USE_FIPS_ENDPOINT in the environment to true.

This change tries to add comperable support for this.

default / current : kms.us-east-1.amazonaws.com
fips endpoint: kms-fips.us-east-1.amazonaws.com

Details:
https://docs.aws.amazon.com/sdkref/latest/guide/feature-endpoints.html

KMS Service Endpoints:
https://docs.aws.amazon.com/general/latest/gr/kms.html#kms_region

@ebourg
Copy link
Owner

ebourg commented Sep 22, 2024

Thank you for the PR, I wasn't aware of the AWS FIPS mode. It would be nice to make the FIPS mode usable even without setting an environment variable. The AmazonSigningService constructor could be modified to accept either a region or a full endpoint URL, or maybe a boolean to enable the FIPS mode.

@sbstjn
Copy link
Author

sbstjn commented Sep 22, 2024

I wanted to simulate the default AWS behaviour, as I don't want to have an additional configuration for jsign about FIPS and for AWS CLI etc.

@@ -259,7 +274,7 @@ void sign(HttpURLConnection conn, AmazonCredentials credentials, byte[] content,
String host = endpoint.getHost();
Matcher matcher = hostnamePattern.matcher(host);
String regionName = matcher.matches() ? matcher.group(2) : "us-east-1";
String serviceName = matcher.matches() ? matcher.group(1) : "kms";
String serviceName = "kms";
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather strip the -fips suffix here, so this method remains generic for any AWS service

Copy link
Author

Choose a reason for hiding this comment

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

But that class is only intended to access KMS or are there other services involved that I miss?

* @param region the AWS region
* @return the endpoint URL
*/
public static String getEndpointUrl(String region) {
Copy link
Owner

Choose a reason for hiding this comment

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

Is it possible to make this method non static and package privarte?

Copy link
Author

Choose a reason for hiding this comment

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

I assumed, if I want to test this individually, it needs to be public. Don't know that much about Java ;) Can give it a try later …

Copy link
Owner

Choose a reason for hiding this comment

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

It remains testable from a test class in the same package if the public keyword is removed, that's the "package private" visibility.

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