You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by bh...@apache.org on 2020/11/03 04:34:28 UTC

[ozone] branch master updated: HDDS-4339. Allow AWSSignatureProcessor init when aws signature is absent. (#1498)

This is an automated email from the ASF dual-hosted git repository.

bharat pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/master by this push:
     new e800303  HDDS-4339. Allow AWSSignatureProcessor init when aws signature is absent. (#1498)
e800303 is described below

commit e800303e2dd5acac46bb133dcaa0c8f59936fa0f
Author: Li Cheng <bl...@gmail.com>
AuthorDate: Tue Nov 3 12:34:14 2020 +0800

    HDDS-4339. Allow AWSSignatureProcessor init when aws signature is absent. (#1498)
    
    Co-authored-by: Li Cheng <ti...@tencent.com>
    Co-authored-by: Bharat Viswanadham <bv...@cloudera.com>
---
 .../hadoop/ozone/s3/AWSSignatureProcessor.java     | 43 +++++++++++----
 .../hadoop/ozone/s3/OzoneClientProducer.java       | 61 +++++++++++++++++-----
 .../apache/hadoop/ozone/s3/SignatureProcessor.java |  2 +
 .../hadoop/ozone/s3/exception/S3ErrorTable.java    |  5 ++
 .../hadoop/ozone/s3/TestOzoneClientProducer.java   | 25 +++++++--
 .../hadoop/ozone/s3/endpoint/TestBucketPut.java    |  5 ++
 .../hadoop/ozone/s3/endpoint/TestRootList.java     |  5 ++
 7 files changed, 117 insertions(+), 29 deletions(-)

diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/AWSSignatureProcessor.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/AWSSignatureProcessor.java
index 0cb82fb..4d45101 100644
--- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/AWSSignatureProcessor.java
+++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/AWSSignatureProcessor.java
@@ -75,6 +75,7 @@ public class AWSSignatureProcessor implements SignatureProcessor {
   private AuthorizationHeaderV4 v4Header;
   private AuthorizationHeaderV2 v2Header;
   private String stringToSign;
+  private Exception exception;
 
   @PostConstruct
   public void init()
@@ -108,23 +109,38 @@ public class AWSSignatureProcessor implements SignatureProcessor {
 
     this.method = context.getMethod();
     String authHeader = headers.get(AUTHORIZATION_HEADER);
-    String[] split = authHeader.split(" ");
-    if (split[0].equals(AuthorizationHeaderV2.IDENTIFIER)) {
-      if (v2Header == null) {
-        v2Header = new AuthorizationHeaderV2(authHeader);
+    try {
+      if (authHeader != null) {
+        String[] split = authHeader.split(" ");
+        if (split[0].equals(AuthorizationHeaderV2.IDENTIFIER)) {
+          if (v2Header == null) {
+            v2Header = new AuthorizationHeaderV2(authHeader);
+          }
+        } else {
+          if (v4Header == null) {
+            v4Header = new AuthorizationHeaderV4(authHeader);
+          }
+          parse();
+        }
+      } else { // no auth header
+        v4Header = null;
+        v2Header = null;
       }
-    } else {
-      if (v4Header == null) {
-        v4Header = new AuthorizationHeaderV4(authHeader);
+    } catch (Exception ex) {
+      // During validation of auth header, create instance and set Exception.
+      // This way it can be handled in OzoneClientProducer creation of
+      // SignatureProcessor instance failure.
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Error during Validation of Auth Header:{}", authHeader);
       }
-      parse();
+      this.exception = ex;
     }
   }
 
 
-  public void parse() throws Exception {
-    StringBuilder strToSign = new StringBuilder();
+  private void parse() 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,
@@ -167,7 +183,8 @@ public class AWSSignatureProcessor implements SignatureProcessor {
   }
 
   @VisibleForTesting
-  public String buildCanonicalRequest() throws OS3Exception {
+  protected String buildCanonicalRequest() throws OS3Exception {
+
     Iterable<String> parts = split("/", uri);
     List<String> encParts = new ArrayList<>();
     for (String p : parts) {
@@ -357,6 +374,10 @@ public class AWSSignatureProcessor implements SignatureProcessor {
     this.v2Header = v2Header;
   }
 
+  public Exception getException() {
+    return this.exception;
+  }
+
   /**
    * A simple map which forces lower case key usage.
    */
diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java
index a3042c1..9d942ed 100644
--- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java
+++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java
@@ -23,7 +23,9 @@ import javax.enterprise.inject.Produces;
 import javax.inject.Inject;
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.security.PrivilegedExceptionAction;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.io.Text;
 import org.apache.hadoop.ozone.OzoneSecurityUtil;
@@ -36,6 +38,8 @@ import org.apache.hadoop.security.token.Token;
 
 import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMTokenProto.Type.S3AUTHINFO;
 import static org.apache.hadoop.ozone.s3.SignatureProcessor.UTF_8;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INTERNAL_ERROR;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.MALFORMED_HEADER;
 import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.S3_AUTHINFO_CREATION_ERROR;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -64,7 +68,7 @@ public class OzoneClientProducer {
 
 
   @Produces
-  public OzoneClient createClient() throws IOException {
+  public OzoneClient createClient() throws OS3Exception, IOException {
     client = getClient(ozoneConfiguration);
     return client;
   }
@@ -74,15 +78,22 @@ public class OzoneClientProducer {
     client.close();
   }
 
-  private OzoneClient getClient(OzoneConfiguration config) throws IOException {
+  private OzoneClient getClient(OzoneConfiguration config)
+      throws OS3Exception {
+    OzoneClient ozoneClient = null;
     try {
+      // Check if any error occurred during creation of signatureProcessor.
+      if (signatureParser.getException() != null) {
+        throw signatureParser.getException();
+      }
       String awsAccessId = signatureParser.getAwsAccessId();
+      validateAccessId(awsAccessId);
+
       UserGroupInformation remoteUser =
           UserGroupInformation.createRemoteUser(awsAccessId);
       if (OzoneSecurityUtil.isSecurityEnabled(config)) {
         LOG.debug("Creating s3 auth info for client.");
         try {
-
           OzoneTokenIdentifier identifier = new OzoneTokenIdentifier();
           identifier.setTokenType(S3AUTHINFO);
           identifier.setStrToSign(signatureParser.getStringToSign());
@@ -98,25 +109,49 @@ public class OzoneClientProducer {
               omService);
           remoteUser.addToken(token);
         } catch (OS3Exception | URISyntaxException ex) {
-          LOG.error("S3 auth info creation failed.");
           throw S3_AUTHINFO_CREATION_ERROR;
         }
-
       }
-      UserGroupInformation.setLoginUser(remoteUser);
-    } catch (Exception e) {
-      LOG.error("Error: ", e);
+      ozoneClient =
+          remoteUser.doAs((PrivilegedExceptionAction<OzoneClient>)() -> {
+            if (omServiceID == null) {
+              return OzoneClientFactory.getRpcClient(ozoneConfiguration);
+            } else {
+              // As in HA case, we need to pass om service ID.
+              return OzoneClientFactory.getRpcClient(omServiceID,
+                  ozoneConfiguration);
+            }
+          });
+    } catch (OS3Exception ex) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Error during Client Creation: ", ex);
+      }
+      throw ex;
+    } catch (Throwable t) {
+      // For any other critical errors during object creation throw Internal
+      // error.
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Error during Client Creation: ", t);
+        throw INTERNAL_ERROR;
+      }
     }
+    return ozoneClient;
+  }
 
-    if (omServiceID == null) {
-      return OzoneClientFactory.getRpcClient(ozoneConfiguration);
-    } else {
-      // As in HA case, we need to pass om service ID.
-      return OzoneClientFactory.getRpcClient(omServiceID, ozoneConfiguration);
+  // ONLY validate aws access id when needed.
+  private void validateAccessId(String awsAccessId) throws Exception {
+    if (awsAccessId == null || awsAccessId.equals("")) {
+      LOG.error("Malformed s3 header. awsAccessID: ", awsAccessId);
+      throw MALFORMED_HEADER;
     }
   }
 
   public void setOzoneConfiguration(OzoneConfiguration config) {
     this.ozoneConfiguration = config;
   }
+
+  @VisibleForTesting
+  public void setSignatureParser(SignatureProcessor signatureParser) {
+    this.signatureParser = signatureParser;
+  }
 }
diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignatureProcessor.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignatureProcessor.java
index b21bfc1..e3cb6af 100644
--- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignatureProcessor.java
+++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/SignatureProcessor.java
@@ -61,4 +61,6 @@ public interface SignatureProcessor {
   String getSignature();
 
   String getAwsAccessId();
+
+  Exception getException();
 }
diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java
index fedc1ba..432b582 100644
--- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java
+++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java
@@ -23,6 +23,7 @@ import org.slf4j.LoggerFactory;
 import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
 import static java.net.HttpURLConnection.HTTP_CONFLICT;
 import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static java.net.HttpURLConnection.HTTP_SERVER_ERROR;
 import static org.apache.hadoop.ozone.s3.util.S3Consts.RANGE_NOT_SATISFIABLE;
 
 /**
@@ -100,6 +101,10 @@ public final class S3ErrorTable {
       "allowed object size. Each part must be at least 5 MB in size, except " +
       "the last part.", HTTP_BAD_REQUEST);
 
+  public static final OS3Exception INTERNAL_ERROR = new OS3Exception(
+      "InternalError", "We encountered an internal error. Please try again.",
+      HTTP_SERVER_ERROR);
+
 
   /**
    * Create a new instance of Error.
diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java
index ac346c7..d1b5e08 100644
--- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java
+++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/TestOzoneClientProducer.java
@@ -21,7 +21,6 @@ import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.core.MultivaluedHashMap;
 import javax.ws.rs.core.MultivaluedMap;
 import javax.ws.rs.core.UriInfo;
-import java.io.IOException;
 import java.net.URI;
 import java.util.Arrays;
 import java.util.Collection;
@@ -29,7 +28,7 @@ import java.util.Collection;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.ozone.OzoneConfigKeys;
 import org.apache.hadoop.ozone.om.OMConfigKeys;
-import org.apache.hadoop.test.LambdaTestUtils;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
 
 import static org.apache.hadoop.ozone.s3.SignatureProcessor.AUTHORIZATION_HEADER;
 import static org.apache.hadoop.ozone.s3.SignatureProcessor.CONTENT_MD5;
@@ -37,6 +36,9 @@ import static org.apache.hadoop.ozone.s3.SignatureProcessor.CONTENT_TYPE;
 import static org.apache.hadoop.ozone.s3.SignatureProcessor.HOST_HEADER;
 import static org.apache.hadoop.ozone.s3.SignatureProcessor.X_AMAZ_DATE;
 import static org.apache.hadoop.ozone.s3.SignatureProcessor.X_AMZ_CONTENT_SHA256;
+import static org.junit.Assert.fail;
+
+import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -84,9 +86,13 @@ public class TestOzoneClientProducer {
   }
 
   @Test
-  public void testGetClientFailure() throws Exception {
-    LambdaTestUtils.intercept(IOException.class, "Couldn't create",
-        () -> producer.createClient());
+  public void testGetClientFailure() {
+    try {
+      producer.createClient();
+      fail("testGetClientFailure");
+    } catch (Exception ex) {
+      Assert.assertTrue(ex instanceof OS3Exception);
+    }
   }
 
   private void setupContext() throws Exception {
@@ -106,6 +112,12 @@ public class TestOzoneClientProducer {
         .thenReturn(authHeader);
     Mockito.when(context.getUriInfo().getQueryParameters())
         .thenReturn(queryMap);
+
+    AWSSignatureProcessor awsSignatureProcessor = new AWSSignatureProcessor();
+    awsSignatureProcessor.setContext(context);
+    awsSignatureProcessor.init();
+
+    producer.setSignatureParser(awsSignatureProcessor);
   }
 
   @Parameterized.Parameters
@@ -137,6 +149,9 @@ public class TestOzoneClientProducer {
             "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
             "20150830T123600Z",
             "application/x-www-form-urlencoded; charset=utf-8"
+        },
+        {
+            null, null, null, null, null, null
         }
     });
   }
diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java
index b4a21e3..6c41509 100644
--- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java
+++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java
@@ -74,6 +74,11 @@ public class TestBucketPut {
       public String getAwsAccessId() {
         return OzoneConsts.OZONE;
       }
+
+      @Override
+      public Exception getException() {
+        return null;
+      }
     });
   }
 
diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java
index 02c3b7c..7c5d2a5 100644
--- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java
+++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestRootList.java
@@ -69,6 +69,11 @@ public class TestRootList {
       public String getAwsAccessId() {
         return OzoneConsts.OZONE;
       }
+
+      @Override
+      public Exception getException() {
+        return null;
+      }
     });
     // List operation should succeed even there is no bucket.
     ListBucketResponse response =


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org