Skip to content

Commit

Permalink
Remov:e Default master key
Browse files Browse the repository at this point in the history
Signed-off-by: Vamsi Manohar <[email protected]>
  • Loading branch information
vmmusings committed Jul 11, 2023
1 parent f5031a7 commit 9a73353
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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");
}
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

}
10 changes: 7 additions & 3 deletions docs/user/ppl/admin/datasources.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,12 @@ testClusters.integTest {
}

// add customized keystore

setting "plugins.query.datasources.encryption.masterkey", "1234567812345678"
keystore 'plugins.query.federation.datasources.config', new File("$projectDir/src/test/resources/", 'datasources.json')
}


run {
useCluster testClusters.integTest
}
12 changes: 11 additions & 1 deletion plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 9a73353

Please sign in to comment.