Gargi-jais11 commented on code in PR #10652:
URL: https://github.com/apache/ozone/pull/10652#discussion_r3655118217
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/AWSV4AuthValidator.java:
##########
@@ -105,18 +98,15 @@ private static byte[] getSigningKey(String key, String
strToSign) {
return kSigning;
}
- /**
- * Validate request by comparing Signature from request. Returns true if
- * aws request is legit else returns false.
- * Signature = HEX(HMAC_SHA256(key, String to Sign))
- *
- * For more details refer to AWS documentation: https://docs.aws.amazon.com
- * /AmazonS3/latest/API/sigv4-streaming.html
- */
public static boolean validateRequest(String strToSign, String signature,
String userKey) {
String expectedSignature = Hex.encode(sign(getSigningKey(userKey,
strToSign), strToSign));
return expectedSignature.equals(signature);
}
+
+ public static boolean validateChunk(String signature, String chunk, byte[]
derivedKey) {
Review Comment:
This seems to be a dead code if u are going to use it in follow up PR's
please add it as TODO or bring this in required PR.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java:
##########
@@ -202,11 +206,22 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager, Execut
// Prepare response. Sets user given full key name in the 'keyName'
// attribute in response object.
int clientVersion = getOmRequest().getVersion();
- omResponse.setCreateKeyResponse(CreateKeyResponse.newBuilder()
+ CreateKeyResponse.Builder createKeyResponseBuilder =
CreateKeyResponse.newBuilder()
.setKeyInfo(omFileInfo.getNetworkProtobuf(keyName, clientVersion,
keyArgs.getLatestVersionLocation()))
.setID(clientID)
- .setOpenVersion(openVersion).build())
+ .setOpenVersion(openVersion);
+ if (getOmRequest().hasS3Authentication() &&
ozoneManager.isSecurityEnabled()
+ && createKeyRequest.hasDerivedKeyPiggyBacking()
+ && createKeyRequest.getDerivedKeyPiggyBacking()
+ ) {
+ OzoneTokenIdentifier s3Token =
S3SecurityUtil.constructS3Token(getOmRequest());
+ if (s3Token.getTokenType().equals(OMTokenProto.Type.S3AUTHINFO)) {
+ byte[] derivedKey =
ozoneManager.getS3DerivedKey(s3Token.getAwsAccessId(), s3Token.getStrToSign());
+
createKeyResponseBuilder.setDerivedKey(ByteString.copyFrom(derivedKey));
+ }
+ }
+ omResponse.setCreateKeyResponse(createKeyResponseBuilder.build())
Review Comment:
1. The same 7-line derived-key block is copy-pasted in **OMKeyCreateRequest
and OMKeyCreateRequestWithFSO.** Any fix in one path can be missed in the other
(FSO is the default layout for new buckets).
2. Right now getS3DerivedKey() runs after addCacheEntry(). If secret lookup
fails, the open key is already in the cache but the client gets an error and
never commits — leaving an orphaned open key until cleanup.
Also, this runs in validateAndUpdateCache, which executes on all OM nodes
during Raft apply, so every follower does a secret lookup even though only the
leader’s response is returned to the client.
**Suggested order:** extract the shared helper first, then use it before
addCacheEntry in both classes. Optionally I think we can add preExecute
validation so invalid piggyback requests fail before Raft submission.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/AWSV4AuthValidator.java:
##########
@@ -75,25 +75,18 @@ private static byte[] sign(byte[] key, String msg) {
}
}
- /**
- * Returns signing key.
- *
- * @param key
- * @param strToSign
- *
- * SignatureKey = HMAC-SHA256(HMAC-SHA256(HMAC-SHA256(HMAC-SHA256("AWS4" +
- * "<YourSecretAccessKey>","20130524"),"us-east-1"),"s3"),"aws4_request")
- *
- * For more details refer to AWS documentation: https://docs.aws.amazon
- * .com/AmazonS3/latest/API/sig-v4-header-based-auth.html
- *
- * */
- private static byte[] getSigningKey(String key, String strToSign) {
- String[] signData = StringUtils.split(StringUtils.split(strToSign,
- '\n')[2], '/');
+ public static byte[] getSigningKey(String key, String strToSign) {
+ String credentialScope = StringUtils.split(strToSign, '\n')[2];
+ return getSigningKeyFromCredentialScope(key, credentialScope);
+ }
+
+ private static byte[] getSigningKeyFromCredentialScope(String key, String
credentialScope) {
+ String[] signData = StringUtils.split(credentialScope, '/');
String dateStamp = signData[0];
String regionName = signData[1];
String serviceName = signData[2];
+ LOG.info("key: {}, dateStamp: {}, regionName: {}, serviceName: {}",
Review Comment:
The key parameter here is the raw AWS Secret Key retrieved from
s3SecretManager. This will appear in OM logs in plain text at INFO level, which
is on by default. This seems to be a credential-leak vulnerability.
Please entirely remove the log line or at most log at TRACE without the key
material.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java:
##########
@@ -351,12 +356,23 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager, Execut
omMetadataManager.getOpenKeyTable(getBucketLayout()).addCacheEntry(
dbOpenKeyName, omKeyInfo, trxnLogIndex);
- // Prepare response
- omResponse.setCreateKeyResponse(CreateKeyResponse.newBuilder()
+ CreateKeyResponse.Builder builder = CreateKeyResponse.newBuilder()
.setKeyInfo(omKeyInfo.getNetworkProtobuf(getOmRequest().getVersion(),
keyArgs.getLatestVersionLocation()))
.setID(clientID)
- .setOpenVersion(openVersion).build())
+ .setOpenVersion(openVersion);
+ if (omRequest.hasS3Authentication() && ozoneManager.isSecurityEnabled()
+ && createKeyRequest.hasDerivedKeyPiggyBacking()
+ && createKeyRequest.getDerivedKeyPiggyBacking()
+ ) {
+ OzoneTokenIdentifier s3Token =
S3SecurityUtil.constructS3Token(omRequest);
+ if (s3Token.getTokenType().equals(OMTokenProto.Type.S3AUTHINFO)) {
+ byte[] derivedKey =
ozoneManager.getS3DerivedKey(s3Token.getAwsAccessId(), s3Token.getStrToSign());
+ builder.setDerivedKey(ByteString.copyFrom(derivedKey));
+ }
Review Comment:
When derivedKeyPiggyBacking=true but the token type is not S3AUTHINFO, the
derived key is silently omitted from the response. S3 Gateway has no signal
that the piggyback was skipped.
```
if (!s3Token.getTokenType().equals(OMTokenProto.Type.S3AUTHINFO)) {
// Piggyback was requested but this token type cannot produce a derived
key.
// S3 Gateway should only set this flag for S3AUTHINFO tokens.
LOG.warn("Derived key piggyback requested but token type is {}, " +
"not S3AUTHINFO. Derived key will not be returned.",
s3Token.getTokenType());
}
byte[] derivedKey = ozoneManager.getS3DerivedKey(
s3Token.getAwsAccessId(), s3Token.getStrToSign());
builder.setDerivedKey(ByteString.copyFrom(derivedKey))
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
