Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Default master encryption key from settings. #1851

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
1 change: 1 addition & 0 deletions integ-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ testClusters.all {
if (System.getProperty("debugJVM") != null) {
jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005'
}
setting "plugins.query.datasources.encryption.masterkey", "1234567812345678"
}

testClusters.integTest {
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
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