[Backport 2.9] Remove Default master encryption key from settings. (#…

…1854) * Remov:e Default master key (#1851) Signed-off-by: Vamsi Manohar <reddyvam@amazon.com> (cherry picked from commit bc02815) * Fix BWC tests Signed-off-by: Vamsi Manohar <reddyvam@amazon.com> --------- Signed-off-by: Vamsi Manohar <reddyvam@amazon.com> Co-authored-by: Vamsi Manohar <reddyvam@amazon.com>
opensearch-project · Jul 12, 2023 · 02da891 · 02da891
1 parent 4f21bf1
commit 02da891
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 8 deletions.
diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/encryptor/EncryptorImpl.java b/datasources/src/main/java/org/opensearch/sql/datasources/encryptor/EncryptorImpl.java
@@ -15,6 +15,7 @@
 import java.util.Base64;
 import javax.crypto.spec.SecretKeySpec;
 import lombok.RequiredArgsConstructor;
+import org.apache.commons.lang3.StringUtils;
 
 @RequiredArgsConstructor
 public class EncryptorImpl implements Encryptor {
@@ -23,7 +24,7 @@ public class EncryptorImpl implements Encryptor {
 
   @Override
   public String encrypt(String plainText) {
-
+    validate(masterKey);
     final AwsCrypto crypto = AwsCrypto.builder()
         .withCommitmentPolicy(CommitmentPolicy.RequireEncryptRequireDecrypt)
         .build();
@@ -39,6 +40,7 @@ public String encrypt(String plainText) {
 
   @Override
   public String decrypt(String encryptedText) {
+    validate(masterKey);
     final AwsCrypto crypto = AwsCrypto.builder()
         .withCommitmentPolicy(CommitmentPolicy.RequireEncryptRequireDecrypt)
         .build();
@@ -52,4 +54,17 @@ public String decrypt(String encryptedText) {
     return new String(decryptedResult.getResult());
   }
 
+  private void validate(String masterKey) {
+    if (StringUtils.isEmpty(masterKey)) {
+      throw new IllegalStateException(
+          "Master key is a required config for using create and update datasource APIs."
+              + "Please set plugins.query.datasources.encryption.masterkey config "
+              + "in opensearch.yml in all the cluster nodes. "
+              + "More details can be found here: "
+              + "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
+              + "admin/datasources.rst#master-key-config-for-encrypting-credential-information");
+    }
+  }
+
+
 }
diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java
@@ -247,7 +247,8 @@ private void reportError(final RestChannel channel, final Exception e, final Res
   private static boolean isClientError(Exception e) {
     return e instanceof NullPointerException
         // NPE is hard to differentiate but more likely caused by bad query
-        || e instanceof IllegalArgumentException;
+        || e instanceof IllegalArgumentException
+        || e instanceof IllegalStateException;
   }
 
 }
diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/encryptor/EncryptorImplTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/encryptor/EncryptorImplTest.java
@@ -84,4 +84,62 @@ public void testDecryptWithDifferentKey() {
       encryptor2.decrypt(encrypted);
     });
   }
+
+  @Test
+  public void testEncryptionAndDecryptionWithNullMasterKey() {
+    String input = "This is a test input";
+    Encryptor encryptor = new EncryptorImpl(null);
+    IllegalStateException illegalStateException
+        = Assertions.assertThrows(IllegalStateException.class,
+              () -> encryptor.encrypt(input));
+    Assertions.assertEquals("Master key is a required config for using create and"
+            + " update datasource APIs."
+            + "Please set plugins.query.datasources.encryption.masterkey config "
+            + "in opensearch.yml in all the cluster nodes. "
+            + "More details can be found here: "
+            + "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
+            + "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
+        illegalStateException.getMessage());
+    illegalStateException
+        = Assertions.assertThrows(IllegalStateException.class,
+              () -> encryptor.decrypt(input));
+    Assertions.assertEquals("Master key is a required config for using create and"
+            + " update datasource APIs."
+            + "Please set plugins.query.datasources.encryption.masterkey config "
+            + "in opensearch.yml in all the cluster nodes. "
+            + "More details can be found here: "
+            + "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
+            + "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
+        illegalStateException.getMessage());
+  }
+
+  @Test
+  public void testEncryptionAndDecryptionWithEmptyMasterKey() {
+    String masterKey = "";
+    String input = "This is a test input";
+    Encryptor encryptor = new EncryptorImpl(masterKey);
+    IllegalStateException illegalStateException
+        = Assertions.assertThrows(IllegalStateException.class,
+              () -> encryptor.encrypt(input));
+    Assertions.assertEquals("Master key is a required config for using create and"
+            + " update datasource APIs."
+            + "Please set plugins.query.datasources.encryption.masterkey config "
+            + "in opensearch.yml in all the cluster nodes. "
+            + "More details can be found here: "
+            + "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
+            + "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
+        illegalStateException.getMessage());
+    illegalStateException
+        = Assertions.assertThrows(IllegalStateException.class,
+              () -> encryptor.decrypt(input));
+    Assertions.assertEquals("Master key is a required config for using create and"
+            + " update datasource APIs."
+            + "Please set plugins.query.datasources.encryption.masterkey config "
+            + "in opensearch.yml in all the cluster nodes. "
+            + "More details can be found here: "
+            + "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
+            + "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
+        illegalStateException.getMessage());
+  }
+
 }
diff --git a/docs/user/ppl/admin/datasources.rst b/docs/user/ppl/admin/datasources.rst
@@ -121,10 +121,14 @@ Only users mapped with roles having above actions are authorized to execute data
 Master Key config for encrypting credential information
 ========================================================
 * When users provide credentials for a data source, the system encrypts and securely stores them in the metadata index. System uses "AES/GCM/NoPadding" symmetric encryption algorithm.
-* Users can set up a master key to use with this encryption method by configuring the plugins.query.datasources.encryption.masterkey setting in the opensearch.yml file.
+* Master key is a required config and users can set this up by configuring the `plugins.query.datasources.encryption.masterkey` setting in the opensearch.yml file.
 * The master key must be 16, 24, or 32 characters long.
-* It's highly recommended that users configure a master key for better security.
-* If users don't provide a master key, the system will default to "0000000000000000".
+* Sample Bash Script to generate a 24 character master key ::
+
+    #!/bin/bash
+    # Generate a 24-character key
+    master_key=$(openssl rand -hex 12)
+    echo "Master Key: $master_key"
 * Sample python script to generate a 24 character master key ::
 
     import random

diff --git a/integ-test/build.gradle b/integ-test/build.gradle
@@ -124,6 +124,7 @@ testClusters.all {
 
 testClusters.integTest {
     plugin ":opensearch-sql-plugin"
+    setting "plugins.query.datasources.encryption.masterkey", "1234567812345678"
 }
 
 testClusters {

diff --git a/integ-test/src/test/java/org/opensearch/sql/datasource/DataSourceAPIsIT.java b/integ-test/src/test/java/org/opensearch/sql/datasource/DataSourceAPIsIT.java
@@ -50,7 +50,10 @@ public void createDataSourceAPITest() {
     //create datasource
     DataSourceMetadata createDSM =
         new DataSourceMetadata("create_prometheus", DataSourceType.PROMETHEUS,
-            ImmutableList.of(), ImmutableMap.of("prometheus.uri", "https://localhost:9090"));
+            ImmutableList.of(), ImmutableMap.of("prometheus.uri", "https://localhost:9090",
+            "prometheus.auth.type","basicauth",
+            "prometheus.auth.username", "username",
+            "prometheus.auth.password", "password"));
     Request createRequest = getCreateDataSourceRequest(createDSM);
     Response response = client().performRequest(createRequest);
     Assert.assertEquals(201, response.getStatusLine().getStatusCode());

diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java
@@ -111,7 +111,6 @@ public class OpenSearchSettings extends Settings {
 
   public static final Setting<String> DATASOURCE_MASTER_SECRET_KEY = Setting.simpleString(
       ENCYRPTION_MASTER_KEY.getKeyValue(),
-      "0000000000000000",
       Setting.Property.NodeScope,
       Setting.Property.Final,
       Setting.Property.Filtered);

diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java
@@ -15,6 +15,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Supplier;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.action.ActionRequest;
@@ -90,7 +91,8 @@
 
 public class SQLPlugin extends Plugin implements ActionPlugin, ScriptPlugin {
 
-  private static final Logger LOG = LogManager.getLogger();
+  private static final Logger LOGGER = LogManager.getLogger(SQLPlugin.class);
+
   private ClusterService clusterService;
   /**
    * Settings should be inited when bootstrap the plugin.
@@ -212,6 +214,14 @@ public ScriptEngine getScriptEngine(Settings settings, Collection<ScriptContext<
   private DataSourceServiceImpl createDataSourceService() {
     String masterKey = OpenSearchSettings
         .DATASOURCE_MASTER_SECRET_KEY.get(clusterService.getSettings());
+    if (StringUtils.isEmpty(masterKey)) {
+      LOGGER.warn("Master key is a required config for using create and update datasource APIs. "
+          + "Please set plugins.query.datasources.encryption.masterkey config "
+          + "in opensearch.yml in all the cluster nodes. "
+          + "More details can be found here: "
+          + "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
+          + "admin/datasources.rst#master-key-config-for-encrypting-credential-information");
+    }
     DataSourceMetadataStorage dataSourceMetadataStorage
         = new OpenSearchDataSourceMetadataStorage(client, clusterService,
             new EncryptorImpl(masterKey));