This is an automated email from the ASF dual-hosted git repository. daim pushed a commit to branch OAK-10093 in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 097a77da1531a415afc7f139123ea16d1414061c Author: Rishabh Kumar <[email protected]> AuthorDate: Thu Jun 8 14:27:30 2023 +0530 OAK-10093 : fixed issues with unit cases and provided steps to create base64 encoded 32 bytes SSE_C keys --- .../jackrabbit/oak/blob/cloud/s3/S3Backend.java | 41 +++++++-------- .../jackrabbit/oak/blob/cloud/s3/S3Constants.java | 4 +- .../oak/blob/cloud/s3/S3RequestDecorator.java | 45 ++++++++++++---- .../jackrabbit/oak/blob/cloud/s3/TestS3Ds.java | 61 ++++++++++++++++------ 4 files changed, 103 insertions(+), 48 deletions(-) diff --git a/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java b/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java index 367a28a1fb..6b145eb0fa 100644 --- a/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java +++ b/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java @@ -39,6 +39,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import com.amazonaws.services.s3.model.GetObjectMetadataRequest; +import com.amazonaws.services.s3.model.GetObjectRequest; import org.apache.commons.io.IOUtils; import org.apache.jackrabbit.core.data.DataIdentifier; import org.apache.jackrabbit.core.data.DataRecord; @@ -328,7 +330,7 @@ public class S3Backend extends AbstractSharedBackend { getClass().getClassLoader()); // check if the same record already exists try { - objectMetaData = s3service.getObjectMetadata(bucket, key); + objectMetaData = s3service.getObjectMetadata(s3ReqDecorator.decorate(new GetObjectMetadataRequest(bucket, key))); } catch (AmazonServiceException ase) { if (!(ase.getStatusCode() == 404 || ase.getStatusCode() == 403)) { throw ase; @@ -389,8 +391,7 @@ public class S3Backend extends AbstractSharedBackend { ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); try { Thread.currentThread().setContextClassLoader(getClass().getClassLoader()); - ObjectMetadata objectMetaData = s3service.getObjectMetadata(bucket, - key); + ObjectMetadata objectMetaData = s3service.getObjectMetadata(s3ReqDecorator.decorate(new GetObjectMetadataRequest(bucket, key))); if (objectMetaData != null) { LOG.trace("exists [{}]: [true] took [{}] ms.", identifier, (System.currentTimeMillis() - start) ); @@ -555,7 +556,7 @@ public class S3Backend extends AbstractSharedBackend { getClass().getClassLoader()); ObjectMetadata meta = s3service.getObjectMetadata(bucket, addMetaKeyPrefix(name)); return new S3DataRecord(this, s3service, bucket, new DataIdentifier(name), - meta.getLastModified().getTime(), meta.getContentLength(), true); + meta.getLastModified().getTime(), meta.getContentLength(), true, s3ReqDecorator); } catch(Exception e) { LOG.error("Error getting metadata record for {}", name, e); } @@ -582,7 +583,7 @@ public class S3Backend extends AbstractSharedBackend { for (final S3ObjectSummary s3ObjSumm : prevObjectListing.getObjectSummaries()) { metadataList.add(new S3DataRecord(this, s3service, bucket, new DataIdentifier(stripMetaKeyPrefix(s3ObjSumm.getKey())), - s3ObjSumm.getLastModified().getTime(), s3ObjSumm.getSize(), true)); + s3ObjSumm.getLastModified().getTime(), s3ObjSumm.getSize(), true, s3ReqDecorator)); } } finally { if (contextClassLoader != null) { @@ -646,7 +647,7 @@ public class S3Backend extends AbstractSharedBackend { public DataRecord apply(S3ObjectSummary input) { return new S3DataRecord(backend, s3service, bucket, new DataIdentifier(getIdentifierName(input.getKey())), - input.getLastModified().getTime(), input.getSize()); + input.getLastModified().getTime(), input.getSize(), s3ReqDecorator); } }); } @@ -659,9 +660,9 @@ public class S3Backend extends AbstractSharedBackend { try { Thread.currentThread().setContextClassLoader(getClass().getClassLoader()); - ObjectMetadata object = s3service.getObjectMetadata(bucket, key); + ObjectMetadata object = s3service.getObjectMetadata(s3ReqDecorator.decorate(new GetObjectMetadataRequest(bucket, key))); S3DataRecord record = new S3DataRecord(this, s3service, bucket, identifier, - object.getLastModified().getTime(), object.getContentLength()); + object.getLastModified().getTime(), object.getContentLength(), s3ReqDecorator); LOG.debug("Identifier [{}]'s getRecord = [{}] took [{}]ms.", identifier, record, (System.currentTimeMillis() - start)); @@ -994,7 +995,8 @@ public class S3Backend extends AbstractSharedBackend { bucket, blobId, lastModified.getTime(), - size + size, + s3ReqDecorator ); } else { @@ -1026,13 +1028,9 @@ public class S3Backend extends AbstractSharedBackend { final Date expiration = new Date(); expiration.setTime(expiration.getTime() + expirySeconds * 1000); - GeneratePresignedUrlRequest request = new GeneratePresignedUrlRequest(bucket, key) + GeneratePresignedUrlRequest request = s3ReqDecorator.decorate(new GeneratePresignedUrlRequest(bucket, key) .withMethod(method) - .withExpiration(expiration); - - if (method != HttpMethod.GET) { - request = s3ReqDecorator.decorate(request); - } + .withExpiration(expiration)); for (Map.Entry<String, String> e : reqParams.entrySet()) { request.addRequestParameter(e.getKey(), e.getValue()); @@ -1169,22 +1167,22 @@ public class S3Backend extends AbstractSharedBackend { private long lastModified; private String bucket; private boolean isMeta; + private final S3RequestDecorator s3RequestDecorator; public S3DataRecord(AbstractSharedBackend backend, AmazonS3Client s3service, String bucket, - DataIdentifier key, long lastModified, - long length) { - this(backend, s3service, bucket, key, lastModified, length, false); + DataIdentifier key, long lastModified, long length, final S3RequestDecorator s3RequestDecorator) { + this(backend, s3service, bucket, key, lastModified, length, false, s3RequestDecorator); } public S3DataRecord(AbstractSharedBackend backend, AmazonS3Client s3service, String bucket, - DataIdentifier key, long lastModified, - long length, boolean isMeta) { + DataIdentifier key, long lastModified, long length, boolean isMeta, final S3RequestDecorator s3RequestDecorator) { super(backend, key); this.s3service = s3service; this.lastModified = lastModified; this.length = length; this.bucket = bucket; this.isMeta = isMeta; + this.s3RequestDecorator = s3RequestDecorator; } @Override @@ -1197,6 +1195,7 @@ public class S3Backend extends AbstractSharedBackend { String id = getKeyName(getIdentifier()); if (isMeta) { id = addMetaKeyPrefix(getIdentifier().toString()); + return s3service.getObject(bucket, id).getObjectContent(); } else { // Don't worry about stream logging for metadata records @@ -1205,7 +1204,7 @@ public class S3Backend extends AbstractSharedBackend { LOG_STREAMS_DOWNLOAD.debug("Binary downloaded from S3 - identifier={}", id, new Exception()); } } - return s3service.getObject(bucket, id).getObjectContent(); + return s3service.getObject(s3RequestDecorator.decorate(new GetObjectRequest(bucket, id))).getObjectContent(); } @Override diff --git a/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Constants.java b/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Constants.java index 62877b9c9b..eb6ca3dce8 100644 --- a/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Constants.java +++ b/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Constants.java @@ -118,8 +118,10 @@ public final class S3Constants { public static final String S3_SSE_KMS_KEYID = "kmsKeyId"; /** - * Constant to set keyID for SSE_C encryption. + * Constant to set base64 encoded keyID for SSE_C encryption. */ + // please use 'openssl rand -base64 -out ssec.key 32' command to + // generate base64 encoded 32 bytes string customer key for SSE_C public static final String S3_SSE_C_KEYID = "sseCustomerKeyId"; /** diff --git a/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3RequestDecorator.java b/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3RequestDecorator.java index 8e66124a20..46b6e5aa79 100644 --- a/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3RequestDecorator.java +++ b/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3RequestDecorator.java @@ -19,8 +19,11 @@ package org.apache.jackrabbit.oak.blob.cloud.s3; import java.util.Properties; +import com.amazonaws.HttpMethod; import com.amazonaws.services.s3.model.CopyObjectRequest; import com.amazonaws.services.s3.model.GeneratePresignedUrlRequest; +import com.amazonaws.services.s3.model.GetObjectMetadataRequest; +import com.amazonaws.services.s3.model.GetObjectRequest; import com.amazonaws.services.s3.model.InitiateMultipartUploadRequest; import com.amazonaws.services.s3.model.ObjectMetadata; import com.amazonaws.services.s3.model.PutObjectRequest; @@ -28,9 +31,9 @@ import com.amazonaws.services.s3.model.SSEAlgorithm; import com.amazonaws.services.s3.model.SSEAwsKeyManagementParams; import com.amazonaws.services.s3.model.SSECustomerKey; +import static com.amazonaws.HttpMethod.GET; import static com.amazonaws.services.s3.model.SSEAlgorithm.AES256; import static com.amazonaws.util.StringUtils.hasValue; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_ENCRYPTION; import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_ENCRYPTION_SSE_C; import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_ENCRYPTION_SSE_KMS; @@ -43,7 +46,6 @@ import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_SSE_KMS_KEY */ public class S3RequestDecorator { DataEncryption dataEncryption = DataEncryption.NONE; - Properties props; SSEAwsKeyManagementParams sseParams; SSECustomerKey sseCustomerKey; @@ -65,7 +67,7 @@ public class S3RequestDecorator { case S3_ENCRYPTION_SSE_C: { final String keyId = props.getProperty(S3_SSE_C_KEYID); if (hasValue(keyId)) { - sseCustomerKey = new SSECustomerKey(keyId.getBytes(UTF_8)); + sseCustomerKey = new SSECustomerKey(keyId); } break; } @@ -73,6 +75,34 @@ public class S3RequestDecorator { } } + /** + * Set encryption in {@link GetObjectMetadataRequest} + */ + public GetObjectMetadataRequest decorate(final GetObjectMetadataRequest request) { + switch (getDataEncryption()) { + case SSE_C: + request.withSSECustomerKey(sseCustomerKey); + break; + case NONE: + break; + } + return request; + } + + /** + * Set encryption in {@link GetObjectRequest} + */ + public GetObjectRequest decorate(final GetObjectRequest request) { + switch (getDataEncryption()) { + case SSE_C: + request.withSSECustomerKey(sseCustomerKey); + break; + case NONE: + break; + } + return request; + } + /** * Set encryption in {@link PutObjectRequest} */ @@ -90,7 +120,6 @@ public class S3RequestDecorator { request.withSSEAwsKeyManagementParams(sseParams); break; case SSE_C: - metadata.setSSEAlgorithm(AES256.getAlgorithm()); request.withSSECustomerKey(sseCustomerKey); break; case NONE: @@ -139,7 +168,6 @@ public class S3RequestDecorator { request.withSSEAwsKeyManagementParams(sseParams); break; case SSE_C: - metadata.setSSEAlgorithm(AES256.getAlgorithm()); request.withSSECustomerKey(sseCustomerKey); break; case NONE: @@ -152,6 +180,7 @@ public class S3RequestDecorator { public GeneratePresignedUrlRequest decorate(GeneratePresignedUrlRequest request) { switch (getDataEncryption()) { case SSE_KMS: + if (request.getMethod() == GET) break; // KMS is not valid for GET Requests String keyId = getSSEParams().getAwsKmsKeyId(); request = request.withSSEAlgorithm(SSEAlgorithm.KMS.getAlgorithm()); if (keyId != null) { @@ -159,7 +188,7 @@ public class S3RequestDecorator { } break; case SSE_C: - request = request.withSSEAlgorithm(AES256).withSSECustomerKey(getSseCustomerKey()); + request = request.withSSECustomerKey(sseCustomerKey); break; } return request; @@ -169,10 +198,6 @@ public class S3RequestDecorator { return this.sseParams; } - private SSECustomerKey getSseCustomerKey() { - return this.sseCustomerKey; - } - private DataEncryption getDataEncryption() { return this.dataEncryption; } diff --git a/oak-blob-cloud/src/test/java/org/apache/jackrabbit/oak/blob/cloud/s3/TestS3Ds.java b/oak-blob-cloud/src/test/java/org/apache/jackrabbit/oak/blob/cloud/s3/TestS3Ds.java index a3b8702551..940ff24618 100644 --- a/oak-blob-cloud/src/test/java/org/apache/jackrabbit/oak/blob/cloud/s3/TestS3Ds.java +++ b/oak-blob-cloud/src/test/java/org/apache/jackrabbit/oak/blob/cloud/s3/TestS3Ds.java @@ -22,12 +22,11 @@ import java.io.InputStream; import java.net.URI; import java.util.Date; import java.util.List; +import java.util.Objects; import java.util.Properties; import javax.jcr.RepositoryException; -import com.amazonaws.services.s3.Headers; -import com.amazonaws.services.s3.model.SSEAlgorithm; import org.apache.jackrabbit.guava.common.collect.Lists; import org.apache.commons.lang3.time.DateUtils; import org.apache.http.HttpEntity; @@ -60,6 +59,20 @@ import org.junit.runners.Parameterized; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static com.amazonaws.services.s3.Headers.SERVER_SIDE_ENCRYPTION; +import static com.amazonaws.services.s3.Headers.SERVER_SIDE_ENCRYPTION_AWS_KMS_KEYID; +import static com.amazonaws.services.s3.Headers.SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM; +import static com.amazonaws.services.s3.Headers.SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY; +import static com.amazonaws.services.s3.Headers.SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY_MD5; +import static com.amazonaws.services.s3.model.SSEAlgorithm.AES256; +import static com.amazonaws.services.s3.model.SSEAlgorithm.KMS; +import static com.amazonaws.util.Base64.decode; +import static com.amazonaws.util.Md5Utils.md5AsBase64; +import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_ENCRYPTION; +import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_ENCRYPTION_SSE_C; +import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_ENCRYPTION_SSE_KMS; +import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_SSE_C_KEYID; +import static org.apache.jackrabbit.oak.blob.cloud.s3.S3Constants.S3_SSE_KMS_KEYID; import static org.apache.jackrabbit.oak.blob.cloud.s3.S3DataStoreUtils.getFixtures; import static org.apache.jackrabbit.oak.blob.cloud.s3.S3DataStoreUtils.getS3Config; import static org.apache.jackrabbit.oak.blob.cloud.s3.S3DataStoreUtils.getS3DataStore; @@ -125,7 +138,7 @@ public class TestS3Ds extends AbstractDataStoreTest { props.setProperty(S3Constants.PRESIGNED_HTTP_UPLOAD_URI_EXPIRY_SECONDS, "60"); props.setProperty(S3Constants.PRESIGNED_URI_ENABLE_ACCELERATION, "60"); props.setProperty(S3Constants.PRESIGNED_HTTP_DOWNLOAD_URI_CACHE_MAX_SIZE, "60"); - props.setProperty(S3Constants.S3_ENCRYPTION, S3Constants.S3_ENCRYPTION_NONE); + props.setProperty(S3_ENCRYPTION, S3Constants.S3_ENCRYPTION_NONE); super.setUp(); } @@ -175,11 +188,11 @@ public class TestS3Ds extends AbstractDataStoreTest { @Test public void testDataMigration() { try { - String encryption = props.getProperty(S3Constants.S3_ENCRYPTION); + String encryption = props.getProperty(S3_ENCRYPTION); //manually close the setup ds and remove encryption ds.close(); - props.remove(S3Constants.S3_ENCRYPTION); + props.remove(S3_ENCRYPTION); ds = createDataStore(); byte[] data = new byte[dataLength]; @@ -190,7 +203,7 @@ public class TestS3Ds extends AbstractDataStoreTest { ds.close(); // turn encryption now anc recreate datastore instance - props.setProperty(S3Constants.S3_ENCRYPTION, encryption); + props.setProperty(S3_ENCRYPTION, encryption); props.setProperty(S3Constants.S3_RENAME_KEYS, "true"); ds = createDataStore(); @@ -252,16 +265,22 @@ public class TestS3Ds extends AbstractDataStoreTest { HttpPut putreq = new HttpPut(puturl); String keyId = null; - String encryptionType = props.getProperty(S3Constants.S3_ENCRYPTION); - - if (encryptionType.equals(S3Constants.S3_ENCRYPTION_SSE_KMS)) { - keyId = props.getProperty(S3Constants.S3_SSE_KMS_KEYID); - putreq.addHeader(new BasicHeader(Headers.SERVER_SIDE_ENCRYPTION, - SSEAlgorithm.KMS.getAlgorithm())); - if(keyId != null) { - putreq.addHeader(new BasicHeader(Headers.SERVER_SIDE_ENCRYPTION_AWS_KMS_KEYID, - keyId)); - } + String encryptionType = props.getProperty(S3_ENCRYPTION); + + switch (encryptionType) { + case S3_ENCRYPTION_SSE_KMS: + keyId = props.getProperty(S3_SSE_KMS_KEYID); + putreq.addHeader(new BasicHeader(SERVER_SIDE_ENCRYPTION, KMS.getAlgorithm())); + if (keyId != null) { + putreq.addHeader(new BasicHeader(SERVER_SIDE_ENCRYPTION_AWS_KMS_KEYID, keyId)); + } + break; + case S3_ENCRYPTION_SSE_C: + keyId = props.getProperty(S3_SSE_C_KEYID); + putreq.addHeader(new BasicHeader(SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, AES256.getAlgorithm())); + putreq.addHeader(new BasicHeader(SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY, keyId)); + putreq.addHeader(new BasicHeader(SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY_MD5, md5AsBase64(decode(keyId)))); + break; } putreq.setEntity(new InputStreamEntity(inputstream , length)); @@ -273,6 +292,16 @@ public class TestS3Ds extends AbstractDataStoreTest { private HttpEntity httpGet(URI uri) throws IOException { HttpGet getreq = new HttpGet(uri); + + final String encryptionType = props.getProperty(S3_ENCRYPTION); + + if (Objects.equals(S3_ENCRYPTION_SSE_C, encryptionType)) { + String keyId = props.getProperty(S3_SSE_C_KEYID); + getreq.addHeader(new BasicHeader(SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, AES256.getAlgorithm())); + getreq.addHeader(new BasicHeader(SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY, keyId)); + getreq.addHeader(new BasicHeader(SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY_MD5, md5AsBase64(decode(keyId)))); + } + CloseableHttpClient httpclient = HttpClients.createDefault(); CloseableHttpResponse res = httpclient.execute(getreq); Assert.assertEquals(200, res.getStatusLine().getStatusCode());
