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.

Sign up for GitHub

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

HDDS-4763. Owner field of S3AUTHINFO type delegation token should be validated #1871

Merged
merged 3 commits into from
Feb 2, 2021
Merged

HDDS-4763. Owner field of S3AUTHINFO type delegation token should be validated #1871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
935a6ca
Owner field of S3AUTHINFO type delegation token should be validated
elek Feb 1, 2021
6255cff
Merge remote-tracking branch 'origin/master' into HDDS-4763
elek Feb 1, 2021
a7a8428
fix findbugs and unit test problems
elek Feb 1, 2021
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
13 changes: 13 additions & 0 deletions ...ger/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,19 @@ public boolean verifySignature(OzoneTokenIdentifier identifier,
private byte[] validateS3AuthInfo(OzoneTokenIdentifier identifier)
throws InvalidToken {
LOG.trace("Validating S3AuthInfo for identifier:{}", identifier);
if (identifier.getOwner() == null) {
throw new InvalidToken(
"Owner is missing from the S3 auth token");
}
if (!identifier.getOwner().toString().equals(identifier.getAwsAccessId())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @elek for working on this. I have one question about the awsaccessId from identifier or owner, they can all be modified by the attacker as well. What we need to check is compare awsaccessId from the identifier.stringtosign using s3 signatureparser (after verify the secret) with the one from the owner/awsid from the identifier.

Copy link
Member Author

Choose a reason for hiding this comment

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

awsaccessId is used as a primary key to find the stored secret in the rockdb. If you use any wrong accessId the signature validation will be failed. If the string2sign contains reference to a different accessId, the validation will be failed as the signature won't be matched.

(At least this is my understanding, but let me know if I missed something...)

Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense to me. I further check the hadoop rpc code and the UGI of token user is taken from the owner field of the identifier. Because S3 token identifier is not protected with a token signature like Hadoop delegation token, the proposed check of owner against awsaccessid is the right fix.

LOG.error(
"Owner and AWSAccessId is different in the S3 token. Possible "
+ " security attack: {}",
identifier);
throw new InvalidToken(
"Invalid S3 identifier: owner=" + identifier.getOwner()
+ ", awsAccessId=" + identifier.getAwsAccessId());
}
String awsSecret;
try {
awsSecret = s3SecretManager.getS3UserSecretString(identifier
Expand Down
28 changes: 15 additions & 13 deletions ...src/test/java/org/apache/hadoop/ozone/security/TestOzoneDelegationTokenSecretManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@

package org.apache.hadoop.ozone.security;

import java.io.File;
import java.io.IOException;
import java.security.KeyPair;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.Signature;
import java.security.cert.X509Certificate;
import java.util.HashMap;
import java.util.Map;

import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.security.x509.SecurityConfig;
import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
Expand All @@ -36,26 +46,16 @@
import org.apache.hadoop.security.token.Token;
import org.apache.hadoop.test.LambdaTestUtils;
import org.apache.hadoop.util.Time;

import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY;
import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMTokenProto.Type.S3AUTHINFO;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import java.io.File;
import java.io.IOException;
import java.security.KeyPair;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.Signature;
import java.security.cert.X509Certificate;
import java.util.HashMap;
import java.util.Map;

import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY;
import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMTokenProto.Type.S3AUTHINFO;

/**
* Test class for {@link OzoneDelegationTokenSecretManager}.
*/
Expand Down Expand Up @@ -342,6 +342,7 @@ public void testValidateS3AUTHINFOSuccess() throws Exception {
"20190221/us-west-1/s3/aws4_request\n" +
"c297c080cce4e0927779823d3fd1f5cae71481a8f7dfc7e18d91851294efc47d");
identifier.setAwsAccessId("testuser1");
identifier.setOwner(new Text("testuser1"));
secretManager.retrievePassword(identifier);
}

Expand All @@ -360,6 +361,7 @@ public void testValidateS3AUTHINFOFailure() throws Exception {
"20190221/us-west-1/s3/aws4_request\n" +
"c297c080cce4e0927779823d3fd1f5cae71481a8f7dfc7e18d91851294efc47d");
identifier.setAwsAccessId("testuser2");
identifier.setOwner(new Text("testuser2"));
// Case 1: User don't have aws secret set.
LambdaTestUtils.intercept(SecretManager.InvalidToken.class, " No S3 " +
"secret found for S3 identifier",
Expand Down