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

Signature created with S3Presigner changed in 2.21.16+ when used with endpointOverride and http address #4697

Closed
WIStudent opened this issue Nov 14, 2023 · 4 comments
Assignees
Labels
bug This issue is a bug. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days.

Comments

@WIStudent
Copy link

Describe the bug

We are using minio on our local dev machines and noticed that the signature of presigned URLs has changed since version 2.21.16 when used with endpointOverride and an http address. Minio currently rejects this new signature.

We are using the following code to generate a presigned URL:

Consumer<GetObjectPresignRequest.Builder> buildPresignRequestFn = builder -> builder
  .signatureDuration(Duration.ofHours(1))
  .getObjectRequest(b -> b.bucket("bucket").key("given/key"));

final var awsCredentialsProvider = StaticCredentialsProvider.create(AwsBasicCredentials.create("minioadmin", "minioadmin"));
final var presigner = S3Presigner.builder()
  .credentialsProvider(awsCredentialsProvider)
  .region(Region.US_EAST_1)
  .endpointOverride(new URI("http://localhost:8082"))
  .build();

final var presignedUrl = presigner.presignGetObject(buildPresignRequestFn).url();

Forcing the X-Amz-Date to 19700101T000000Z using mocks results in the signature 501491751374ffbf1170c5808aaf827f51d6cc070ac4a6776fbbf87c7ce7a160 when used with 2.21.15 or older and 2683d70a5695b830615bd63e99a3cfa991596998576f0be9a010663493eeb1d0 when used with 2.21.16 or newer.

Expected Behavior

Versions <= 2.21.15 and >= 2.21.16 should produce the same signature.

Current Behavior

Versions <= 2.21.15 and >= 2.21.16 produce different signatures for http endpoints.

Reproduction Steps

import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.s3.presigner.S3Presigner;
import software.amazon.awssdk.services.s3.presigner.model.GetObjectPresignRequest;

import java.net.URI;
import java.net.URISyntaxException;
import java.time.Clock;
import java.time.Duration;
import java.time.Instant;
import java.util.function.Consumer;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.*;

class NewSignatureTest {

  @Test
  void signature() throws URISyntaxException {
    Consumer<GetObjectPresignRequest.Builder> buildPresignRequestFn = builder -> builder
        .signatureDuration(Duration.ofHours(1))
        .getObjectRequest(b -> b.bucket("bucket").key("given/key"));

    final var fakeNow = Instant.ofEpochMilli(600);

    final var fakeClock = mock(Clock.class);
    when(fakeClock.instant()).thenReturn(fakeNow);

    try (
      var instant = mockStatic(Instant.class, Mockito.CALLS_REAL_METHODS);
      var clock = mockStatic(Clock.class, CALLS_REAL_METHODS)
    ) {
      instant.when(Instant::now).thenReturn(fakeNow);
      clock.when(Clock::systemUTC).thenReturn(fakeClock);

      final var awsCredentialsProvider = StaticCredentialsProvider.create(AwsBasicCredentials.create("minioadmin", "minioadmin"));
      final var presigner = S3Presigner.builder()
        .credentialsProvider(awsCredentialsProvider)
        .region(Region.US_EAST_1)
        .endpointOverride(new URI("http://localhost:8082"))
        .build();

      final var presignedUrl = presigner.presignGetObject(buildPresignRequestFn).url();

      assertThat(presignedUrl)
        .hasProtocol("http")
        .hasHost("bucket.localhost")
        .hasPort(8082)
        .hasPath("/given/key")
        .hasParameter("X-Amz-Algorithm", "AWS4-HMAC-SHA256")
        .hasParameter("X-Amz-Date", "19700101T000000Z")
        .hasParameter("X-Amz-SignedHeaders", "host")
        .hasParameter("X-Amz-Expires", "3600")
        .hasParameter("X-Amz-Credential", "minioadmin/19700101/us-east-1/s3/aws4_request")
        .hasParameter("X-Amz-Signature", "501491751374ffbf1170c5808aaf827f51d6cc070ac4a6776fbbf87c7ce7a160");
    }
  }
}

Possible Solution

I debugged into it and noticed that contentHash on line 118 in V4RequestSigner has the value e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 (sha256 hash of an empty byte array) when used with an http endpoint and UNSIGNED-PAYLOAD when used with an https endpoint. I think the contentHash used to be UNSIGNED-PAYLOAD for both http and https endpoints in 2.21.15 and older.

Additional Information/Context

No response

AWS Java SDK version used

2.21.22

JDK version used

openjdk version "17.0.9" 2023-10-17 LTS

Operating System and version

Windows 11 with WSL

@WIStudent WIStudent added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 14, 2023
@debora-ito
Copy link
Member

@WIStudent thank you for reaching out. We are investigating.

@debora-ito
Copy link
Member

@WIStudent a fix was released and is included in version 2.21.25. Please let us know if you still see the issue after upgrading.

@debora-ito debora-ito added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Nov 17, 2023
@WIStudent
Copy link
Author

Updating to 2.21.25 fixed the issue.

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days.
Projects
None yet
Development

No branches or pull requests

2 participants