Metadata file read fix to include reading trailing metadata during fu…

…ll content read Signed-off-by: Vikas Bansal <43470111+vikasvb90@users.noreply.github.com>
vikasvb90 · Sep 4, 2023 · 5ca5e9d · 5ca5e9d
1 parent 9ca9fc4
commit 5ca5e9d
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 56 deletions.
diff --git a/server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobContainer.java b/server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobContainer.java
@@ -157,7 +157,7 @@ private Map<String, BlobMetadata> convertToEncryptedMetadataMap(Map<String, Blob
             .collect(
                 Collectors.toMap(
                     Map.Entry::getKey,
-                    entry -> new EncryptedBlobMetadata(entry.getValue(), cryptoHandler, getEncryptedHeaderContentSupplier(entry.getKey()))
+                    entry -> new EncryptedBlobMetadata<>(entry.getValue(), cryptoHandler, getEncryptedHeaderContentSupplier(entry.getKey()))
                 )
             );
 
@@ -176,7 +176,7 @@ public void listBlobsByPrefixInSortedOrder(
                 if (metadataList != null) {
                     List<BlobMetadata> encryptedMetadata = metadataList.stream()
                         .map(
-                            blobMetadata -> new EncryptedBlobMetadata(
+                            blobMetadata -> new EncryptedBlobMetadata<>(
                                 blobMetadata,
                                 cryptoHandler,
                                 getEncryptedHeaderContentSupplier(blobMetadata.name())

diff --git a/server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobMetadata.java b/server/src/main/java/org/opensearch/common/blobstore/EncryptedBlobMetadata.java
@@ -38,12 +38,12 @@ public String name() {
 
     @Override
     public long length() {
-        Object cryptoContext;
+        U cryptoContext;
         try {
             cryptoContext = cryptoHandler.loadEncryptionMetadata(encryptedHeaderContentSupplier);
         } catch (IOException ex) {
             throw new RuntimeException(ex);
         }
-        return cryptoHandler.estimateDecryptedLength((U) cryptoContext, delegate.length());
+        return cryptoHandler.estimateDecryptedLength(cryptoContext, delegate.length());
     }
 }
diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java
@@ -87,7 +87,6 @@
 import org.opensearch.common.util.PageCacheRecycler;
 import org.opensearch.common.util.concurrent.OpenSearchExecutors;
 import org.opensearch.common.util.concurrent.ThreadContext;
-import org.opensearch.crypto.CryptoManagerRegistry;
 import org.opensearch.discovery.DiscoveryModule;
 import org.opensearch.discovery.HandshakingTransportAddressConnector;
 import org.opensearch.discovery.PeerFinder;
@@ -670,12 +669,7 @@ public void apply(Settings value, Settings current, Settings previous) {
 
                 // Remote cluster state settings
                 RemoteClusterStateService.REMOTE_CLUSTER_STATE_ENABLED_SETTING,
-                RemoteClusterStateService.REMOTE_CLUSTER_STATE_REPOSITORY_SETTING,
-
-                // Crypto settings
-                CryptoManagerRegistry.CRYPTO_KEY_REFRESH_INTERVAL,
-                CryptoManagerRegistry.CRYPTO_ALGORITHM,
-                CryptoManagerRegistry.CRYPTO_KEY_CACHE_SIZE
+                RemoteClusterStateService.REMOTE_CLUSTER_STATE_REPOSITORY_SETTING
             )
         )
     );

diff --git a/server/src/main/java/org/opensearch/crypto/CryptoManagerRegistry.java b/server/src/main/java/org/opensearch/crypto/CryptoManagerRegistry.java
@@ -14,7 +14,6 @@
 import org.opensearch.cluster.metadata.CryptoMetadata;
 import org.opensearch.common.SetOnce;
 import org.opensearch.common.crypto.MasterKeyProvider;
-import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.encryption.CryptoManager;
@@ -25,7 +24,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.function.Function;
 
 /**
  * During node bootstrap, installed key provider extensions responsible for generating data keys are loaded.
@@ -44,51 +42,14 @@ public class CryptoManagerRegistry {
     private static volatile CryptoManagerRegistry instance;
     private static final Object lock = new Object();
 
-    /**
-     * The crypto algorithm to be used by {@link CryptoManager} to encrypt data.
-     */
-    public static final Setting<String> CRYPTO_ALGORITHM = new Setting<>(
-        "crypto.algorithm",
-        "ALG_AES_256_GCM_HKDF_SHA512_COMMIT_KEY",
-        Function.identity(),
-        Setting.Property.NodeScope
-    );
-
-    /**
-     * Refresh interval for the rotation of crypto key used in encrypting data.
-     */
-    public static final Setting<TimeValue> CRYPTO_KEY_REFRESH_INTERVAL = Setting.timeSetting(
-        "crypto.key.refresh_interval",
-        TimeValue.timeValueDays(2),
-        TimeValue.timeValueHours(1),
-        TimeValue.timeValueDays(10),
-        Setting.Property.NodeScope
-    );
-
-    /**
-     * Size of cache used for encryption keys.
-     */
-    public static final Setting<Integer> CRYPTO_KEY_CACHE_SIZE = Setting.intSetting(
-        "crypto.key.cache_size",
-        500,
-        100,
-        Setting.Property.NodeScope
-    );
-
     /**
      * Initializes the registry with crypto factories for the installed crypto key providers.
      *
      * @param cryptoPlugins The list of installed crypto key provider plugins.
      * @param settings Crypto settings.
      */
     protected CryptoManagerRegistry(List<CryptoKeyProviderPlugin> cryptoPlugins, Settings settings) {
-        cryptoManagerFactory.set(
-            new CryptoManagerFactory(
-                CRYPTO_ALGORITHM.get(settings),
-                CRYPTO_KEY_REFRESH_INTERVAL.get(settings),
-                CRYPTO_KEY_CACHE_SIZE.get(settings)
-            )
-        );
+        cryptoManagerFactory.set(new CryptoManagerFactory("ALG_AES_256_GCM_HKDF_SHA512_COMMIT_KEY", TimeValue.timeValueDays(2), 500));
         registry.set(loadCryptoFactories(cryptoPlugins));
     }
 

diff --git a/server/src/main/java/org/opensearch/index/store/RemoteDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteDirectory.java
@@ -145,6 +145,19 @@ public void onFailure(Exception e) {
         }
     }
 
+    /**
+     * Reads entire content from remote file. This should be used to read small files as it loads entire content
+     * in memory.
+     * @param fileName Name of file
+     * @return Content read
+     * @throws IOException if read fails with IO error
+     */
+    public byte[] readAllBytes(String fileName) throws IOException {
+        try (InputStream inputStream = blobContainer.readBlob(fileName)) {
+            return inputStream.readAllBytes();
+        }
+    }
+
     /**
      * Removes an existing file in the directory.
      *

diff --git a/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java b/server/src/main/java/org/opensearch/index/store/RemoteSegmentStoreDirectory.java
@@ -190,11 +190,8 @@ public RemoteSegmentMetadata readLatestMetadataFile() throws IOException {
     }
 
     private RemoteSegmentMetadata readMetadataFile(String metadataFilename) throws IOException {
-        try (IndexInput indexInput = remoteMetadataDirectory.openInput(metadataFilename, IOContext.DEFAULT)) {
-            byte[] metadataBytes = new byte[(int) indexInput.length()];
-            indexInput.readBytes(metadataBytes, 0, (int) indexInput.length());
-            return metadataStreamWrapper.readStream(new ByteArrayIndexInput(metadataFilename, metadataBytes));
-        }
+        byte[] metadataBytes = remoteMetadataDirectory.readAllBytes(metadataFilename);
+        return metadataStreamWrapper.readStream(new ByteArrayIndexInput(metadataFilename, metadataBytes));
     }
 
     /**