Remov:e Default master key · opensearch-project/sql@449cc17 · GitHub
Skip to content

Commit 449cc17

Browse files
vamsimanoharvamsimanohar
committed
Remov:e Default master key
Signed-off-by: Vamsi Manohar <reddyvam@amazon.com>
1 parent f5031a7 commit 449cc17

7 files changed

Lines changed: 96 additions & 7 deletions

File tree

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

Lines changed: 16 additions & 1 deletion

‎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: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,58 @@ 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 datasource APIs. "
96+
+ "Please set plugins.query.datasources.encryption.masterkey config "
97+
+ "in opensearch.yml in all the cluster nodes. "
98+
+ "More details can be found here: "
99+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
100+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
101+
illegalStateException.getMessage());
102+
illegalStateException
103+
= Assertions.assertThrows(IllegalStateException.class,
104+
() -> encryptor.decrypt(input));
105+
Assertions.assertEquals("Master key is a required config for using datasource APIs. "
106+
+ "Please set plugins.query.datasources.encryption.masterkey config "
107+
+ "in opensearch.yml in all the cluster nodes. "
108+
+ "More details can be found here: "
109+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
110+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
111+
illegalStateException.getMessage());
112+
}
113+
114+
@Test
115+
public void testEncryptionAndDecryptionWithEmptyMasterKey() {
116+
String masterKey = "";
117+
String input = "This is a test input";
118+
Encryptor encryptor = new EncryptorImpl(masterKey);
119+
IllegalStateException illegalStateException
120+
= Assertions.assertThrows(IllegalStateException.class,
121+
() -> encryptor.encrypt(input));
122+
Assertions.assertEquals("Master key is a required config for using datasource APIs. "
123+
+ "Please set plugins.query.datasources.encryption.masterkey config "
124+
+ "in opensearch.yml in all the cluster nodes. "
125+
+ "More details can be found here: "
126+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
127+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
128+
illegalStateException.getMessage());
129+
illegalStateException
130+
= Assertions.assertThrows(IllegalStateException.class,
131+
() -> encryptor.decrypt(input));
132+
Assertions.assertEquals("Master key is a required config for using datasource APIs. "
133+
+ "Please set plugins.query.datasources.encryption.masterkey config "
134+
+ "in opensearch.yml in all the cluster nodes. "
135+
+ "More details can be found here: "
136+
+ "https://github.com/opensearch-project/sql/blob/main/docs/user/ppl/"
137+
+ "admin/datasources.rst#master-key-config-for-encrypting-credential-information",
138+
illegalStateException.getMessage());
139+
}
140+
87141
}

‎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

‎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/build.gradle‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,15 @@ testClusters.integTest {
253253
}
254254

255255
// add customized keystore
256+
256257
keystore 'plugins.query.federation.datasources.config', new File("$projectDir/src/test/resources/", 'datasources.json')
257258
}
258259

260+
testClusters.integTest.nodes.each { node ->
261+
node.setting("plugins.query.datasources.encryption.masterkey",
262+
"1234567812345678")
263+
}
264+
259265
run {
260266
useCluster testClusters.integTest
261267
}

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

Lines changed: 11 additions & 1 deletion

0 commit comments

Comments
 (0)