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

Introduce aws sigv4a request signer #303

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ lazy val flintCore = (project in file("flint-core"))
exclude ("com.fasterxml.jackson.core", "jackson-databind"),
"com.amazonaws" % "aws-java-sdk-cloudwatch" % "1.12.593"
exclude("com.fasterxml.jackson.core", "jackson-databind"),
"software.amazon.awssdk" % "auth-crt" % "2.25.23",
"org.scalactic" %% "scalactic" % "3.2.15" % "test",
"org.scalatest" %% "scalatest" % "3.2.15" % "test",
"org.scalatest" %% "scalatest-flatspec" % "3.2.15" % "test",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.flint.core.auth;

import com.amazonaws.auth.AWSSessionCredentials;
import com.amazonaws.auth.AWSCredentialsProvider;
import com.amazonaws.services.glue.model.InvalidStateException;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpEntityEnclosingRequest;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
import org.apache.http.HttpRequestInterceptor;
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.entity.BasicHttpEntity;
import org.apache.http.message.BasicHeader;
import org.apache.http.protocol.HttpContext;
import software.amazon.awssdk.auth.credentials.AwsSessionCredentials;
import software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute;
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
import software.amazon.awssdk.core.signer.Signer;
import software.amazon.awssdk.http.SdkHttpFullRequest;
import software.amazon.awssdk.http.SdkHttpMethod;
import software.amazon.awssdk.regions.Region;

import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.logging.Level;
import java.util.logging.Logger;

import static org.apache.http.protocol.HttpCoreContext.HTTP_TARGET_HOST;
import static org.opensearch.flint.core.auth.AWSRequestSigningApacheInterceptor.nvpToMapParams;
import static org.opensearch.flint.core.auth.AWSRequestSigningApacheInterceptor.skipHeader;

