From 320be0edf8aee1d96cc2e00500db61354e685d8d Mon Sep 17 00:00:00 2001 From: Jordan Zimmerman Date: Thu, 16 May 2024 22:12:12 +0100 Subject: [PATCH] Remove MinioHeaders - it's unnecessary. Just return the auth value --- .../server/credentials/SigningController.java | 8 ++--- .../trino/s3/proxy/server/minio/Signer.java | 11 +++---- .../server/minio/emulation/MinioHeaders.java | 31 ------------------- .../proxy/server/rest/TrinoS3ProxyClient.java | 30 ++++-------------- .../proxy/server/TestSigningController.java | 9 +++--- 5 files changed, 18 insertions(+), 71 deletions(-) delete mode 100644 trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/minio/emulation/MinioHeaders.java diff --git a/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/credentials/SigningController.java b/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/credentials/SigningController.java index e254a6ea..0504ac21 100644 --- a/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/credentials/SigningController.java +++ b/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/credentials/SigningController.java @@ -24,7 +24,6 @@ import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; import java.util.List; -import java.util.Map; import java.util.Optional; import static java.util.Objects.requireNonNull; @@ -83,13 +82,12 @@ public boolean validateRequest(String method, MultivaluedMap req return Scope.fromHeaders(requestHeaders).map(scope -> { // TODO CredentialsEntry credentials = credentialsController.credentials(scope.accessKey).orElseThrow().emulated(); - Map signedRequestHeaders = signedRequestHeaders(scope, credentials, method, requestHeaders, encodedPath, encodedQuery); - String requestAuthorization = signedRequestHeaders.get("Authorization"); + String requestAuthorization = signedRequestHeaders(scope, credentials, method, requestHeaders, encodedPath, encodedQuery); return scope.authorization.equals(requestAuthorization); }).orElse(false); } - public Map signedRequestHeaders(Scope scope, CredentialsEntry credentials, String method, MultivaluedMap requestHeaders, String encodedPath, String encodedQuery) + public String signedRequestHeaders(Scope scope, CredentialsEntry credentials, String method, MultivaluedMap requestHeaders, String encodedPath, String encodedQuery) { MinioUrl minioUrl = MinioUrl.build(encodedPath, encodedQuery); MinioRequest minioRequest = MinioRequest.build(requestHeaders, method, minioUrl); @@ -98,7 +96,7 @@ public Map signedRequestHeaders(Scope scope, CredentialsEntry cr String sha256 = minioRequest.headerValue("x-amz-content-sha256").orElseThrow(); try { - return Signer.signV4S3(minioRequest, scope.region, credentials.accessKey(), credentials.secretKey(), sha256).headers(); + return Signer.signV4S3(minioRequest, scope.region, credentials.accessKey(), credentials.secretKey(), sha256); } catch (NoSuchAlgorithmException | InvalidKeyException e) { // TODO diff --git a/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/minio/Signer.java b/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/minio/Signer.java index e721a762..cd7ddddc 100644 --- a/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/minio/Signer.java +++ b/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/minio/Signer.java @@ -21,9 +21,8 @@ import com.google.common.collect.Multimap; import com.google.common.collect.MultimapBuilder; import com.google.common.io.BaseEncoding; -import io.trino.s3.proxy.server.minio.emulation.MinioHeaders; -import io.trino.s3.proxy.server.minio.emulation.MinioUrl; import io.trino.s3.proxy.server.minio.emulation.MinioRequest; +import io.trino.s3.proxy.server.minio.emulation.MinioUrl; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -301,7 +300,7 @@ public static String getChunkSignature( /** * Returns signed request object for given request, region, access key and secret key. */ - private static MinioHeaders signV4( + private static String signV4( String serviceName, MinioRequest request, String region, @@ -320,13 +319,13 @@ private static MinioHeaders signV4( signer.setSignature(); signer.setAuthorization(); - return MinioHeaders.of("Authorization", signer.authorization); + return signer.authorization; } /** * Returns signed request of given request for S3 service. */ - public static MinioHeaders signV4S3( + public static String signV4S3( MinioRequest request, String region, String accessKey, String secretKey, String contentSha256) throws NoSuchAlgorithmException, InvalidKeyException { @@ -336,7 +335,7 @@ public static MinioHeaders signV4S3( /** * Returns signed request of given request for STS service. */ - public static MinioHeaders signV4Sts( + public static String signV4Sts( MinioRequest request, String region, String accessKey, String secretKey, String contentSha256) throws NoSuchAlgorithmException, InvalidKeyException { diff --git a/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/minio/emulation/MinioHeaders.java b/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/minio/emulation/MinioHeaders.java deleted file mode 100644 index fd2db5b7..00000000 --- a/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/minio/emulation/MinioHeaders.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.trino.s3.proxy.server.minio.emulation; - -import com.google.common.collect.ImmutableMap; - -import java.util.Map; - -public record MinioHeaders(Map headers) -{ - public MinioHeaders - { - headers = ImmutableMap.copyOf(headers); - } - - public static MinioHeaders of(String name, String value) - { - return new MinioHeaders(ImmutableMap.of(name, value)); - } -} diff --git a/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/rest/TrinoS3ProxyClient.java b/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/rest/TrinoS3ProxyClient.java index e6cc21e5..8e2d7705 100644 --- a/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/rest/TrinoS3ProxyClient.java +++ b/trino-s3-proxy/src/main/java/io/trino/s3/proxy/server/rest/TrinoS3ProxyClient.java @@ -33,12 +33,8 @@ import java.lang.annotation.Target; import java.net.URI; import java.time.Duration; -import java.util.Map; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.util.concurrent.MoreExecutors.shutdownAndAwaitTermination; @@ -119,33 +115,19 @@ public void proxyRequest(ContainerRequest request, AsyncResponse asyncResponse, String encodedPath = firstNonNull(uri.getRawPath(), ""); String encodedQuery = firstNonNull(uri.getRawQuery(), ""); - Map signedRequestHeaders = signingController.signedRequestHeaders(scope, credentials, request.getMethod(), requestHeaders, encodedPath, encodedQuery); - signedRequestHeaders.forEach(requestBuilder::addHeader); + String authorization = signingController.signedRequestHeaders(scope, credentials, request.getMethod(), requestHeaders, encodedPath, encodedQuery); + requestBuilder.addHeader("Authorization", authorization); requestHeaders.forEach((name, values) -> { - if (!signedRequestHeaders.containsKey(name)) { + if (!name.equals("Authorization")) { values.forEach(value -> requestBuilder.addHeader(name, value)); } }); - try { - Request proxyRequest = requestBuilder.build(); + Request proxyRequest = requestBuilder.build(); - // TODO config a mask time to wait - Duration maxWaitForResponse = Duration.ofHours(1); - executorService.submit(() -> httpClient.execute(proxyRequest, new StreamingResponseHandler(asyncResponse, maxWaitForResponse))).get(maxWaitForResponse.toMillis(), TimeUnit.MILLISECONDS); - } - catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new WebApplicationException(Response.Status.SERVICE_UNAVAILABLE); - } - catch (ExecutionException e) { - // TODO - throw new WebApplicationException(e, Response.Status.INTERNAL_SERVER_ERROR); - } - catch (TimeoutException e) { - throw new WebApplicationException(Response.Status.REQUEST_TIMEOUT); - } + // TODO config a max time to wait + executorService.submit(() -> httpClient.execute(proxyRequest, new StreamingResponseHandler(asyncResponse, Duration.ofHours(1)))); } private String buildHost(String bucket, String region) diff --git a/trino-s3-proxy/src/test/java/io/trino/s3/proxy/server/TestSigningController.java b/trino-s3-proxy/src/test/java/io/trino/s3/proxy/server/TestSigningController.java index c3ce0c3d..1f72d9c7 100644 --- a/trino-s3-proxy/src/test/java/io/trino/s3/proxy/server/TestSigningController.java +++ b/trino-s3-proxy/src/test/java/io/trino/s3/proxy/server/TestSigningController.java @@ -22,7 +22,6 @@ import jakarta.ws.rs.core.MultivaluedMap; import org.junit.jupiter.api.Test; -import java.util.Map; import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; @@ -60,9 +59,9 @@ public void testRootLs() requestHeaders.putSingle("User-Agent", "aws-cli/2.15.16 Python/3.11.7 Darwin/22.6.0 source/x86_64 prompt/off command/s3.ls"); requestHeaders.putSingle("Accept-Encoding", "identity"); - Map signedHeaders = signingController.signedRequestHeaders(new Scope("dummy", "THIS_IS_AN_ACCESS_KEY", "us-east-1"), CREDENTIALS.emulated(), "GET", requestHeaders, "/", ""); + String authorization = signingController.signedRequestHeaders(new Scope("dummy", "THIS_IS_AN_ACCESS_KEY", "us-east-1"), CREDENTIALS.emulated(), "GET", requestHeaders, "/", ""); - assertThat(signedHeaders).contains(Map.entry("Authorization", "AWS4-HMAC-SHA256 Credential=THIS_IS_AN_ACCESS_KEY/20240516/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-security-token, Signature=9a19c251bf4e1533174e80da59fa57c65b3149b611ec9a4104f6944767c25704")); + assertThat(authorization).isEqualTo("AWS4-HMAC-SHA256 Credential=THIS_IS_AN_ACCESS_KEY/20240516/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-security-token, Signature=9a19c251bf4e1533174e80da59fa57c65b3149b611ec9a4104f6944767c25704"); } @Test @@ -79,8 +78,8 @@ public void testBucketLs() requestHeaders.putSingle("User-Agent", "aws-cli/2.15.16 Python/3.11.7 Darwin/22.6.0 source/x86_64 prompt/off command/s3.ls"); requestHeaders.putSingle("Accept-Encoding", "identity"); - Map signedHeaders = signingController.signedRequestHeaders(new Scope("dummy", "THIS_IS_AN_ACCESS_KEY", "us-east-1"), CREDENTIALS.emulated(), "GET", requestHeaders, "/mybucket", "list-type=2&prefix=foo%2Fbar&delimiter=%2F&encoding-type=url"); + String authorization = signingController.signedRequestHeaders(new Scope("dummy", "THIS_IS_AN_ACCESS_KEY", "us-east-1"), CREDENTIALS.emulated(), "GET", requestHeaders, "/mybucket", "list-type=2&prefix=foo%2Fbar&delimiter=%2F&encoding-type=url"); - assertThat(signedHeaders).contains(Map.entry("Authorization", "AWS4-HMAC-SHA256 Credential=THIS_IS_AN_ACCESS_KEY/20240516/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-security-token, Signature=222d7b7fcd4d5560c944e8fecd9424ee3915d131c3ad9e000d65db93e87946c4")); + assertThat(authorization).isEqualTo("AWS4-HMAC-SHA256 Credential=THIS_IS_AN_ACCESS_KEY/20240516/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-security-token, Signature=222d7b7fcd4d5560c944e8fecd9424ee3915d131c3ad9e000d65db93e87946c4"); } }