Remvoe Default master key

Signed-off-by: Vamsi Manohar <[email protected]>

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 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");
+    }
+  }
+
+
 }

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;
   }
 
 }

datasources/src/test/java/org/opensearch/sql/datasources/encryptor/EncryptorImplTest.java

@@ -84,4 +84,58 @@ 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 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 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 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 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());
+  }
+
 }

docs/user/ppl/admin/datasources.rst

@@ -121,10 +121,8 @@ 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 python script to generate a 24 character master key ::
 
     import random

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);

plugin/build.gradle

@@ -253,9 +253,15 @@ testClusters.integTest {
     }
 
     // add customized keystore
+
     keystore 'plugins.query.federation.datasources.config', new File("$projectDir/src/test/resources/", 'datasources.json')
 }
 
+testClusters.integTest.nodes.each { node ->
+    node.setting("plugins.query.datasources.encryption.masterkey",
+            "1234567812345678")
+}
+
 run {
     useCluster testClusters.integTest
 }

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 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));