bharatviswa504 commented on a change in pull request #1628: URL: https://github.com/apache/ozone/pull/1628#discussion_r599473226
########## File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/AWSSignatureProcessor.java ########## @@ -0,0 +1,205 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.hadoop.ozone.s3.signature; + +import javax.enterprise.context.RequestScoped; +import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.MultivaluedMap; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.hadoop.ozone.s3.HeaderPreprocessor; +import org.apache.hadoop.ozone.s3.exception.OS3Exception; +import org.apache.hadoop.ozone.s3.signature.SignatureInfo.Version; + +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Parser to process AWS V2 & V4 auth request. Creates string to sign and auth + * header. For more details refer to AWS documentation https://docs.aws + * .amazon.com/general/latest/gr/sigv4-create-canonical-request.html. + **/ +@RequestScoped +public class AWSSignatureProcessor implements SignatureProcessor { + + private static final Logger LOG = + LoggerFactory.getLogger(AWSSignatureProcessor.class); + + @Context + private ContainerRequestContext context; + + public SignatureInfo parseSignature() throws OS3Exception { + + LowerCaseKeyStringMap headers = Review comment: Question: why do we convert header keys to lowercase? ########## File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java ########## @@ -125,7 +127,7 @@ public static String createSignatureBase( headers, queryParams, !signatureInfo.isSignPayload()); - + System.out.println(canonicalRequest); Review comment: Can we do Log here? I see it is already logged when debug enabled. do we want to remove this? ########## File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java ########## @@ -138,6 +146,21 @@ private OzoneClient getClient(OzoneConfiguration config) return ozoneClient; } + @NotNull + @VisibleForTesting + OzoneClient createOzoneClient() throws IOException { + if (omServiceID == null) { + return OzoneClientFactory.getRpcClient(ozoneConfiguration); + } else { + // As in HA case, we need to pass om service ID. + return OzoneClientFactory.getRpcClient(omServiceID, + ozoneConfiguration); + } + } + + private void getSignatureInfo() { Review comment: Minor: Unused method. ########## File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/signature/StringToSignProducer.java ########## @@ -0,0 +1,325 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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 org.apache.hadoop.ozone.s3.signature; + +import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.core.MultivaluedMap; +import java.io.UnsupportedEncodingException; +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URLEncoder; +import java.net.UnknownHostException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.time.LocalDate; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.hadoop.ozone.s3.exception.OS3Exception; +import org.apache.hadoop.ozone.s3.signature.AWSSignatureProcessor.LowerCaseKeyStringMap; +import org.apache.hadoop.util.StringUtils; + +import com.google.common.annotations.VisibleForTesting; +import static java.time.temporal.ChronoUnit.SECONDS; +import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.S3_AUTHINFO_CREATION_ERROR; +import org.apache.kerby.util.Hex; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Stateless utility to create stringToSign, the base of the signature. + */ +public final class StringToSignProducer { + + public static final String X_AMZ_CONTENT_SHA256 = "X-Amz-Content-SHA256"; + public static final String X_AMAZ_DATE = "X-Amz-Date"; + private static final Logger LOG = + LoggerFactory.getLogger(StringToSignProducer.class); + private static final Charset UTF_8 = StandardCharsets.UTF_8; + private static final String NEWLINE = "\n"; + private static final String HOST = "host"; + private static final String UNSIGNED_PAYLOAD = "UNSIGNED-PAYLOAD"; + /** + * Seconds in a week, which is the max expiration time Sig-v4 accepts. + */ + private static final long PRESIGN_URL_MAX_EXPIRATION_SECONDS = + 60 * 60 * 24 * 7; + public static final DateTimeFormatter TIME_FORMATTER = + DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'") + .withZone(ZoneOffset.UTC); + + private StringToSignProducer() { + } + + public static String createSignatureBase( + SignatureInfo signatureInfo, + ContainerRequestContext context + ) throws Exception { + return createSignatureBase(signatureInfo, + context.getUriInfo().getRequestUri().getScheme(), + context.getMethod(), + context.getUriInfo().getRequestUri().getPath(), + LowerCaseKeyStringMap.fromHeaderMap(context.getHeaders()), + fromMultiValueToSingleValueMap( + context.getUriInfo().getQueryParameters())); + } + + @VisibleForTesting + public static String createSignatureBase( + SignatureInfo signatureInfo, + String scheme, + String method, + String uri, + LowerCaseKeyStringMap headers, + Map<String, String> queryParams + ) throws Exception { + StringBuilder strToSign = new StringBuilder(); + // According to AWS sigv4 documentation, authorization header should be + // in following format. + // Authorization: algorithm Credential=access key ID/credential scope, + // SignedHeaders=SignedHeaders, Signature=signature + + // Construct String to sign in below format. + // StringToSign = + // Algorithm + \n + + // RequestDateTime + \n + + // CredentialScope + \n + + // HashedCanonicalRequest + String credentialScope = signatureInfo.getCredentialScope(); + + // If the absolute path is empty, use a forward slash (/) + uri = (uri.trim().length() > 0) ? uri : "/"; + // Encode URI and preserve forward slashes + strToSign.append(signatureInfo.getAlgorithm() + NEWLINE); + strToSign.append(signatureInfo.getDateTime() + NEWLINE); + strToSign.append(credentialScope + NEWLINE); + + String canonicalRequest = buildCanonicalRequest( + scheme, + method, + uri, + signatureInfo.getSignedHeaders(), + headers, + queryParams, + !signatureInfo.isSignPayload()); + System.out.println(canonicalRequest); + strToSign.append(hash(canonicalRequest)); + if (LOG.isDebugEnabled()) { + LOG.debug("canonicalRequest:[{}]", canonicalRequest); + LOG.debug("StringToSign:[{}]", strToSign); + } + + return strToSign.toString(); + } + + public static Map<String, String> fromMultiValueToSingleValueMap( + MultivaluedMap<String, String> queryParameters + ) { + Map<String, String> result = new HashMap<>(); + for (String key : queryParameters.keySet()) { + result.put(key, queryParameters.getFirst(key)); + } + return result; + } + + public static String hash(String payload) throws NoSuchAlgorithmException { + MessageDigest md = MessageDigest.getInstance("SHA-256"); + md.update(payload.getBytes(UTF_8)); + return Hex.encode(md.digest()).toLowerCase(); + } + + @VisibleForTesting + public static String buildCanonicalRequest( + String schema, + String method, + String uri, + String signedHeaders, + Map<String, String> headers, + Map<String, String> queryParams, + boolean unsignedPayload + ) throws OS3Exception { + + Iterable<String> parts = split("/", uri); + List<String> encParts = new ArrayList<>(); + for (String p : parts) { + encParts.add(urlEncode(p)); + } + String canonicalUri = join("/", encParts); + + String canonicalQueryStr = getQueryParamString(queryParams); + + StringBuilder canonicalHeaders = new StringBuilder(); + + for (String header : StringUtils.getStringCollection(signedHeaders, ";")) { + canonicalHeaders.append(header.toLowerCase()); + canonicalHeaders.append(":"); + if (headers.containsKey(header)) { + String headerValue = headers.get(header); + canonicalHeaders.append(headerValue); + canonicalHeaders.append(NEWLINE); + + // Set for testing purpose only to skip date and host validation. + validateSignedHeader(schema, header, headerValue); + + } else { + throw new RuntimeException("Header " + header + " not present in " + + "request but requested to be signed."); + } + } + + String payloadHash; + if (UNSIGNED_PAYLOAD.equals( + headers.get(X_AMZ_CONTENT_SHA256)) || unsignedPayload) { + payloadHash = UNSIGNED_PAYLOAD; + } else { + payloadHash = headers.get(X_AMZ_CONTENT_SHA256); + } + String canonicalRequest = method + NEWLINE + + canonicalUri + NEWLINE + + canonicalQueryStr + NEWLINE + + canonicalHeaders + NEWLINE + + signedHeaders + NEWLINE + + payloadHash; + return canonicalRequest; + } + + /** + * String join that also works with empty strings. + * + * @return joined string + */ + private static String join(String glue, List<String> parts) { + StringBuilder result = new StringBuilder(); + boolean addSeparator = false; + for (String p : parts) { + if (addSeparator) { + result.append(glue); + } + result.append(p); + addSeparator = true; + } + return result.toString(); + } + + /** + * Returns matching strings. + * + * @param regex Regular expression to split by + * @param whole The string to split + * @return pieces + */ + private static Iterable<String> split(String regex, String whole) { + Pattern p = Pattern.compile(regex); + Matcher m = p.matcher(whole); + List<String> result = new ArrayList<>(); + int pos = 0; + while (m.find()) { + result.add(whole.substring(pos, m.start())); + pos = m.end(); + } + result.add(whole.substring(pos)); + return result; + } + + private static String urlEncode(String str) { + try { + + return URLEncoder.encode(str, UTF_8.name()) + .replaceAll("\\+", "%20") Review comment: Why are we doing this? https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-query-string-auth.html Here there is an example urlEncode method is given, don't see such replace here? ########## File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java ########## @@ -57,6 +57,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; + Review comment: Minor: Unnecessary change ########## File path: hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/signature/TestStringToSignProducer.java ########## @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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 org.apache.hadoop.ozone.s3.signature; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.hadoop.ozone.s3.HeaderPreprocessor; +import org.apache.hadoop.ozone.s3.exception.OS3Exception; +import org.apache.hadoop.ozone.s3.signature.AWSSignatureProcessor.LowerCaseKeyStringMap; + +import org.junit.Assert; +import org.junit.Test; + +/** + * Test string2sign creation. + */ +public class TestStringToSignProducer { + + @Test + public void test() throws Exception { + + LowerCaseKeyStringMap headers = new LowerCaseKeyStringMap(); + headers.put("Content-Length", "123"); + headers.put("Host", "0.0.0.0:9878"); + headers.put("X-AMZ-Content-Sha256", "Content-SHA"); + headers.put("X-AMZ-Date", "123"); + headers.put("Content-Type", "ozone/mpu"); + headers.put(HeaderPreprocessor.ORIGINAL_CONTENT_TYPE, "streaming"); + + String authHeader = + "AWS4-HMAC-SHA256 Credential=AKIAJWFJK62WUTKNFJJA/20181009/us-east-1" + + "/s3/aws4_request, " + + "SignedHeaders=host;x-amz-content-sha256;x-amz-date;" + + "content-type, " + + "Signature" + + + "=db81b057718d7c1b3b8dffa29933099551c51d787b3b13b9e0f9ebed45982bf2"; + + headers.put("Authorization", + authHeader); + + Map<String, String> queryParameters = new HashMap<>(); + + final SignatureInfo signatureInfo = + new AuthorizationV4HeaderParser(authHeader, "123") { + @Override + public void validateDateRange(Credential credentialObj) + throws OS3Exception { + //NOOP + } + }.parseSignature(); + + headers.fixContentType(); + + final String signatureBase = + StringToSignProducer.createSignatureBase( + signatureInfo, + "http", + "GET", + "/buckets", + headers, + queryParameters); + + Assert.assertEquals( + "String to sign is invalid", Review comment: Not understood here message is String to sign is invalid But signatureBase is matching with signature. Could you explain how/what is being tested here and how the value is generated -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
