Merge pull request #2 from gbbafna/crypto-comments

PR comments addressed
vikasvb90 · Aug 30, 2023 · 9fe9806 · 9fe9806
2 parents 02633e9 + c115f15
commit 9fe9806
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 49 deletions.
diff --git a/...nsearch/common/crypto/CryptoProvider.java → ...ensearch/common/crypto/CryptoHandler.java b/...nsearch/common/crypto/CryptoProvider.java → ...ensearch/common/crypto/CryptoHandler.java
@@ -16,16 +16,19 @@
 /**
  * Crypto provider abstractions for encryption and decryption of data. Allows registering multiple providers
  * for defining different ways of encrypting or decrypting data.
+ *
+ * T - Encryption Metadata / CryptoContext
+ * U - Parsed Encryption Metadata / CryptoContext
  */
-public interface CryptoProvider {
+public interface CryptoHandler<T, U> {
 
     /**
      * To initialise or create a new crypto metadata to be used in encryption. This is needed to set the context before
      * beginning encryption.
      *
      * @return crypto metadata instance
      */
-    Object initEncryptionMetadata();
+    T initEncryptionMetadata();
 
     /**
      * To load crypto metadata to be used in encryption from content header.
@@ -34,7 +37,7 @@ public interface CryptoProvider {
      *
      * @return crypto metadata instance used in decryption.
      */
-    Object loadEncryptionMetadata(EncryptedHeaderContentSupplier encryptedHeaderContentSupplier) throws IOException;
+    U loadEncryptionMetadata(EncryptedHeaderContentSupplier encryptedHeaderContentSupplier) throws IOException;
 
     /**
      * Few encryption algorithms have certain conditions on the unit of content to be encrypted. This requires the
@@ -46,7 +49,7 @@ public interface CryptoProvider {
      * @param contentSize Size of the raw content
      * @return Adjusted size of the content.
      */
-    long adjustContentSizeForPartialEncryption(Object cryptoContext, long contentSize);
+    long adjustContentSizeForPartialEncryption(T cryptoContext, long contentSize);
 
     /**
      * Estimate length of the encrypted content. It should only be used to determine length of entire content after
@@ -56,15 +59,15 @@ public interface CryptoProvider {
      * @param contentLength Size of the raw content
      * @return Calculated size of the encrypted content.
      */
-    long estimateEncryptedLengthOfEntireContent(Object cryptoContext, long contentLength);
+    long estimateEncryptedLengthOfEntireContent(T cryptoContext, long contentLength);
 
     /**
      * For given encrypted content length, estimate the length of the decrypted content.
      * @param cryptoContext crypto metadata instance consisting of encryption metadata used in encryption.
      * @param contentLength Size of the encrypted content
      * @return Calculated size of the decrypted content.
      */
-    long estimateDecryptedLength(Object cryptoContext, long contentLength);
+    long estimateDecryptedLength(U cryptoContext, long contentLength);
 
     /**
      * Wraps a raw InputStream with encrypting stream
@@ -73,7 +76,7 @@ public interface CryptoProvider {
      * @param stream Raw InputStream to encrypt
      * @return encrypting stream wrapped around raw InputStream.
      */
-    InputStreamContainer createEncryptingStream(Object encryptionMetadata, InputStreamContainer stream);
+    InputStreamContainer createEncryptingStream(T encryptionMetadata, InputStreamContainer stream);
 
     /**
      * Provides encrypted stream for a raw stream emitted for a part of content.
@@ -84,7 +87,7 @@ public interface CryptoProvider {
      * @param streamIdx Index of the current stream.
      * @return Encrypted stream for the provided raw stream.
      */
-    InputStreamContainer createEncryptingStreamOfPart(Object cryptoContext, InputStreamContainer stream, int totalStreams, int streamIdx);
+    InputStreamContainer createEncryptingStreamOfPart(T cryptoContext, InputStreamContainer stream, int totalStreams, int streamIdx);
 
     /**
      * This method accepts an encrypted stream and provides a decrypting wrapper.
@@ -107,5 +110,5 @@ public interface CryptoProvider {
      * @param startPosOfRawContent starting position in the raw/decrypted content
      * @param endPosOfRawContent ending position in the raw/decrypted content
      */
-    DecryptedRangedStreamProvider createDecryptingStreamOfRange(Object cryptoContext, long startPosOfRawContent, long endPosOfRawContent);
+    DecryptedRangedStreamProvider createDecryptingStreamOfRange(U cryptoContext, long startPosOfRawContent, long endPosOfRawContent);
 }
diff --git a/libs/encryption-sdk/src/main/java/org/opensearch/encryption/CryptoManager.java b/libs/encryption-sdk/src/main/java/org/opensearch/encryption/CryptoManager.java
@@ -8,13 +8,13 @@
 
 package org.opensearch.encryption;
 
-import org.opensearch.common.crypto.CryptoProvider;
+import org.opensearch.common.crypto.CryptoHandler;
 import org.opensearch.common.util.concurrent.RefCounted;
 
 /**
  * Crypto plugin interface used for encryption and decryption.
  */
-public interface CryptoManager extends RefCounted {
+public interface CryptoManager<T, U> extends RefCounted {
 
     /**
      * @return key provider type
@@ -29,5 +29,5 @@ public interface CryptoManager extends RefCounted {
     /**
      * @return Crypto provider for encrypting or decrypting raw content.
      */
-    CryptoProvider getCryptoProvider();
+    CryptoHandler<T, U> getCryptoProvider();
 }
diff --git a/libs/encryption-sdk/src/main/java/org/opensearch/encryption/CryptoManagerFactory.java b/libs/encryption-sdk/src/main/java/org/opensearch/encryption/CryptoManagerFactory.java
@@ -8,7 +8,10 @@
 
 package org.opensearch.encryption;
 
-import org.opensearch.common.crypto.CryptoProvider;
+import com.amazonaws.encryptionsdk.CryptoAlgorithm;
+import com.amazonaws.encryptionsdk.caching.CachingCryptoMaterialsManager;
+import com.amazonaws.encryptionsdk.caching.LocalCryptoMaterialsCache;
+import org.opensearch.common.crypto.CryptoHandler;
 import org.opensearch.common.crypto.MasterKeyProvider;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.util.concurrent.AbstractRefCounted;
@@ -17,10 +20,6 @@
 import java.security.SecureRandom;
 import java.util.concurrent.TimeUnit;
 
-import com.amazonaws.encryptionsdk.CryptoAlgorithm;
-import com.amazonaws.encryptionsdk.caching.CachingCryptoMaterialsManager;
-import com.amazonaws.encryptionsdk.caching.LocalCryptoMaterialsCache;
-
 public class CryptoManagerFactory {
 
     private final int dataKeyCacheSize;
@@ -50,7 +49,7 @@ private String validateAndGetAlgorithmId(String algorithm) {
         }
     }
 
-    public CryptoManager getOrCreateCryptoManager(
+    public CryptoManager<? , ?> getOrCreateCryptoManager(
         MasterKeyProvider keyProvider,
         String keyProviderName,
         String keyProviderType,
@@ -61,17 +60,17 @@ public CryptoManager getOrCreateCryptoManager(
             keyProviderName,
             validateAndGetAlgorithmId(algorithm)
         );
-        CryptoProvider cryptoProvider = createCryptoProvider(algorithm, materialsManager, keyProvider);
-        return createCryptoManager(cryptoProvider, keyProviderType, keyProviderName, onClose);
+        CryptoHandler<? , ?> cryptoHandler = createCryptoProvider(algorithm, materialsManager, keyProvider);
+        return createCryptoManager(cryptoHandler, keyProviderType, keyProviderName, onClose);
     }
 
     // package private for tests
-    CryptoProvider createCryptoProvider(
+    CryptoHandler<? , ?> createCryptoProvider(
         String algorithm,
         CachingCryptoMaterialsManager materialsManager,
         MasterKeyProvider masterKeyProvider
     ) {
-        return new NoOpCryptoProvider();
+        return new NoOpCryptoHandler();
     }
 
     // Package private for tests
@@ -90,8 +89,8 @@ CachingCryptoMaterialsManager createMaterialsManager(MasterKeyProvider masterKey
     }
 
     // package private for tests
-    CryptoManager createCryptoManager(CryptoProvider cryptoProvider, String keyProviderType, String keyProviderName, Runnable onClose) {
-        return new CryptoManagerImpl(keyProviderName, keyProviderType) {
+    <T, U> CryptoManager<?, ?> createCryptoManager(CryptoHandler<T, U> cryptoHandler, String keyProviderType, String keyProviderName, Runnable onClose) {
+        return new CryptoManagerImpl<T, U>(keyProviderName, keyProviderType) {
             @Override
             protected void closeInternal() {
                 onClose.run();
@@ -108,13 +107,13 @@ public String name() {
             }
 
             @Override
-            public CryptoProvider getCryptoProvider() {
-                return cryptoProvider;
+            public CryptoHandler<T, U> getCryptoProvider() {
+                return cryptoHandler;
             }
         };
     }
 
-    private static abstract class CryptoManagerImpl extends AbstractRefCounted implements CryptoManager {
+    private static abstract class CryptoManagerImpl<T, U> extends AbstractRefCounted implements CryptoManager<T, U> {
         public CryptoManagerImpl(String keyProviderName, String keyProviderType) {
             super(keyProviderName + "-" + keyProviderType);
         }

diff --git a/...search/encryption/NoOpCryptoProvider.java → ...nsearch/encryption/NoOpCryptoHandler.java b/...search/encryption/NoOpCryptoProvider.java → ...nsearch/encryption/NoOpCryptoHandler.java
@@ -8,15 +8,15 @@
 
 package org.opensearch.encryption;
 
-import org.opensearch.common.crypto.CryptoProvider;
+import org.opensearch.common.crypto.CryptoHandler;
 import org.opensearch.common.crypto.DecryptedRangedStreamProvider;
 import org.opensearch.common.crypto.EncryptedHeaderContentSupplier;
 import org.opensearch.common.io.InputStreamContainer;
 
 import java.io.IOException;
 import java.io.InputStream;
 
-public class NoOpCryptoProvider implements CryptoProvider {
+public class NoOpCryptoHandler implements CryptoHandler<Object, Object> {
 
     /**
      * No op - Initialises metadata store used in encryption.

diff --git a/libs/encryption-sdk/src/test/java/org/opensearch/encryption/CryptoManagerFactoryTests.java b/libs/encryption-sdk/src/test/java/org/opensearch/encryption/CryptoManagerFactoryTests.java
@@ -8,16 +8,15 @@
 
 package org.opensearch.encryption;
 
-import org.opensearch.common.crypto.CryptoProvider;
+import com.amazonaws.encryptionsdk.caching.CachingCryptoMaterialsManager;
+import org.junit.Before;
+import org.opensearch.common.crypto.CryptoHandler;
 import org.opensearch.common.crypto.MasterKeyProvider;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.test.OpenSearchTestCase;
-import org.junit.Before;
 
 import java.util.Collections;
 
-import com.amazonaws.encryptionsdk.caching.CachingCryptoMaterialsManager;
-
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -38,7 +37,7 @@ public void testGetOrCreateCryptoManager() {
         MasterKeyProvider mockKeyProvider = mock(MasterKeyProvider.class);
         when(mockKeyProvider.getEncryptionContext()).thenReturn(Collections.emptyMap());
 
-        CryptoManager cryptoManager = cryptoManagerFactory.getOrCreateCryptoManager(
+        CryptoManager<? , ?> cryptoManager = cryptoManagerFactory.getOrCreateCryptoManager(
             mockKeyProvider,
             "keyProviderName",
             "keyProviderType",
@@ -53,13 +52,13 @@ public void testCreateCryptoProvider() {
         MasterKeyProvider mockKeyProvider = mock(MasterKeyProvider.class);
         when(mockKeyProvider.getEncryptionContext()).thenReturn(Collections.emptyMap());
 
-        CryptoProvider cryptoProvider = cryptoManagerFactory.createCryptoProvider(
+        CryptoHandler<? , ?> cryptoHandler = cryptoManagerFactory.createCryptoProvider(
             "ALG_AES_256_GCM_HKDF_SHA512_COMMIT_KEY_ECDSA_P384",
             mockMaterialsManager,
             mockKeyProvider
         );
 
-        assertNotNull(cryptoProvider);
+        assertNotNull(cryptoHandler);
     }
 
     public void testCreateMaterialsManager() {
@@ -76,9 +75,9 @@ public void testCreateMaterialsManager() {
     }
 
     public void testCreateCryptoManager() {
-        CryptoProvider mockCryptoProvider = mock(CryptoProvider.class);
-        CryptoManager cryptoManager = cryptoManagerFactory.createCryptoManager(
-            mockCryptoProvider,
+        CryptoHandler<? , ?> mockCryptoHandler = mock(CryptoHandler.class);
+        CryptoManager<? , ?> cryptoManager = cryptoManagerFactory.createCryptoManager(
+            mockCryptoHandler,
             "keyProviderName",
             "keyProviderType",
             null

diff --git a/...h/encryption/NoOpCryptoProviderTests.java → ...ch/encryption/NoOpCryptoHandlerTests.java b/...h/encryption/NoOpCryptoProviderTests.java → ...ch/encryption/NoOpCryptoHandlerTests.java
@@ -17,44 +17,44 @@
 import java.io.IOException;
 import java.io.InputStream;
 
-public class NoOpCryptoProviderTests extends OpenSearchTestCase {
+public class NoOpCryptoHandlerTests extends OpenSearchTestCase {
 
     public void testInitEncryptionMetadata() {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         Object encryptionMetadata = cryptoProvider.initEncryptionMetadata();
         assertNotNull(encryptionMetadata);
     }
 
     public void testAdjustContentSizeForPartialEncryption() {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         long originalSize = 1000L;
         long adjustedSize = cryptoProvider.adjustContentSizeForPartialEncryption(new Object(), originalSize);
         assertEquals(originalSize, adjustedSize);
     }
 
     public void testEstimateEncryptedLengthOfEntireContent() {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         long originalSize = 2000L;
         long estimatedSize = cryptoProvider.estimateEncryptedLengthOfEntireContent(new Object(), originalSize);
         assertEquals(originalSize, estimatedSize);
     }
 
     public void testEstimateDecryptedLength() {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         long originalSize = 1500L;
         long estimatedSize = cryptoProvider.estimateDecryptedLength(new Object(), originalSize);
         assertEquals(originalSize, estimatedSize);
     }
 
     public void testCreateEncryptingStream() {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         InputStreamContainer inputStream = randomStream();
         InputStreamContainer encryptedStream = cryptoProvider.createEncryptingStream(new Object(), inputStream);
         assertEquals(inputStream, encryptedStream);
     }
 
     public void testCreateEncryptingStreamOfPart() {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         InputStreamContainer inputStream = randomStream();
         InputStreamContainer encryptedStream = cryptoProvider.createEncryptingStreamOfPart(new Object(), inputStream, 2, 1);
         assertEquals(inputStream, encryptedStream);
@@ -68,21 +68,21 @@ private InputStreamContainer randomStream() {
     }
 
     public void testLoadEncryptionMetadata() throws IOException {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         EncryptedHeaderContentSupplier supplier = (start, length) -> { throw new UnsupportedOperationException("Not implemented"); };
         Object encryptionMetadata = cryptoProvider.loadEncryptionMetadata(supplier);
         assertNotNull(encryptionMetadata);
     }
 
     public void testCreateDecryptingStream() {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         InputStream encryptedStream = randomStream().getInputStream();
         InputStream decryptedStream = cryptoProvider.createDecryptingStream(encryptedStream);
         assertEquals(encryptedStream, decryptedStream);
     }
 
     public void testCreateDecryptingStreamOfRange() {
-        NoOpCryptoProvider cryptoProvider = new NoOpCryptoProvider();
+        NoOpCryptoHandler cryptoProvider = new NoOpCryptoHandler();
         Object cryptoContext = new Object();
         long startPos = 0L;
         long endPos = 100L;