From 2d953c53548d5f17174d8ff90270d1a9c1461c23 Mon Sep 17 00:00:00 2001 From: Rob Block Date: Mon, 18 Mar 2019 15:49:59 -0700 Subject: [PATCH 01/15] Add CloudBlobCryptoService interface for use in CloudBlobStore. Enhance KeyManagementService to take single context string as input. --- .../router/KeyManagementService.java | 16 +++++++ .../CloudBlobCryptoService.java | 44 +++++++++++++++++++ .../CloudBlobStore.java | 20 ++++++--- .../CloudBlobStoreTest.java | 7 +-- .../TestCloudBlobCryptoService.java | 34 ++++++++++++++ .../SingleKeyManagementService.java | 10 +++++ 6 files changed, 122 insertions(+), 9 deletions(-) create mode 100644 ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java create mode 100644 ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java diff --git a/ambry-api/src/main/java/com.github.ambry/router/KeyManagementService.java b/ambry-api/src/main/java/com.github.ambry/router/KeyManagementService.java index 1a3eca5058..53e23fc8d8 100644 --- a/ambry-api/src/main/java/com.github.ambry/router/KeyManagementService.java +++ b/ambry-api/src/main/java/com.github.ambry/router/KeyManagementService.java @@ -37,6 +37,13 @@ public interface KeyManagementService extends Closeable { */ void register(short accountId, short containerId) throws GeneralSecurityException; + /** + * Registers with KMS to create key for a unique context. + * @param context refers to the key context to register + * @throws {@link GeneralSecurityException} on KMS unavailability or duplicate registration + */ + void register(String context) throws GeneralSecurityException; + /** * Fetches the key associated with the pair of AccountId and ContainerId. User is expected to have * registered using {@link #register(short, short)} for this pair before fetching keys. @@ -47,6 +54,15 @@ public interface KeyManagementService extends Closeable { */ T getKey(short accountId, short containerId) throws GeneralSecurityException; + /** + * Fetches the key associated with the specified context. User is expected to have + * registered using {@link #register(String)} for this context before fetching keys. + * @param context refers to the context for which key is expected + * @return T the key associated with the context + * @throws {@link GeneralSecurityException} on KMS unavailability or if key is not registered + */ + T getKey(String context) throws GeneralSecurityException; + /** * Generate and return a random key (of type T) * @return a random key (of type T) diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java new file mode 100644 index 0000000000..134d668172 --- /dev/null +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java @@ -0,0 +1,44 @@ +/** + * Copyright 2019 LinkedIn Corp. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + */ +package com.github.ambry.cloud; + +import com.github.ambry.commons.BlobId; +import java.nio.ByteBuffer; +import java.security.GeneralSecurityException; + + +/** + * Crypto Service to assist in encrypting and decrypting blob chunks transferred between + * Ambry and the cloud backup. + */ +public interface CloudBlobCryptoService { + + /** + * Encrypts the blob buffer being uploaded to cloud storage. + * @param blobId the {@link BlobId} associated with the buffer. + * @param buffer The {@link ByteBuffer} that needs to be encrypted. + * @return The encrypted buffer. + * @throws {@link GeneralSecurityException} on any exception with encryption. + */ + public ByteBuffer encrypt(BlobId blobId, ByteBuffer buffer) throws GeneralSecurityException; + + /** + * Decrypts the blob buffer being downloaded from cloud storage. + * @param blobId the {@link BlobId} associated with the buffer. + * @param buffer The {@link ByteBuffer} that needs to be decrypted. + * @return The decrypted buffer. + * @throws {@link GeneralSecurityException} on any exception with decryption + */ + public ByteBuffer decrypt(BlobId blobId, ByteBuffer buffer) throws GeneralSecurityException; +} diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 3a8b7ec638..80d66b8f93 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -15,7 +15,6 @@ import com.github.ambry.clustermap.PartitionId; import com.github.ambry.commons.BlobId; -import com.github.ambry.config.VerifiableProperties; import com.github.ambry.store.FindInfo; import com.github.ambry.store.FindToken; import com.github.ambry.store.MessageInfo; @@ -33,6 +32,7 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.ReadableByteChannel; +import java.security.GeneralSecurityException; import java.util.EnumSet; import java.util.HashSet; import java.util.List; @@ -47,19 +47,22 @@ */ class CloudBlobStore implements Store { - private final Logger logger = LoggerFactory.getLogger(getClass()); + private static final Logger logger = LoggerFactory.getLogger(CloudBlobStore.class); private final PartitionId partitionId; private final CloudDestination cloudDestination; + private final CloudBlobCryptoService cryptoService; private boolean started; /** * Constructor for CloudBlobStore * @param partitionId partition associated with BlobStore. - * @param verProps the cloud store properties. + * @param cloudDestination the {@link CloudDestination}. + * @param cryptoService the {@link CloudBlobCryptoService} to use for encryption. */ - CloudBlobStore(PartitionId partitionId, VerifiableProperties verProps, CloudDestination cloudDestination) { + CloudBlobStore(PartitionId partitionId, CloudDestination cloudDestination, CloudBlobCryptoService cryptoService) { this.cloudDestination = cloudDestination; this.partitionId = partitionId; + this.cryptoService = cryptoService; } @Override @@ -96,11 +99,16 @@ public void put(MessageWriteSet messageSetToWrite) throws StoreException { * @param size the number of bytes to upload. * @throws CloudStorageException if the upload failed. */ - private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) throws CloudStorageException { + private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) + throws CloudStorageException, IOException, GeneralSecurityException { boolean performUpload = messageInfo.getExpirationTimeInMs() == Utils.Infinite_Time && !messageInfo.isDeleted(); - // May need to encrypt buffer before upload if (performUpload) { BlobId blobId = (BlobId) messageInfo.getStoreKey(); + // TODO: would be more efficient to call blobId.isEncrypted() + if (!BlobId.isEncrypted(blobId.getID())) { + // Need to encrypt the buffer before upload + messageBuf = cryptoService.encrypt(blobId, messageBuf); + } CloudBlobMetadata blobMetadata = new CloudBlobMetadata(blobId, messageInfo.getOperationTimeMs(), messageInfo.getExpirationTimeInMs(), messageInfo.getSize()); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java index b5c6554d28..8741c5f4b2 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java @@ -49,6 +49,7 @@ */ public class CloudBlobStoreTest { + private final CloudBlobCryptoService cryptoService = new TestCloudBlobCryptoService(); private Store store; private CloudDestination dest; private PartitionId partitionId; @@ -61,7 +62,7 @@ public class CloudBlobStoreTest { public void setup() throws Exception { dest = mock(CloudDestination.class); partitionId = new MockPartitionId(); - store = new CloudBlobStore(partitionId, null, dest); + store = new CloudBlobStore(partitionId, dest, cryptoService); store.start(); } @@ -135,7 +136,7 @@ public void testFindMissingKeys() throws Exception { @Test public void testStoreNotStarted() throws Exception { // Create store and don't start it. - CloudBlobStore idleStore = new CloudBlobStore(partitionId, null, dest); + CloudBlobStore idleStore = new CloudBlobStore(partitionId, dest, cryptoService); List keys = Collections.singletonList(getUniqueId(refAccountId, refContainerId)); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); addBlobToSet(messageWriteSet, 10, Utils.Infinite_Time, refAccountId, refContainerId); @@ -167,7 +168,7 @@ public void testExceptionalDest() throws Exception { new CloudStorageException("ouch")); when(exDest.deleteBlob(any(BlobId.class), anyLong())).thenThrow(new CloudStorageException("ouch")); when(exDest.getBlobMetadata(anyList())).thenThrow(new CloudStorageException("ouch")); - CloudBlobStore exStore = new CloudBlobStore(partitionId, null, exDest); + CloudBlobStore exStore = new CloudBlobStore(partitionId, exDest, cryptoService); exStore.start(); List keys = Collections.singletonList(getUniqueId(refAccountId, refContainerId)); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java new file mode 100644 index 0000000000..a9b11da838 --- /dev/null +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java @@ -0,0 +1,34 @@ +/** + * Copyright 2019 LinkedIn Corp. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + */ +package com.github.ambry.cloud; + +import com.github.ambry.commons.BlobId; +import java.nio.ByteBuffer; +import java.security.GeneralSecurityException; + + +/** + * Dummy implementation for testing. + */ +class TestCloudBlobCryptoService implements CloudBlobCryptoService { + @Override + public ByteBuffer encrypt(BlobId blobId, ByteBuffer buffer) throws GeneralSecurityException { + return buffer; + } + + @Override + public ByteBuffer decrypt(BlobId blobId, ByteBuffer buffer) throws GeneralSecurityException { + return buffer; + } +} diff --git a/ambry-router/src/main/java/com.github.ambry.router/SingleKeyManagementService.java b/ambry-router/src/main/java/com.github.ambry.router/SingleKeyManagementService.java index 108584b765..9050aba6ec 100644 --- a/ambry-router/src/main/java/com.github.ambry.router/SingleKeyManagementService.java +++ b/ambry-router/src/main/java/com.github.ambry.router/SingleKeyManagementService.java @@ -58,6 +58,11 @@ public void register(short accountId, short containerId) { // no op } + @Override + public void register(String context) throws GeneralSecurityException { + // no op + } + /** * Fetches the key associated with the pair of AccountId and ContainerId. {@link SingleKeyManagementService} returns * the default key for all {@link #getKey(short, short)}} @@ -67,6 +72,11 @@ public void register(short accountId, short containerId) { */ @Override public SecretKeySpec getKey(short accountId, short containerId) throws GeneralSecurityException { + return getKey(accountId + ":" + containerId); + } + + @Override + public SecretKeySpec getKey(String context) throws GeneralSecurityException { if (enabled) { return secretKeySpec; } else { From a8804bced40b9c24e1a03f7be84b0afe984960d7 Mon Sep 17 00:00:00 2001 From: David Harju Date: Fri, 29 Mar 2019 15:51:19 -0700 Subject: [PATCH 02/15] first commited files --- .../CloudBlobMetadata.java | 17 +++++++++++++++++ .../com.github.ambry.cloud/CloudBlobStore.java | 1 + .../azure/AzureCloudDestination.java | 1 + .../BlobIdTransformer.java | 1 + 4 files changed, 20 insertions(+) diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java index 010273a61f..c6d3f90a11 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java @@ -28,6 +28,7 @@ public class CloudBlobMetadata { public static final String FIELD_EXPIRATION_TIME = "expirationTime"; public static final String FIELD_ACCOUNT_ID = "accountId"; public static final String FIELD_CONTAINER_ID = "containerId"; + public static final String FIELD_ENCRYPTION_CONTEXT = "encryptionContext"; private String id; private String partitionId; @@ -38,6 +39,7 @@ public class CloudBlobMetadata { private int containerId; private long expirationTime; private long deletionTime; + private String encryptionContext; /** * Default constructor (for JSONSerializer). @@ -213,6 +215,21 @@ public CloudBlobMetadata setContainerId(int containerId) { return this; } + /** + * @return the encryption context. + */ + public String getEncryptionContext() { return encryptionContext;} + + /** + * Set the encryption context. + * @param encryptionContext the encryption context of the blob. + * @return this instance. + */ + public CloudBlobMetadata setEncryptionContext(String encryptionContext) { + this.encryptionContext = encryptionContext; + return this; + } + @Override public boolean equals(Object o) { if (!(o instanceof CloudBlobMetadata)) { diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 80d66b8f93..5f17b41e27 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -105,6 +105,7 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) if (performUpload) { BlobId blobId = (BlobId) messageInfo.getStoreKey(); // TODO: would be more efficient to call blobId.isEncrypted() + if (!BlobId.isEncrypted(blobId.getID())) { // Need to encrypt the buffer before upload messageBuf = cryptoService.encrypt(blobId, messageBuf); diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java index 24388c526c..59011396d7 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java @@ -280,6 +280,7 @@ private static HashMap getMetadataMap(CloudBlobMetadata cloudBlo map.put(CloudBlobMetadata.FIELD_EXPIRATION_TIME, String.valueOf(cloudBlobMetadata.getExpirationTime())); map.put(CloudBlobMetadata.FIELD_ACCOUNT_ID, String.valueOf(cloudBlobMetadata.getAccountId())); map.put(CloudBlobMetadata.FIELD_CONTAINER_ID, String.valueOf(cloudBlobMetadata.getContainerId())); + map.put(CloudBlobMetadata.FIELD_ENCRYPTION_CONTEXT, String.valueOf(cloudBlobMetadata.getEncryptionContext())); return map; } } diff --git a/ambry-replication/src/main/java/com.github.ambry.replication/BlobIdTransformer.java b/ambry-replication/src/main/java/com.github.ambry.replication/BlobIdTransformer.java index 306a471d4b..baf4c7f5a3 100644 --- a/ambry-replication/src/main/java/com.github.ambry.replication/BlobIdTransformer.java +++ b/ambry-replication/src/main/java/com.github.ambry.replication/BlobIdTransformer.java @@ -216,6 +216,7 @@ private Message newMessage(InputStream inputStream, StoreKey newKey, MessageInfo PutMessageFormatInputStream putMessageFormatInputStream = new PutMessageFormatInputStream(newKey, blobEncryptionKey, newProperties, userMetaData, blobDataBytes, blobData.getSize(), blobData.getBlobType()); + // Reuse the original CRC if present in the oldMessageInfo. This is important to ensure that messages that are // received via replication are sent to the store with proper CRCs (which the store needs to detect duplicate // messages). As an additional guard, here the original CRC is only reused if the key's ID in string form is the From f718475729da6362d40cb71c6492dcea504d3496 Mon Sep 17 00:00:00 2001 From: David Harju Date: Sat, 30 Mar 2019 16:44:54 -0700 Subject: [PATCH 03/15] changed name of field --- .../src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java | 2 +- .../src/main/java/com.github.ambry.cloud/CloudBlobStore.java | 2 ++ .../com.github.ambry.cloud/azure/AzureCloudDestination.java | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java index c6d3f90a11..7d90266dee 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java @@ -28,7 +28,7 @@ public class CloudBlobMetadata { public static final String FIELD_EXPIRATION_TIME = "expirationTime"; public static final String FIELD_ACCOUNT_ID = "accountId"; public static final String FIELD_CONTAINER_ID = "containerId"; - public static final String FIELD_ENCRYPTION_CONTEXT = "encryptionContext"; + public static final String FIELD_SHOULD_ENCRYPT = "shouldEncrypt"; private String id; private String partitionId; diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 5f17b41e27..8c4e136951 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -106,6 +106,8 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) BlobId blobId = (BlobId) messageInfo.getStoreKey(); // TODO: would be more efficient to call blobId.isEncrypted() + + if (!BlobId.isEncrypted(blobId.getID())) { // Need to encrypt the buffer before upload messageBuf = cryptoService.encrypt(blobId, messageBuf); diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java index 59011396d7..a60666a96f 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java @@ -280,7 +280,7 @@ private static HashMap getMetadataMap(CloudBlobMetadata cloudBlo map.put(CloudBlobMetadata.FIELD_EXPIRATION_TIME, String.valueOf(cloudBlobMetadata.getExpirationTime())); map.put(CloudBlobMetadata.FIELD_ACCOUNT_ID, String.valueOf(cloudBlobMetadata.getAccountId())); map.put(CloudBlobMetadata.FIELD_CONTAINER_ID, String.valueOf(cloudBlobMetadata.getContainerId())); - map.put(CloudBlobMetadata.FIELD_ENCRYPTION_CONTEXT, String.valueOf(cloudBlobMetadata.getEncryptionContext())); + map.put(CloudBlobMetadata.FIELD_SHOULD_ENCRYPT, String.valueOf(cloudBlobMetadata.getEncryptionContext())); return map; } } From b694d4278f221d17b9067661c38f5e6068e1deb3 Mon Sep 17 00:00:00 2001 From: David Harju Date: Sun, 31 Mar 2019 13:46:42 -0700 Subject: [PATCH 04/15] Added implementation and tests for AzureCloudBlobCryptoService --- .../CloudBlobCryptoService.java | 7 +- .../CloudBlobMetadata.java | 16 +- .../CloudBlobStore.java | 11 +- .../azure/AzureCloudBlobCryptoService.java | 158 ++++++++++++++++++ .../azure/AzureCloudDestination.java | 2 +- .../CloudBlobStoreTest.java | 2 +- .../TestCloudBlobCryptoService.java | 5 +- .../AzureCloudBlobCryptoServiceTest.java | 149 +++++++++++++++++ .../azure/AzureCloudDestinationTest.java | 4 +- .../azure/AzureIntegrationTest.java | 4 +- .../GCMCryptoService.java | 4 +- .../SingleKeyManagementService.java | 2 +- build.gradle | 3 +- 13 files changed, 335 insertions(+), 32 deletions(-) create mode 100644 ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoService.java create mode 100644 ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceTest.java diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java index 134d668172..9cf8cdddf2 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java @@ -13,7 +13,6 @@ */ package com.github.ambry.cloud; -import com.github.ambry.commons.BlobId; import java.nio.ByteBuffer; import java.security.GeneralSecurityException; @@ -26,19 +25,17 @@ public interface CloudBlobCryptoService { /** * Encrypts the blob buffer being uploaded to cloud storage. - * @param blobId the {@link BlobId} associated with the buffer. * @param buffer The {@link ByteBuffer} that needs to be encrypted. * @return The encrypted buffer. * @throws {@link GeneralSecurityException} on any exception with encryption. */ - public ByteBuffer encrypt(BlobId blobId, ByteBuffer buffer) throws GeneralSecurityException; + public ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException; /** * Decrypts the blob buffer being downloaded from cloud storage. - * @param blobId the {@link BlobId} associated with the buffer. * @param buffer The {@link ByteBuffer} that needs to be decrypted. * @return The decrypted buffer. * @throws {@link GeneralSecurityException} on any exception with decryption */ - public ByteBuffer decrypt(BlobId blobId, ByteBuffer buffer) throws GeneralSecurityException; + public ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException; } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java index 7d90266dee..26311894f5 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java @@ -28,7 +28,7 @@ public class CloudBlobMetadata { public static final String FIELD_EXPIRATION_TIME = "expirationTime"; public static final String FIELD_ACCOUNT_ID = "accountId"; public static final String FIELD_CONTAINER_ID = "containerId"; - public static final String FIELD_SHOULD_ENCRYPT = "shouldEncrypt"; + public static final String FIELD_USES_CLOUD_ENCRYPTION = "usesCloudEncryption"; private String id; private String partitionId; @@ -39,7 +39,7 @@ public class CloudBlobMetadata { private int containerId; private long expirationTime; private long deletionTime; - private String encryptionContext; + private boolean usesCloudEncryption; /** * Default constructor (for JSONSerializer). @@ -54,7 +54,7 @@ public CloudBlobMetadata() { * @param expirationTime The blob expiration time. * @param size The blob size. */ - public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size) { + public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size, boolean usesCloudEncryption) { this.id = blobId.getID(); this.partitionId = blobId.getPartition().toPathString(); this.accountId = blobId.getAccountId(); @@ -216,17 +216,17 @@ public CloudBlobMetadata setContainerId(int containerId) { } /** - * @return the encryption context. + * @return whether the blob was cloud encrypted. */ - public String getEncryptionContext() { return encryptionContext;} + public boolean getUsesCloudEncryption() { return usesCloudEncryption;} /** * Set the encryption context. - * @param encryptionContext the encryption context of the blob. + * @param usesCloudEncryption whether {@link CloudBlobCryptoService} was used on the blob. * @return this instance. */ - public CloudBlobMetadata setEncryptionContext(String encryptionContext) { - this.encryptionContext = encryptionContext; + public CloudBlobMetadata setUsesCloudEncryption(boolean usesCloudEncryption) { + this.usesCloudEncryption = usesCloudEncryption; return this; } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 8c4e136951..02c6d9ad1a 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -105,16 +105,15 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) if (performUpload) { BlobId blobId = (BlobId) messageInfo.getStoreKey(); // TODO: would be more efficient to call blobId.isEncrypted() - - - - if (!BlobId.isEncrypted(blobId.getID())) { + boolean usesCloudEncryption = false; + if (blobId.getVersion() < 4 || !BlobId.isEncrypted(blobId.getID())) { // Need to encrypt the buffer before upload - messageBuf = cryptoService.encrypt(blobId, messageBuf); + messageBuf = cryptoService.encrypt(messageBuf); + usesCloudEncryption = true; } CloudBlobMetadata blobMetadata = new CloudBlobMetadata(blobId, messageInfo.getOperationTimeMs(), messageInfo.getExpirationTimeInMs(), - messageInfo.getSize()); + messageInfo.getSize(), usesCloudEncryption); cloudDestination.uploadBlob(blobId, size, blobMetadata, new ByteBufferInputStream(messageBuf)); } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoService.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoService.java new file mode 100644 index 0000000000..105bdbac99 --- /dev/null +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoService.java @@ -0,0 +1,158 @@ +/* + * Copyright 2019 LinkedIn Corp. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + */ +package com.github.ambry.cloud.azure; + +import com.github.ambry.cloud.CloudBlobCryptoService; +import com.github.ambry.router.CryptoService; +import com.github.ambry.router.KeyManagementService; +import com.github.ambry.utils.Crc32; +import java.nio.ByteBuffer; +import java.security.GeneralSecurityException; + + +/** + * Implementation of CloudBlobCryptoService, encrypts byte buffers by + * 1. generating a random encryption key + * 2. encrypting that random key with a key from KeyManagementService + * that is associated with the given context string + * 3. encrypting a data byte buffer with the generated random key + * 4. returning a byte buffer containing the serialized encrypted key + * and encrypted data (see {@link EncryptedKeyAndEncryptedData} below + * for serialization description + */ +public class AzureCloudBlobCryptoService implements CloudBlobCryptoService { + + private final CryptoService cryptoService; + private final KeyManagementService kms; + //Use this context to look up encryption key for the data encryption key + private final String context; + + public AzureCloudBlobCryptoService(CryptoService cryptoService, KeyManagementService kms, String context) { + this.cryptoService = cryptoService; + this.kms = kms; + this.context = context; + } + + @Override + public ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException { + Object key = kms.getRandomKey(); + Object keyKey = kms.getKey(context); + ByteBuffer encryptedKey = cryptoService.encryptKey(key, keyKey); + ByteBuffer encryptedDataBuffer = cryptoService.encrypt(buffer, key); + ByteBuffer output = ByteBuffer.allocate(EncryptedKeyAndEncryptedData.OVERHEAD_LENGTH + encryptedKey.array().length + + encryptedDataBuffer.array().length); + EncryptedKeyAndEncryptedData.serialize(output, new EncryptedKeyAndEncryptedData(encryptedKey, encryptedDataBuffer)); + return output; + } + + @Override + public ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException { + EncryptedKeyAndEncryptedData encryptedKeyAndEncryptedData = EncryptedKeyAndEncryptedData.deserialize(buffer); + Object blobKeyKey = kms.getKey(context); + Object key = cryptoService.decryptKey(encryptedKeyAndEncryptedData.encryptedKey, blobKeyKey); + return cryptoService.decrypt(encryptedKeyAndEncryptedData.encryptedData, key); + } + + /** + * POJO class for encrypted key and encrypted data byte buffers, also ser/deser for + * encrypted key + encrypted data + * + * + * - - - - - - - - - - - - - - - - - - -- - -- - - - - - -- - - - - -- - - - - - + * | | | | | | | + * | version | Encrypted | Encrypted | Encrypted | Encrypted|Crc | + * |(2 bytes)| Key Size | Data Size | Key | Data |(8 bytes) | + * | | | | | (m bytes)| | + * | | (4 bytes) | (8 bytes) | (n bytes) | | | + * | | | | | | | + * - - - - - - - - - - - - - - - - - - -- - -- - - - - - -- - - - - -- - - - - - + * + * version - The version of the message header + * + * encrypted key - The size of the encrypted key (should be >= 0). + * size + * + * encrypted data - The size of the encrypted data (should be >= 0). + * size 8 bytes for future proofing, but current implementation only + * supports int sized sizes + * + * encrypted key - The encrypted key + * + * encrypted data - The encrypted data + * + * crc - The crc of the message + * + */ + static class EncryptedKeyAndEncryptedData { + + private static short CURRENT_VERSION = 1; + private static int VERSION_FIELD_SIZE = 2; + private static int ENCRYPTED_KEY_SIZE_FIELD_SIZE = 4; + private static int ENCRYPTED_DATA_SIZE_FIELD_SIZE = 8; + static int INITIAL_MESSAGE_LENGTH = + VERSION_FIELD_SIZE + ENCRYPTED_KEY_SIZE_FIELD_SIZE + ENCRYPTED_DATA_SIZE_FIELD_SIZE; + private static int CRC_FIELD_LENGTH = 8; + static int OVERHEAD_LENGTH = INITIAL_MESSAGE_LENGTH + CRC_FIELD_LENGTH; + + private ByteBuffer encryptedKey; + private ByteBuffer encryptedData; + + EncryptedKeyAndEncryptedData(ByteBuffer encryptedKey, ByteBuffer encryptedData) { + this.encryptedKey = encryptedKey; + this.encryptedData = encryptedData; + } + + static void serialize(ByteBuffer outputBuffer, EncryptedKeyAndEncryptedData encryptedKeyAndEncryptedData) { + int startOffset = outputBuffer.position(); + outputBuffer.putShort(CURRENT_VERSION); + int encryptedKeySize = encryptedKeyAndEncryptedData.encryptedKey.array().length; + int encryptedDataSize = encryptedKeyAndEncryptedData.encryptedData.array().length; + outputBuffer.putInt(encryptedKeySize); + outputBuffer.putLong(encryptedDataSize); + outputBuffer.put(encryptedKeyAndEncryptedData.encryptedKey); + outputBuffer.put(encryptedKeyAndEncryptedData.encryptedData); + Crc32 crc = new Crc32(); + crc.update(outputBuffer.array(), startOffset, INITIAL_MESSAGE_LENGTH + encryptedKeySize + encryptedDataSize); + outputBuffer.putLong(crc.getValue()); + } + + static EncryptedKeyAndEncryptedData deserialize(ByteBuffer buffer) throws GeneralSecurityException { + int startOffset = buffer.position(); + Crc32 crc = new Crc32(); + short version = buffer.getShort(); + if (version != 1) { + throw new GeneralSecurityException("Unrecognized version"); + } + int encryptedKeySize = buffer.getInt(); + int encryptedDataSize = (int) buffer.getLong(); + if (encryptedKeySize < 0) { + throw new GeneralSecurityException("Encrypted key size is less than 0"); + } + if (encryptedDataSize < 0) { + throw new GeneralSecurityException("Encrypted data size is less than 0"); + } + byte[] encryptedKey = new byte[encryptedKeySize]; + byte[] encryptedData = new byte[encryptedDataSize]; + buffer.get(encryptedKey); + buffer.get(encryptedData); + crc.update(buffer.array(), startOffset, INITIAL_MESSAGE_LENGTH + encryptedKeySize + encryptedDataSize); + long expectedCrc = buffer.getLong(); + long actualCrc = crc.getValue(); + if (actualCrc != expectedCrc) { + throw new GeneralSecurityException("Encrypted blob is corrupt"); + } + return new EncryptedKeyAndEncryptedData(ByteBuffer.wrap(encryptedKey), ByteBuffer.wrap(encryptedData)); + } + } +} diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java index a60666a96f..fbff92d5a4 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java @@ -280,7 +280,7 @@ private static HashMap getMetadataMap(CloudBlobMetadata cloudBlo map.put(CloudBlobMetadata.FIELD_EXPIRATION_TIME, String.valueOf(cloudBlobMetadata.getExpirationTime())); map.put(CloudBlobMetadata.FIELD_ACCOUNT_ID, String.valueOf(cloudBlobMetadata.getAccountId())); map.put(CloudBlobMetadata.FIELD_CONTAINER_ID, String.valueOf(cloudBlobMetadata.getContainerId())); - map.put(CloudBlobMetadata.FIELD_SHOULD_ENCRYPT, String.valueOf(cloudBlobMetadata.getEncryptionContext())); + map.put(CloudBlobMetadata.FIELD_USES_CLOUD_ENCRYPTION, String.valueOf(cloudBlobMetadata.getUsesCloudEncryption())); return map; } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java index 8741c5f4b2..cee33af84f 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java @@ -121,7 +121,7 @@ public void testFindMissingKeys() throws Exception { // Blob with metadata BlobId existentBlobId = getUniqueId(refAccountId, refContainerId); keys.add(existentBlobId); - metadataMap.put(existentBlobId.getID(), new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024)); + metadataMap.put(existentBlobId.getID(), new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024, false)); // Blob without metadata BlobId nonexistentBlobId = getUniqueId(refAccountId, refContainerId); keys.add(nonexistentBlobId); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java index a9b11da838..91b5d55dde 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java @@ -13,7 +13,6 @@ */ package com.github.ambry.cloud; -import com.github.ambry.commons.BlobId; import java.nio.ByteBuffer; import java.security.GeneralSecurityException; @@ -23,12 +22,12 @@ */ class TestCloudBlobCryptoService implements CloudBlobCryptoService { @Override - public ByteBuffer encrypt(BlobId blobId, ByteBuffer buffer) throws GeneralSecurityException { + public ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException { return buffer; } @Override - public ByteBuffer decrypt(BlobId blobId, ByteBuffer buffer) throws GeneralSecurityException { + public ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException { return buffer; } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceTest.java new file mode 100644 index 0000000000..731a3fc5b9 --- /dev/null +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceTest.java @@ -0,0 +1,149 @@ +/* + * Copyright 2019 LinkedIn Corp. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + */ +package com.github.ambry.cloud.azure; + +import com.github.ambry.config.CryptoServiceConfig; +import com.github.ambry.config.KMSConfig; +import com.github.ambry.config.VerifiableProperties; +import com.github.ambry.router.CryptoService; +import com.github.ambry.router.GCMCryptoService; +import com.github.ambry.router.KeyManagementService; +import com.github.ambry.router.SingleKeyManagementService; +import com.github.ambry.utils.TestUtils; +import java.nio.ByteBuffer; +import java.security.GeneralSecurityException; +import java.util.Properties; +import org.junit.Test; + +import static org.junit.Assert.*; + + +/** + * Tests AzureCloudBlobCryptoService + */ +public class AzureCloudBlobCryptoServiceTest { + + private AzureCloudBlobCryptoService azureCloudBlobCryptoService; + private static final int TWO_FIFTY_SIX_BITS_IN_BYTES = 32; + + public AzureCloudBlobCryptoServiceTest() throws GeneralSecurityException { + CryptoService cryptoService = + new GCMCryptoService(new CryptoServiceConfig(new VerifiableProperties(new Properties()))); + KeyManagementService kms = new SingleKeyManagementService(new KMSConfig(new VerifiableProperties(new Properties())), + TestUtils.getRandomKey(TWO_FIFTY_SIX_BITS_IN_BYTES)); + azureCloudBlobCryptoService = new AzureCloudBlobCryptoService(cryptoService, kms, "any_context"); + } + + /** + * Tests basic encrypt -> decrypt functionality + * @throws Exception + */ + @Test + public void testBasicFunctionality() throws Exception { + byte[] payload = TestUtils.getRandomBytes(10000); + byte[] encryptedPayload = azureCloudBlobCryptoService.encrypt(ByteBuffer.wrap(payload)).array(); + assertFalse(payload.length == encryptedPayload.length); + assertNotSubset("Payload is a subset of the 'encrypted' payload", encryptedPayload, payload); + byte[] decryptedPayload = azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(encryptedPayload)).array(); + assertNotSame("Payload and decrypted payload are the same array instance", payload, decryptedPayload); + assertArrayEquals("Decrypted payload different from original payload", payload, decryptedPayload); + } + + /** + * Tests that all data validation checks work for decryption + * @throws GeneralSecurityException + */ + @Test + public void testBadDecryptInput() throws GeneralSecurityException { + byte[] garbage = TestUtils.getRandomBytes(10000); + //zero out version, encrypted key size, encrypted data size fields + for (int i = 0; i < AzureCloudBlobCryptoService.EncryptedKeyAndEncryptedData.INITIAL_MESSAGE_LENGTH; i++) { + garbage[i] = 0; + } + //Version check should fail + try { + azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(garbage)).array(); + fail("Should have thrown exception"); + } catch (GeneralSecurityException e) { + //expected + } + //Version check pass, encrypted key size check should fail + garbage[1] = 1; + garbage[2] = -128; + try { + azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(garbage)).array(); + fail("Should have thrown exception"); + } catch (GeneralSecurityException e) { + //expected + } + //Version check, encrypted key size check pass, encrypted data size check fail + garbage[2] = 0; + garbage[5] = 79; + garbage[6] = -128; + garbage[10] = -128; + try { + azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(garbage)).array(); + fail("Should have thrown exception"); + } catch (GeneralSecurityException e) { + //expected + } + //Version check, encrypted key size check, encrypted data size check pass, + //crc check fail + garbage[6] = 0; + garbage[10] = 0; + garbage[13] = 79; + try { + azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(garbage)).array(); + fail("Should have thrown exception"); + } catch (GeneralSecurityException e) { + //expected + } + } + + /** + * Throws an exception if the smaller array is a subset of the larger array + * @param message message sent with exception + * @param arr0 + * @param arr1 + * @throws Exception + */ + private void assertNotSubset(String message, byte[] arr0, byte[] arr1) throws Exception { + boolean found; + if (arr0 == null || arr1 == null) { + throw new Exception("Array arguments should not be null"); + } + if (arr0.length == 0 || arr1.length == 0) { + throw new Exception("Array arguments should not have 0 length"); + } + byte[] largerArray = arr0.length > arr1.length ? arr0 : arr1; + byte[] smallerArray = largerArray == arr0 ? arr1 : arr0; + + for (int i = 0; i <= largerArray.length - smallerArray.length; i++) { + //could add rolling hash function to make comparison more efficient + //but overkill in this testing application + found = true; + int ix = i; + for (int j = 0; j < smallerArray.length; j++) { + if (largerArray[ix] != smallerArray[j]) { + found = false; + break; + } + ix++; + } + if (found) { + throw new Exception(message); + } + } + } +} diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java index d6a09232b4..4aa45753bc 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java @@ -152,7 +152,7 @@ public void testUpdateNotExists() throws Exception { @Test public void testQueryMetadata() throws Exception { QueryIterable mockIterable = mock(QueryIterable.class); - CloudBlobMetadata inputMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize); + CloudBlobMetadata inputMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, false); List docList = Collections.singletonList(new Document(objectMapper.writeValueAsString(inputMetadata))); when(mockIterable.iterator()).thenReturn(docList.iterator()); FeedResponse feedResponse = mock(FeedResponse.class); @@ -228,7 +228,7 @@ public void testUpdateDocClientException() throws Exception { */ private boolean uploadDefaultBlob() throws CloudStorageException { InputStream inputStream = getBlobInputStream(blobSize); - CloudBlobMetadata metadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize); + CloudBlobMetadata metadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, false); return azureDest.uploadBlob(blobId, blobSize, metadata, inputStream); } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java index 85615e749e..b526c3ffed 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java @@ -84,7 +84,7 @@ public void testNormalFlow() throws Exception { BlobDataType.DATACHUNK); InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata cloudBlobMetadata = - new CloudBlobMetadata(blobId, System.currentTimeMillis(), Utils.Infinite_Time, blobSize); + new CloudBlobMetadata(blobId, System.currentTimeMillis(), Utils.Infinite_Time, blobSize, false); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); // Try to upload same blob again @@ -117,7 +117,7 @@ public void testBatchQuery() throws Exception { BlobDataType.DATACHUNK); blobIdList.add(blobId); InputStream inputStream = getBlobInputStream(blobSize); - CloudBlobMetadata cloudBlobMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize); + CloudBlobMetadata cloudBlobMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, false); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); } diff --git a/ambry-router/src/main/java/com.github.ambry.router/GCMCryptoService.java b/ambry-router/src/main/java/com.github.ambry.router/GCMCryptoService.java index e85f55acff..8321bce3d9 100644 --- a/ambry-router/src/main/java/com.github.ambry.router/GCMCryptoService.java +++ b/ambry-router/src/main/java/com.github.ambry.router/GCMCryptoService.java @@ -38,7 +38,7 @@ * {@link CryptoService} which is capable of encrypting or decrypting bytes based on the given key. * This implementation uses GCM for encryption and decryption */ -class GCMCryptoService implements CryptoService { +public class GCMCryptoService implements CryptoService { private static final int VERSION_FIELD_SIZE_IN_BYTES = Short.BYTES; private static final short KEY_RECORD_VERSION_V_1 = 1; @@ -50,7 +50,7 @@ class GCMCryptoService implements CryptoService { private static final Logger logger = LoggerFactory.getLogger(GCMCryptoService.class); - GCMCryptoService(CryptoServiceConfig cryptoServiceConfig) { + public GCMCryptoService(CryptoServiceConfig cryptoServiceConfig) { config = cryptoServiceConfig; ivValSize = cryptoServiceConfig.cryptoServiceIvSizeInBytes; Security.addProvider(new BouncyCastleProvider()); diff --git a/ambry-router/src/main/java/com.github.ambry.router/SingleKeyManagementService.java b/ambry-router/src/main/java/com.github.ambry.router/SingleKeyManagementService.java index 9050aba6ec..8e0156b165 100644 --- a/ambry-router/src/main/java/com.github.ambry.router/SingleKeyManagementService.java +++ b/ambry-router/src/main/java/com.github.ambry.router/SingleKeyManagementService.java @@ -40,7 +40,7 @@ public class SingleKeyManagementService implements KeyManagementService Date: Sun, 31 Mar 2019 15:47:38 -0700 Subject: [PATCH 05/15] Integration code --- .../com.github.ambry/config/CloudConfig.java | 49 +++++++++++++++++++ .../CloudBackupManager.java | 10 +++- .../CloudBlobCryptoService.java | 4 +- .../CloudBlobCryptoServiceFactory.java | 6 +++ .../CloudBlobStore.java | 5 +- .../AzureCloudBlobCryptoServiceFactory.java | 36 ++++++++++++++ .../CloudBlobStoreTest.java | 6 +-- .../ServerTestUtil.java | 7 ++- 8 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java create mode 100644 ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java diff --git a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java index 2e48fbfbb8..5283ea2360 100644 --- a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java +++ b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java @@ -24,6 +24,18 @@ public class CloudConfig { public static final String CLOUD_DESTINATION_FACTORY_CLASS = "cloud.destination.factory.class"; public static final String DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS = "com.github.ambry.cloud.azure.AzureCloudDestinationFactory"; + public static final String CRYPTO_SERVICE_FACTORY_CLASS = "vcr.crypto.service.factory.class"; + public static final String DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS = + "com.github.ambry.router.GCMCryptoServiceFactory"; + public static final String KMS_SERVICE_FACTORY_CLASS = "vcr.kms.service.factory.class"; + public static final String DEFAULT_KMS_SERVICE_FACTORY_CLASS = + "com.github.ambry.router.SingleKeyManagementServiceFactory"; + public static final String KMS_SERVICE_KEY_CONTEXT = "vcr.kms.service.key.context"; + public static final String DEFAULT_KMS_SERVICE_KEY_CONTEXT = "vcr.kms.service.key.context"; + public static final String CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = + "vcr.cloud.blob.crypto.service.factory.class"; + public static final String DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = + "com.github.ambry.cloud.azure.AzureCloudBlobCryptoServiceFactory"; @Config("vcr.cluster.zk.connect.string") @Default(VCR_CLUSTER_ZK_CONNECT_STRING) @@ -57,6 +69,34 @@ public class CloudConfig { @Default(DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS) public final String cloudDestinationFactoryClass; + /** + * The crypto service factory class name. + */ + @Config(CRYPTO_SERVICE_FACTORY_CLASS) + @Default(DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS) + public final String cryptoServiceFactoryClass; + + /** + * The kms service factory class name. + */ + @Config(KMS_SERVICE_FACTORY_CLASS) + @Default(DEFAULT_KMS_SERVICE_FACTORY_CLASS) + public final String kmsServiceFactoryClass; + + /** + * The kms destination factory class name. + */ + @Config(KMS_SERVICE_KEY_CONTEXT) + @Default(DEFAULT_KMS_SERVICE_KEY_CONTEXT) + public final String kmsServiceKeyContext; + + /** + * The cloud blob crypto service factory class name. + */ + @Config(CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS) + @Default(DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS) + public final String cloudBlobCryptoServiceFactoryClass; + public CloudConfig(VerifiableProperties verifiableProperties) { vcrClusterZkConnectString = @@ -67,5 +107,14 @@ public CloudConfig(VerifiableProperties verifiableProperties) { verifiableProperties.getString("vcr.replica.mount.path.prefix", VCR_REPLICA_MOUNT_PATH_PREFIX); cloudDestinationFactoryClass = verifiableProperties.getString(CLOUD_DESTINATION_FACTORY_CLASS, DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS); + cryptoServiceFactoryClass = + verifiableProperties.getString(CRYPTO_SERVICE_FACTORY_CLASS, DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS); + kmsServiceFactoryClass = + verifiableProperties.getString(KMS_SERVICE_FACTORY_CLASS, DEFAULT_KMS_SERVICE_FACTORY_CLASS); + kmsServiceKeyContext = + verifiableProperties.getString(KMS_SERVICE_KEY_CONTEXT, DEFAULT_KMS_SERVICE_KEY_CONTEXT); + cloudBlobCryptoServiceFactoryClass = + verifiableProperties.getString(CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS, + DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS); } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java index fe41cc5c83..c300fbc837 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java @@ -14,6 +14,7 @@ package com.github.ambry.cloud; import com.codahale.metrics.MetricRegistry; +import com.github.ambry.cloud.azure.AzureCloudBlobCryptoService; import com.github.ambry.clustermap.ClusterMap; import com.github.ambry.clustermap.PartitionId; import com.github.ambry.clustermap.ReplicaId; @@ -34,6 +35,7 @@ import com.github.ambry.store.StoreKeyConverterFactory; import com.github.ambry.store.StoreKeyFactory; import com.github.ambry.utils.SystemTime; +import com.github.ambry.utils.Utils; import java.util.ArrayList; import java.util.List; import java.util.concurrent.ScheduledExecutorService; @@ -49,17 +51,21 @@ public CloudBackupManager(CloudConfig cloudConfig, ReplicationConfig replication ClusterMap clusterMap, VirtualReplicatorCluster virtualReplicatorCluster, CloudDestinationFactory cloudDestinationFactory, ScheduledExecutorService scheduler, ConnectionPool connectionPool, MetricRegistry metricRegistry, NotificationSystem requestNotification, - StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName) throws ReplicationException { + StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName, + CloudBlobCryptoServiceFactory cloudBlobCryptoServiceFactory) throws ReplicationException { super(replicationConfig, clusterMapConfig, storeKeyFactory, clusterMap, scheduler, virtualReplicatorCluster.getCurrentDataNodeId(), connectionPool, metricRegistry, requestNotification, storeKeyConverterFactory, transformerClassName); CloudDestination cloudDestination = cloudDestinationFactory.getCloudDestination(); List partitionIds = virtualReplicatorCluster.getAssignedPartitionIds(); + for (PartitionId partitionId : partitionIds) { ReplicaId cloudReplica = new CloudReplica(cloudConfig, partitionId, virtualReplicatorCluster.getCurrentDataNodeId()); - Store cloudStore = new CloudBlobStore(partitionId, null, cloudDestination); + + Store cloudStore = new CloudBlobStore(partitionId, null, cloudDestination, + cloudBlobCryptoServiceFactory.getCloudBlobCryptoService()); try { cloudStore.start(); } catch (StoreException e) { diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java index 9cf8cdddf2..a08f44b586 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java @@ -29,7 +29,7 @@ public interface CloudBlobCryptoService { * @return The encrypted buffer. * @throws {@link GeneralSecurityException} on any exception with encryption. */ - public ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException; + ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException; /** * Decrypts the blob buffer being downloaded from cloud storage. @@ -37,5 +37,5 @@ public interface CloudBlobCryptoService { * @return The decrypted buffer. * @throws {@link GeneralSecurityException} on any exception with decryption */ - public ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException; + ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException; } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java new file mode 100644 index 0000000000..7648535efb --- /dev/null +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java @@ -0,0 +1,6 @@ +package com.github.ambry.cloud; + +public interface CloudBlobCryptoServiceFactory { + + CloudBlobCryptoService getCloudBlobCryptoService(); +} diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 02c6d9ad1a..6e8aedf50d 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -15,6 +15,7 @@ import com.github.ambry.clustermap.PartitionId; import com.github.ambry.commons.BlobId; +import com.github.ambry.config.VerifiableProperties; import com.github.ambry.store.FindInfo; import com.github.ambry.store.FindToken; import com.github.ambry.store.MessageInfo; @@ -59,7 +60,7 @@ class CloudBlobStore implements Store { * @param cloudDestination the {@link CloudDestination}. * @param cryptoService the {@link CloudBlobCryptoService} to use for encryption. */ - CloudBlobStore(PartitionId partitionId, CloudDestination cloudDestination, CloudBlobCryptoService cryptoService) { + CloudBlobStore(PartitionId partitionId, VerifiableProperties verProps, CloudDestination cloudDestination, CloudBlobCryptoService cryptoService) { this.cloudDestination = cloudDestination; this.partitionId = partitionId; this.cryptoService = cryptoService; @@ -106,7 +107,7 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) BlobId blobId = (BlobId) messageInfo.getStoreKey(); // TODO: would be more efficient to call blobId.isEncrypted() boolean usesCloudEncryption = false; - if (blobId.getVersion() < 4 || !BlobId.isEncrypted(blobId.getID())) { + if (cryptoService != null && (blobId.getVersion() < 4 || !BlobId.isEncrypted(blobId.getID()))) { // Need to encrypt the buffer before upload messageBuf = cryptoService.encrypt(messageBuf); usesCloudEncryption = true; diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java new file mode 100644 index 0000000000..6a43d9f319 --- /dev/null +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java @@ -0,0 +1,36 @@ +package com.github.ambry.cloud.azure; + +import com.codahale.metrics.MetricRegistry; +import com.github.ambry.cloud.CloudBlobCryptoService; +import com.github.ambry.cloud.CloudBlobCryptoServiceFactory; +import com.github.ambry.config.CloudConfig; +import com.github.ambry.config.VerifiableProperties; +import com.github.ambry.router.CryptoServiceFactory; +import com.github.ambry.router.KeyManagementServiceFactory; +import com.github.ambry.utils.Utils; +import java.security.GeneralSecurityException; + + +public class AzureCloudBlobCryptoServiceFactory implements CloudBlobCryptoServiceFactory { + + private CryptoServiceFactory cryptoServiceFactory; + private KeyManagementServiceFactory keyManagementServiceFactory; + private String context; + + public AzureCloudBlobCryptoServiceFactory(VerifiableProperties verifiableProperties, String clusterName, MetricRegistry metricRegistry) + throws ReflectiveOperationException { + CloudConfig cloudConfig = new CloudConfig(verifiableProperties); + cryptoServiceFactory = Utils.getObj(cloudConfig.cryptoServiceFactoryClass, verifiableProperties, metricRegistry); + keyManagementServiceFactory = Utils.getObj(cloudConfig.kmsServiceFactoryClass, verifiableProperties, clusterName, metricRegistry); + context = cloudConfig.kmsServiceKeyContext; + } + + @Override + public CloudBlobCryptoService getCloudBlobCryptoService() { + try { + return new AzureCloudBlobCryptoService(cryptoServiceFactory.getCryptoService(), keyManagementServiceFactory.getKeyManagementService(), context); + } catch (GeneralSecurityException e) { + throw new IllegalStateException(e); + } + } +} diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java index cee33af84f..029a19f2cc 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java @@ -62,7 +62,7 @@ public class CloudBlobStoreTest { public void setup() throws Exception { dest = mock(CloudDestination.class); partitionId = new MockPartitionId(); - store = new CloudBlobStore(partitionId, dest, cryptoService); + store = new CloudBlobStore(partitionId, null, dest, cryptoService); store.start(); } @@ -136,7 +136,7 @@ public void testFindMissingKeys() throws Exception { @Test public void testStoreNotStarted() throws Exception { // Create store and don't start it. - CloudBlobStore idleStore = new CloudBlobStore(partitionId, dest, cryptoService); + CloudBlobStore idleStore = new CloudBlobStore(partitionId,null, dest, cryptoService); List keys = Collections.singletonList(getUniqueId(refAccountId, refContainerId)); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); addBlobToSet(messageWriteSet, 10, Utils.Infinite_Time, refAccountId, refContainerId); @@ -168,7 +168,7 @@ public void testExceptionalDest() throws Exception { new CloudStorageException("ouch")); when(exDest.deleteBlob(any(BlobId.class), anyLong())).thenThrow(new CloudStorageException("ouch")); when(exDest.getBlobMetadata(anyList())).thenThrow(new CloudStorageException("ouch")); - CloudBlobStore exStore = new CloudBlobStore(partitionId, exDest, cryptoService); + CloudBlobStore exStore = new CloudBlobStore(partitionId, null, exDest, cryptoService); exStore.start(); List keys = Collections.singletonList(getUniqueId(refAccountId, refContainerId)); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); diff --git a/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java b/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java index bb74611bce..fb1a0d82a0 100644 --- a/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java +++ b/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java @@ -17,6 +17,7 @@ import com.github.ambry.account.AccountService; import com.github.ambry.account.InMemAccountService; import com.github.ambry.cloud.CloudBackupManager; +import com.github.ambry.cloud.CloudBlobCryptoServiceFactory; import com.github.ambry.cloud.CloudBlobMetadata; import com.github.ambry.cloud.CloudDataNode; import com.github.ambry.cloud.CloudDestinationFactory; @@ -610,6 +611,7 @@ static void endToEndBackupManagerTest(MockCluster cluster, DataNodeId dataNode, props.setProperty("clustermap.port", "12309"); props.setProperty("vcr.ssl.port", "12310"); props.setProperty("vcr.cluster.name", "VCRCluster"); + props.setProperty("kms.default.container.key", TestUtils.getRandomKey(32)); VerifiableProperties vProps = new VerifiableProperties(props); ClusterMapConfig clusterMapConfig = new ClusterMapConfig(vProps); CloudConfig cloudConfig = new CloudConfig(vProps); @@ -629,11 +631,14 @@ static void endToEndBackupManagerTest(MockCluster cluster, DataNodeId dataNode, new BlockingChannelConnectionPool(connectionPoolConfig, clientSSLConfig, clusterMapConfig, clusterMap.getMetricRegistry()); connectionPool.start(); + CloudBlobCryptoServiceFactory cloudBlobCryptoServiceFactory = Utils.getObj(cloudConfig.cloudBlobCryptoServiceFactoryClass, + vProps, "ambry", clusterMap.getMetricRegistry()); CloudBackupManager cloudBackupManager = new CloudBackupManager(cloudConfig, replicationConfig, clusterMapConfig, storeConfig, Utils.getObj(storeConfig.storeKeyFactory, clusterMap), clusterMap, virtualReplicatorCluster, mockCloudDestinationFactory, scheduler, connectionPool, clusterMap.getMetricRegistry(), null, - new StoreKeyConverterFactoryImpl(null, null), serverConfig.serverMessageTransformer); + new StoreKeyConverterFactoryImpl(null, null), serverConfig.serverMessageTransformer, + cloudBlobCryptoServiceFactory); cloudBackupManager.start(); // Waiting for backup done assertTrue("Did not backup all blobs in 2 minutes", latchBasedInMemoryCloudDestination.await(2, TimeUnit.MINUTES)); From b47497984f1c9bd967bff4d719cf379b95436f65 Mon Sep 17 00:00:00 2001 From: David Harju Date: Sun, 31 Mar 2019 15:54:21 -0700 Subject: [PATCH 06/15] Licenses and java doc --- .../CloudBlobCryptoServiceFactory.java | 13 +++++++++++++ .../AzureCloudBlobCryptoServiceFactory.java | 16 ++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java index 7648535efb..eeb2c1622f 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java @@ -1,3 +1,16 @@ +/* + * Copyright 2019 LinkedIn Corp. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + */ package com.github.ambry.cloud; public interface CloudBlobCryptoServiceFactory { diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java index 6a43d9f319..d0be41856c 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java @@ -1,3 +1,16 @@ +/* + * Copyright 2019 LinkedIn Corp. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + */ package com.github.ambry.cloud.azure; import com.codahale.metrics.MetricRegistry; @@ -11,6 +24,9 @@ import java.security.GeneralSecurityException; +/** + * Factory for constructing {@link AzureCloudBlobCryptoService} instances. + */ public class AzureCloudBlobCryptoServiceFactory implements CloudBlobCryptoServiceFactory { private CryptoServiceFactory cryptoServiceFactory; From 57b521570736266e2aff38032a9ef3b21dbe1468 Mon Sep 17 00:00:00 2001 From: David Harju Date: Sun, 31 Mar 2019 15:59:06 -0700 Subject: [PATCH 07/15] file reformatting --- .../java/com.github.ambry/config/CloudConfig.java | 14 +++++--------- .../com.github.ambry.cloud/CloudBlobMetadata.java | 7 +++++-- .../com.github.ambry.cloud/CloudBlobStore.java | 3 ++- .../azure/AzureCloudBlobCryptoServiceFactory.java | 10 ++++++---- .../com.github.ambry.cloud/CloudBlobStoreTest.java | 5 +++-- .../azure/AzureCloudDestinationTest.java | 3 ++- .../azure/AzureIntegrationTest.java | 3 ++- .../BlobIdTransformer.java | 1 - .../com.github.ambry.server/ServerTestUtil.java | 4 ++-- 9 files changed, 27 insertions(+), 23 deletions(-) diff --git a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java index 5283ea2360..a56dc14304 100644 --- a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java +++ b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java @@ -25,15 +25,13 @@ public class CloudConfig { public static final String DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS = "com.github.ambry.cloud.azure.AzureCloudDestinationFactory"; public static final String CRYPTO_SERVICE_FACTORY_CLASS = "vcr.crypto.service.factory.class"; - public static final String DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS = - "com.github.ambry.router.GCMCryptoServiceFactory"; + public static final String DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS = "com.github.ambry.router.GCMCryptoServiceFactory"; public static final String KMS_SERVICE_FACTORY_CLASS = "vcr.kms.service.factory.class"; public static final String DEFAULT_KMS_SERVICE_FACTORY_CLASS = "com.github.ambry.router.SingleKeyManagementServiceFactory"; public static final String KMS_SERVICE_KEY_CONTEXT = "vcr.kms.service.key.context"; public static final String DEFAULT_KMS_SERVICE_KEY_CONTEXT = "vcr.kms.service.key.context"; - public static final String CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = - "vcr.cloud.blob.crypto.service.factory.class"; + public static final String CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = "vcr.cloud.blob.crypto.service.factory.class"; public static final String DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = "com.github.ambry.cloud.azure.AzureCloudBlobCryptoServiceFactory"; @@ -111,10 +109,8 @@ public CloudConfig(VerifiableProperties verifiableProperties) { verifiableProperties.getString(CRYPTO_SERVICE_FACTORY_CLASS, DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS); kmsServiceFactoryClass = verifiableProperties.getString(KMS_SERVICE_FACTORY_CLASS, DEFAULT_KMS_SERVICE_FACTORY_CLASS); - kmsServiceKeyContext = - verifiableProperties.getString(KMS_SERVICE_KEY_CONTEXT, DEFAULT_KMS_SERVICE_KEY_CONTEXT); - cloudBlobCryptoServiceFactoryClass = - verifiableProperties.getString(CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS, - DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS); + kmsServiceKeyContext = verifiableProperties.getString(KMS_SERVICE_KEY_CONTEXT, DEFAULT_KMS_SERVICE_KEY_CONTEXT); + cloudBlobCryptoServiceFactoryClass = verifiableProperties.getString(CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS, + DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS); } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java index 26311894f5..1abe8ebd75 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java @@ -54,7 +54,8 @@ public CloudBlobMetadata() { * @param expirationTime The blob expiration time. * @param size The blob size. */ - public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size, boolean usesCloudEncryption) { + public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size, + boolean usesCloudEncryption) { this.id = blobId.getID(); this.partitionId = blobId.getPartition().toPathString(); this.accountId = blobId.getAccountId(); @@ -218,7 +219,9 @@ public CloudBlobMetadata setContainerId(int containerId) { /** * @return whether the blob was cloud encrypted. */ - public boolean getUsesCloudEncryption() { return usesCloudEncryption;} + public boolean getUsesCloudEncryption() { + return usesCloudEncryption; + } /** * Set the encryption context. diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 6e8aedf50d..0a91f6fc83 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -60,7 +60,8 @@ class CloudBlobStore implements Store { * @param cloudDestination the {@link CloudDestination}. * @param cryptoService the {@link CloudBlobCryptoService} to use for encryption. */ - CloudBlobStore(PartitionId partitionId, VerifiableProperties verProps, CloudDestination cloudDestination, CloudBlobCryptoService cryptoService) { + CloudBlobStore(PartitionId partitionId, VerifiableProperties verProps, CloudDestination cloudDestination, + CloudBlobCryptoService cryptoService) { this.cloudDestination = cloudDestination; this.partitionId = partitionId; this.cryptoService = cryptoService; diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java index d0be41856c..99fdfdc295 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java @@ -33,18 +33,20 @@ public class AzureCloudBlobCryptoServiceFactory implements CloudBlobCryptoServic private KeyManagementServiceFactory keyManagementServiceFactory; private String context; - public AzureCloudBlobCryptoServiceFactory(VerifiableProperties verifiableProperties, String clusterName, MetricRegistry metricRegistry) - throws ReflectiveOperationException { + public AzureCloudBlobCryptoServiceFactory(VerifiableProperties verifiableProperties, String clusterName, + MetricRegistry metricRegistry) throws ReflectiveOperationException { CloudConfig cloudConfig = new CloudConfig(verifiableProperties); cryptoServiceFactory = Utils.getObj(cloudConfig.cryptoServiceFactoryClass, verifiableProperties, metricRegistry); - keyManagementServiceFactory = Utils.getObj(cloudConfig.kmsServiceFactoryClass, verifiableProperties, clusterName, metricRegistry); + keyManagementServiceFactory = + Utils.getObj(cloudConfig.kmsServiceFactoryClass, verifiableProperties, clusterName, metricRegistry); context = cloudConfig.kmsServiceKeyContext; } @Override public CloudBlobCryptoService getCloudBlobCryptoService() { try { - return new AzureCloudBlobCryptoService(cryptoServiceFactory.getCryptoService(), keyManagementServiceFactory.getKeyManagementService(), context); + return new AzureCloudBlobCryptoService(cryptoServiceFactory.getCryptoService(), + keyManagementServiceFactory.getKeyManagementService(), context); } catch (GeneralSecurityException e) { throw new IllegalStateException(e); } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java index 029a19f2cc..b8cfbc4f35 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java @@ -121,7 +121,8 @@ public void testFindMissingKeys() throws Exception { // Blob with metadata BlobId existentBlobId = getUniqueId(refAccountId, refContainerId); keys.add(existentBlobId); - metadataMap.put(existentBlobId.getID(), new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024, false)); + metadataMap.put(existentBlobId.getID(), + new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024, false)); // Blob without metadata BlobId nonexistentBlobId = getUniqueId(refAccountId, refContainerId); keys.add(nonexistentBlobId); @@ -136,7 +137,7 @@ public void testFindMissingKeys() throws Exception { @Test public void testStoreNotStarted() throws Exception { // Create store and don't start it. - CloudBlobStore idleStore = new CloudBlobStore(partitionId,null, dest, cryptoService); + CloudBlobStore idleStore = new CloudBlobStore(partitionId, null, dest, cryptoService); List keys = Collections.singletonList(getUniqueId(refAccountId, refContainerId)); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); addBlobToSet(messageWriteSet, 10, Utils.Infinite_Time, refAccountId, refContainerId); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java index b0e95b41d8..8406ab1ab6 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java @@ -184,7 +184,8 @@ public void testInitClientException() throws Exception { props.setProperty(AzureCloudConfig.COSMOS_ENDPOINT, "http://ambry.cosmos.com"); props.setProperty(AzureCloudConfig.COSMOS_COLLECTION_LINK, "ambry/metadata"); props.setProperty(AzureCloudConfig.COSMOS_KEY, "dummykey"); - CloudDestinationFactory factory = new AzureCloudDestinationFactory(new VerifiableProperties(props), new MetricRegistry()); + CloudDestinationFactory factory = + new AzureCloudDestinationFactory(new VerifiableProperties(props), new MetricRegistry()); try { factory.getCloudDestination(); fail("Expected exception to be thrown"); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java index 6eb6b8c915..8997184cee 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java @@ -118,7 +118,8 @@ public void testBatchQuery() throws Exception { BlobDataType.DATACHUNK); blobIdList.add(blobId); InputStream inputStream = getBlobInputStream(blobSize); - CloudBlobMetadata cloudBlobMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, false); + CloudBlobMetadata cloudBlobMetadata = + new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, false); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); } diff --git a/ambry-replication/src/main/java/com.github.ambry.replication/BlobIdTransformer.java b/ambry-replication/src/main/java/com.github.ambry.replication/BlobIdTransformer.java index baf4c7f5a3..306a471d4b 100644 --- a/ambry-replication/src/main/java/com.github.ambry.replication/BlobIdTransformer.java +++ b/ambry-replication/src/main/java/com.github.ambry.replication/BlobIdTransformer.java @@ -216,7 +216,6 @@ private Message newMessage(InputStream inputStream, StoreKey newKey, MessageInfo PutMessageFormatInputStream putMessageFormatInputStream = new PutMessageFormatInputStream(newKey, blobEncryptionKey, newProperties, userMetaData, blobDataBytes, blobData.getSize(), blobData.getBlobType()); - // Reuse the original CRC if present in the oldMessageInfo. This is important to ensure that messages that are // received via replication are sent to the store with proper CRCs (which the store needs to detect duplicate // messages). As an additional guard, here the original CRC is only reused if the key's ID in string form is the diff --git a/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java b/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java index fb1a0d82a0..9ef277b10c 100644 --- a/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java +++ b/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java @@ -631,8 +631,8 @@ static void endToEndBackupManagerTest(MockCluster cluster, DataNodeId dataNode, new BlockingChannelConnectionPool(connectionPoolConfig, clientSSLConfig, clusterMapConfig, clusterMap.getMetricRegistry()); connectionPool.start(); - CloudBlobCryptoServiceFactory cloudBlobCryptoServiceFactory = Utils.getObj(cloudConfig.cloudBlobCryptoServiceFactoryClass, - vProps, "ambry", clusterMap.getMetricRegistry()); + CloudBlobCryptoServiceFactory cloudBlobCryptoServiceFactory = + Utils.getObj(cloudConfig.cloudBlobCryptoServiceFactoryClass, vProps, "ambry", clusterMap.getMetricRegistry()); CloudBackupManager cloudBackupManager = new CloudBackupManager(cloudConfig, replicationConfig, clusterMapConfig, storeConfig, Utils.getObj(storeConfig.storeKeyFactory, clusterMap), clusterMap, virtualReplicatorCluster, From a5b8b9c0909ddac5ceea23cba9ebcdf9bc8a5483 Mon Sep 17 00:00:00 2001 From: David Harju Date: Wed, 3 Apr 2019 16:30:03 -0700 Subject: [PATCH 08/15] Review comments --- .../src/main/java/com.github.ambry/config/CloudConfig.java | 2 +- .../main/java/com.github.ambry.cloud/CloudBackupManager.java | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java index a56dc14304..5097563370 100644 --- a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java +++ b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java @@ -30,7 +30,7 @@ public class CloudConfig { public static final String DEFAULT_KMS_SERVICE_FACTORY_CLASS = "com.github.ambry.router.SingleKeyManagementServiceFactory"; public static final String KMS_SERVICE_KEY_CONTEXT = "vcr.kms.service.key.context"; - public static final String DEFAULT_KMS_SERVICE_KEY_CONTEXT = "vcr.kms.service.key.context"; + public static final String DEFAULT_KMS_SERVICE_KEY_CONTEXT = "default-backup"; public static final String CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = "vcr.cloud.blob.crypto.service.factory.class"; public static final String DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = "com.github.ambry.cloud.azure.AzureCloudBlobCryptoServiceFactory"; diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java index c300fbc837..b7d29eeb20 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java @@ -14,7 +14,6 @@ package com.github.ambry.cloud; import com.codahale.metrics.MetricRegistry; -import com.github.ambry.cloud.azure.AzureCloudBlobCryptoService; import com.github.ambry.clustermap.ClusterMap; import com.github.ambry.clustermap.PartitionId; import com.github.ambry.clustermap.ReplicaId; @@ -35,7 +34,6 @@ import com.github.ambry.store.StoreKeyConverterFactory; import com.github.ambry.store.StoreKeyFactory; import com.github.ambry.utils.SystemTime; -import com.github.ambry.utils.Utils; import java.util.ArrayList; import java.util.List; import java.util.concurrent.ScheduledExecutorService; From f061fc0d8ab5cc370e8f79ddec837c5a50080cfe Mon Sep 17 00:00:00 2001 From: David Harju Date: Thu, 4 Apr 2019 15:43:23 -0700 Subject: [PATCH 09/15] Major review comment changes --- .../com.github.ambry/config/CloudConfig.java | 39 ++++------ .../CloudBackupManager.java | 5 +- ...Service.java => CloudBlobCryptoAgent.java} | 8 +- ....java => CloudBlobCryptoAgentFactory.java} | 7 +- ...a => CloudBlobCryptoAgentFactoryImpl.java} | 20 ++--- ...ice.java => CloudBlobCryptoAgentImpl.java} | 17 ++-- .../CloudBlobMetadata.java | 77 ++++++++++++++++--- .../CloudBlobStore.java | 19 +++-- .../azure/AzureCloudDestination.java | 4 +- ...java => CloudBlobCryptoAgentImplTest.java} | 15 ++-- .../CloudBlobStoreTest.java | 5 +- ...ice.java => TestCloudBlobCryptoAgent.java} | 7 +- .../azure/AzureCloudDestinationTest.java | 6 +- .../azure/AzureIntegrationTest.java | 6 +- .../ServerTestUtil.java | 6 +- 15 files changed, 157 insertions(+), 84 deletions(-) rename ambry-cloud/src/main/java/com.github.ambry.cloud/{CloudBlobCryptoService.java => CloudBlobCryptoAgent.java} (90%) rename ambry-cloud/src/main/java/com.github.ambry.cloud/{CloudBlobCryptoServiceFactory.java => CloudBlobCryptoAgentFactory.java} (70%) rename ambry-cloud/src/main/java/com.github.ambry.cloud/{azure/AzureCloudBlobCryptoServiceFactory.java => CloudBlobCryptoAgentFactoryImpl.java} (62%) rename ambry-cloud/src/main/java/com.github.ambry.cloud/{azure/AzureCloudBlobCryptoService.java => CloudBlobCryptoAgentImpl.java} (93%) rename ambry-cloud/src/test/java/com.github.ambry.cloud/{azure/AzureCloudBlobCryptoServiceTest.java => CloudBlobCryptoAgentImplTest.java} (90%) rename ambry-cloud/src/test/java/com.github.ambry.cloud/{TestCloudBlobCryptoService.java => TestCloudBlobCryptoAgent.java} (86%) diff --git a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java index 5097563370..c4602419dc 100644 --- a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java +++ b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java @@ -24,16 +24,13 @@ public class CloudConfig { public static final String CLOUD_DESTINATION_FACTORY_CLASS = "cloud.destination.factory.class"; public static final String DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS = "com.github.ambry.cloud.azure.AzureCloudDestinationFactory"; - public static final String CRYPTO_SERVICE_FACTORY_CLASS = "vcr.crypto.service.factory.class"; - public static final String DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS = "com.github.ambry.router.GCMCryptoServiceFactory"; - public static final String KMS_SERVICE_FACTORY_CLASS = "vcr.kms.service.factory.class"; - public static final String DEFAULT_KMS_SERVICE_FACTORY_CLASS = - "com.github.ambry.router.SingleKeyManagementServiceFactory"; - public static final String KMS_SERVICE_KEY_CONTEXT = "vcr.kms.service.key.context"; + public static final String KMS_SERVICE_KEY_CONTEXT = "kms.service.key.context"; public static final String DEFAULT_KMS_SERVICE_KEY_CONTEXT = "default-backup"; - public static final String CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = "vcr.cloud.blob.crypto.service.factory.class"; + public static final String CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = "cloud.blob.crypto.service.factory.class"; public static final String DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS = - "com.github.ambry.cloud.azure.AzureCloudBlobCryptoServiceFactory"; + "com.github.ambry.cloud.CloudBlobCryptoAgentFactoryImpl"; + public static final String VCR_ENCRYPTION_FORMAT = "vcr.encryption.format"; + public static final String DEFAULT_VCR_ENCRYPTION_FORMAT = "DEFAULT"; @Config("vcr.cluster.zk.connect.string") @Default(VCR_CLUSTER_ZK_CONNECT_STRING) @@ -67,20 +64,6 @@ public class CloudConfig { @Default(DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS) public final String cloudDestinationFactoryClass; - /** - * The crypto service factory class name. - */ - @Config(CRYPTO_SERVICE_FACTORY_CLASS) - @Default(DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS) - public final String cryptoServiceFactoryClass; - - /** - * The kms service factory class name. - */ - @Config(KMS_SERVICE_FACTORY_CLASS) - @Default(DEFAULT_KMS_SERVICE_FACTORY_CLASS) - public final String kmsServiceFactoryClass; - /** * The kms destination factory class name. */ @@ -95,6 +78,13 @@ public class CloudConfig { @Default(DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS) public final String cloudBlobCryptoServiceFactoryClass; + /** + * VCR Encryption format + */ + @Config(VCR_ENCRYPTION_FORMAT) + @Default(DEFAULT_VCR_ENCRYPTION_FORMAT) + public final String vcrEncryptionFormat; + public CloudConfig(VerifiableProperties verifiableProperties) { vcrClusterZkConnectString = @@ -105,12 +95,9 @@ public CloudConfig(VerifiableProperties verifiableProperties) { verifiableProperties.getString("vcr.replica.mount.path.prefix", VCR_REPLICA_MOUNT_PATH_PREFIX); cloudDestinationFactoryClass = verifiableProperties.getString(CLOUD_DESTINATION_FACTORY_CLASS, DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS); - cryptoServiceFactoryClass = - verifiableProperties.getString(CRYPTO_SERVICE_FACTORY_CLASS, DEFAULT_CRYPTO_SERVICE_FACTORY_CLASS); - kmsServiceFactoryClass = - verifiableProperties.getString(KMS_SERVICE_FACTORY_CLASS, DEFAULT_KMS_SERVICE_FACTORY_CLASS); kmsServiceKeyContext = verifiableProperties.getString(KMS_SERVICE_KEY_CONTEXT, DEFAULT_KMS_SERVICE_KEY_CONTEXT); cloudBlobCryptoServiceFactoryClass = verifiableProperties.getString(CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS, DEFAULT_CLOUD_BLOB_CRYPTO_SERVICE_FACTORY_CLASS); + vcrEncryptionFormat = verifiableProperties.getString(VCR_ENCRYPTION_FORMAT, DEFAULT_VCR_ENCRYPTION_FORMAT); } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java index b7d29eeb20..d0a5676a11 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java @@ -50,7 +50,7 @@ public CloudBackupManager(CloudConfig cloudConfig, ReplicationConfig replication CloudDestinationFactory cloudDestinationFactory, ScheduledExecutorService scheduler, ConnectionPool connectionPool, MetricRegistry metricRegistry, NotificationSystem requestNotification, StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName, - CloudBlobCryptoServiceFactory cloudBlobCryptoServiceFactory) throws ReplicationException { + CloudBlobCryptoAgentFactory cloudBlobCryptoAgentFactory) throws ReplicationException { super(replicationConfig, clusterMapConfig, storeKeyFactory, clusterMap, scheduler, virtualReplicatorCluster.getCurrentDataNodeId(), connectionPool, metricRegistry, requestNotification, @@ -63,7 +63,8 @@ public CloudBackupManager(CloudConfig cloudConfig, ReplicationConfig replication new CloudReplica(cloudConfig, partitionId, virtualReplicatorCluster.getCurrentDataNodeId()); Store cloudStore = new CloudBlobStore(partitionId, null, cloudDestination, - cloudBlobCryptoServiceFactory.getCloudBlobCryptoService()); + cloudBlobCryptoAgentFactory.getCloudBlobCryptoAgent( + CloudBlobMetadata.VcrEncryptionFormat.valueOf(cloudConfig.vcrEncryptionFormat))); try { cloudStore.start(); } catch (StoreException e) { diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgent.java similarity index 90% rename from ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java rename to ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgent.java index a08f44b586..e5ed4894d5 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoService.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgent.java @@ -21,7 +21,7 @@ * Crypto Service to assist in encrypting and decrypting blob chunks transferred between * Ambry and the cloud backup. */ -public interface CloudBlobCryptoService { +public interface CloudBlobCryptoAgent { /** * Encrypts the blob buffer being uploaded to cloud storage. @@ -38,4 +38,10 @@ public interface CloudBlobCryptoService { * @throws {@link GeneralSecurityException} on any exception with decryption */ ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException; + + /** + * Returns the encryption context (if any) + * @return + */ + String getEncryptionContext(); } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactory.java similarity index 70% rename from ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java rename to ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactory.java index eeb2c1622f..d30fc973db 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoServiceFactory.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactory.java @@ -13,7 +13,10 @@ */ package com.github.ambry.cloud; -public interface CloudBlobCryptoServiceFactory { +/** + * Factory for constructing {@link CloudBlobCryptoAgent} instances. + */ +public interface CloudBlobCryptoAgentFactory { - CloudBlobCryptoService getCloudBlobCryptoService(); + CloudBlobCryptoAgent getCloudBlobCryptoAgent(CloudBlobMetadata.VcrEncryptionFormat vcrEncryptionFormat); } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java similarity index 62% rename from ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java rename to ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java index 99fdfdc295..e1b953942b 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceFactory.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java @@ -11,12 +11,11 @@ * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. */ -package com.github.ambry.cloud.azure; +package com.github.ambry.cloud; import com.codahale.metrics.MetricRegistry; -import com.github.ambry.cloud.CloudBlobCryptoService; -import com.github.ambry.cloud.CloudBlobCryptoServiceFactory; import com.github.ambry.config.CloudConfig; +import com.github.ambry.config.RouterConfig; import com.github.ambry.config.VerifiableProperties; import com.github.ambry.router.CryptoServiceFactory; import com.github.ambry.router.KeyManagementServiceFactory; @@ -25,27 +24,28 @@ /** - * Factory for constructing {@link AzureCloudBlobCryptoService} instances. + * Factory for constructing {@link CloudBlobCryptoAgentImpl} instances. */ -public class AzureCloudBlobCryptoServiceFactory implements CloudBlobCryptoServiceFactory { +public class CloudBlobCryptoAgentFactoryImpl implements CloudBlobCryptoAgentFactory { private CryptoServiceFactory cryptoServiceFactory; private KeyManagementServiceFactory keyManagementServiceFactory; private String context; - public AzureCloudBlobCryptoServiceFactory(VerifiableProperties verifiableProperties, String clusterName, + public CloudBlobCryptoAgentFactoryImpl(VerifiableProperties verifiableProperties, String clusterName, MetricRegistry metricRegistry) throws ReflectiveOperationException { CloudConfig cloudConfig = new CloudConfig(verifiableProperties); - cryptoServiceFactory = Utils.getObj(cloudConfig.cryptoServiceFactoryClass, verifiableProperties, metricRegistry); + RouterConfig routerConfig = new RouterConfig(verifiableProperties); + cryptoServiceFactory = Utils.getObj(routerConfig.routerCryptoServiceFactory, verifiableProperties, metricRegistry); keyManagementServiceFactory = - Utils.getObj(cloudConfig.kmsServiceFactoryClass, verifiableProperties, clusterName, metricRegistry); + Utils.getObj(routerConfig.routerKeyManagementServiceFactory, verifiableProperties, clusterName, metricRegistry); context = cloudConfig.kmsServiceKeyContext; } @Override - public CloudBlobCryptoService getCloudBlobCryptoService() { + public CloudBlobCryptoAgent getCloudBlobCryptoAgent(CloudBlobMetadata.VcrEncryptionFormat vcrEncryptionFormat) { try { - return new AzureCloudBlobCryptoService(cryptoServiceFactory.getCryptoService(), + return new CloudBlobCryptoAgentImpl(cryptoServiceFactory.getCryptoService(), keyManagementServiceFactory.getKeyManagementService(), context); } catch (GeneralSecurityException e) { throw new IllegalStateException(e); diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoService.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java similarity index 93% rename from ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoService.java rename to ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java index 105bdbac99..9a93550ab5 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoService.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java @@ -11,9 +11,9 @@ * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. */ -package com.github.ambry.cloud.azure; +package com.github.ambry.cloud; -import com.github.ambry.cloud.CloudBlobCryptoService; +import com.github.ambry.cloud.CloudBlobCryptoAgent; import com.github.ambry.router.CryptoService; import com.github.ambry.router.KeyManagementService; import com.github.ambry.utils.Crc32; @@ -22,7 +22,7 @@ /** - * Implementation of CloudBlobCryptoService, encrypts byte buffers by + * Implementation of CloudBlobCryptoAgent, encrypts byte buffers by * 1. generating a random encryption key * 2. encrypting that random key with a key from KeyManagementService * that is associated with the given context string @@ -31,14 +31,14 @@ * and encrypted data (see {@link EncryptedKeyAndEncryptedData} below * for serialization description */ -public class AzureCloudBlobCryptoService implements CloudBlobCryptoService { +public class CloudBlobCryptoAgentImpl implements CloudBlobCryptoAgent { private final CryptoService cryptoService; private final KeyManagementService kms; //Use this context to look up encryption key for the data encryption key private final String context; - public AzureCloudBlobCryptoService(CryptoService cryptoService, KeyManagementService kms, String context) { + public CloudBlobCryptoAgentImpl(CryptoService cryptoService, KeyManagementService kms, String context) { this.cryptoService = cryptoService; this.kms = kms; this.context = context; @@ -64,6 +64,11 @@ public ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException { return cryptoService.decrypt(encryptedKeyAndEncryptedData.encryptedData, key); } + @Override + public String getEncryptionContext() { + return context; + } + /** * POJO class for encrypted key and encrypted data byte buffers, also ser/deser for * encrypted key + encrypted data @@ -150,7 +155,7 @@ static EncryptedKeyAndEncryptedData deserialize(ByteBuffer buffer) throws Genera long expectedCrc = buffer.getLong(); long actualCrc = crc.getValue(); if (actualCrc != expectedCrc) { - throw new GeneralSecurityException("Encrypted blob is corrupt"); + throw new GeneralSecurityException("Encrypted blob is corrupt. ExpectedCRC: "+expectedCrc+" ActualCRC: "+actualCrc); } return new EncryptedKeyAndEncryptedData(ByteBuffer.wrap(encryptedKey), ByteBuffer.wrap(encryptedData)); } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java index 1abe8ebd75..0ee18279e5 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java @@ -28,7 +28,9 @@ public class CloudBlobMetadata { public static final String FIELD_EXPIRATION_TIME = "expirationTime"; public static final String FIELD_ACCOUNT_ID = "accountId"; public static final String FIELD_CONTAINER_ID = "containerId"; - public static final String FIELD_USES_CLOUD_ENCRYPTION = "usesCloudEncryption"; + public static final String FIELD_ENCRYPTION_TYPE = "encryptionType"; + public static final String FIELD_KMS_CONTEXT = "kmsContext"; + public static final String FIELD_ENCRYPTION_FORMAT = "vcrEncryptionFormat"; private String id; private String partitionId; @@ -39,7 +41,19 @@ public class CloudBlobMetadata { private int containerId; private long expirationTime; private long deletionTime; - private boolean usesCloudEncryption; + private String kmsContext; + private EncryptionType encryptionType; + private VcrEncryptionFormat vcrEncryptionFormat; + + /** + * Different encryption types for cloud stored blobs + */ + public enum EncryptionType {ROUTER, VCR, CLOUD;} + + /** + * Different encryption formats for VCR encryption + */ + public enum VcrEncryptionFormat {DEFAULT;} /** * Default constructor (for JSONSerializer). @@ -53,9 +67,11 @@ public CloudBlobMetadata() { * @param creationTime The blob creation time. * @param expirationTime The blob expiration time. * @param size The blob size. + * @param kmsContext + * @param encryptionType */ - public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size, - boolean usesCloudEncryption) { + public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size, String kmsContext, + EncryptionType encryptionType, VcrEncryptionFormat vcrEncryptionFormat) { this.id = blobId.getID(); this.partitionId = blobId.getPartition().toPathString(); this.accountId = blobId.getAccountId(); @@ -65,6 +81,9 @@ public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, this.uploadTime = System.currentTimeMillis(); this.deletionTime = Utils.Infinite_Time; this.size = size; + this.kmsContext = kmsContext; + this.encryptionType = encryptionType; + this.vcrEncryptionFormat = vcrEncryptionFormat; } /** @@ -217,19 +236,53 @@ public CloudBlobMetadata setContainerId(int containerId) { } /** - * @return whether the blob was cloud encrypted. + * @return the kms context */ - public boolean getUsesCloudEncryption() { - return usesCloudEncryption; + public String getKmsContext() { + return kmsContext; } /** - * Set the encryption context. - * @param usesCloudEncryption whether {@link CloudBlobCryptoService} was used on the blob. - * @return this instance. + * Sets the kms context + * @param kmsContext + * @return + */ + public CloudBlobMetadata setKmsContext(String kmsContext) { + this.kmsContext = kmsContext; + return this; + } + + /** + * @return the encryption type + */ + public EncryptionType getEncryptionType() { + return encryptionType; + } + + /** + * Sets the encryption type + * @param encryptionType + * @return + */ + public CloudBlobMetadata setEncryptionType(EncryptionType encryptionType) { + this.encryptionType = encryptionType; + return this; + } + + /** + * @return the VCR encryption format + */ + public VcrEncryptionFormat getVcrEncryptionFormat() { + return vcrEncryptionFormat; + } + + /** + * Sets the VCR encryption format + * @param vcrEncryptionFormat + * @return */ - public CloudBlobMetadata setUsesCloudEncryption(boolean usesCloudEncryption) { - this.usesCloudEncryption = usesCloudEncryption; + public CloudBlobMetadata setVcrEncryptionFormat(VcrEncryptionFormat vcrEncryptionFormat) { + this.vcrEncryptionFormat = vcrEncryptionFormat; return this; } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 0a91f6fc83..b99fb1ef8e 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -51,17 +51,17 @@ class CloudBlobStore implements Store { private static final Logger logger = LoggerFactory.getLogger(CloudBlobStore.class); private final PartitionId partitionId; private final CloudDestination cloudDestination; - private final CloudBlobCryptoService cryptoService; + private final CloudBlobCryptoAgent cryptoService; private boolean started; /** * Constructor for CloudBlobStore * @param partitionId partition associated with BlobStore. * @param cloudDestination the {@link CloudDestination}. - * @param cryptoService the {@link CloudBlobCryptoService} to use for encryption. + * @param cryptoService the {@link CloudBlobCryptoAgent} to use for encryption. */ CloudBlobStore(PartitionId partitionId, VerifiableProperties verProps, CloudDestination cloudDestination, - CloudBlobCryptoService cryptoService) { + CloudBlobCryptoAgent cryptoService) { this.cloudDestination = cloudDestination; this.partitionId = partitionId; this.cryptoService = cryptoService; @@ -107,15 +107,20 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) if (performUpload) { BlobId blobId = (BlobId) messageInfo.getStoreKey(); // TODO: would be more efficient to call blobId.isEncrypted() - boolean usesCloudEncryption = false; - if (cryptoService != null && (blobId.getVersion() < 4 || !BlobId.isEncrypted(blobId.getID()))) { + String kmsContext = cryptoService.getEncryptionContext(); + CloudBlobMetadata.EncryptionType encryptionType; + if (cryptoService != null && (blobId.getVersion() < BlobId.BLOB_ID_V4 || !BlobId.isEncrypted(blobId.getID()))) { // Need to encrypt the buffer before upload messageBuf = cryptoService.encrypt(messageBuf); - usesCloudEncryption = true; + encryptionType = CloudBlobMetadata.EncryptionType.VCR; + } + else { + kmsContext = blobId.getAccountId()+":"+blobId.getContainerId(); + encryptionType = CloudBlobMetadata.EncryptionType.ROUTER; } CloudBlobMetadata blobMetadata = new CloudBlobMetadata(blobId, messageInfo.getOperationTimeMs(), messageInfo.getExpirationTimeInMs(), - messageInfo.getSize(), usesCloudEncryption); + messageInfo.getSize(), kmsContext, encryptionType, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); cloudDestination.uploadBlob(blobId, size, blobMetadata, new ByteBufferInputStream(messageBuf)); } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java index 5b77450360..0fc06492a7 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java @@ -278,7 +278,9 @@ private static HashMap getMetadataMap(CloudBlobMetadata cloudBlo map.put(CloudBlobMetadata.FIELD_EXPIRATION_TIME, String.valueOf(cloudBlobMetadata.getExpirationTime())); map.put(CloudBlobMetadata.FIELD_ACCOUNT_ID, String.valueOf(cloudBlobMetadata.getAccountId())); map.put(CloudBlobMetadata.FIELD_CONTAINER_ID, String.valueOf(cloudBlobMetadata.getContainerId())); - map.put(CloudBlobMetadata.FIELD_USES_CLOUD_ENCRYPTION, String.valueOf(cloudBlobMetadata.getUsesCloudEncryption())); + map.put(CloudBlobMetadata.FIELD_ENCRYPTION_TYPE, cloudBlobMetadata.getEncryptionType().name()); + map.put(CloudBlobMetadata.FIELD_KMS_CONTEXT, String.valueOf(cloudBlobMetadata.getKmsContext())); + map.put(CloudBlobMetadata.FIELD_ENCRYPTION_FORMAT, cloudBlobMetadata.getVcrEncryptionFormat().name()); return map; } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobCryptoAgentImplTest.java similarity index 90% rename from ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceTest.java rename to ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobCryptoAgentImplTest.java index 731a3fc5b9..11e596ba47 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudBlobCryptoServiceTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobCryptoAgentImplTest.java @@ -11,8 +11,9 @@ * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. */ -package com.github.ambry.cloud.azure; +package com.github.ambry.cloud; +import com.github.ambry.cloud.CloudBlobCryptoAgentImpl; import com.github.ambry.config.CryptoServiceConfig; import com.github.ambry.config.KMSConfig; import com.github.ambry.config.VerifiableProperties; @@ -30,19 +31,19 @@ /** - * Tests AzureCloudBlobCryptoService + * Tests CloudBlobCryptoAgentImpl */ -public class AzureCloudBlobCryptoServiceTest { +public class CloudBlobCryptoAgentImplTest { - private AzureCloudBlobCryptoService azureCloudBlobCryptoService; + private CloudBlobCryptoAgentImpl azureCloudBlobCryptoService; private static final int TWO_FIFTY_SIX_BITS_IN_BYTES = 32; - public AzureCloudBlobCryptoServiceTest() throws GeneralSecurityException { + public CloudBlobCryptoAgentImplTest() throws GeneralSecurityException { CryptoService cryptoService = new GCMCryptoService(new CryptoServiceConfig(new VerifiableProperties(new Properties()))); KeyManagementService kms = new SingleKeyManagementService(new KMSConfig(new VerifiableProperties(new Properties())), TestUtils.getRandomKey(TWO_FIFTY_SIX_BITS_IN_BYTES)); - azureCloudBlobCryptoService = new AzureCloudBlobCryptoService(cryptoService, kms, "any_context"); + azureCloudBlobCryptoService = new CloudBlobCryptoAgentImpl(cryptoService, kms, "any_context"); } /** @@ -68,7 +69,7 @@ public void testBasicFunctionality() throws Exception { public void testBadDecryptInput() throws GeneralSecurityException { byte[] garbage = TestUtils.getRandomBytes(10000); //zero out version, encrypted key size, encrypted data size fields - for (int i = 0; i < AzureCloudBlobCryptoService.EncryptedKeyAndEncryptedData.INITIAL_MESSAGE_LENGTH; i++) { + for (int i = 0; i < CloudBlobCryptoAgentImpl.EncryptedKeyAndEncryptedData.INITIAL_MESSAGE_LENGTH; i++) { garbage[i] = 0; } //Version check should fail diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java index b8cfbc4f35..3d644eb14a 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java @@ -49,7 +49,7 @@ */ public class CloudBlobStoreTest { - private final CloudBlobCryptoService cryptoService = new TestCloudBlobCryptoService(); + private final CloudBlobCryptoAgent cryptoService = new TestCloudBlobCryptoAgent(); private Store store; private CloudDestination dest; private PartitionId partitionId; @@ -122,7 +122,8 @@ public void testFindMissingKeys() throws Exception { BlobId existentBlobId = getUniqueId(refAccountId, refContainerId); keys.add(existentBlobId); metadataMap.put(existentBlobId.getID(), - new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024, false)); + new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024, null, + CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT)); // Blob without metadata BlobId nonexistentBlobId = getUniqueId(refAccountId, refContainerId); keys.add(nonexistentBlobId); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoAgent.java similarity index 86% rename from ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java rename to ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoAgent.java index 91b5d55dde..36c689a63a 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoService.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoAgent.java @@ -20,7 +20,7 @@ /** * Dummy implementation for testing. */ -class TestCloudBlobCryptoService implements CloudBlobCryptoService { +class TestCloudBlobCryptoAgent implements CloudBlobCryptoAgent { @Override public ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException { return buffer; @@ -30,4 +30,9 @@ public ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException { public ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException { return buffer; } + + @Override + public String getEncryptionContext() { + return null; + } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java index 8406ab1ab6..9e3c1e247b 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java @@ -154,7 +154,8 @@ public void testUpdateNotExists() throws Exception { @Test public void testQueryMetadata() throws Exception { QueryIterable mockIterable = mock(QueryIterable.class); - CloudBlobMetadata inputMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, false); + CloudBlobMetadata inputMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, null, + CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); List docList = Collections.singletonList(new Document(objectMapper.writeValueAsString(inputMetadata))); when(mockIterable.iterator()).thenReturn(docList.iterator()); FeedResponse feedResponse = mock(FeedResponse.class); @@ -240,7 +241,8 @@ public void testUpdateDocClientException() throws Exception { */ private boolean uploadDefaultBlob() throws CloudStorageException { InputStream inputStream = getBlobInputStream(blobSize); - CloudBlobMetadata metadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, false); + CloudBlobMetadata metadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, null, + CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); return azureDest.uploadBlob(blobId, blobSize, metadata, inputStream); } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java index 8997184cee..cce2be5291 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java @@ -85,7 +85,8 @@ public void testNormalFlow() throws Exception { BlobDataType.DATACHUNK); InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata cloudBlobMetadata = - new CloudBlobMetadata(blobId, System.currentTimeMillis(), Utils.Infinite_Time, blobSize, false); + new CloudBlobMetadata(blobId, System.currentTimeMillis(), Utils.Infinite_Time, blobSize, null, + CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); // Try to upload same blob again @@ -119,7 +120,8 @@ public void testBatchQuery() throws Exception { blobIdList.add(blobId); InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata cloudBlobMetadata = - new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, false); + new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, null, + CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); } diff --git a/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java b/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java index 9ef277b10c..b7ad53f0ea 100644 --- a/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java +++ b/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java @@ -17,7 +17,7 @@ import com.github.ambry.account.AccountService; import com.github.ambry.account.InMemAccountService; import com.github.ambry.cloud.CloudBackupManager; -import com.github.ambry.cloud.CloudBlobCryptoServiceFactory; +import com.github.ambry.cloud.CloudBlobCryptoAgentFactory; import com.github.ambry.cloud.CloudBlobMetadata; import com.github.ambry.cloud.CloudDataNode; import com.github.ambry.cloud.CloudDestinationFactory; @@ -631,14 +631,14 @@ static void endToEndBackupManagerTest(MockCluster cluster, DataNodeId dataNode, new BlockingChannelConnectionPool(connectionPoolConfig, clientSSLConfig, clusterMapConfig, clusterMap.getMetricRegistry()); connectionPool.start(); - CloudBlobCryptoServiceFactory cloudBlobCryptoServiceFactory = + CloudBlobCryptoAgentFactory cloudBlobCryptoAgentFactory = Utils.getObj(cloudConfig.cloudBlobCryptoServiceFactoryClass, vProps, "ambry", clusterMap.getMetricRegistry()); CloudBackupManager cloudBackupManager = new CloudBackupManager(cloudConfig, replicationConfig, clusterMapConfig, storeConfig, Utils.getObj(storeConfig.storeKeyFactory, clusterMap), clusterMap, virtualReplicatorCluster, mockCloudDestinationFactory, scheduler, connectionPool, clusterMap.getMetricRegistry(), null, new StoreKeyConverterFactoryImpl(null, null), serverConfig.serverMessageTransformer, - cloudBlobCryptoServiceFactory); + cloudBlobCryptoAgentFactory); cloudBackupManager.start(); // Waiting for backup done assertTrue("Did not backup all blobs in 2 minutes", latchBasedInMemoryCloudDestination.await(2, TimeUnit.MINUTES)); From 068cab7c3fdcab011f6253165f77d8d746189132 Mon Sep 17 00:00:00 2001 From: Rob Block Date: Sun, 14 Apr 2019 21:29:09 -0700 Subject: [PATCH 10/15] Rename inner format class to EncryptedDataPayload, encryptionType -> encryptionOrigin Add encryption metrics --- .../com.github.ambry/config/CloudConfig.java | 40 +++++---- .../com.github.ambry/config/RouterConfig.java | 15 ++-- .../CloudBackupManager.java | 5 +- .../CloudBlobCryptoAgentFactory.java | 2 +- .../CloudBlobCryptoAgentFactoryImpl.java | 8 +- .../CloudBlobCryptoAgentImpl.java | 90 ++++++++++++------- .../CloudBlobMetadata.java | 78 +++++++--------- .../CloudBlobStore.java | 59 ++++++++---- .../com.github.ambry.cloud/VcrMetrics.java | 16 +++- .../com.github.ambry.cloud/VcrServer.java | 2 +- .../azure/AzureCloudDestination.java | 3 +- .../CloudBlobCryptoAgentImplTest.java | 19 ++-- .../CloudBlobStoreTest.java | 81 +++++++++++++---- .../com.github.ambry.cloud/VcrServerTest.java | 2 + .../azure/AzureCloudDestinationTest.java | 8 +- .../azure/AzureIntegrationTest.java | 8 +- 16 files changed, 270 insertions(+), 166 deletions(-) diff --git a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java index 3446b18850..73956295bc 100644 --- a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java +++ b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java @@ -25,6 +25,9 @@ public class CloudConfig { public static final String VCR_SSL_PORT = "vcr.ssl.port"; public static final String VCR_REPLICA_MOUNT_PATH_PREFIX = "vcr.replica.mount.path.prefix"; public static final String VCR_REQUIRE_ENCRYPTION = "vcr.require.encryption"; + public static final String VCR_KMS_FACTORY = "vcr.key.management.service.factory"; + public static final String VCR_CRYPTO_SERVICE_FACTORY = "vcr.crypto.service.factory"; + public static final String VCR_MIN_TTL_DAYS = "vcr.min.ttl.days"; public static final String VCR_ASSIGNED_PARTITIONS = "vcr.assigned.partitions"; @@ -87,14 +90,14 @@ public class CloudConfig { public final String vcrReplicaMountPathPrefix; /** - * The cloud destination factory class name. + * Require blobs to be encrypted prior to cloud upload? */ - @Config(CLOUD_DESTINATION_FACTORY_CLASS) - @Default(DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS) - public final String cloudDestinationFactoryClass; + @Config(VCR_REQUIRE_ENCRYPTION) + @Default("false") + public final boolean vcrRequireEncryption; /** - * The kms destination factory class name. + * The kms service factory class name. */ @Config(KMS_SERVICE_KEY_CONTEXT) @Default(DEFAULT_KMS_SERVICE_KEY_CONTEXT) @@ -108,18 +111,18 @@ public class CloudConfig { public final String cloudBlobCryptoAgentFactoryClass; /** - * VCR Encryption format + * The KeyManagementServiceFactory that will be used to fetch {@link com.github.ambry.router.KeyManagementService} */ - @Config(VCR_ENCRYPTION_FORMAT) - @Default(DEFAULT_VCR_ENCRYPTION_FORMAT) - public final String vcrEncryptionFormat; + @Config(VCR_KMS_FACTORY) + @Default(RouterConfig.DEFAULT_KMS_FACTORY) + public final String vcrKeyManagementServiceFactory; /** - * Require blobs to be encrypted prior to cloud upload? + * The CryptoServiceFactory that will be used to fetch {@link com.github.ambry.router.CryptoService} */ - @Config(VCR_REQUIRE_ENCRYPTION) - @Default("false") - public final boolean vcrRequireEncryption; + @Config(VCR_CRYPTO_SERVICE_FACTORY) + @Default(RouterConfig.DEFAULT_CRYPTO_SERVICE_FACTORY) + public final String vcrCryptoServiceFactory; /** * Minimum TTL in days required for blobs to be uploaded to cloud. @@ -141,10 +144,6 @@ public CloudConfig(VerifiableProperties verifiableProperties) { DEFAULT_VIRTUAL_REPLICATOR_CLUSTER_FACTORY_CLASS); cloudDestinationFactoryClass = verifiableProperties.getString(CLOUD_DESTINATION_FACTORY_CLASS, DEFAULT_CLOUD_DESTINATION_FACTORY_CLASS); - kmsServiceKeyContext = verifiableProperties.getString(KMS_SERVICE_KEY_CONTEXT, DEFAULT_KMS_SERVICE_KEY_CONTEXT); - cloudBlobCryptoAgentFactoryClass = verifiableProperties.getString(CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS, - DEFAULT_CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS); - vcrEncryptionFormat = verifiableProperties.getString(VCR_ENCRYPTION_FORMAT, DEFAULT_VCR_ENCRYPTION_FORMAT); vcrAssignedPartitions = verifiableProperties.getString(VCR_ASSIGNED_PARTITIONS, null); vcrClusterZkConnectString = verifiableProperties.getString(VCR_CLUSTER_ZK_CONNECT_STRING, DEFAULT_VCR_CLUSTER_ZK_CONNECT_STRING); @@ -153,7 +152,12 @@ public CloudConfig(VerifiableProperties verifiableProperties) { vcrReplicaMountPathPrefix = verifiableProperties.getString(VCR_REPLICA_MOUNT_PATH_PREFIX, DEFAULT_VCR_REPLICA_MOUNT_PATH_PREFIX); vcrRequireEncryption = verifiableProperties.getBoolean(VCR_REQUIRE_ENCRYPTION, false); + vcrKeyManagementServiceFactory = verifiableProperties.getString(VCR_KMS_FACTORY, RouterConfig.DEFAULT_KMS_FACTORY); + vcrCryptoServiceFactory = + verifiableProperties.getString(VCR_CRYPTO_SERVICE_FACTORY, RouterConfig.DEFAULT_CRYPTO_SERVICE_FACTORY); + kmsServiceKeyContext = verifiableProperties.getString(KMS_SERVICE_KEY_CONTEXT, DEFAULT_KMS_SERVICE_KEY_CONTEXT); + cloudBlobCryptoAgentFactoryClass = verifiableProperties.getString(CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS, + DEFAULT_CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS); vcrMinTtlDays = verifiableProperties.getInt(VCR_MIN_TTL_DAYS, DEFAULT_MIN_TTL_DAYS); - vcrAssignedPartitions = verifiableProperties.getString(VCR_ASSIGNED_PARTITIONS, null); } } diff --git a/ambry-api/src/main/java/com.github.ambry/config/RouterConfig.java b/ambry-api/src/main/java/com.github.ambry/config/RouterConfig.java index 51e9355f3b..9285aea8c0 100644 --- a/ambry-api/src/main/java/com.github.ambry/config/RouterConfig.java +++ b/ambry-api/src/main/java/com.github.ambry/config/RouterConfig.java @@ -21,6 +21,9 @@ */ public class RouterConfig { + public static final String DEFAULT_KMS_FACTORY = "com.github.ambry.router.SingleKeyManagementServiceFactory"; + public static final String DEFAULT_CRYPTO_SERVICE_FACTORY = "com.github.ambry.router.GCMCryptoServiceFactory"; + /** * Number of independent scaling units for the router. */ @@ -188,14 +191,14 @@ public class RouterConfig { * The KeyManagementServiceFactory that will be used to fetch {@link com.github.ambry.router.KeyManagementService} */ @Config("router.key.management.service.factory") - @Default("com.github.ambry.router.SingleKeyManagementServiceFactory") + @Default(DEFAULT_KMS_FACTORY) public final String routerKeyManagementServiceFactory; /** * The CryptoServiceFactory that will be used to fetch {@link com.github.ambry.router.CryptoService} */ @Config("router.crypto.service.factory") - @Default("com.github.ambry.router.GCMCryptoServiceFactory") + @Default(DEFAULT_CRYPTO_SERVICE_FACTORY) public final String routerCryptoServiceFactory; /** @@ -264,10 +267,10 @@ public RouterConfig(VerifiableProperties verifiableProperties) { routerBlobidCurrentVersion = verifiableProperties.getShortFromAllowedValues("router.blobid.current.version", (short) 6, new Short[]{1, 2, 3, 4, 5, 6}); - routerKeyManagementServiceFactory = verifiableProperties.getString("router.key.management.service.factory", - "com.github.ambry.router.SingleKeyManagementServiceFactory"); - routerCryptoServiceFactory = verifiableProperties.getString("router.crypto.service.factory", - "com.github.ambry.router.GCMCryptoServiceFactory"); + routerKeyManagementServiceFactory = + verifiableProperties.getString("router.key.management.service.factory", DEFAULT_KMS_FACTORY); + routerCryptoServiceFactory = + verifiableProperties.getString("router.crypto.service.factory", DEFAULT_CRYPTO_SERVICE_FACTORY); routerCryptoJobsWorkerCount = verifiableProperties.getIntInRange("router.crypto.jobs.worker.count", 1, 1, Integer.MAX_VALUE); routerTtlUpdateRequestParallelism = diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java index d6aefc123f..fe37727cc5 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java @@ -51,7 +51,7 @@ public CloudBackupManager(CloudConfig cloudConfig, ReplicationConfig replication CloudDestinationFactory cloudDestinationFactory, ScheduledExecutorService scheduler, ConnectionPool connectionPool, MetricRegistry metricRegistry, NotificationSystem requestNotification, StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName, - CloudBlobCryptoAgentFactory cloudBlobCryptoAgentFactory) throws ReplicationException { + CloudBlobCryptoAgentFactory cloudBlobCryptoAgentFactory, VcrMetrics vcrMetrics) throws ReplicationException { super(replicationConfig, clusterMapConfig, storeKeyFactory, clusterMap, scheduler, virtualReplicatorCluster.getCurrentDataNodeId(), Collections.emptyList(), connectionPool, metricRegistry, @@ -63,8 +63,7 @@ public CloudBackupManager(CloudConfig cloudConfig, ReplicationConfig replication ReplicaId cloudReplica = new CloudReplica(cloudConfig, partitionId, virtualReplicatorCluster.getCurrentDataNodeId()); Store cloudStore = new CloudBlobStore(partitionId, cloudConfig, cloudDestination, - cloudBlobCryptoAgentFactory.getCloudBlobCryptoAgent( - CloudBlobMetadata.VcrEncryptionFormat.valueOf(cloudConfig.vcrEncryptionFormat))); + cloudBlobCryptoAgentFactory.getCloudBlobCryptoAgent(), vcrMetrics); try { cloudStore.start(); } catch (StoreException e) { diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactory.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactory.java index d30fc973db..6469781f9b 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactory.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactory.java @@ -18,5 +18,5 @@ */ public interface CloudBlobCryptoAgentFactory { - CloudBlobCryptoAgent getCloudBlobCryptoAgent(CloudBlobMetadata.VcrEncryptionFormat vcrEncryptionFormat); + CloudBlobCryptoAgent getCloudBlobCryptoAgent(); } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java index e1b953942b..81b13b3346 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java @@ -15,7 +15,6 @@ import com.codahale.metrics.MetricRegistry; import com.github.ambry.config.CloudConfig; -import com.github.ambry.config.RouterConfig; import com.github.ambry.config.VerifiableProperties; import com.github.ambry.router.CryptoServiceFactory; import com.github.ambry.router.KeyManagementServiceFactory; @@ -35,15 +34,14 @@ public class CloudBlobCryptoAgentFactoryImpl implements CloudBlobCryptoAgentFact public CloudBlobCryptoAgentFactoryImpl(VerifiableProperties verifiableProperties, String clusterName, MetricRegistry metricRegistry) throws ReflectiveOperationException { CloudConfig cloudConfig = new CloudConfig(verifiableProperties); - RouterConfig routerConfig = new RouterConfig(verifiableProperties); - cryptoServiceFactory = Utils.getObj(routerConfig.routerCryptoServiceFactory, verifiableProperties, metricRegistry); + cryptoServiceFactory = Utils.getObj(cloudConfig.vcrCryptoServiceFactory, verifiableProperties, metricRegistry); keyManagementServiceFactory = - Utils.getObj(routerConfig.routerKeyManagementServiceFactory, verifiableProperties, clusterName, metricRegistry); + Utils.getObj(cloudConfig.vcrKeyManagementServiceFactory, verifiableProperties, clusterName, metricRegistry); context = cloudConfig.kmsServiceKeyContext; } @Override - public CloudBlobCryptoAgent getCloudBlobCryptoAgent(CloudBlobMetadata.VcrEncryptionFormat vcrEncryptionFormat) { + public CloudBlobCryptoAgent getCloudBlobCryptoAgent() { try { return new CloudBlobCryptoAgentImpl(cryptoServiceFactory.getCryptoService(), keyManagementServiceFactory.getKeyManagementService(), context); diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java index 9a93550ab5..5ef9498992 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java @@ -13,7 +13,6 @@ */ package com.github.ambry.cloud; -import com.github.ambry.cloud.CloudBlobCryptoAgent; import com.github.ambry.router.CryptoService; import com.github.ambry.router.KeyManagementService; import com.github.ambry.utils.Crc32; @@ -28,7 +27,7 @@ * that is associated with the given context string * 3. encrypting a data byte buffer with the generated random key * 4. returning a byte buffer containing the serialized encrypted key - * and encrypted data (see {@link EncryptedKeyAndEncryptedData} below + * and encrypted data (see {@link EncryptedDataPayload} below * for serialization description */ public class CloudBlobCryptoAgentImpl implements CloudBlobCryptoAgent { @@ -47,21 +46,21 @@ public CloudBlobCryptoAgentImpl(CryptoService cryptoService, KeyManagementServic @Override public ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException { Object key = kms.getRandomKey(); - Object keyKey = kms.getKey(context); - ByteBuffer encryptedKey = cryptoService.encryptKey(key, keyKey); + Object contextKey = kms.getKey(context); + ByteBuffer encryptedKey = cryptoService.encryptKey(key, contextKey); ByteBuffer encryptedDataBuffer = cryptoService.encrypt(buffer, key); - ByteBuffer output = ByteBuffer.allocate(EncryptedKeyAndEncryptedData.OVERHEAD_LENGTH + encryptedKey.array().length - + encryptedDataBuffer.array().length); - EncryptedKeyAndEncryptedData.serialize(output, new EncryptedKeyAndEncryptedData(encryptedKey, encryptedDataBuffer)); + ByteBuffer output = ByteBuffer.allocate( + EncryptedDataPayload.OVERHEAD_LENGTH + encryptedKey.array().length + encryptedDataBuffer.array().length); + EncryptedDataPayload.serialize(output, new EncryptedDataPayload(encryptedKey, encryptedDataBuffer)); return output; } @Override public ByteBuffer decrypt(ByteBuffer buffer) throws GeneralSecurityException { - EncryptedKeyAndEncryptedData encryptedKeyAndEncryptedData = EncryptedKeyAndEncryptedData.deserialize(buffer); + EncryptedDataPayload encryptedDataPayload = EncryptedDataPayload.deserialize(buffer); Object blobKeyKey = kms.getKey(context); - Object key = cryptoService.decryptKey(encryptedKeyAndEncryptedData.encryptedKey, blobKeyKey); - return cryptoService.decrypt(encryptedKeyAndEncryptedData.encryptedData, key); + Object key = cryptoService.decryptKey(encryptedDataPayload.encryptedKey, blobKeyKey); + return cryptoService.decrypt(encryptedDataPayload.encryptedData, key); } @Override @@ -70,9 +69,7 @@ public String getEncryptionContext() { } /** - * POJO class for encrypted key and encrypted data byte buffers, also ser/deser for - * encrypted key + encrypted data - * + * POJO class representing payload for encrypted key and data payload, with ser/deser methods. * * - - - - - - - - - - - - - - - - - - -- - -- - - - - - -- - - - - -- - - - - - * | | | | | | | @@ -99,7 +96,7 @@ public String getEncryptionContext() { * crc - The crc of the message * */ - static class EncryptedKeyAndEncryptedData { + static class EncryptedDataPayload { private static short CURRENT_VERSION = 1; private static int VERSION_FIELD_SIZE = 2; @@ -113,34 +110,49 @@ static class EncryptedKeyAndEncryptedData { private ByteBuffer encryptedKey; private ByteBuffer encryptedData; - EncryptedKeyAndEncryptedData(ByteBuffer encryptedKey, ByteBuffer encryptedData) { + /** + * Constructor. + * @param encryptedKey the buffer containing the encrypted key. + * @param encryptedData the buffer containing the encrypted data. + */ + EncryptedDataPayload(ByteBuffer encryptedKey, ByteBuffer encryptedData) { this.encryptedKey = encryptedKey; this.encryptedData = encryptedData; } - static void serialize(ByteBuffer outputBuffer, EncryptedKeyAndEncryptedData encryptedKeyAndEncryptedData) { + /** + * Serialize the {@link EncryptedDataPayload} into an output buffer. + * @param outputBuffer the {@link ByteBuffer} to write to. + * @param encryptedDataPayload the {@link EncryptedDataPayload} to serialize. + */ + static void serialize(ByteBuffer outputBuffer, EncryptedDataPayload encryptedDataPayload) { int startOffset = outputBuffer.position(); outputBuffer.putShort(CURRENT_VERSION); - int encryptedKeySize = encryptedKeyAndEncryptedData.encryptedKey.array().length; - int encryptedDataSize = encryptedKeyAndEncryptedData.encryptedData.array().length; + int encryptedKeySize = encryptedDataPayload.encryptedKey.array().length; + int encryptedDataSize = encryptedDataPayload.encryptedData.array().length; outputBuffer.putInt(encryptedKeySize); outputBuffer.putLong(encryptedDataSize); - outputBuffer.put(encryptedKeyAndEncryptedData.encryptedKey); - outputBuffer.put(encryptedKeyAndEncryptedData.encryptedData); + outputBuffer.put(encryptedDataPayload.encryptedKey); + outputBuffer.put(encryptedDataPayload.encryptedData); Crc32 crc = new Crc32(); crc.update(outputBuffer.array(), startOffset, INITIAL_MESSAGE_LENGTH + encryptedKeySize + encryptedDataSize); outputBuffer.putLong(crc.getValue()); } - static EncryptedKeyAndEncryptedData deserialize(ByteBuffer buffer) throws GeneralSecurityException { - int startOffset = buffer.position(); + /** + * Deserialize an {@link EncryptedDataPayload} from an input buffer. + * @param inputBuffer the {@link ByteBuffer} to read from. + * @return the deserialized {@link EncryptedDataPayload}. + */ + static EncryptedDataPayload deserialize(ByteBuffer inputBuffer) throws GeneralSecurityException { + int startOffset = inputBuffer.position(); Crc32 crc = new Crc32(); - short version = buffer.getShort(); + short version = inputBuffer.getShort(); if (version != 1) { - throw new GeneralSecurityException("Unrecognized version"); + throw new GeneralSecurityException("Unrecognized version: " + version); } - int encryptedKeySize = buffer.getInt(); - int encryptedDataSize = (int) buffer.getLong(); + int encryptedKeySize = inputBuffer.getInt(); + int encryptedDataSize = (int) inputBuffer.getLong(); if (encryptedKeySize < 0) { throw new GeneralSecurityException("Encrypted key size is less than 0"); } @@ -149,15 +161,29 @@ static EncryptedKeyAndEncryptedData deserialize(ByteBuffer buffer) throws Genera } byte[] encryptedKey = new byte[encryptedKeySize]; byte[] encryptedData = new byte[encryptedDataSize]; - buffer.get(encryptedKey); - buffer.get(encryptedData); - crc.update(buffer.array(), startOffset, INITIAL_MESSAGE_LENGTH + encryptedKeySize + encryptedDataSize); - long expectedCrc = buffer.getLong(); + try { + inputBuffer.get(encryptedKey); + } catch (Exception e) { + throw new GeneralSecurityException("Reading encrypted key from buffer", e); + } + try { + inputBuffer.get(encryptedData); + } catch (Exception e) { + throw new GeneralSecurityException("Reading encrypted data from buffer", e); + } + crc.update(inputBuffer.array(), startOffset, INITIAL_MESSAGE_LENGTH + encryptedKeySize + encryptedDataSize); + long expectedCrc; + try { + expectedCrc = inputBuffer.getLong(); + } catch (Exception e) { + throw new GeneralSecurityException("Reading crc from buffer", e); + } long actualCrc = crc.getValue(); if (actualCrc != expectedCrc) { - throw new GeneralSecurityException("Encrypted blob is corrupt. ExpectedCRC: "+expectedCrc+" ActualCRC: "+actualCrc); + throw new GeneralSecurityException( + "Encrypted blob is corrupt. ExpectedCRC: " + expectedCrc + " ActualCRC: " + actualCrc); } - return new EncryptedKeyAndEncryptedData(ByteBuffer.wrap(encryptedKey), ByteBuffer.wrap(encryptedData)); + return new EncryptedDataPayload(ByteBuffer.wrap(encryptedKey), ByteBuffer.wrap(encryptedData)); } } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java index 0ee18279e5..bffce70dd6 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java @@ -28,9 +28,8 @@ public class CloudBlobMetadata { public static final String FIELD_EXPIRATION_TIME = "expirationTime"; public static final String FIELD_ACCOUNT_ID = "accountId"; public static final String FIELD_CONTAINER_ID = "containerId"; - public static final String FIELD_ENCRYPTION_TYPE = "encryptionType"; + public static final String FIELD_ENCRYPTION_ORIGIN = "encryptionOrigin"; public static final String FIELD_KMS_CONTEXT = "kmsContext"; - public static final String FIELD_ENCRYPTION_FORMAT = "vcrEncryptionFormat"; private String id; private String partitionId; @@ -41,19 +40,24 @@ public class CloudBlobMetadata { private int containerId; private long expirationTime; private long deletionTime; + private EncryptionOrigin encryptionOrigin; private String kmsContext; - private EncryptionType encryptionType; - private VcrEncryptionFormat vcrEncryptionFormat; /** * Different encryption types for cloud stored blobs */ - public enum EncryptionType {ROUTER, VCR, CLOUD;} + public enum EncryptionOrigin { - /** - * Different encryption formats for VCR encryption - */ - public enum VcrEncryptionFormat {DEFAULT;} + /** No encryption */ + NONE, + /** Encrypted by Router */ + ROUTER, + /** Encrypted by VCR */ + VCR, + /** Encrypted by Cloud Data Store */ + CLOUD + + } /** * Default constructor (for JSONSerializer). @@ -67,11 +71,11 @@ public CloudBlobMetadata() { * @param creationTime The blob creation time. * @param expirationTime The blob expiration time. * @param size The blob size. - * @param kmsContext - * @param encryptionType + * @param encryptionOrigin The blob's encryption origin. + * @param kmsContext The KMS context used to encrypt the blob. Only used when encryptionOrigin = VCR. */ - public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size, String kmsContext, - EncryptionType encryptionType, VcrEncryptionFormat vcrEncryptionFormat) { + public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size, + EncryptionOrigin encryptionOrigin, String kmsContext) { this.id = blobId.getID(); this.partitionId = blobId.getPartition().toPathString(); this.accountId = blobId.getAccountId(); @@ -81,9 +85,8 @@ public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, this.uploadTime = System.currentTimeMillis(); this.deletionTime = Utils.Infinite_Time; this.size = size; + this.encryptionOrigin = encryptionOrigin; this.kmsContext = kmsContext; - this.encryptionType = encryptionType; - this.vcrEncryptionFormat = vcrEncryptionFormat; } /** @@ -236,53 +239,36 @@ public CloudBlobMetadata setContainerId(int containerId) { } /** - * @return the kms context - */ - public String getKmsContext() { - return kmsContext; - } - - /** - * Sets the kms context - * @param kmsContext - * @return - */ - public CloudBlobMetadata setKmsContext(String kmsContext) { - this.kmsContext = kmsContext; - return this; - } - - /** - * @return the encryption type + * @return the {@link EncryptionOrigin} */ - public EncryptionType getEncryptionType() { - return encryptionType; + public EncryptionOrigin getEncryptionOrigin() { + return encryptionOrigin; } /** - * Sets the encryption type - * @param encryptionType + * Sets the encryption origin + * @param encryptionOrigin the {@link EncryptionOrigin}. * @return */ - public CloudBlobMetadata setEncryptionType(EncryptionType encryptionType) { - this.encryptionType = encryptionType; + public CloudBlobMetadata setEncryptionOrigin(EncryptionOrigin encryptionOrigin) { + this.encryptionOrigin = encryptionOrigin; return this; } /** - * @return the VCR encryption format + * @return the KMS context */ - public VcrEncryptionFormat getVcrEncryptionFormat() { - return vcrEncryptionFormat; + public String getKmsContext() { + return kmsContext; } /** - * Sets the VCR encryption format - * @param vcrEncryptionFormat + * Sets the KMS context + * @param kmsContext the KMS context used for encryption. * @return */ - public CloudBlobMetadata setVcrEncryptionFormat(VcrEncryptionFormat vcrEncryptionFormat) { - this.vcrEncryptionFormat = vcrEncryptionFormat; + public CloudBlobMetadata setKmsContext(String kmsContext) { + this.kmsContext = kmsContext; return this; } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 70b5732918..f9fe68dcf9 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -13,6 +13,7 @@ */ package com.github.ambry.cloud; +import com.codahale.metrics.Timer; import com.github.ambry.clustermap.PartitionId; import com.github.ambry.commons.BlobId; import com.github.ambry.config.CloudConfig; @@ -44,6 +45,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static com.github.ambry.cloud.CloudBlobMetadata.EncryptionOrigin; /** * The blob store that reflects data in a cloud storage. @@ -53,8 +55,9 @@ class CloudBlobStore implements Store { private static final Logger logger = LoggerFactory.getLogger(CloudBlobStore.class); private final PartitionId partitionId; private final CloudDestination cloudDestination; - private final CloudBlobCryptoAgent cryptoService; private final CloudConfig cloudConfig; + private final CloudBlobCryptoAgent cryptoAgent; + private final VcrMetrics vcrMetrics; private final long minTtlMillis; private boolean started; @@ -64,15 +67,17 @@ class CloudBlobStore implements Store { * @param cloudDestination the {@link CloudDestination}. * @param cloudConfig the {@link CloudConfig} to use. * @param cloudDestination the {@link CloudDestination} to use. - * @param cryptoService the {@link CloudBlobCryptoAgent} to use for encryption. + * @param cryptoAgent the {@link CloudBlobCryptoAgent} to use for encryption. + * @param vcrMetrics the {@link VcrMetrics} to use. */ CloudBlobStore(PartitionId partitionId, CloudConfig cloudConfig, CloudDestination cloudDestination, - CloudBlobCryptoAgent cryptoService) { + CloudBlobCryptoAgent cryptoAgent, VcrMetrics vcrMetrics) { this.cloudDestination = Objects.requireNonNull(cloudDestination, "cloudDestination is required"); this.partitionId = Objects.requireNonNull(partitionId, "partitionId is required"); this.cloudConfig = Objects.requireNonNull(cloudConfig, "cloudConfig is required"); + this.vcrMetrics = Objects.requireNonNull(vcrMetrics, "vcrMetrics is required"); minTtlMillis = TimeUnit.DAYS.toMillis(cloudConfig.vcrMinTtlDays); - this.cryptoService = cryptoService; + this.cryptoAgent = cryptoAgent; } @Override @@ -109,25 +114,47 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) throws CloudStorageException, IOException, GeneralSecurityException { if (shouldUpload(messageInfo)) { BlobId blobId = (BlobId) messageInfo.getStoreKey(); - // TODO: would be more efficient to call blobId.isEncrypted() - String kmsContext = cryptoService.getEncryptionContext(); - CloudBlobMetadata.EncryptionType encryptionType; - if (cloudConfig.vcrRequireEncryption && cryptoService != null && (blobId.getVersion() < BlobId.BLOB_ID_V4 || !BlobId.isEncrypted(blobId.getID()))) { - // Need to encrypt the buffer before upload - messageBuf = cryptoService.encrypt(messageBuf); - encryptionType = CloudBlobMetadata.EncryptionType.VCR; - } - else { - kmsContext = blobId.getAccountId()+":"+blobId.getContainerId(); - encryptionType = CloudBlobMetadata.EncryptionType.ROUTER; + boolean isRouterEncrypted = isRouterEncrypted(blobId); + String kmsContext = null; + EncryptionOrigin encryptionOrigin = isRouterEncrypted ? EncryptionOrigin.ROUTER : EncryptionOrigin.NONE; + if (cloudConfig.vcrRequireEncryption) { + if (isRouterEncrypted) { + // Nothing further needed + } else { + // Need to encrypt the buffer before upload + if (cryptoAgent != null) { + Timer.Context encryptionTimer = vcrMetrics.blobEncryptionTime.time(); + messageBuf = cryptoAgent.encrypt(messageBuf); + encryptionTimer.stop(); + vcrMetrics.blobEncryptionCount.inc(); + encryptionOrigin = EncryptionOrigin.VCR; + kmsContext = cryptoAgent.getEncryptionContext(); + } else { + // Cannot upload this blob since we can't satisfy encryption requirements + vcrMetrics.skipUnencryptedBlobsCount.inc(); + return; + } + } + } else { + // encryption not required, upload as is } CloudBlobMetadata blobMetadata = new CloudBlobMetadata(blobId, messageInfo.getOperationTimeMs(), messageInfo.getExpirationTimeInMs(), - messageInfo.getSize(), kmsContext, encryptionType, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); + messageInfo.getSize(), encryptionOrigin, kmsContext); cloudDestination.uploadBlob(blobId, size, blobMetadata, new ByteBufferInputStream(messageBuf)); } } + /** + * Utility to check whether a blob was already encrypted by the router. + * @param blobId the blob to check. + * @return True if the blob is encrypted, otherwise false. + */ + private static boolean isRouterEncrypted(BlobId blobId) throws IOException { + // TODO: would be more efficient to call blobId.isEncrypted() + return blobId.getVersion() >= BlobId.BLOB_ID_V4 && BlobId.isEncrypted(blobId.getID()); + } + /** * Utility to decide whether a blob should be uploaded. * @param messageInfo The {@link MessageInfo} containing the blob metadata. diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java index 062d6cf26d..eb398689d6 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java @@ -13,8 +13,10 @@ */ package com.github.ambry.cloud; +import com.codahale.metrics.Counter; import com.codahale.metrics.Histogram; import com.codahale.metrics.MetricRegistry; +import com.codahale.metrics.Timer; public class VcrMetrics { @@ -22,9 +24,21 @@ public class VcrMetrics { public final Histogram vcrStartTimeInMs; public final Histogram vcrShutdownTimeInMs; + // Encryption metrics + public final Counter blobEncryptionCount; + public final Counter blobDecryptionCount; + public final Timer blobEncryptionTime; + public final Timer blobDecryptionTime; + public final Counter skipUnencryptedBlobsCount; + public VcrMetrics(MetricRegistry registry) { - // TODO: add metrics to registry vcrStartTimeInMs = registry.histogram(MetricRegistry.name(VcrServer.class, "VcrStartTimeInMs")); vcrShutdownTimeInMs = registry.histogram(MetricRegistry.name(VcrServer.class, "VcrShutdownTimeInMs")); + blobEncryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobEncryptionCount")); + blobDecryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobDet4acryptionCount")); + blobEncryptionTime = registry.timer(MetricRegistry.name(CloudBlobStore.class, "BlobEncryptionTime")); + blobDecryptionTime = registry.timer(MetricRegistry.name(CloudBlobStore.class, "BlobDecryptionTime")); + skipUnencryptedBlobsCount = + registry.counter(MetricRegistry.name(CloudBlobStore.class, "SkipUnencryptedBlobsCount")); } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java index bdfbdc3116..991358ba98 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java @@ -151,7 +151,7 @@ public void startup() throws InstantiationException { cloudBackupManager = new CloudBackupManager(cloudConfig, replicationConfig, clusterMapConfig, storeConfig, storeKeyFactory, clusterMap, virtualReplicatorCluster, cloudDestinationFactory, scheduler, connectionPool, registry, - notificationSystem, storeKeyConverterFactory, serverConfig.serverMessageTransformer, cloudBlobCryptoAgentFactory); + notificationSystem, storeKeyConverterFactory, serverConfig.serverMessageTransformer, cloudBlobCryptoAgentFactory, metrics); cloudBackupManager.start(); DataNodeId currentNode = virtualReplicatorCluster.getCurrentDataNodeId(); diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java index eaf659b679..b25c1784dc 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java @@ -358,9 +358,8 @@ private static HashMap getMetadataMap(CloudBlobMetadata cloudBlo map.put(CloudBlobMetadata.FIELD_EXPIRATION_TIME, String.valueOf(cloudBlobMetadata.getExpirationTime())); map.put(CloudBlobMetadata.FIELD_ACCOUNT_ID, String.valueOf(cloudBlobMetadata.getAccountId())); map.put(CloudBlobMetadata.FIELD_CONTAINER_ID, String.valueOf(cloudBlobMetadata.getContainerId())); - map.put(CloudBlobMetadata.FIELD_ENCRYPTION_TYPE, cloudBlobMetadata.getEncryptionType().name()); + map.put(CloudBlobMetadata.FIELD_ENCRYPTION_ORIGIN, cloudBlobMetadata.getEncryptionOrigin().name()); map.put(CloudBlobMetadata.FIELD_KMS_CONTEXT, String.valueOf(cloudBlobMetadata.getKmsContext())); - map.put(CloudBlobMetadata.FIELD_ENCRYPTION_FORMAT, cloudBlobMetadata.getVcrEncryptionFormat().name()); return map; } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobCryptoAgentImplTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobCryptoAgentImplTest.java index 11e596ba47..9ab1e1ce2c 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobCryptoAgentImplTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobCryptoAgentImplTest.java @@ -13,7 +13,6 @@ */ package com.github.ambry.cloud; -import com.github.ambry.cloud.CloudBlobCryptoAgentImpl; import com.github.ambry.config.CryptoServiceConfig; import com.github.ambry.config.KMSConfig; import com.github.ambry.config.VerifiableProperties; @@ -35,7 +34,7 @@ */ public class CloudBlobCryptoAgentImplTest { - private CloudBlobCryptoAgentImpl azureCloudBlobCryptoService; + private CloudBlobCryptoAgentImpl cryptoAgent; private static final int TWO_FIFTY_SIX_BITS_IN_BYTES = 32; public CloudBlobCryptoAgentImplTest() throws GeneralSecurityException { @@ -43,7 +42,7 @@ public CloudBlobCryptoAgentImplTest() throws GeneralSecurityException { new GCMCryptoService(new CryptoServiceConfig(new VerifiableProperties(new Properties()))); KeyManagementService kms = new SingleKeyManagementService(new KMSConfig(new VerifiableProperties(new Properties())), TestUtils.getRandomKey(TWO_FIFTY_SIX_BITS_IN_BYTES)); - azureCloudBlobCryptoService = new CloudBlobCryptoAgentImpl(cryptoService, kms, "any_context"); + cryptoAgent = new CloudBlobCryptoAgentImpl(cryptoService, kms, "any_context"); } /** @@ -53,10 +52,10 @@ public CloudBlobCryptoAgentImplTest() throws GeneralSecurityException { @Test public void testBasicFunctionality() throws Exception { byte[] payload = TestUtils.getRandomBytes(10000); - byte[] encryptedPayload = azureCloudBlobCryptoService.encrypt(ByteBuffer.wrap(payload)).array(); + byte[] encryptedPayload = cryptoAgent.encrypt(ByteBuffer.wrap(payload)).array(); assertFalse(payload.length == encryptedPayload.length); assertNotSubset("Payload is a subset of the 'encrypted' payload", encryptedPayload, payload); - byte[] decryptedPayload = azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(encryptedPayload)).array(); + byte[] decryptedPayload = cryptoAgent.decrypt(ByteBuffer.wrap(encryptedPayload)).array(); assertNotSame("Payload and decrypted payload are the same array instance", payload, decryptedPayload); assertArrayEquals("Decrypted payload different from original payload", payload, decryptedPayload); } @@ -69,12 +68,12 @@ public void testBasicFunctionality() throws Exception { public void testBadDecryptInput() throws GeneralSecurityException { byte[] garbage = TestUtils.getRandomBytes(10000); //zero out version, encrypted key size, encrypted data size fields - for (int i = 0; i < CloudBlobCryptoAgentImpl.EncryptedKeyAndEncryptedData.INITIAL_MESSAGE_LENGTH; i++) { + for (int i = 0; i < CloudBlobCryptoAgentImpl.EncryptedDataPayload.INITIAL_MESSAGE_LENGTH; i++) { garbage[i] = 0; } //Version check should fail try { - azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(garbage)).array(); + cryptoAgent.decrypt(ByteBuffer.wrap(garbage)).array(); fail("Should have thrown exception"); } catch (GeneralSecurityException e) { //expected @@ -83,7 +82,7 @@ public void testBadDecryptInput() throws GeneralSecurityException { garbage[1] = 1; garbage[2] = -128; try { - azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(garbage)).array(); + cryptoAgent.decrypt(ByteBuffer.wrap(garbage)).array(); fail("Should have thrown exception"); } catch (GeneralSecurityException e) { //expected @@ -94,7 +93,7 @@ public void testBadDecryptInput() throws GeneralSecurityException { garbage[6] = -128; garbage[10] = -128; try { - azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(garbage)).array(); + cryptoAgent.decrypt(ByteBuffer.wrap(garbage)).array(); fail("Should have thrown exception"); } catch (GeneralSecurityException e) { //expected @@ -105,7 +104,7 @@ public void testBadDecryptInput() throws GeneralSecurityException { garbage[10] = 0; garbage[13] = 79; try { - azureCloudBlobCryptoService.decrypt(ByteBuffer.wrap(garbage)).array(); + cryptoAgent.decrypt(ByteBuffer.wrap(garbage)).array(); fail("Should have thrown exception"); } catch (GeneralSecurityException e) { //expected diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java index 013c5afd22..c2a79708c5 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java @@ -13,6 +13,7 @@ */ package com.github.ambry.cloud; +import com.codahale.metrics.MetricRegistry; import com.github.ambry.clustermap.MockPartitionId; import com.github.ambry.clustermap.PartitionId; import com.github.ambry.commons.BlobId; @@ -52,11 +53,12 @@ */ public class CloudBlobStoreTest { - private final CloudBlobCryptoAgent cryptoService = new TestCloudBlobCryptoAgent(); + private final CloudBlobCryptoAgent cryptoAgent = new TestCloudBlobCryptoAgent(); private Store store; private CloudDestination dest; private PartitionId partitionId; private CloudConfig cloudConfig; + private VcrMetrics vcrMetrics; private Random random = new Random(); private short refAccountId = 50; private short refContainerId = 100; @@ -64,40 +66,79 @@ public class CloudBlobStoreTest { @Before public void setup() throws Exception { - dest = mock(CloudDestination.class); partitionId = new MockPartitionId(); + } + + /** + * Setup the cloud blobstore. + * @param requireEncryption value of requireEncryption flag in CloudConfig. + * @param withCryptoAgent whether to set a {@link CloudBlobCryptoAgent} in the cloud store. + * @param start whether to start the store. + */ + private void setupCloudStore(boolean requireEncryption, boolean withCryptoAgent, boolean start) throws Exception { Properties props = new Properties(); // Require encryption for uploading - props.setProperty(CloudConfig.VCR_REQUIRE_ENCRYPTION, "true"); + props.setProperty(CloudConfig.VCR_REQUIRE_ENCRYPTION, Boolean.toString(requireEncryption)); cloudConfig = new CloudConfig(new VerifiableProperties(props)); - store = new CloudBlobStore(partitionId, cloudConfig, dest, cryptoService); - store.start(); + dest = mock(CloudDestination.class); + vcrMetrics = new VcrMetrics(new MetricRegistry()); + store = new CloudBlobStore(partitionId, cloudConfig, dest, withCryptoAgent ? cryptoAgent : null, vcrMetrics); + if (start) { + store.start(); + } } /** Test the CloudBlobStore put method. */ @Test public void testStorePuts() throws Exception { + testStorePuts(false, false); + testStorePuts(false, true); + testStorePuts(true, false); + testStorePuts(true, true); + } + + private void testStorePuts(boolean requireEncryption, boolean withCryptoAgent) throws Exception { + setupCloudStore(requireEncryption, withCryptoAgent, true); // Put blobs with and without expiration and encryption MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); - int count = 10; + int count = 1; long expireTime = System.currentTimeMillis() + 10000; + int expectedUploads = 0; + int expectedEncryptions = 0; + int expectedUnencryptedSkips = 0; for (int j = 0; j < count; j++) { long size = Math.abs(random.nextLong()) % 10000; - // Permanent and encrypted, should be uploaded + // Permanent and encrypted, should be uploaded and not reencrypted addBlobToSet(messageWriteSet, size, Utils.Infinite_Time, refAccountId, refContainerId, true); - // Permanent and unencrypted, should not be uploaded + expectedUploads++; + // Permanent and unencrypted addBlobToSet(messageWriteSet, size, Utils.Infinite_Time, refAccountId, refContainerId, false); - // Short TTL and encrypted, should not be uploaded + if (withCryptoAgent) { + expectedUploads++; + if (requireEncryption) { + expectedEncryptions++; + } + } else { + if (requireEncryption) { + expectedUnencryptedSkips++; + } else { + expectedUploads++; + } + } + // Short TTL and encrypted, should not be uploaded nor encrypted addBlobToSet(messageWriteSet, size, expireTime, refAccountId, refContainerId, true); } store.put(messageWriteSet); - verify(dest, times(count)).uploadBlob(any(BlobId.class), anyLong(), any(CloudBlobMetadata.class), + verify(dest, times(expectedUploads)).uploadBlob(any(BlobId.class), anyLong(), any(CloudBlobMetadata.class), any(InputStream.class)); + assertEquals("Unexpected encryption count", expectedEncryptions, vcrMetrics.blobEncryptionCount.getCount()); + assertEquals("Unexpected skip count", expectedUnencryptedSkips, vcrMetrics.skipUnencryptedBlobsCount.getCount()); } /** Test the CloudBlobStore delete method. */ @Test public void testStoreDeletes() throws Exception { + setupCloudStore(true, true, true); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); int count = 10; for (int j = 0; j < count; j++) { @@ -111,6 +152,7 @@ public void testStoreDeletes() throws Exception { /** Test the CloudBlobStore updateTtl method. */ @Test public void testStoreTtlUpdates() throws Exception { + setupCloudStore(true, true, true); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); int count = 10; for (int j = 0; j < count; j++) { @@ -125,6 +167,7 @@ public void testStoreTtlUpdates() throws Exception { /** Test the CloudBlobStore findMissingKeys method. */ @Test public void testFindMissingKeys() throws Exception { + setupCloudStore(true, true, true); int count = 10; List keys = new ArrayList<>(); Map metadataMap = new HashMap<>(); @@ -133,8 +176,8 @@ public void testFindMissingKeys() throws Exception { BlobId existentBlobId = getUniqueId(); keys.add(existentBlobId); metadataMap.put(existentBlobId.getID(), - new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024, null, - CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT)); + new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024, + CloudBlobMetadata.EncryptionOrigin.ROUTER, null)); // Blob without metadata BlobId nonexistentBlobId = getUniqueId(); keys.add(nonexistentBlobId); @@ -149,24 +192,24 @@ public void testFindMissingKeys() throws Exception { @Test public void testStoreNotStarted() throws Exception { // Create store and don't start it. - CloudBlobStore idleStore = new CloudBlobStore(partitionId, cloudConfig, dest, cryptoService); + setupCloudStore(true, true, false); List keys = Collections.singletonList(getUniqueId()); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); addBlobToSet(messageWriteSet, 10, Utils.Infinite_Time, refAccountId, refContainerId, true); try { - idleStore.put(messageWriteSet); + store.put(messageWriteSet); fail("Store put should have failed."); } catch (StoreException e) { assertEquals(StoreErrorCodes.Store_Not_Started, e.getErrorCode()); } try { - idleStore.delete(messageWriteSet); + store.delete(messageWriteSet); fail("Store delete should have failed."); } catch (StoreException e) { assertEquals(StoreErrorCodes.Store_Not_Started, e.getErrorCode()); } try { - idleStore.findMissingKeys(keys); + store.findMissingKeys(keys); fail("Store findMissingKeys should have failed."); } catch (StoreException e) { assertEquals(StoreErrorCodes.Store_Not_Started, e.getErrorCode()); @@ -181,7 +224,11 @@ public void testExceptionalDest() throws Exception { new CloudStorageException("ouch")); when(exDest.deleteBlob(any(BlobId.class), anyLong())).thenThrow(new CloudStorageException("ouch")); when(exDest.getBlobMetadata(anyList())).thenThrow(new CloudStorageException("ouch")); - CloudBlobStore exStore = new CloudBlobStore(partitionId, cloudConfig, exDest, cryptoService); + Properties props = new Properties(); + props.setProperty(CloudConfig.VCR_REQUIRE_ENCRYPTION, "false"); + cloudConfig = new CloudConfig(new VerifiableProperties(props)); + vcrMetrics = new VcrMetrics(new MetricRegistry()); + CloudBlobStore exStore = new CloudBlobStore(partitionId, cloudConfig, exDest, cryptoAgent, vcrMetrics); exStore.start(); List keys = Collections.singletonList(getUniqueId()); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/VcrServerTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/VcrServerTest.java index 2810847c07..5caecaa963 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/VcrServerTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/VcrServerTest.java @@ -17,6 +17,7 @@ import com.github.ambry.clustermap.MockClusterMap; import com.github.ambry.config.VerifiableProperties; import com.github.ambry.notification.NotificationSystem; +import com.github.ambry.utils.TestUtils; import java.util.Collections; import java.util.Properties; import org.junit.Before; @@ -60,6 +61,7 @@ public void testVCRServer() throws Exception { props.setProperty("server.scheduler.num.of.threads", "1"); props.setProperty("num.io.threads", "1"); props.setProperty("vcr.assigned.partitions", "0,1"); + props.setProperty("kms.default.container.key", TestUtils.getRandomKey(16)); CloudDestinationFactory cloudDestinationFactory = new LatchBasedInMemoryCloudDestinationFactory(new LatchBasedInMemoryCloudDestination(Collections.emptyList())); VerifiableProperties verifiableProperties = new VerifiableProperties(props); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java index 295431b360..6557f77629 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java @@ -183,8 +183,8 @@ public void testUpdateNotExists() throws Exception { @Test public void testQueryMetadata() throws Exception { QueryIterable mockIterable = mock(QueryIterable.class); - CloudBlobMetadata inputMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, null, - CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); + CloudBlobMetadata inputMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, + CloudBlobMetadata.EncryptionOrigin.NONE, null); List docList = Collections.singletonList(new Document(objectMapper.writeValueAsString(inputMetadata))); when(mockIterable.iterator()).thenReturn(docList.iterator()); FeedResponse feedResponse = mock(FeedResponse.class); @@ -345,8 +345,8 @@ private void verifyUpdateErrorMetrics(int numUpdates, boolean isDocument) { */ private boolean uploadDefaultBlob() throws CloudStorageException { InputStream inputStream = getBlobInputStream(blobSize); - CloudBlobMetadata metadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, null, - CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); + CloudBlobMetadata metadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, + CloudBlobMetadata.EncryptionOrigin.NONE, null); return azureDest.uploadBlob(blobId, blobSize, metadata, inputStream); } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java index cce2be5291..dde7ea89b8 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java @@ -85,8 +85,8 @@ public void testNormalFlow() throws Exception { BlobDataType.DATACHUNK); InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata cloudBlobMetadata = - new CloudBlobMetadata(blobId, System.currentTimeMillis(), Utils.Infinite_Time, blobSize, null, - CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); + new CloudBlobMetadata(blobId, System.currentTimeMillis(), Utils.Infinite_Time, blobSize, + CloudBlobMetadata.EncryptionOrigin.NONE, null); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); // Try to upload same blob again @@ -120,8 +120,8 @@ public void testBatchQuery() throws Exception { blobIdList.add(blobId); InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata cloudBlobMetadata = - new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, null, - CloudBlobMetadata.EncryptionType.ROUTER, CloudBlobMetadata.VcrEncryptionFormat.DEFAULT); + new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, + CloudBlobMetadata.EncryptionOrigin.NONE, null); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); } From 906b91a274827db124f6e2e051142b971e1d5854 Mon Sep 17 00:00:00 2001 From: Rob Block Date: Mon, 15 Apr 2019 19:50:52 -0700 Subject: [PATCH 11/15] Added cryptoAgentFactory to CloudBlobMetadata --- .../com.github.ambry/config/CloudConfig.java | 2 +- .../CloudBackupManager.java | 2 +- .../CloudBlobMetadata.java | 54 +++++++++++++------ .../CloudBlobStore.java | 13 +++-- .../azure/AzureCloudDestination.java | 3 +- .../CloudBlobStoreTest.java | 17 ++++-- .../azure/AzureCloudDestinationTest.java | 4 +- .../azure/AzureIntegrationTest.java | 9 +++- .../ServerTestUtil.java | 1 - 9 files changed, 74 insertions(+), 31 deletions(-) diff --git a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java index 73956295bc..73d23ca967 100644 --- a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java +++ b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java @@ -104,7 +104,7 @@ public class CloudConfig { public final String kmsServiceKeyContext; /** - * The cloud blob crypto service factory class name. + * The cloud blob crypto agent factory class name. */ @Config(CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS) @Default(DEFAULT_CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS) diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java index fe37727cc5..a0b63cfe90 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java @@ -63,7 +63,7 @@ public CloudBackupManager(CloudConfig cloudConfig, ReplicationConfig replication ReplicaId cloudReplica = new CloudReplica(cloudConfig, partitionId, virtualReplicatorCluster.getCurrentDataNodeId()); Store cloudStore = new CloudBlobStore(partitionId, cloudConfig, cloudDestination, - cloudBlobCryptoAgentFactory.getCloudBlobCryptoAgent(), vcrMetrics); + cloudBlobCryptoAgentFactory, vcrMetrics); try { cloudStore.start(); } catch (StoreException e) { diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java index bffce70dd6..3e793219db 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java @@ -29,7 +29,8 @@ public class CloudBlobMetadata { public static final String FIELD_ACCOUNT_ID = "accountId"; public static final String FIELD_CONTAINER_ID = "containerId"; public static final String FIELD_ENCRYPTION_ORIGIN = "encryptionOrigin"; - public static final String FIELD_KMS_CONTEXT = "kmsContext"; + public static final String FIELD_VCR_KMS_CONTEXT = "vcrKmsContext"; + public static final String FIELD_CRYPTO_AGENT_FACTORY = "cryptoAgentFactory"; private String id; private String partitionId; @@ -41,7 +42,8 @@ public class CloudBlobMetadata { private long expirationTime; private long deletionTime; private EncryptionOrigin encryptionOrigin; - private String kmsContext; + private String vcrKmsContext; + private String cryptoAgentFactory; /** * Different encryption types for cloud stored blobs @@ -72,10 +74,12 @@ public CloudBlobMetadata() { * @param expirationTime The blob expiration time. * @param size The blob size. * @param encryptionOrigin The blob's encryption origin. - * @param kmsContext The KMS context used to encrypt the blob. Only used when encryptionOrigin = VCR. + * @param vcrKmsContext The KMS context used to encrypt the blob. Only used when encryptionOrigin = VCR. + * @param cryptoAgentFactory The class name of the {@link CloudBlobCryptoAgentFactory} used to encrypt the blob. + * Only used when encryptionOrigin = VCR. */ public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, long size, - EncryptionOrigin encryptionOrigin, String kmsContext) { + EncryptionOrigin encryptionOrigin, String vcrKmsContext, String cryptoAgentFactory) { this.id = blobId.getID(); this.partitionId = blobId.getPartition().toPathString(); this.accountId = blobId.getAccountId(); @@ -86,7 +90,8 @@ public CloudBlobMetadata(BlobId blobId, long creationTime, long expirationTime, this.deletionTime = Utils.Infinite_Time; this.size = size; this.encryptionOrigin = encryptionOrigin; - this.kmsContext = kmsContext; + this.vcrKmsContext = vcrKmsContext; + this.cryptoAgentFactory = cryptoAgentFactory; } /** @@ -239,16 +244,16 @@ public CloudBlobMetadata setContainerId(int containerId) { } /** - * @return the {@link EncryptionOrigin} + * @return the {@link EncryptionOrigin}. */ public EncryptionOrigin getEncryptionOrigin() { return encryptionOrigin; } /** - * Sets the encryption origin + * Sets the encryption origin. * @param encryptionOrigin the {@link EncryptionOrigin}. - * @return + * @return this instance. */ public CloudBlobMetadata setEncryptionOrigin(EncryptionOrigin encryptionOrigin) { this.encryptionOrigin = encryptionOrigin; @@ -256,19 +261,36 @@ public CloudBlobMetadata setEncryptionOrigin(EncryptionOrigin encryptionOrigin) } /** - * @return the KMS context + * @return the VCR KMS context. + */ + public String getVcrKmsContext() { + return vcrKmsContext; + } + + /** + * Sets the VCR KMS context. + * @param vcrKmsContext the KMS context used for encryption. + * @return this instance. + */ + public CloudBlobMetadata setVcrKmsContext(String vcrKmsContext) { + this.vcrKmsContext = vcrKmsContext; + return this; + } + + /** + * @return the VCR crypto agent factory class name. */ - public String getKmsContext() { - return kmsContext; + public String getCryptoAgentFactory() { + return cryptoAgentFactory; } /** - * Sets the KMS context - * @param kmsContext the KMS context used for encryption. - * @return + * Sets the VCR crypto agent factory class name. + * @param cryptoAgentFactory the class name of the {@link CloudBlobCryptoAgentFactory} used for encryption. + * @return this instance. */ - public CloudBlobMetadata setKmsContext(String kmsContext) { - this.kmsContext = kmsContext; + public CloudBlobMetadata setCryptoAgentFactory(String cryptoAgentFactory) { + this.cryptoAgentFactory = cryptoAgentFactory; return this; } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index f9fe68dcf9..23a8125a6a 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -39,6 +39,7 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -56,6 +57,7 @@ class CloudBlobStore implements Store { private final PartitionId partitionId; private final CloudDestination cloudDestination; private final CloudConfig cloudConfig; + private final CloudBlobCryptoAgentFactory cryptoAgentFactory; private final CloudBlobCryptoAgent cryptoAgent; private final VcrMetrics vcrMetrics; private final long minTtlMillis; @@ -67,17 +69,18 @@ class CloudBlobStore implements Store { * @param cloudDestination the {@link CloudDestination}. * @param cloudConfig the {@link CloudConfig} to use. * @param cloudDestination the {@link CloudDestination} to use. - * @param cryptoAgent the {@link CloudBlobCryptoAgent} to use for encryption. + * @param cryptoAgentFactory the {@link CloudBlobCryptoAgentFactory} to use for encryption. * @param vcrMetrics the {@link VcrMetrics} to use. */ CloudBlobStore(PartitionId partitionId, CloudConfig cloudConfig, CloudDestination cloudDestination, - CloudBlobCryptoAgent cryptoAgent, VcrMetrics vcrMetrics) { + CloudBlobCryptoAgentFactory cryptoAgentFactory, VcrMetrics vcrMetrics) { this.cloudDestination = Objects.requireNonNull(cloudDestination, "cloudDestination is required"); this.partitionId = Objects.requireNonNull(partitionId, "partitionId is required"); this.cloudConfig = Objects.requireNonNull(cloudConfig, "cloudConfig is required"); this.vcrMetrics = Objects.requireNonNull(vcrMetrics, "vcrMetrics is required"); minTtlMillis = TimeUnit.DAYS.toMillis(cloudConfig.vcrMinTtlDays); - this.cryptoAgent = cryptoAgent; + this.cryptoAgentFactory = cryptoAgentFactory; + this.cryptoAgent = cryptoAgentFactory != null ? cryptoAgentFactory.getCloudBlobCryptoAgent() : null; } @Override @@ -116,6 +119,7 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) BlobId blobId = (BlobId) messageInfo.getStoreKey(); boolean isRouterEncrypted = isRouterEncrypted(blobId); String kmsContext = null; + String cryptoAgentFactoryClass = null; EncryptionOrigin encryptionOrigin = isRouterEncrypted ? EncryptionOrigin.ROUTER : EncryptionOrigin.NONE; if (cloudConfig.vcrRequireEncryption) { if (isRouterEncrypted) { @@ -129,6 +133,7 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) vcrMetrics.blobEncryptionCount.inc(); encryptionOrigin = EncryptionOrigin.VCR; kmsContext = cryptoAgent.getEncryptionContext(); + cryptoAgentFactoryClass = this.cryptoAgentFactory.getClass().getName(); } else { // Cannot upload this blob since we can't satisfy encryption requirements vcrMetrics.skipUnencryptedBlobsCount.inc(); @@ -140,7 +145,7 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) } CloudBlobMetadata blobMetadata = new CloudBlobMetadata(blobId, messageInfo.getOperationTimeMs(), messageInfo.getExpirationTimeInMs(), - messageInfo.getSize(), encryptionOrigin, kmsContext); + messageInfo.getSize(), encryptionOrigin, kmsContext, cryptoAgentFactoryClass); cloudDestination.uploadBlob(blobId, size, blobMetadata, new ByteBufferInputStream(messageBuf)); } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java index b25c1784dc..467307ebec 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java @@ -359,7 +359,8 @@ private static HashMap getMetadataMap(CloudBlobMetadata cloudBlo map.put(CloudBlobMetadata.FIELD_ACCOUNT_ID, String.valueOf(cloudBlobMetadata.getAccountId())); map.put(CloudBlobMetadata.FIELD_CONTAINER_ID, String.valueOf(cloudBlobMetadata.getContainerId())); map.put(CloudBlobMetadata.FIELD_ENCRYPTION_ORIGIN, cloudBlobMetadata.getEncryptionOrigin().name()); - map.put(CloudBlobMetadata.FIELD_KMS_CONTEXT, String.valueOf(cloudBlobMetadata.getKmsContext())); + map.put(CloudBlobMetadata.FIELD_VCR_KMS_CONTEXT, String.valueOf(cloudBlobMetadata.getVcrKmsContext())); + map.put(CloudBlobMetadata.FIELD_CRYPTO_AGENT_FACTORY, String.valueOf(cloudBlobMetadata.getCryptoAgentFactory())); return map; } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java index c2a79708c5..adb4170a9d 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java @@ -54,6 +54,7 @@ public class CloudBlobStoreTest { private final CloudBlobCryptoAgent cryptoAgent = new TestCloudBlobCryptoAgent(); + private final CloudBlobCryptoAgentFactory cryptoAgentFactory = new TestCryptoAgentFactory(); private Store store; private CloudDestination dest; private PartitionId partitionId; @@ -82,7 +83,7 @@ private void setupCloudStore(boolean requireEncryption, boolean withCryptoAgent, cloudConfig = new CloudConfig(new VerifiableProperties(props)); dest = mock(CloudDestination.class); vcrMetrics = new VcrMetrics(new MetricRegistry()); - store = new CloudBlobStore(partitionId, cloudConfig, dest, withCryptoAgent ? cryptoAgent : null, vcrMetrics); + store = new CloudBlobStore(partitionId, cloudConfig, dest, withCryptoAgent ? cryptoAgentFactory : null, vcrMetrics); if (start) { store.start(); } @@ -177,7 +178,7 @@ public void testFindMissingKeys() throws Exception { keys.add(existentBlobId); metadataMap.put(existentBlobId.getID(), new CloudBlobMetadata(existentBlobId, operationTime, Utils.Infinite_Time, 1024, - CloudBlobMetadata.EncryptionOrigin.ROUTER, null)); + CloudBlobMetadata.EncryptionOrigin.ROUTER, null, null)); // Blob without metadata BlobId nonexistentBlobId = getUniqueId(); keys.add(nonexistentBlobId); @@ -228,7 +229,7 @@ public void testExceptionalDest() throws Exception { props.setProperty(CloudConfig.VCR_REQUIRE_ENCRYPTION, "false"); cloudConfig = new CloudConfig(new VerifiableProperties(props)); vcrMetrics = new VcrMetrics(new MetricRegistry()); - CloudBlobStore exStore = new CloudBlobStore(partitionId, cloudConfig, exDest, cryptoAgent, vcrMetrics); + CloudBlobStore exStore = new CloudBlobStore(partitionId, cloudConfig, exDest, cryptoAgentFactory, vcrMetrics); exStore.start(); List keys = Collections.singletonList(getUniqueId()); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); @@ -294,4 +295,14 @@ private BlobId getUniqueId(short accountId, short containerId, boolean encrypted return new BlobId(BLOB_ID_V6, BlobIdType.NATIVE, dataCenterId, accountId, containerId, partitionId, encrypted, BlobDataType.DATACHUNK); } + + /** + * Factory to create TestCloudBlobCryptoAgent + */ + private class TestCryptoAgentFactory implements CloudBlobCryptoAgentFactory { + @Override + public CloudBlobCryptoAgent getCloudBlobCryptoAgent() { + return cryptoAgent; + } + } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java index 6557f77629..e4a1de3516 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java @@ -184,7 +184,7 @@ public void testUpdateNotExists() throws Exception { public void testQueryMetadata() throws Exception { QueryIterable mockIterable = mock(QueryIterable.class); CloudBlobMetadata inputMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, - CloudBlobMetadata.EncryptionOrigin.NONE, null); + CloudBlobMetadata.EncryptionOrigin.NONE, null, null); List docList = Collections.singletonList(new Document(objectMapper.writeValueAsString(inputMetadata))); when(mockIterable.iterator()).thenReturn(docList.iterator()); FeedResponse feedResponse = mock(FeedResponse.class); @@ -346,7 +346,7 @@ private void verifyUpdateErrorMetrics(int numUpdates, boolean isDocument) { private boolean uploadDefaultBlob() throws CloudStorageException { InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata metadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, - CloudBlobMetadata.EncryptionOrigin.NONE, null); + CloudBlobMetadata.EncryptionOrigin.NONE, null, null); return azureDest.uploadBlob(blobId, blobSize, metadata, inputStream); } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java index dde7ea89b8..770d559c8a 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java @@ -19,6 +19,7 @@ import com.github.ambry.clustermap.MockPartitionId; import com.github.ambry.clustermap.PartitionId; import com.github.ambry.commons.BlobId; +import com.github.ambry.config.CloudConfig; import com.github.ambry.config.VerifiableProperties; import com.github.ambry.utils.TestUtils; import com.github.ambry.utils.Utils; @@ -59,6 +60,8 @@ public class AzureIntegrationTest { private static final Logger logger = LoggerFactory.getLogger(AzureIntegrationTest.class); + private final String vcrKmsContext = "backup-default"; + private final String cryptoAgentFactory = CloudConfig.DEFAULT_CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS; private AzureCloudDestination azureDest; private int blobSize = 1024; private byte dataCenterId = 66; @@ -86,7 +89,7 @@ public void testNormalFlow() throws Exception { InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata cloudBlobMetadata = new CloudBlobMetadata(blobId, System.currentTimeMillis(), Utils.Infinite_Time, blobSize, - CloudBlobMetadata.EncryptionOrigin.NONE, null); + CloudBlobMetadata.EncryptionOrigin.NONE, vcrKmsContext, cryptoAgentFactory); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); // Try to upload same blob again @@ -121,7 +124,7 @@ public void testBatchQuery() throws Exception { InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata cloudBlobMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, - CloudBlobMetadata.EncryptionOrigin.NONE, null); + CloudBlobMetadata.EncryptionOrigin.NONE, vcrKmsContext, cryptoAgentFactory); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); } @@ -136,6 +139,8 @@ public void testBatchQuery() throws Exception { assertEquals("Unexpected metadata containerId", containerId, metadata.getContainerId()); assertEquals("Unexpected metadata partitionId", partitionId.toPathString(), metadata.getPartitionId()); assertEquals("Unexpected metadata creationTime", creationTime, metadata.getCreationTime()); + assertEquals("Unexpected metadata vcrKmsContext", vcrKmsContext, metadata.getVcrKmsContext()); + assertEquals("Unexpected metadata cryptoAgentFactory", vcrKmsContext, metadata.getCryptoAgentFactory()); } // Cleanup diff --git a/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java b/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java index 5c38a36979..7bde948c73 100644 --- a/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java +++ b/ambry-server/src/integration-test/java/com.github.ambry.server/ServerTestUtil.java @@ -17,7 +17,6 @@ import com.github.ambry.account.AccountService; import com.github.ambry.account.InMemAccountService; import com.github.ambry.cloud.CloudBackupManager; -import com.github.ambry.cloud.CloudBlobCryptoAgentFactory; import com.github.ambry.cloud.CloudBlobMetadata; import com.github.ambry.cloud.CloudDestinationFactory; import com.github.ambry.cloud.LatchBasedInMemoryCloudDestination; From 81eadab1aced4c7068d9bfc97a280a0583a5a5cb Mon Sep 17 00:00:00 2001 From: Rob Block Date: Thu, 18 Apr 2019 19:01:22 -0700 Subject: [PATCH 12/15] Address review comments --- .../com.github.ambry/config/CloudConfig.java | 2 - .../CloudBackupManager.java | 14 ++-- .../CloudBlobCryptoAgentFactoryImpl.java | 6 +- .../CloudBlobCryptoAgentImpl.java | 69 +++++++--------- .../CloudBlobStore.java | 55 +++++++------ .../com.github.ambry.cloud/VcrMetrics.java | 13 ++- .../com.github.ambry.cloud/VcrServer.java | 9 ++- .../CloudBlobStoreTest.java | 81 +++++++++---------- .../TestCloudBlobCryptoAgentFactory.java | 33 ++++++++ 9 files changed, 155 insertions(+), 127 deletions(-) create mode 100644 ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoAgentFactory.java diff --git a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java index 4d87c97430..fa6d326cbc 100644 --- a/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java +++ b/ambry-api/src/main/java/com.github.ambry/config/CloudConfig.java @@ -42,8 +42,6 @@ public class CloudConfig { public static final String CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS = "cloud.blob.crypto.agent.factory.class"; public static final String DEFAULT_CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS = "com.github.ambry.cloud.CloudBlobCryptoAgentFactoryImpl"; - public static final String VCR_ENCRYPTION_FORMAT = "vcr.encryption.format"; - public static final String DEFAULT_VCR_ENCRYPTION_FORMAT = "DEFAULT"; private static final String DEFAULT_VCR_CLUSTER_ZK_CONNECT_STRING = "localhost:2181"; private static final String DEFAULT_VCR_CLUSTER_NAME = "VCRCluster"; private static final String DEFAULT_VCR_REPLICA_MOUNT_PATH_PREFIX = "/tmp/vcr/"; diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java index a0b63cfe90..1dfc45ac44 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java @@ -22,6 +22,7 @@ import com.github.ambry.config.ClusterMapConfig; import com.github.ambry.config.ReplicationConfig; import com.github.ambry.config.StoreConfig; +import com.github.ambry.config.VerifiableProperties; import com.github.ambry.network.ConnectionPool; import com.github.ambry.notification.NotificationSystem; import com.github.ambry.replication.DiskTokenPersistor; @@ -45,13 +46,13 @@ */ public class CloudBackupManager extends ReplicationEngine { - public CloudBackupManager(CloudConfig cloudConfig, ReplicationConfig replicationConfig, - ClusterMapConfig clusterMapConfig, StoreConfig storeConfig, StoreKeyFactory storeKeyFactory, - ClusterMap clusterMap, VirtualReplicatorCluster virtualReplicatorCluster, + public CloudBackupManager(VerifiableProperties properties, CloudConfig cloudConfig, + ReplicationConfig replicationConfig, ClusterMapConfig clusterMapConfig, StoreConfig storeConfig, + StoreKeyFactory storeKeyFactory, ClusterMap clusterMap, VirtualReplicatorCluster virtualReplicatorCluster, CloudDestinationFactory cloudDestinationFactory, ScheduledExecutorService scheduler, ConnectionPool connectionPool, MetricRegistry metricRegistry, NotificationSystem requestNotification, - StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName, - CloudBlobCryptoAgentFactory cloudBlobCryptoAgentFactory, VcrMetrics vcrMetrics) throws ReplicationException { + StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName, VcrMetrics vcrMetrics) + throws ReplicationException { super(replicationConfig, clusterMapConfig, storeKeyFactory, clusterMap, scheduler, virtualReplicatorCluster.getCurrentDataNodeId(), Collections.emptyList(), connectionPool, metricRegistry, @@ -62,8 +63,7 @@ public CloudBackupManager(CloudConfig cloudConfig, ReplicationConfig replication for (PartitionId partitionId : partitionIds) { ReplicaId cloudReplica = new CloudReplica(cloudConfig, partitionId, virtualReplicatorCluster.getCurrentDataNodeId()); - Store cloudStore = new CloudBlobStore(partitionId, cloudConfig, cloudDestination, - cloudBlobCryptoAgentFactory, vcrMetrics); + Store cloudStore = new CloudBlobStore(properties, partitionId, cloudDestination, vcrMetrics); try { cloudStore.start(); } catch (StoreException e) { diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java index 81b13b3346..365e084e3a 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentFactoryImpl.java @@ -27,9 +27,9 @@ */ public class CloudBlobCryptoAgentFactoryImpl implements CloudBlobCryptoAgentFactory { - private CryptoServiceFactory cryptoServiceFactory; - private KeyManagementServiceFactory keyManagementServiceFactory; - private String context; + private final CryptoServiceFactory cryptoServiceFactory; + private final KeyManagementServiceFactory keyManagementServiceFactory; + private final String context; public CloudBlobCryptoAgentFactoryImpl(VerifiableProperties verifiableProperties, String clusterName, MetricRegistry metricRegistry) throws ReflectiveOperationException { diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java index 5ef9498992..540384476e 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobCryptoAgentImpl.java @@ -52,6 +52,7 @@ public ByteBuffer encrypt(ByteBuffer buffer) throws GeneralSecurityException { ByteBuffer output = ByteBuffer.allocate( EncryptedDataPayload.OVERHEAD_LENGTH + encryptedKey.array().length + encryptedDataBuffer.array().length); EncryptedDataPayload.serialize(output, new EncryptedDataPayload(encryptedKey, encryptedDataBuffer)); + output.flip(); return output; } @@ -105,10 +106,10 @@ static class EncryptedDataPayload { static int INITIAL_MESSAGE_LENGTH = VERSION_FIELD_SIZE + ENCRYPTED_KEY_SIZE_FIELD_SIZE + ENCRYPTED_DATA_SIZE_FIELD_SIZE; private static int CRC_FIELD_LENGTH = 8; - static int OVERHEAD_LENGTH = INITIAL_MESSAGE_LENGTH + CRC_FIELD_LENGTH; + private static int OVERHEAD_LENGTH = INITIAL_MESSAGE_LENGTH + CRC_FIELD_LENGTH; - private ByteBuffer encryptedKey; - private ByteBuffer encryptedData; + private final ByteBuffer encryptedKey; + private final ByteBuffer encryptedData; /** * Constructor. @@ -122,7 +123,7 @@ static class EncryptedDataPayload { /** * Serialize the {@link EncryptedDataPayload} into an output buffer. - * @param outputBuffer the {@link ByteBuffer} to write to. + * @param outputBuffer the {@link ByteBuffer} to write to. Currently, only HeapByteBuffer is supported. * @param encryptedDataPayload the {@link EncryptedDataPayload} to serialize. */ static void serialize(ByteBuffer outputBuffer, EncryptedDataPayload encryptedDataPayload) { @@ -141,49 +142,41 @@ static void serialize(ByteBuffer outputBuffer, EncryptedDataPayload encryptedDat /** * Deserialize an {@link EncryptedDataPayload} from an input buffer. - * @param inputBuffer the {@link ByteBuffer} to read from. + * @param inputBuffer the {@link ByteBuffer} to read from. Currently, only HeapByteBuffer is supported. * @return the deserialized {@link EncryptedDataPayload}. + * @throws GeneralSecurityException if the buffer does not have the expected size or format. */ static EncryptedDataPayload deserialize(ByteBuffer inputBuffer) throws GeneralSecurityException { int startOffset = inputBuffer.position(); Crc32 crc = new Crc32(); - short version = inputBuffer.getShort(); - if (version != 1) { - throw new GeneralSecurityException("Unrecognized version: " + version); - } - int encryptedKeySize = inputBuffer.getInt(); - int encryptedDataSize = (int) inputBuffer.getLong(); - if (encryptedKeySize < 0) { - throw new GeneralSecurityException("Encrypted key size is less than 0"); - } - if (encryptedDataSize < 0) { - throw new GeneralSecurityException("Encrypted data size is less than 0"); - } - byte[] encryptedKey = new byte[encryptedKeySize]; - byte[] encryptedData = new byte[encryptedDataSize]; try { + short version = inputBuffer.getShort(); + if (version != 1) { + throw new GeneralSecurityException("Unrecognized version: " + version); + } + int encryptedKeySize = inputBuffer.getInt(); + int encryptedDataSize = (int) inputBuffer.getLong(); + if (encryptedKeySize < 0) { + throw new GeneralSecurityException("Encrypted key size is less than 0"); + } + if (encryptedDataSize < 0) { + throw new GeneralSecurityException("Encrypted data size is less than 0"); + } + byte[] encryptedKey = new byte[encryptedKeySize]; + byte[] encryptedData = new byte[encryptedDataSize]; inputBuffer.get(encryptedKey); - } catch (Exception e) { - throw new GeneralSecurityException("Reading encrypted key from buffer", e); - } - try { inputBuffer.get(encryptedData); - } catch (Exception e) { - throw new GeneralSecurityException("Reading encrypted data from buffer", e); - } - crc.update(inputBuffer.array(), startOffset, INITIAL_MESSAGE_LENGTH + encryptedKeySize + encryptedDataSize); - long expectedCrc; - try { - expectedCrc = inputBuffer.getLong(); - } catch (Exception e) { - throw new GeneralSecurityException("Reading crc from buffer", e); - } - long actualCrc = crc.getValue(); - if (actualCrc != expectedCrc) { - throw new GeneralSecurityException( - "Encrypted blob is corrupt. ExpectedCRC: " + expectedCrc + " ActualCRC: " + actualCrc); + crc.update(inputBuffer.array(), startOffset, INITIAL_MESSAGE_LENGTH + encryptedKeySize + encryptedDataSize); + long expectedCrc = inputBuffer.getLong(); + long actualCrc = crc.getValue(); + if (actualCrc != expectedCrc) { + throw new GeneralSecurityException( + "Encrypted blob is corrupt. ExpectedCRC: " + expectedCrc + " ActualCRC: " + actualCrc); + } + return new EncryptedDataPayload(ByteBuffer.wrap(encryptedKey), ByteBuffer.wrap(encryptedData)); + } catch (RuntimeException rex) { + throw new GeneralSecurityException("Reading payload from buffer", rex); } - return new EncryptedDataPayload(ByteBuffer.wrap(encryptedKey), ByteBuffer.wrap(encryptedData)); } } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java index 451ef5bcdd..dd093d1c65 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobStore.java @@ -17,6 +17,8 @@ import com.github.ambry.clustermap.PartitionId; import com.github.ambry.commons.BlobId; import com.github.ambry.config.CloudConfig; +import com.github.ambry.config.ClusterMapConfig; +import com.github.ambry.config.VerifiableProperties; import com.github.ambry.store.FindInfo; import com.github.ambry.store.FindToken; import com.github.ambry.store.MessageInfo; @@ -39,14 +41,14 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static com.github.ambry.cloud.CloudBlobMetadata.EncryptionOrigin; +import static com.github.ambry.cloud.CloudBlobMetadata.*; + /** * The blob store that reflects data in a cloud storage. @@ -56,31 +58,40 @@ class CloudBlobStore implements Store { private static final Logger logger = LoggerFactory.getLogger(CloudBlobStore.class); private final PartitionId partitionId; private final CloudDestination cloudDestination; - private final CloudConfig cloudConfig; private final CloudBlobCryptoAgentFactory cryptoAgentFactory; private final CloudBlobCryptoAgent cryptoAgent; private final VcrMetrics vcrMetrics; private final long minTtlMillis; + private final boolean requireEncryption; private boolean started; /** * Constructor for CloudBlobStore + * @param properties the {@link VerifiableProperties} to use. * @param partitionId partition associated with BlobStore. - * @param cloudDestination the {@link CloudDestination}. - * @param cloudConfig the {@link CloudConfig} to use. * @param cloudDestination the {@link CloudDestination} to use. - * @param cryptoAgentFactory the {@link CloudBlobCryptoAgentFactory} to use for encryption. * @param vcrMetrics the {@link VcrMetrics} to use. + * @throws IllegalStateException if construction failed. */ - CloudBlobStore(PartitionId partitionId, CloudConfig cloudConfig, CloudDestination cloudDestination, - CloudBlobCryptoAgentFactory cryptoAgentFactory, VcrMetrics vcrMetrics) { + CloudBlobStore(VerifiableProperties properties, PartitionId partitionId, CloudDestination cloudDestination, + VcrMetrics vcrMetrics) throws IllegalStateException { + + CloudConfig cloudConfig = new CloudConfig(properties); + ClusterMapConfig clusterMapConfig = new ClusterMapConfig(properties); this.cloudDestination = Objects.requireNonNull(cloudDestination, "cloudDestination is required"); this.partitionId = Objects.requireNonNull(partitionId, "partitionId is required"); - this.cloudConfig = Objects.requireNonNull(cloudConfig, "cloudConfig is required"); this.vcrMetrics = Objects.requireNonNull(vcrMetrics, "vcrMetrics is required"); minTtlMillis = TimeUnit.DAYS.toMillis(cloudConfig.vcrMinTtlDays); - this.cryptoAgentFactory = cryptoAgentFactory; - this.cryptoAgent = cryptoAgentFactory != null ? cryptoAgentFactory.getCloudBlobCryptoAgent() : null; + requireEncryption = cloudConfig.vcrRequireEncryption; + + String cryptoAgentFactoryClass = cloudConfig.cloudBlobCryptoAgentFactoryClass; + try { + cryptoAgentFactory = Utils.getObj(cryptoAgentFactoryClass, properties, clusterMapConfig.clusterMapClusterName, + vcrMetrics.getMetricRegistry()); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException("Unable to construct factory " + cryptoAgentFactoryClass, e); + } + this.cryptoAgent = cryptoAgentFactory.getCloudBlobCryptoAgent(); } @Override @@ -121,24 +132,18 @@ private void putBlob(MessageInfo messageInfo, ByteBuffer messageBuf, long size) String kmsContext = null; String cryptoAgentFactoryClass = null; EncryptionOrigin encryptionOrigin = isRouterEncrypted ? EncryptionOrigin.ROUTER : EncryptionOrigin.NONE; - if (cloudConfig.vcrRequireEncryption) { + if (requireEncryption) { if (isRouterEncrypted) { // Nothing further needed } else { // Need to encrypt the buffer before upload - if (cryptoAgent != null) { - Timer.Context encryptionTimer = vcrMetrics.blobEncryptionTime.time(); - messageBuf = cryptoAgent.encrypt(messageBuf); - encryptionTimer.stop(); - vcrMetrics.blobEncryptionCount.inc(); - encryptionOrigin = EncryptionOrigin.VCR; - kmsContext = cryptoAgent.getEncryptionContext(); - cryptoAgentFactoryClass = this.cryptoAgentFactory.getClass().getName(); - } else { - // Cannot upload this blob since we can't satisfy encryption requirements - vcrMetrics.skipUnencryptedBlobsCount.inc(); - return; - } + Timer.Context encryptionTimer = vcrMetrics.blobEncryptionTime.time(); + messageBuf = cryptoAgent.encrypt(messageBuf); + encryptionTimer.stop(); + vcrMetrics.blobEncryptionCount.inc(); + encryptionOrigin = EncryptionOrigin.VCR; + kmsContext = cryptoAgent.getEncryptionContext(); + cryptoAgentFactoryClass = this.cryptoAgentFactory.getClass().getName(); } } else { // encryption not required, upload as is diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java index eb398689d6..391743be26 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java @@ -21,6 +21,8 @@ public class VcrMetrics { + private final MetricRegistry registry; + public final Histogram vcrStartTimeInMs; public final Histogram vcrShutdownTimeInMs; @@ -29,16 +31,21 @@ public class VcrMetrics { public final Counter blobDecryptionCount; public final Timer blobEncryptionTime; public final Timer blobDecryptionTime; - public final Counter skipUnencryptedBlobsCount; public VcrMetrics(MetricRegistry registry) { + this.registry = registry; vcrStartTimeInMs = registry.histogram(MetricRegistry.name(VcrServer.class, "VcrStartTimeInMs")); vcrShutdownTimeInMs = registry.histogram(MetricRegistry.name(VcrServer.class, "VcrShutdownTimeInMs")); blobEncryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobEncryptionCount")); blobDecryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobDet4acryptionCount")); blobEncryptionTime = registry.timer(MetricRegistry.name(CloudBlobStore.class, "BlobEncryptionTime")); blobDecryptionTime = registry.timer(MetricRegistry.name(CloudBlobStore.class, "BlobDecryptionTime")); - skipUnencryptedBlobsCount = - registry.counter(MetricRegistry.name(CloudBlobStore.class, "SkipUnencryptedBlobsCount")); + } + + /** + * @return the {@link MetricRegistry} where these metrics are registered. + */ + public MetricRegistry getMetricRegistry() { + return registry; } } diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java index 991358ba98..c42600efd5 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java @@ -147,11 +147,12 @@ public void startup() throws InstantiationException { StoreKeyConverterFactory storeKeyConverterFactory = Utils.getObj(serverConfig.serverStoreKeyConverterFactory, properties, registry); CloudBlobCryptoAgentFactory cloudBlobCryptoAgentFactory = - Utils.getObj(cloudConfig.cloudBlobCryptoAgentFactoryClass, properties, clusterMapConfig.clusterMapClusterName, clusterMap.getMetricRegistry()); + Utils.getObj(cloudConfig.cloudBlobCryptoAgentFactoryClass, properties, clusterMapConfig.clusterMapClusterName, + clusterMap.getMetricRegistry()); cloudBackupManager = - new CloudBackupManager(cloudConfig, replicationConfig, clusterMapConfig, storeConfig, storeKeyFactory, - clusterMap, virtualReplicatorCluster, cloudDestinationFactory, scheduler, connectionPool, registry, - notificationSystem, storeKeyConverterFactory, serverConfig.serverMessageTransformer, cloudBlobCryptoAgentFactory, metrics); + new CloudBackupManager(properties, cloudConfig, replicationConfig, clusterMapConfig, storeConfig, + storeKeyFactory, clusterMap, virtualReplicatorCluster, cloudDestinationFactory, scheduler, connectionPool, + registry, notificationSystem, storeKeyConverterFactory, serverConfig.serverMessageTransformer, metrics); cloudBackupManager.start(); DataNodeId currentNode = virtualReplicatorCluster.getCurrentDataNodeId(); diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java index adb4170a9d..a97ba7bbb3 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/CloudBlobStoreTest.java @@ -53,12 +53,9 @@ */ public class CloudBlobStoreTest { - private final CloudBlobCryptoAgent cryptoAgent = new TestCloudBlobCryptoAgent(); - private final CloudBlobCryptoAgentFactory cryptoAgentFactory = new TestCryptoAgentFactory(); private Store store; private CloudDestination dest; private PartitionId partitionId; - private CloudConfig cloudConfig; private VcrMetrics vcrMetrics; private Random random = new Random(); private short refAccountId = 50; @@ -73,40 +70,51 @@ public void setup() throws Exception { /** * Setup the cloud blobstore. * @param requireEncryption value of requireEncryption flag in CloudConfig. - * @param withCryptoAgent whether to set a {@link CloudBlobCryptoAgent} in the cloud store. * @param start whether to start the store. */ - private void setupCloudStore(boolean requireEncryption, boolean withCryptoAgent, boolean start) throws Exception { - Properties props = new Properties(); + private void setupCloudStore(boolean requireEncryption, boolean start) throws Exception { + Properties properties = new Properties(); + // Required clustermap properties + setBasicProperties(properties); // Require encryption for uploading - props.setProperty(CloudConfig.VCR_REQUIRE_ENCRYPTION, Boolean.toString(requireEncryption)); - cloudConfig = new CloudConfig(new VerifiableProperties(props)); + properties.setProperty(CloudConfig.VCR_REQUIRE_ENCRYPTION, Boolean.toString(requireEncryption)); + properties.setProperty(CloudConfig.CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS, + TestCloudBlobCryptoAgentFactory.class.getName()); + VerifiableProperties verifiableProperties = new VerifiableProperties(properties); dest = mock(CloudDestination.class); vcrMetrics = new VcrMetrics(new MetricRegistry()); - store = new CloudBlobStore(partitionId, cloudConfig, dest, withCryptoAgent ? cryptoAgentFactory : null, vcrMetrics); + store = new CloudBlobStore(verifiableProperties, partitionId, dest, vcrMetrics); if (start) { store.start(); } } + /** + * Method to set basic required properties + * @param properties the Properties to set + */ + private void setBasicProperties(Properties properties) { + properties.setProperty("clustermap.cluster.name", "dev"); + properties.setProperty("clustermap.datacenter.name", "DC1"); + properties.setProperty("clustermap.host.name", "localhost"); + properties.setProperty("kms.default.container.key", TestUtils.getRandomKey(64)); + } + /** Test the CloudBlobStore put method. */ @Test public void testStorePuts() throws Exception { - testStorePuts(false, false); - testStorePuts(false, true); - testStorePuts(true, false); - testStorePuts(true, true); + testStorePuts(false); + testStorePuts(true); } - private void testStorePuts(boolean requireEncryption, boolean withCryptoAgent) throws Exception { - setupCloudStore(requireEncryption, withCryptoAgent, true); + private void testStorePuts(boolean requireEncryption) throws Exception { + setupCloudStore(requireEncryption, true); // Put blobs with and without expiration and encryption MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); - int count = 1; + int count = 5; long expireTime = System.currentTimeMillis() + 10000; int expectedUploads = 0; int expectedEncryptions = 0; - int expectedUnencryptedSkips = 0; for (int j = 0; j < count; j++) { long size = Math.abs(random.nextLong()) % 10000; // Permanent and encrypted, should be uploaded and not reencrypted @@ -114,17 +122,9 @@ private void testStorePuts(boolean requireEncryption, boolean withCryptoAgent) t expectedUploads++; // Permanent and unencrypted addBlobToSet(messageWriteSet, size, Utils.Infinite_Time, refAccountId, refContainerId, false); - if (withCryptoAgent) { - expectedUploads++; - if (requireEncryption) { - expectedEncryptions++; - } - } else { - if (requireEncryption) { - expectedUnencryptedSkips++; - } else { - expectedUploads++; - } + expectedUploads++; + if (requireEncryption) { + expectedEncryptions++; } // Short TTL and encrypted, should not be uploaded nor encrypted addBlobToSet(messageWriteSet, size, expireTime, refAccountId, refContainerId, true); @@ -133,13 +133,12 @@ private void testStorePuts(boolean requireEncryption, boolean withCryptoAgent) t verify(dest, times(expectedUploads)).uploadBlob(any(BlobId.class), anyLong(), any(CloudBlobMetadata.class), any(InputStream.class)); assertEquals("Unexpected encryption count", expectedEncryptions, vcrMetrics.blobEncryptionCount.getCount()); - assertEquals("Unexpected skip count", expectedUnencryptedSkips, vcrMetrics.skipUnencryptedBlobsCount.getCount()); } /** Test the CloudBlobStore delete method. */ @Test public void testStoreDeletes() throws Exception { - setupCloudStore(true, true, true); + setupCloudStore(true, true); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); int count = 10; for (int j = 0; j < count; j++) { @@ -153,7 +152,7 @@ public void testStoreDeletes() throws Exception { /** Test the CloudBlobStore updateTtl method. */ @Test public void testStoreTtlUpdates() throws Exception { - setupCloudStore(true, true, true); + setupCloudStore(true, true); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); int count = 10; for (int j = 0; j < count; j++) { @@ -168,7 +167,7 @@ public void testStoreTtlUpdates() throws Exception { /** Test the CloudBlobStore findMissingKeys method. */ @Test public void testFindMissingKeys() throws Exception { - setupCloudStore(true, true, true); + setupCloudStore(true, true); int count = 10; List keys = new ArrayList<>(); Map metadataMap = new HashMap<>(); @@ -193,7 +192,7 @@ public void testFindMissingKeys() throws Exception { @Test public void testStoreNotStarted() throws Exception { // Create store and don't start it. - setupCloudStore(true, true, false); + setupCloudStore(true, false); List keys = Collections.singletonList(getUniqueId()); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); addBlobToSet(messageWriteSet, 10, Utils.Infinite_Time, refAccountId, refContainerId, true); @@ -226,10 +225,12 @@ public void testExceptionalDest() throws Exception { when(exDest.deleteBlob(any(BlobId.class), anyLong())).thenThrow(new CloudStorageException("ouch")); when(exDest.getBlobMetadata(anyList())).thenThrow(new CloudStorageException("ouch")); Properties props = new Properties(); + setBasicProperties(props); props.setProperty(CloudConfig.VCR_REQUIRE_ENCRYPTION, "false"); - cloudConfig = new CloudConfig(new VerifiableProperties(props)); + props.setProperty(CloudConfig.CLOUD_BLOB_CRYPTO_AGENT_FACTORY_CLASS, + TestCloudBlobCryptoAgentFactory.class.getName()); vcrMetrics = new VcrMetrics(new MetricRegistry()); - CloudBlobStore exStore = new CloudBlobStore(partitionId, cloudConfig, exDest, cryptoAgentFactory, vcrMetrics); + CloudBlobStore exStore = new CloudBlobStore(new VerifiableProperties(props), partitionId, exDest, vcrMetrics); exStore.start(); List keys = Collections.singletonList(getUniqueId()); MockMessageWriteSet messageWriteSet = new MockMessageWriteSet(); @@ -295,14 +296,4 @@ private BlobId getUniqueId(short accountId, short containerId, boolean encrypted return new BlobId(BLOB_ID_V6, BlobIdType.NATIVE, dataCenterId, accountId, containerId, partitionId, encrypted, BlobDataType.DATACHUNK); } - - /** - * Factory to create TestCloudBlobCryptoAgent - */ - private class TestCryptoAgentFactory implements CloudBlobCryptoAgentFactory { - @Override - public CloudBlobCryptoAgent getCloudBlobCryptoAgent() { - return cryptoAgent; - } - } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoAgentFactory.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoAgentFactory.java new file mode 100644 index 0000000000..931185fd58 --- /dev/null +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/TestCloudBlobCryptoAgentFactory.java @@ -0,0 +1,33 @@ +/** + * Copyright 2019 LinkedIn Corp. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + */ +package com.github.ambry.cloud; + +import com.codahale.metrics.MetricRegistry; +import com.github.ambry.config.VerifiableProperties; + + +/** + * Factory to create TestCloudBlobCryptoAgent + */ +public class TestCloudBlobCryptoAgentFactory implements CloudBlobCryptoAgentFactory { + private final CloudBlobCryptoAgent cryptoAgent; + public TestCloudBlobCryptoAgentFactory(VerifiableProperties verifiableProperties, String clusterName, + MetricRegistry metricRegistry) { + this.cryptoAgent = new TestCloudBlobCryptoAgent(); + } + @Override + public CloudBlobCryptoAgent getCloudBlobCryptoAgent() { + return cryptoAgent; + } +} From d35d007d19348cd2bfd5758a5ec7e40089944be3 Mon Sep 17 00:00:00 2001 From: Rob Block Date: Mon, 22 Apr 2019 11:43:46 -0700 Subject: [PATCH 13/15] Address review comments, remove startup/shutdown metrics --- .../com.github.ambry.cloud/CloudBackupManager.java | 5 +++-- .../main/java/com.github.ambry.cloud/VcrMetrics.java | 5 ----- .../main/java/com.github.ambry.cloud/VcrServer.java | 9 +-------- .../azure/AzureIntegrationTest.java | 11 ++++++----- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java index 1dfc45ac44..f061e3bd1f 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java @@ -51,14 +51,15 @@ public CloudBackupManager(VerifiableProperties properties, CloudConfig cloudConf StoreKeyFactory storeKeyFactory, ClusterMap clusterMap, VirtualReplicatorCluster virtualReplicatorCluster, CloudDestinationFactory cloudDestinationFactory, ScheduledExecutorService scheduler, ConnectionPool connectionPool, MetricRegistry metricRegistry, NotificationSystem requestNotification, - StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName, VcrMetrics vcrMetrics) - throws ReplicationException { + StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName) throws ReplicationException { super(replicationConfig, clusterMapConfig, storeKeyFactory, clusterMap, scheduler, virtualReplicatorCluster.getCurrentDataNodeId(), Collections.emptyList(), connectionPool, metricRegistry, requestNotification, storeKeyConverterFactory, transformerClassName); + CloudDestination cloudDestination = cloudDestinationFactory.getCloudDestination(); List partitionIds = virtualReplicatorCluster.getAssignedPartitionIds(); + VcrMetrics vcrMetrics = new VcrMetrics(metricRegistry); for (PartitionId partitionId : partitionIds) { ReplicaId cloudReplica = diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java index 391743be26..c0b512ffb6 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java @@ -23,9 +23,6 @@ public class VcrMetrics { private final MetricRegistry registry; - public final Histogram vcrStartTimeInMs; - public final Histogram vcrShutdownTimeInMs; - // Encryption metrics public final Counter blobEncryptionCount; public final Counter blobDecryptionCount; @@ -34,8 +31,6 @@ public class VcrMetrics { public VcrMetrics(MetricRegistry registry) { this.registry = registry; - vcrStartTimeInMs = registry.histogram(MetricRegistry.name(VcrServer.class, "VcrStartTimeInMs")); - vcrShutdownTimeInMs = registry.histogram(MetricRegistry.name(VcrServer.class, "VcrShutdownTimeInMs")); blobEncryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobEncryptionCount")); blobDecryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobDet4acryptionCount")); blobEncryptionTime = registry.timer(MetricRegistry.name(CloudBlobStore.class, "BlobEncryptionTime")); diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java index c42600efd5..cdff0c3366 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrServer.java @@ -71,7 +71,6 @@ public class VcrServer { private ConnectionPool connectionPool = null; private final NotificationSystem notificationSystem; private CloudDestinationFactory cloudDestinationFactory; - private VcrMetrics metrics = null; /** * VcrServer constructor. @@ -112,7 +111,6 @@ public void startup() throws InstantiationException { logger.info("Setting up JMX."); long startTime = SystemTime.getInstance().milliseconds(); registry = clusterMap.getMetricRegistry(); - this.metrics = new VcrMetrics(registry); reporter = JmxReporter.forRegistry(registry).build(); reporter.start(); @@ -146,13 +144,10 @@ public void startup() throws InstantiationException { StoreKeyConverterFactory storeKeyConverterFactory = Utils.getObj(serverConfig.serverStoreKeyConverterFactory, properties, registry); - CloudBlobCryptoAgentFactory cloudBlobCryptoAgentFactory = - Utils.getObj(cloudConfig.cloudBlobCryptoAgentFactoryClass, properties, clusterMapConfig.clusterMapClusterName, - clusterMap.getMetricRegistry()); cloudBackupManager = new CloudBackupManager(properties, cloudConfig, replicationConfig, clusterMapConfig, storeConfig, storeKeyFactory, clusterMap, virtualReplicatorCluster, cloudDestinationFactory, scheduler, connectionPool, - registry, notificationSystem, storeKeyConverterFactory, serverConfig.serverMessageTransformer, metrics); + registry, notificationSystem, storeKeyConverterFactory, serverConfig.serverMessageTransformer); cloudBackupManager.start(); DataNodeId currentNode = virtualReplicatorCluster.getCurrentDataNodeId(); @@ -167,7 +162,6 @@ public void startup() throws InstantiationException { // TODO: for recovery, need AmbryRequests and RequestHandlerPool long processingTime = SystemTime.getInstance().milliseconds() - startTime; - metrics.vcrStartTimeInMs.update(processingTime); logger.info("VCR startup time in Ms " + processingTime); } catch (Exception e) { logger.error("Error during VCR startup", e); @@ -217,7 +211,6 @@ public void shutdown() { } finally { shutdownLatch.countDown(); long processingTime = SystemTime.getInstance().milliseconds() - startTime; - metrics.vcrShutdownTimeInMs.update(processingTime); logger.info("VCR shutdown time in Ms " + processingTime); } } diff --git a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java index 27226290d0..5ad440531b 100644 --- a/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java +++ b/ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureIntegrationTest.java @@ -97,7 +97,7 @@ public void testNormalFlow() throws Exception { InputStream inputStream = getBlobInputStream(blobSize); CloudBlobMetadata cloudBlobMetadata = new CloudBlobMetadata(blobId, System.currentTimeMillis(), Utils.Infinite_Time, blobSize, - CloudBlobMetadata.EncryptionOrigin.NONE, vcrKmsContext, cryptoAgentFactory); + CloudBlobMetadata.EncryptionOrigin.VCR, vcrKmsContext, cryptoAgentFactory); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); // Try to upload same blob again @@ -130,9 +130,8 @@ public void testBatchQuery() throws Exception { BlobDataType.DATACHUNK); blobIdList.add(blobId); InputStream inputStream = getBlobInputStream(blobSize); - CloudBlobMetadata cloudBlobMetadata = - new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, - CloudBlobMetadata.EncryptionOrigin.NONE, vcrKmsContext, cryptoAgentFactory); + CloudBlobMetadata cloudBlobMetadata = new CloudBlobMetadata(blobId, creationTime, Utils.Infinite_Time, blobSize, + CloudBlobMetadata.EncryptionOrigin.VCR, vcrKmsContext, cryptoAgentFactory); assertTrue("Expected upload to return true", azureDest.uploadBlob(blobId, blobSize, cloudBlobMetadata, inputStream)); } @@ -147,8 +146,10 @@ public void testBatchQuery() throws Exception { assertEquals("Unexpected metadata containerId", containerId, metadata.getContainerId()); assertEquals("Unexpected metadata partitionId", partitionId.toPathString(), metadata.getPartitionId()); assertEquals("Unexpected metadata creationTime", creationTime, metadata.getCreationTime()); + assertEquals("Unexpected metadata encryption origin", CloudBlobMetadata.EncryptionOrigin.VCR, + metadata.getEncryptionOrigin()); assertEquals("Unexpected metadata vcrKmsContext", vcrKmsContext, metadata.getVcrKmsContext()); - assertEquals("Unexpected metadata cryptoAgentFactory", vcrKmsContext, metadata.getCryptoAgentFactory()); + assertEquals("Unexpected metadata cryptoAgentFactory", cryptoAgentFactory, metadata.getCryptoAgentFactory()); } // Cleanup From 8ed696abdc172d85397342e75c33126834cecd1b Mon Sep 17 00:00:00 2001 From: Rob Block Date: Mon, 22 Apr 2019 13:35:55 -0700 Subject: [PATCH 14/15] Removed CLOUD encryptionOrigin and clarified javadoc. --- .../java/com.github.ambry.cloud/CloudBlobMetadata.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java index 3e793219db..4877b06fd9 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBlobMetadata.java @@ -46,18 +46,17 @@ public class CloudBlobMetadata { private String cryptoAgentFactory; /** - * Different encryption types for cloud stored blobs + * Possible values of encryption origin for cloud stored blobs. + * Only considers encryption initiated by Ambry. */ public enum EncryptionOrigin { - /** No encryption */ + /** Not encrypted by Ambry */ NONE, /** Encrypted by Router */ ROUTER, /** Encrypted by VCR */ - VCR, - /** Encrypted by Cloud Data Store */ - CLOUD + VCR } From 80b21639a68c67df1c04760e03741efa6c7a3c98 Mon Sep 17 00:00:00 2001 From: Rob Block Date: Tue, 23 Apr 2019 12:53:34 -0700 Subject: [PATCH 15/15] Fix a few typos per review comments. --- .../main/java/com.github.ambry.cloud/CloudBackupManager.java | 1 - .../src/main/java/com.github.ambry.cloud/VcrMetrics.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java index f061e3bd1f..dc8f589db8 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/CloudBackupManager.java @@ -52,7 +52,6 @@ public CloudBackupManager(VerifiableProperties properties, CloudConfig cloudConf CloudDestinationFactory cloudDestinationFactory, ScheduledExecutorService scheduler, ConnectionPool connectionPool, MetricRegistry metricRegistry, NotificationSystem requestNotification, StoreKeyConverterFactory storeKeyConverterFactory, String transformerClassName) throws ReplicationException { - super(replicationConfig, clusterMapConfig, storeKeyFactory, clusterMap, scheduler, virtualReplicatorCluster.getCurrentDataNodeId(), Collections.emptyList(), connectionPool, metricRegistry, requestNotification, storeKeyConverterFactory, transformerClassName); diff --git a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java index c0b512ffb6..b4a034e6b5 100644 --- a/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java +++ b/ambry-cloud/src/main/java/com.github.ambry.cloud/VcrMetrics.java @@ -32,7 +32,7 @@ public class VcrMetrics { public VcrMetrics(MetricRegistry registry) { this.registry = registry; blobEncryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobEncryptionCount")); - blobDecryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobDet4acryptionCount")); + blobDecryptionCount = registry.counter(MetricRegistry.name(CloudBlobStore.class, "BlobDecryptionCount")); blobEncryptionTime = registry.timer(MetricRegistry.name(CloudBlobStore.class, "BlobEncryptionTime")); blobDecryptionTime = registry.timer(MetricRegistry.name(CloudBlobStore.class, "BlobDecryptionTime")); }