Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
@RequestScoped
public class AWSSignatureProcessor implements SignatureProcessor {

private final static Logger LOG =
private static final Logger LOG =
LoggerFactory.getLogger(AWSSignatureProcessor.class);

@Context
Expand All @@ -58,9 +58,11 @@ public SignatureInfo parseSignature() throws OS3Exception {
String authHeader = headers.get("Authorization");

List<SignatureParser> signatureParsers = new ArrayList<>();
signatureParsers.add(new AuthorizationV4HeaderParser(authHeader));
signatureParsers.add(new AuthorizationV4HeaderParser(authHeader,
headers.get(StringToSignProducer.X_AMAZ_DATE)));
signatureParsers.add(new AuthorizationV4QueryParser(
context.getUriInfo().getQueryParameters()));
StringToSignProducer.fromMultiValueToSingleValueMap(
context.getUriInfo().getQueryParameters())));
signatureParsers.add(new AuthorizationV2HeaderParser(authHeader));

SignatureInfo signatureInfo = null;
Expand All @@ -73,7 +75,7 @@ public SignatureInfo parseSignature() throws OS3Exception {
if (signatureInfo == null) {
signatureInfo = new SignatureInfo(
Version.NONE,
"", "", "", "", "", "", false
"", "", "", "", "", "", "", false
);
}
return signatureInfo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public SignatureInfo parseSignature() throws OS3Exception {
return new SignatureInfo(
Version.V2,
"",
"",
accessKeyID,
signature,
"",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ public class AuthorizationV4HeaderParser implements SignatureParser {

private String authHeader;

public AuthorizationV4HeaderParser(String authHeader) {
private String dateHeader;

public AuthorizationV4HeaderParser(String authHeader, String dateHeader) {
this.authHeader = authHeader;
this.dateHeader = dateHeader;
}

/**
Expand Down Expand Up @@ -91,6 +94,7 @@ public SignatureInfo parseSignature() throws OS3Exception {
return new SignatureInfo(
Version.V4,
credentialObj.getDate(),
dateHeader,
credentialObj.getAccessKeyID(),
signature,
signedHeaders,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@
*/
package org.apache.hadoop.ozone.s3.signature;

import javax.ws.rs.core.MultivaluedMap;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.time.ZonedDateTime;
import java.util.Map;

import org.apache.hadoop.ozone.s3.exception.OS3Exception;
import org.apache.hadoop.ozone.s3.signature.SignatureInfo.Version;

import com.google.common.annotations.VisibleForTesting;
import static java.time.temporal.ChronoUnit.SECONDS;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -38,10 +41,10 @@ public class AuthorizationV4QueryParser implements SignatureParser {
private static final Logger LOG =
LoggerFactory.getLogger(AuthorizationV4QueryParser.class);

private final MultivaluedMap<String, String> queryParameters;
private final Map<String, String> queryParameters;

public AuthorizationV4QueryParser(
MultivaluedMap<String, String> queryParameters
Map<String, String> queryParameters
) {
this.queryParameters = queryParameters;
}
Expand All @@ -53,30 +56,43 @@ public SignatureInfo parseSignature() throws OS3Exception {
return null;
}

final String dateString = queryParameters.getFirst("X-Amz-Date");
final String expiresString = queryParameters.getFirst("X-Amz-Expires");
if (expiresString != null && expiresString.length() > 0) {
final Long expires =
Long.valueOf(expiresString);
validateDateAndExpires();

if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
throw new IllegalArgumentException("Pre-signed S3 url is expired");
}
}
final String rawCredential = queryParameters.get("X-Amz-Credential");

Credential credential =
new Credential(queryParameters.getFirst("X-Amz-Credential"));
null;
try {
credential = new Credential(URLDecoder.decode(rawCredential, "UTF-8"));
} catch (UnsupportedEncodingException e) {
throw new IllegalArgumentException(
"X-Amz-Credential is not proper URL encoded");
}

return new SignatureInfo(
Version.V4,
dateString,
credential.getDate(),
queryParameters.get("X-Amz-Date"),
credential.getAccessKeyID(),
queryParameters.getFirst("X-Amz-Signature"),
queryParameters.getFirst("X-Amz-SignedHeaders"),
queryParameters.get("X-Amz-Signature"),
queryParameters.get("X-Amz-SignedHeaders"),
credential.createScope(),
queryParameters.getFirst("X-Amz-Algorithm"),
queryParameters.get("X-Amz-Algorithm"),
false
);
}

@VisibleForTesting
protected void validateDateAndExpires() {
final String dateString = queryParameters.get("X-Amz-Date");
final String expiresString = queryParameters.get("X-Amz-Expires");
if (expiresString != null && expiresString.length() > 0) {
final Long expires = Long.valueOf(expiresString);

if (ZonedDateTime.parse(dateString, StringToSignProducer.TIME_FORMATTER)
.plus(expires, SECONDS).isBefore(ZonedDateTime.now())) {
throw new IllegalArgumentException("Pre-signed S3 url is expired");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,16 @@ public class SignatureInfo {

private Version version;

/**
* Information comes from the credential (Date only).
*/
private String date;

/**
* Information comes from header/query param (full timestamp).
*/
private String dateTime;

private String awsAccessId;

private String signature;
Expand All @@ -40,9 +48,11 @@ public class SignatureInfo {

private boolean signPayload = true;

@SuppressWarnings("checkstyle:ParameterNumber")
public SignatureInfo(
Version version,
String date,
String dateTime,
String awsAccessId,
String signature,
String signedHeaders,
Expand All @@ -52,6 +62,7 @@ public SignatureInfo(
) {
this.version = version;
this.date = date;
this.dateTime = dateTime;
this.awsAccessId = awsAccessId;
this.signature = signature;
this.signedHeaders = signedHeaders;
Expand Down Expand Up @@ -92,6 +103,10 @@ public boolean isSignPayload() {
return signPayload;
}

public String getDateTime() {
return dateTime;
}

public enum Version {
NONE, V4, V2;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
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;
Expand Down Expand Up @@ -84,7 +85,8 @@ public static String createSignatureBase(
context.getMethod(),
context.getUriInfo().getRequestUri().getPath(),
LowerCaseKeyStringMap.fromHeaderMap(context.getHeaders()),
context.getUriInfo().getQueryParameters());
fromMultiValueToSingleValueMap(
context.getUriInfo().getQueryParameters()));
}

@VisibleForTesting
Expand All @@ -94,7 +96,7 @@ public static String createSignatureBase(
String method,
String uri,
LowerCaseKeyStringMap headers,
MultivaluedMap<String, String> queryParams
Map<String, String> queryParams
) throws Exception {
StringBuilder strToSign = new StringBuilder();
// According to AWS sigv4 documentation, authorization header should be
Expand All @@ -114,7 +116,7 @@ public static String createSignatureBase(
uri = (uri.trim().length() > 0) ? uri : "/";
// Encode URI and preserve forward slashes
strToSign.append(signatureInfo.getAlgorithm() + NEWLINE);
strToSign.append(headers.get(X_AMAZ_DATE) + NEWLINE);
strToSign.append(signatureInfo.getDateTime() + NEWLINE);
strToSign.append(credentialScope + NEWLINE);

String canonicalRequest = buildCanonicalRequest(
Expand All @@ -125,7 +127,7 @@ public static String createSignatureBase(
headers,
queryParams,
!signatureInfo.isSignPayload());

System.out.println(canonicalRequest);
strToSign.append(hash(canonicalRequest));
if (LOG.isDebugEnabled()) {
LOG.debug("canonicalRequest:[{}]", canonicalRequest);
Expand All @@ -135,6 +137,16 @@ public static String createSignatureBase(
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));
Expand All @@ -148,7 +160,7 @@ public static String buildCanonicalRequest(
String uri,
String signedHeaders,
Map<String, String> headers,
MultivaluedMap<String, String> queryParams,
Map<String, String> queryParams,
boolean unsignedPayload
) throws OS3Exception {

Expand Down Expand Up @@ -193,7 +205,6 @@ public static String buildCanonicalRequest(
+ canonicalHeaders + NEWLINE
+ signedHeaders + NEWLINE
+ payloadHash;

return canonicalRequest;
}

Expand Down Expand Up @@ -247,24 +258,27 @@ private static String urlEncode(String str) {
}

private static String getQueryParamString(
MultivaluedMap<String, String> queryMap
Map<String, String> queryMap
) {
List<String> params = new ArrayList<>(queryMap.keySet());

// Sort by name, then by value
Collections.sort(params, (o1, o2) -> o1.equals(o2) ?
queryMap.getFirst(o1).compareTo(queryMap.getFirst(o2)) :
queryMap.get(o1).compareTo(queryMap.get(o2)) :
o1.compareTo(o2));

StringBuilder result = new StringBuilder();
for (String p : params) {
if (result.length() > 0) {
result.append("&");
}
result.append(urlEncode(p));
result.append('=');
if (!p.equals("X-Amz-Signature")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: This check is not there in the old code, newly added. Any reason for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the code which creates the canonical string of the query parameters to sign it later.

If the signature itself is added as a query parameter, it should be excluded from the string itself (to have a predictable string to sign).

TLDR: earlier we didn't support X-Amz-Signature query parameter therefore we ignored this exclusion.


result.append(urlEncode(queryMap.getFirst(p)));
if (result.length() > 0) {
result.append("&");
}
result.append(urlEncode(p));
result.append('=');

result.append(urlEncode(queryMap.get(p)));
}
}
return result.toString();
}
Expand Down
Loading