/**
* Interceptor for signing AWS requests according to Signature Version 4A.
* This interceptor processes HTTP requests, signs them with AWS credentials,
* and updates the request headers to include the signature.
*/
public class AWSRequestSigV4ASigningApacheInterceptor implements HttpRequestInterceptor {
private static final Logger LOG = Logger.getLogger(AWSRequestSigV4ASigningApacheInterceptor.class.getName());

private static final String HTTPS_PROTOCOL = "https";
private static final int HTTPS_PORT = 443;

private final String service;
private final String region;
private final Signer signer;
private final AWSCredentialsProvider awsCredentialsProvider;

/**
* Constructs an interceptor for AWS request signing with metadata access.
*
* @param service The AWS service name.
* @param region The AWS region for signing.
* @param signer The signer implementation.
* @param awsCredentialsProvider The credentials provider for metadata access.
*/
public AWSRequestSigV4ASigningApacheInterceptor(String service, String region, Signer signer, AWSCredentialsProvider awsCredentialsProvider) {
this.service = service;
this.region = region;
this.signer = signer;
this.awsCredentialsProvider = awsCredentialsProvider;
}

/**
* Processes and signs an HTTP request, updating its headers with the signature.
*
* @param request the HTTP request to process and sign.
* @param context the HTTP context associated with the request.
* @throws IOException if an I/O error occurs during request processing.
*/
@Override
public void process(HttpRequest request, HttpContext context) throws IOException {
SdkHttpFullRequest requestToSign = buildSdkHttpRequest(request, context);
SdkHttpFullRequest signedRequest = signRequest(requestToSign);
updateRequestHeaders(request, signedRequest.headers());
updateRequestEntity(request, signedRequest);
}

/**
* Builds an {@link SdkHttpFullRequest} from the Apache {@link HttpRequest}.
*
* @param request the HTTP request to process and sign.
* @param context the HTTP context associated with the request.
* @return an SDK HTTP request ready to be signed.
* @throws IOException if an error occurs while building the request.
*/
private SdkHttpFullRequest buildSdkHttpRequest(HttpRequest request, HttpContext context) throws IOException {
noCharger marked this conversation as resolved.
Show resolved Hide resolved
URIBuilder uriBuilder = parseUri(request);
SdkHttpFullRequest.Builder builder = SdkHttpFullRequest.builder()
.method(SdkHttpMethod.fromValue(request.getRequestLine().getMethod()))
noCharger marked this conversation as resolved.
Show resolved Hide resolved
.protocol(HTTPS_PROTOCOL)
.port(HTTPS_PORT)
.headers(headerArrayToMap(request.getAllHeaders()))
.rawQueryParameters(nvpToMapParams(uriBuilder.getQueryParams()));

HttpHost host = (HttpHost) context.getAttribute(HTTP_TARGET_HOST);
if (host == null) {
throw new InvalidStateException("Host must not be null");
}
builder.host(host.getHostName());
try {
builder.encodedPath(uriBuilder.build().getRawPath());
} catch (URISyntaxException e) {
throw new IOException("Invalid URI", e);
}
setRequestEntity(request, builder);
return builder.build();
}

/**
* Sets the request entity for the {@link SdkHttpFullRequest.Builder} if the original request contains an entity.
* This is used for requests that have a body, such as POST or PUT requests.
*
* @param request the original HTTP request.
* @param builder the SDK HTTP request builder.
*/
private void setRequestEntity(HttpRequest request, SdkHttpFullRequest.Builder builder) {
if (request instanceof HttpEntityEnclosingRequest) {
HttpEntity entity = ((HttpEntityEnclosingRequest) request).getEntity();
if (entity != null) {
builder.contentStreamProvider(() -> {
try {
return entity.getContent();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
});
}
}
}

private URIBuilder parseUri(HttpRequest request) throws IOException {
try {
return new URIBuilder(request.getRequestLine().getUri());
} catch (URISyntaxException e) {
throw new IOException("Invalid URI", e);
}
}

/**
* Signs the given SDK HTTP request using the provided AWS credentials and signer.
*
* @param request the SDK HTTP request to sign.
* @return a signed SDK HTTP request.
*/
private SdkHttpFullRequest signRequest(SdkHttpFullRequest request) {
AWSSessionCredentials sessionCredentials = (AWSSessionCredentials) awsCredentialsProvider.getCredentials();
AwsSessionCredentials awsCredentials = AwsSessionCredentials.create(
sessionCredentials.getAWSAccessKeyId(),
sessionCredentials.getAWSSecretKey(),
sessionCredentials.getSessionToken()
);

ExecutionAttributes executionAttributes = new ExecutionAttributes()
.putAttribute(AwsSignerExecutionAttribute.AWS_CREDENTIALS, awsCredentials)
.putAttribute(AwsSignerExecutionAttribute.SERVICE_SIGNING_NAME, service)
.putAttribute(AwsSignerExecutionAttribute.SIGNING_REGION, Region.of(region));

try {
return signer.sign(request, executionAttributes);
} catch (Exception e) {
LOG.log(Level.SEVERE, "Error Sigv4a signing the request", e);
throw e;
}
}

/**
* Updates the HTTP request headers with the signed headers.
*
* @param request the original HTTP request.
* @param signedHeaders the headers after signing.
*/
private void updateRequestHeaders(HttpRequest request, Map<String, List<String>> signedHeaders) {
Header[] headers = convertHeaderMapToArray(signedHeaders);
request.setHeaders(headers);
}

/**
* Updates the request entity based on the signed request. This is used to update the request body after signing.
*
* @param request the original HTTP request.
* @param signedRequest the signed SDK HTTP request.
*/
private void updateRequestEntity(HttpRequest request, SdkHttpFullRequest signedRequest) {
if (request instanceof HttpEntityEnclosingRequest) {
HttpEntityEnclosingRequest httpEntityEnclosingRequest = (HttpEntityEnclosingRequest) request;
signedRequest.contentStreamProvider().ifPresent(provider -> {
InputStream contentStream = provider.newStream();
BasicHttpEntity basicHttpEntity = new BasicHttpEntity();
basicHttpEntity.setContent(contentStream);
signedRequest.firstMatchingHeader("Content-Length").ifPresent(value ->
basicHttpEntity.setContentLength(Long.parseLong(value)));
signedRequest.firstMatchingHeader("Content-Type").ifPresent(basicHttpEntity::setContentType);
httpEntityEnclosingRequest.setEntity(basicHttpEntity);
});
}
}

/**
* Converts an array of {@link Header} objects into a map, consolidating multiple values for the same header name.
*
* @param headers the array of {@link Header} objects to convert.
* @return a map where each key is a header name and each value is a list of header values.
*/
private static Map<String, List<String>> headerArrayToMap(final Header[] headers) {
Map<String, List<String>> headersMap = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
for (Header header : headers) {
if (!skipHeader(header)) {
headersMap.computeIfAbsent(header.getName(), k -> new ArrayList<>()).add(header.getValue());
}
}
return headersMap;
}

/**
* Converts a map of headers back into an array of {@link Header} objects.
*
* @param mapHeaders the map of headers to convert.
* @return an array of {@link Header} objects.
*/
private Header[] convertHeaderMapToArray(final Map<String, List<String>> mapHeaders) {
return mapHeaders.entrySet().stream()
.map(entry -> new BasicHeader(entry.getKey(), String.join(",", entry.getValue())))
.toArray(Header[]::new);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public void process(final HttpRequest request, final HttpContext context)
* @param params list of HTTP query params as NameValuePairs
* @return a multimap of HTTP query params
*/
private static Map<String, List<String>> nvpToMapParams(final List<NameValuePair> params) {
static Map<String, List<String>> nvpToMapParams(final List<NameValuePair> params) {
Map<String, List<String>> parameterMap = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
for (NameValuePair nvp : params) {
List<String> argsList =
Expand Down Expand Up @@ -154,10 +154,11 @@ private static Map<String, String> headerArrayToMap(final Header[] headers) {
* @param header header line to check
* @return true if the given header should be excluded when signing
*/
private static boolean skipHeader(final Header header) {
static boolean skipHeader(final Header header) {
return ("content-length".equalsIgnoreCase(header.getName())
&& "0".equals(header.getValue())) // Strip Content-Length: 0
|| "host".equalsIgnoreCase(header.getName()); // Host comes from endpoint
&& "0".equals(header.getValue())) // Strip Content-Length: 0
|| "host".equalsIgnoreCase(header.getName()) // Host comes from endpoint
|| "connection".equalsIgnoreCase(header.getName()); // Skip setting Connection manually
Comment on lines +157 to +161

Choose a reason for hiding this comment

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

nit: better to check if header is against a set of headers

}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.flint.core.auth;

import com.amazonaws.auth.AWS4Signer;
import com.amazonaws.auth.AWSCredentialsProvider;
import com.amazonaws.auth.Signer;
import org.apache.http.HttpException;
import org.apache.http.HttpRequest;
import org.apache.http.HttpRequestInterceptor;
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.protocol.HttpContext;
import org.jetbrains.annotations.TestOnly;
import org.opensearch.common.Strings;
import software.amazon.awssdk.authcrt.signer.AwsCrtV4aSigner;

import java.io.IOException;
import java.net.URISyntaxException;
Expand All @@ -19,33 +27,45 @@ public class ResourceBasedAWSRequestSigningApacheInterceptor implements HttpRequ

private final String service;
private final String metadataAccessIdentifier;
final AWSRequestSigningApacheInterceptor primaryInterceptor;
final AWSRequestSigningApacheInterceptor metadataAccessInterceptor;
final HttpRequestInterceptor primaryInterceptor;
final HttpRequestInterceptor metadataAccessInterceptor;

/**
* Constructs an interceptor for AWS request signing with optional metadata access.
*
* @param service The AWS service name.
* @param signer The AWS request signer.
* @param region The AWS region for signing.
* @param primaryCredentialsProvider The credentials provider for general access.
* @param metadataAccessCredentialsProvider The credentials provider for metadata access.
* @param metadataAccessIdentifier Identifier for operations requiring metadata access.
*/
public ResourceBasedAWSRequestSigningApacheInterceptor(final String service,
final Signer signer,
final String region,
final AWSCredentialsProvider primaryCredentialsProvider,
final AWSCredentialsProvider metadataAccessCredentialsProvider,
final String metadataAccessIdentifier) {
this(service,
new AWSRequestSigningApacheInterceptor(service, signer, primaryCredentialsProvider),
new AWSRequestSigningApacheInterceptor(service, signer, metadataAccessCredentialsProvider),
metadataAccessIdentifier);
if (Strings.isNullOrEmpty(service)) {
throw new IllegalArgumentException("Service name must not be null or empty.");
}
if (Strings.isNullOrEmpty(region)) {
throw new IllegalArgumentException("Region must not be null or empty.");
}
this.service = service;
this.metadataAccessIdentifier = metadataAccessIdentifier;
AWS4Signer signer = new AWS4Signer();
noCharger marked this conversation as resolved.
Show resolved Hide resolved
signer.setServiceName(service);
signer.setRegionName(region);
this.primaryInterceptor = new AWSRequestSigningApacheInterceptor(service, signer, primaryCredentialsProvider);
this.metadataAccessInterceptor = primaryCredentialsProvider.equals(metadataAccessCredentialsProvider)
? this.primaryInterceptor
: new AWSRequestSigV4ASigningApacheInterceptor(service, region, AwsCrtV4aSigner.builder().build(), metadataAccessCredentialsProvider);
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a builder for AwsCrtV4asigner? Waht are the parameters available? Are we good with defaults. I see
plain constructor for AWS4Signer signer = new AWS4Signer();

Also when should we use AwsCrtV4asigner insread of AWS4Signer. what are the downsides of making AwsCrtV4asigner default.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why is there a builder for AwsCrtV4asigner? Waht are the parameters available? Are we good with defaults. I see plain constructor for AWS4Signer signer = new AWS4Signer();

Also when should we use AwsCrtV4asigner insread of AWS4Signer. what are the downsides of making AwsCrtV4asigner default.

AwsCrtV4aSigner is an interface that employs the builder pattern to instantiate. However, the AWS4Signer class has its own getter and setter.

@SdkPublicApi
@Immutable
@ThreadSafe
public interface AwsCrtV4aSigner extends Signer, Presigner {

    /**
     * Create a default Aws4aSigner.
     */
    static AwsCrtV4aSigner create() {
        return DefaultAwsCrtV4aSigner.create();
    }

    static Builder builder() {
        return DefaultAwsCrtV4aSigner.builder();
    }

    interface Builder {
        /**
         * The region scope that this signer will default to if not provided explicitly when the signer is invoked.
         *
         * @param defaultRegionScope The default region scope.
         * @return This builder for method chaining.
         */
        Builder defaultRegionScope(RegionScope defaultRegionScope);

        AwsCrtV4aSigner build();
    }
}

The main difference is that AWS4Signer utilizes AWS4-HMAC-SHA256, whereas AWSCrtV4asigner uses AWS4-ECDSA-P256-SHA256:

  • Algorithm: The primary difference is the algorithm used for signing requests: HMAC with SHA-256 in the former, and ECDSA with P-256 and SHA-256 in the latter.
  • Security: Both are considered secure, but ECDSA might offer better performance with equivalent levels of security due to the efficiency of elliptic curve cryptography.
  • Implementation: AWS4Signer is from aws-java-sdk V1, whereas AwsCrtV4asigner is from V2. They are incompatible with one another.

For the long run, we could use AwsCrtV4asigner as the default. However, in this PR, I would like to confine the blast radius to only metadata access using a specified credential provider.

Copy link
Collaborator

Choose a reason for hiding this comment

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

long term plan is replace signer with signer4a?, and deprecated AWSRequestSigningApacheInterceptor? In case the major difference of singer vs singer 4a is With AWS Signature Version 4A, the signature does not include Region-specific information and is calculated using the AWS4-ECDSA-P256-SHA256 algorithm.
I think we should use one Interceptor instead of two.

if it is correct, could u create a issue to track future maintain works.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

long term plan is replace signer with signer4a?, and deprecated AWSRequestSigningApacheInterceptor? In case the major difference of singer vs singer 4a is With AWS Signature Version 4A, the signature does not include Region-specific information and is calculated using the AWS4-ECDSA-P256-SHA256 algorithm. I think we should use one Interceptor instead of two.

if it is correct, could u create a issue to track future maintain works.

#321

}

// Test constructor allowing injection of mock interceptors
@TestOnly
ResourceBasedAWSRequestSigningApacheInterceptor(final String service,
final AWSRequestSigningApacheInterceptor primaryInterceptor,
final AWSRequestSigningApacheInterceptor metadataAccessInterceptor,
final HttpRequestInterceptor primaryInterceptor,
final HttpRequestInterceptor metadataAccessInterceptor,
final String metadataAccessIdentifier) {
this.service = service == null ? "unknown" : service;
this.primaryInterceptor = primaryInterceptor;
Expand Down Expand Up @@ -94,6 +114,6 @@ private String parseUriToPath(HttpRequest request) throws IOException {
* @return true if the operation requires metadata access credentials, false otherwise.
*/
private boolean isMetadataAccess(String resourcePath) {
return resourcePath.contains(metadataAccessIdentifier);
return !Strings.isNullOrEmpty(metadataAccessIdentifier) && resourcePath.contains(metadataAccessIdentifier);
}
}
Loading
Loading