Skip to content

Commit bc02815

Browse files
vamsimanoharvamsimanohar
authored
Remov:e Default master key (opensearch-project#1851)
Signed-off-by: Vamsi Manohar <[email protected]>
1 parent f5031a7 commit bc02815

File tree

8 files changed

+99
-8
lines changed

8 files changed

+99
-8
lines changed

‎datasources/src/main/java/org/opensearch/sql/datasources/encryptor/EncryptorImpl.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.util.Base64;
1616
import javax.crypto.spec.SecretKeySpec;
1717
import lombok.RequiredArgsConstructor;
18+
import org.apache.commons.lang3.StringUtils;
1819

1920
@RequiredArgsConstructor
2021
public class EncryptorImpl implements Encryptor {
@@ -23,7 +24,7 @@ public class EncryptorImpl implements Encryptor {
2324

2425
@Override
2526
public String encrypt(String plainText) {
26-
27+
validate(masterKey);
2728
final AwsCrypto crypto = AwsCrypto.builder()
2829
.withCommitmentPolicy(CommitmentPolicy.RequireEncryptRequireDecrypt)
2930
.build();
@@ -39,6 +40,7 @@ public String encrypt(String plainText) {
3940

4041
@Override
4142
public String decrypt(String encryptedText) {
43+
validate(masterKey);
4244
final AwsCrypto crypto = AwsCrypto.builder()
4345
.withCommitmentPolicy(CommitmentPolicy.RequireEncryptRequireDecrypt)
4446
.build();
@@ -52,4 +54,17 @@ public String decrypt(String encryptedText) {
5254
return new String(decryptedResult.getResult());
5355
}
5456

57+
private void validate(String masterKey) {
58+
if (StringUtils.isEmpty(masterKey)) {
59+
throw new IllegalStateException(
60+
"Master key is a required config for using create and update datasource APIs."
61+
+ "Please set plugins.query.datasources.encryption.masterkey config "
62+
+ "in opensearch.yml in all the cluster nodes. "
63+
+ "More details can be found here: "
64+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
65+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information");
66+
}
67+
}
68+
69+
5570
}

‎datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,8 @@ private void reportError(final RestChannel channel, final Exception e, final Res
247247
private static boolean isClientError(Exception e) {
248248
return e instanceof NullPointerException
249249
// NPE is hard to differentiate but more likely caused by bad query
250-
|| e instanceof IllegalArgumentException;
250+
|| e instanceof IllegalArgumentException
251+
|| e instanceof IllegalStateException;
251252
}
252253

253254
}

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,62 @@ public void testDecryptWithDifferentKey() {
8484
encryptor2.decrypt(encrypted);
8585
});
8686
}
87+
88+
@Test
89+
public void testEncryptionAndDecryptionWithNullMasterKey() {
90+
String input = "This is a test input";
91+
Encryptor encryptor = new EncryptorImpl(null);
92+
IllegalStateException illegalStateException
93+
= Assertions.assertThrows(IllegalStateException.class,
94+
() -> encryptor.encrypt(input));
95+
Assertions.assertEquals("Master key is a required config for using create and"
96+
+ " update datasource APIs."
97+
+ "Please set plugins.query.datasources.encryption.masterkey config "
98+
+ "in opensearch.yml in all the cluster nodes. "
99+
+ "More details can be found here: "
100+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
101+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
102+
illegalStateException.getMessage());
103+
illegalStateException
104+
= Assertions.assertThrows(IllegalStateException.class,
105+
() -> encryptor.decrypt(input));
106+
Assertions.assertEquals("Master key is a required config for using create and"
107+
+ " update datasource APIs."
108+
+ "Please set plugins.query.datasources.encryption.masterkey config "
109+
+ "in opensearch.yml in all the cluster nodes. "
110+
+ "More details can be found here: "
111+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
112+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
113+
illegalStateException.getMessage());
114+
}
115+
116+
@Test
117+
public void testEncryptionAndDecryptionWithEmptyMasterKey() {
118+
String masterKey = "";
119+
String input = "This is a test input";
120+
Encryptor encryptor = new EncryptorImpl(masterKey);
121+
IllegalStateException illegalStateException
122+
= Assertions.assertThrows(IllegalStateException.class,
123+
() -> encryptor.encrypt(input));
124+
Assertions.assertEquals("Master key is a required config for using create and"
125+
+ " update datasource APIs."
126+
+ "Please set plugins.query.datasources.encryption.masterkey config "
127+
+ "in opensearch.yml in all the cluster nodes. "
128+
+ "More details can be found here: "
129+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
130+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
131+
illegalStateException.getMessage());
132+
illegalStateException
133+
= Assertions.assertThrows(IllegalStateException.class,
134+
() -> encryptor.decrypt(input));
135+
Assertions.assertEquals("Master key is a required config for using create and"
136+
+ " update datasource APIs."
137+
+ "Please set plugins.query.datasources.encryption.masterkey config "
138+
+ "in opensearch.yml in all the cluster nodes. "
139+
+ "More details can be found here: "
140+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
141+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
142+
illegalStateException.getMessage());
143+
}
144+
87145
}

‎docs/user/ppl/admin/datasources.rst

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,14 @@ Only users mapped with roles having above actions are authorized to execute data
121121
Master Key config for encrypting credential information
122122
========================================================
123123
* 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.
124-
* 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.
124+
* 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.
125125
* The master key must be 16, 24, or 32 characters long.
126-
* It's highly recommended that users configure a master key for better security.
127-
* If users don't provide a master key, the system will default to "0000000000000000".
126+
* Sample Bash Script to generate a 24 character master key ::
127+
128+
#!/bin/bash
129+
# Generate a 24-character key
130+
master_key=$(openssl rand -hex 12)
131+
echo "Master Key: $master_key"
128132
* Sample python script to generate a 24 character master key ::
129133

130134
import random

‎integ-test/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ testClusters.all {
129129
if (System.getProperty("debugJVM") != null) {
130130
jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005'
131131
}
132+
setting "plugins.query.datasources.encryption.masterkey", "1234567812345678"
132133
}
133134

134135
testClusters.integTest {

‎integ-test/src/test/java/org/opensearch/sql/datasource/DataSourceAPIsIT.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ public void createDataSourceAPITest() {
5050
//create datasource
5151
DataSourceMetadata createDSM =
5252
new DataSourceMetadata("create_prometheus", DataSourceType.PROMETHEUS,
53-
ImmutableList.of(), ImmutableMap.of("prometheus.uri", "https://localhost:9090"));
53+
ImmutableList.of(), ImmutableMap.of("prometheus.uri", "https://localhost:9090",
54+
"prometheus.auth.type","basicauth",
55+
"prometheus.auth.username", "username",
56+
"prometheus.auth.password", "password"));
5457
Request createRequest = getCreateDataSourceRequest(createDSM);
5558
Response response = client().performRequest(createRequest);
5659
Assert.assertEquals(201, response.getStatusLine().getStatusCode());

‎opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ public class OpenSearchSettings extends Settings {
111111

112112
public static final Setting<String> DATASOURCE_MASTER_SECRET_KEY = Setting.simpleString(
113113
ENCYRPTION_MASTER_KEY.getKeyValue(),
114-
"0000000000000000",
115114
Setting.Property.NodeScope,
116115
Setting.Property.Final,
117116
Setting.Property.Filtered);

‎plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.util.List;
1616
import java.util.Objects;
1717
import java.util.function.Supplier;
18+
import org.apache.commons.lang3.StringUtils;
1819
import org.apache.logging.log4j.LogManager;
1920
import org.apache.logging.log4j.Logger;
2021
import org.opensearch.action.ActionRequest;
@@ -90,7 +91,8 @@
9091

9192
public class SQLPlugin extends Plugin implements ActionPlugin, ScriptPlugin {
9293

93-
private static final Logger LOG = LogManager.getLogger();
94+
private static final Logger LOGGER = LogManager.getLogger(SQLPlugin.class);
95+
9496
private ClusterService clusterService;
9597
/**
9698
* Settings should be inited when bootstrap the plugin.
@@ -212,6 +214,14 @@ public ScriptEngine getScriptEngine(Settings settings, Collection<ScriptContext<
212214
private DataSourceServiceImpl createDataSourceService() {
213215
String masterKey = OpenSearchSettings
214216
.DATASOURCE_MASTER_SECRET_KEY.get(clusterService.getSettings());
217+
if (StringUtils.isEmpty(masterKey)) {
218+
LOGGER.warn("Master key is a required config for using create and update datasource APIs. "
219+
+ "Please set plugins.query.datasources.encryption.masterkey config "
220+
+ "in opensearch.yml in all the cluster nodes. "
221+
+ "More details can be found here: "
222+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
223+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information");
224+
}
215225
DataSourceMetadataStorage dataSourceMetadataStorage
216226
= new OpenSearchDataSourceMetadataStorage(client, clusterService,
217227
new EncryptorImpl(masterKey));

0 commit comments

Comments
 (0)