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

Adding x-amz-content-sha256 header for signed requests #339

Merged
Merged
Show file tree
Hide file tree
Changes from 5 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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Fix version and build ([#254](https://github.com/opensearch-project/opensearch-java/pull/254))
- Update Gradle to 7.6 ([#309](https://github.com/opensearch-project/opensearch-java/pull/309))
- Prevent SPI calls at runtime ([#293](https://github.com/opensearch-project/opensearch-java/pull/293))
- Add support for OpenSearch Serverless ([#339](https://github.com/opensearch-project/opensearch-java/pull/339))

### Deprecated

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ public void close() {
}

@CheckForNull
private <RequestT> OpenSearchRequestBodyBuffer prepareRequestBody(
protected <RequestT> OpenSearchRequestBodyBuffer prepareRequestBody(
RequestT request,
Endpoint<RequestT, ?, ?> endpoint,
TransportOptions options
Expand All @@ -321,7 +321,7 @@ private <RequestT> OpenSearchRequestBodyBuffer prepareRequestBody(
return null;
}

private <RequestT> SdkHttpFullRequest prepareRequest(
protected <RequestT> SdkHttpFullRequest prepareRequest(
RequestT request,
Endpoint<RequestT, ?, ?> endpoint,
@CheckForNull TransportOptions options,
Expand Down Expand Up @@ -363,6 +363,9 @@ private <RequestT> SdkHttpFullRequest prepareRequest(
}
req.putHeader("Content-Length", String.valueOf(body.getContentLength()));
req.contentStreamProvider(body::getInputStream);
// To add the "X-Amz-Content-Sha256" header, it needs to set as required.
// It is a required header for Amazon OpenSearch Serverless.
req.putHeader("x-amz-content-sha256", "required");
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it took me a while to find too. We use Aws4Signer which does not have the sign method overriden like Aws4UnsignedPayloadSigner. According to documentation, the Aws4UnsignedPayloadSigner is similar to Aws4Signer but just adds UNSIGNED-PAYLOAD when protocol is HTTPS. Can we use Aws4UnsignedPayloadSigner?

Copy link
Member

Choose a reason for hiding this comment

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

If it works we sure can I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried the Aws4UnsignedPayloadSigner but it does not work for Amazon OpenSearch Service since it signs the payload with UNSIGNED_PAYLOAD over https protocol. Works for Amazon OpenSearch Serverless. I can PR this change on their repo may be as a new signer class but for now looks like we might have to use the hard-coded "required".

}

boolean responseCompression = Optional.ofNullable(options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,19 @@
import org.junit.Assert;
import org.opensearch.client.opensearch.OpenSearchAsyncClient;
import org.opensearch.client.opensearch.OpenSearchClient;
import org.opensearch.client.opensearch._types.ErrorResponse;
import org.opensearch.client.opensearch._types.OpType;
import org.opensearch.client.opensearch._types.OpenSearchException;
import org.opensearch.client.opensearch._types.Refresh;
import org.opensearch.client.opensearch.core.IndexRequest;
import org.opensearch.client.opensearch.core.IndexResponse;
import org.opensearch.client.opensearch.core.SearchResponse;
import org.opensearch.client.opensearch.indices.CreateIndexRequest;
import org.opensearch.client.opensearch.indices.CreateIndexResponse;
import org.opensearch.client.opensearch.indices.OpenSearchIndicesClient;
import org.opensearch.client.transport.JsonEndpoint;
import org.opensearch.client.util.OpenSearchRequestBodyBuffer;
import software.amazon.awssdk.http.SdkHttpFullRequest;

import java.util.List;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -150,4 +155,22 @@ public void testDoubleWrappedException() throws Exception {
// error message contains the actual error, not a generic [http_exception]
Assert.assertTrue(exception.getMessage().contains("[resource_already_exists_exception]"));
}

@Test
public void testContentShaHeader() throws Exception {
CreateIndexRequest request = new CreateIndexRequest.Builder().index(TEST_INDEX).build();
JsonEndpoint<CreateIndexRequest, CreateIndexResponse, ErrorResponse> endpoint =
(JsonEndpoint<CreateIndexRequest, CreateIndexResponse, ErrorResponse>) CreateIndexRequest._ENDPOINT;
AwsSdk2TransportMock transport = new AwsSdk2TransportMock(
getHttpClient(),
getTestClusterHost(),
getTestClusterServiceName(),
getTestClusterRegion(),
getTransportOptions().build()
);
OpenSearchRequestBodyBuffer requestBody = transport.prepareRequestBody(request, endpoint, transport.options());
SdkHttpFullRequest clientReq = transport.prepareRequest(request, endpoint, transport.options(), requestBody);
// Headers of the signed request contains "x-amz-content-sha256" since it is set to required
Assert.assertTrue(clientReq.headers().containsKey("x-amz-content-sha256"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.client.opensearch.integTest.aws;

import org.opensearch.client.transport.Endpoint;
import org.opensearch.client.transport.TransportOptions;
import org.opensearch.client.transport.aws.AwsSdk2Transport;
import org.opensearch.client.transport.aws.AwsSdk2TransportOptions;
import org.opensearch.client.util.OpenSearchRequestBodyBuffer;
import software.amazon.awssdk.http.SdkHttpClient;
import software.amazon.awssdk.http.SdkHttpFullRequest;
import software.amazon.awssdk.regions.Region;

import java.io.IOException;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

public class AwsSdk2TransportMock extends AwsSdk2Transport {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that "mock" is correct naming here. That implies something that "fakes" a certain operation, which this does not.


public AwsSdk2TransportMock(
@Nonnull SdkHttpClient httpClient,
@Nonnull String host,
@Nonnull String signingServiceName,
@Nonnull Region signingRegion,
@CheckForNull AwsSdk2TransportOptions options) {
super(httpClient, host, signingServiceName, signingRegion, options);
}

@Override
public <RequestT> OpenSearchRequestBodyBuffer prepareRequestBody(RequestT request,
Endpoint<RequestT, ?, ?> endpoint, TransportOptions options) throws IOException {
return super.prepareRequestBody(request, endpoint, options);
}
@Override
public <RequestT> SdkHttpFullRequest prepareRequest(RequestT request, Endpoint<RequestT, ?, ?> endpoint,
TransportOptions options, OpenSearchRequestBodyBuffer body) {
return super.prepareRequest(request, endpoint, options, body);
}
}