Skip to content

Commit 582a8a7

Browse files
iigoninbennygoerzigKarstenSchnitterKai Sternad
committed
add tests
Signed-off-by: Iwan Igonin <iigonin@sternad.de> Co-authored-by: Benny Goerzig <benny.goerzig@sap.com> Co-authored-by: Karsten Schnitter <k.schnitter@sap.com> Co-authored-by: Kai Sternad <k.sternad@sternad.de>
1 parent b625e85 commit 582a8a7

7 files changed

Lines changed: 156 additions & 50 deletions

File tree

‎src/main/java/org/opensearch/security/ssl/util/SSLConfigConstants.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Arrays;
2222
import java.util.Collections;
2323
import java.util.List;
24+
import java.util.Locale;
2425
import java.util.function.Function;
2526

2627
import org.bouncycastle.crypto.CryptoServicesRegistrar;
@@ -47,7 +48,7 @@ public final class SSLConfigConstants {
4748
public static final String ENFORCE_CERT_RELOAD_DN_VERIFICATION = "enforce_cert_reload_dn_verification";
4849
public static final String DEFAULT_STORE_TYPE = CryptoServicesRegistrar.isInApprovedOnlyMode()
4950
? "BCFKS"
50-
: KeyStore.getDefaultType().toUpperCase();
51+
: KeyStore.getDefaultType().toUpperCase(Locale.ROOT);
5152
public static final String SSL_PREFIX = "plugins.security.ssl.";
5253

5354
public static final String KEYSTORE_TYPE = "keystore_type";

‎src/main/java/org/opensearch/security/support/PemKeyReader.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,6 @@ public static String extractStoreType(String storePath, String storeType) {
367367
}
368368

369369
private static String detectStoreType(String path) {
370-
371370
try (InputStream raw = new BufferedInputStream(new FileInputStream(path))) {
372371
raw.mark(32);
373372
byte[] magic = new byte[4];
@@ -400,7 +399,7 @@ private static String detectStoreType(String path) {
400399
}
401400
}
402401

403-
private static KeyStore newEmptyStore() throws Exception {
402+
static KeyStore newEmptyStore() throws Exception {
404403
var ks = KeyStore.getInstance(DEFAULT_STORE_TYPE);
405404
ks.load(null, null);
406405
return ks;

‎src/test/java/org/opensearch/security/SecurityAdminTests.java‎

Lines changed: 29 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.io.PrintStream;
2424
import java.util.ArrayList;
2525
import java.util.List;
26-
import java.util.Objects;
2726

2827
import org.apache.http.HttpStatus;
2928
import org.junit.Assert;
@@ -59,9 +58,9 @@ public void testSecurityAdmin() throws Exception {
5958

6059
List<String> argsAsList = new ArrayList<>();
6160
argsAsList.add("-ts");
62-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
61+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
6362
argsAsList.add("-ks");
64-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
63+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
6564
argsAsList.add("-p");
6665
argsAsList.add(String.valueOf(clusterInfo.httpPort));
6766
argsAsList.add("-cn");
@@ -95,17 +94,11 @@ public void testSecurityAdminHostnameVerificationEnforced() throws Exception {
9594

9695
List<String> argsAsList = new ArrayList<>();
9796
argsAsList.add("-cacert");
98-
argsAsList.add(
99-
Objects.requireNonNull(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "root-ca.pem")).toFile().getAbsolutePath()
100-
);
97+
argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "root-ca.pem").toFile().getAbsolutePath());
10198
argsAsList.add("-cert");
102-
argsAsList.add(
103-
Objects.requireNonNull(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "kirk.crt.pem")).toFile().getAbsolutePath()
104-
);
99+
argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "kirk.crt.pem").toFile().getAbsolutePath());
105100
argsAsList.add("-key");
106-
argsAsList.add(
107-
Objects.requireNonNull(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "kirk.key.pem")).toFile().getAbsolutePath()
108-
);
101+
argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "kirk.key.pem").toFile().getAbsolutePath());
109102
argsAsList.add("-p");
110103
argsAsList.add(String.valueOf(clusterInfo.httpPort));
111104
argsAsList.add("-icl");
@@ -138,17 +131,11 @@ public void testSecurityAdminHostnameVerificationNotEnforced() throws Exception
138131

139132
List<String> argsAsList = new ArrayList<>();
140133
argsAsList.add("-cacert");
141-
argsAsList.add(
142-
Objects.requireNonNull(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "root-ca.pem")).toFile().getAbsolutePath()
143-
);
134+
argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "root-ca.pem").toFile().getAbsolutePath());
144135
argsAsList.add("-cert");
145-
argsAsList.add(
146-
Objects.requireNonNull(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "kirk.crt.pem")).toFile().getAbsolutePath()
147-
);
136+
argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "kirk.crt.pem").toFile().getAbsolutePath());
148137
argsAsList.add("-key");
149-
argsAsList.add(
150-
Objects.requireNonNull(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "kirk.key.pem")).toFile().getAbsolutePath()
151-
);
138+
argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "kirk.key.pem").toFile().getAbsolutePath());
152139
argsAsList.add("-p");
153140
argsAsList.add(String.valueOf(clusterInfo.httpPort));
154141
argsAsList.add("-icl");
@@ -172,9 +159,9 @@ public void testSecurityAdminInvalidCert() throws Exception {
172159

173160
List<String> argsAsList = new ArrayList<>();
174161
argsAsList.add("-ts");
175-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
162+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
176163
argsAsList.add("-ks");
177-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
164+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
178165
argsAsList.add("-p");
179166
argsAsList.add(String.valueOf(clusterInfo.httpPort));
180167
argsAsList.add("-cn");
@@ -191,9 +178,9 @@ public void testSecurityAdminInvalidCert() throws Exception {
191178

192179
argsAsList = new ArrayList<>();
193180
argsAsList.add("-ts");
194-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
181+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
195182
argsAsList.add("-ks");
196-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "spock-keystore")).toFile().getAbsolutePath());
183+
argsAsList.add(FileHelper.resolveStorePath(prefix + "spock-keystore").toFile().getAbsolutePath());
197184
argsAsList.add("-p");
198185
argsAsList.add(String.valueOf(clusterInfo.httpPort));
199186
argsAsList.add("-cn");
@@ -209,9 +196,9 @@ public void testSecurityAdminInvalidCert() throws Exception {
209196

210197
argsAsList = new ArrayList<>();
211198
argsAsList.add("-ts");
212-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
199+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
213200
argsAsList.add("-ks");
214-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "node-0-keystore")).toFile().getAbsolutePath());
201+
argsAsList.add(FileHelper.resolveStorePath(prefix + "node-0-keystore").toFile().getAbsolutePath());
215202
argsAsList.add("-p");
216203
argsAsList.add(String.valueOf(clusterInfo.httpPort));
217204
argsAsList.add("-cn");
@@ -238,9 +225,9 @@ public void testSecurityAdminRegularUpdate() throws Exception {
238225

239226
List<String> argsAsList = new ArrayList<>();
240227
argsAsList.add("-ts");
241-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
228+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
242229
argsAsList.add("-ks");
243-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
230+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
244231
argsAsList.add("-p");
245232
argsAsList.add(String.valueOf(clusterInfo.httpPort));
246233
argsAsList.add("-cn");
@@ -273,9 +260,9 @@ public void testSecurityAdminSingularV7Updates() throws Exception {
273260

274261
List<String> argsAsList = new ArrayList<>();
275262
argsAsList.add("-ts");
276-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
263+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
277264
argsAsList.add("-ks");
278-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
265+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
279266
argsAsList.add("-p");
280267
argsAsList.add(String.valueOf(clusterInfo.httpPort));
281268
argsAsList.add("-cn");
@@ -291,9 +278,9 @@ public void testSecurityAdminSingularV7Updates() throws Exception {
291278

292279
argsAsList = new ArrayList<>();
293280
argsAsList.add("-ts");
294-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
281+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
295282
argsAsList.add("-ks");
296-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
283+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
297284
argsAsList.add("-p");
298285
argsAsList.add(String.valueOf(clusterInfo.httpPort));
299286
argsAsList.add("-cn");
@@ -309,9 +296,9 @@ public void testSecurityAdminSingularV7Updates() throws Exception {
309296

310297
argsAsList = new ArrayList<>();
311298
argsAsList.add("-ts");
312-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
299+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
313300
argsAsList.add("-ks");
314-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
301+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
315302
argsAsList.add("-p");
316303
argsAsList.add(String.valueOf(clusterInfo.httpPort));
317304
argsAsList.add("-cn");
@@ -347,19 +334,15 @@ public void testSecurityAdminInvalidYml() throws Exception {
347334

348335
List<String> argsAsList = new ArrayList<>();
349336
argsAsList.add("-ts");
350-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
337+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
351338
argsAsList.add("-ks");
352-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
339+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
353340
argsAsList.add("-p");
354341
argsAsList.add(String.valueOf(clusterInfo.httpPort));
355342
argsAsList.add("-cn");
356343
argsAsList.add(clusterInfo.clustername);
357344
argsAsList.add("-f");
358-
argsAsList.add(
359-
Objects.requireNonNull(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "roles_invalidxcontent.yml"))
360-
.toFile()
361-
.getAbsolutePath()
362-
);
345+
argsAsList.add(FileHelper.getAbsoluteFilePathFromClassPath(prefix + "roles_invalidxcontent.yml").toFile().getAbsolutePath());
363346
argsAsList.add("-t");
364347
argsAsList.add("roles");
365348
argsAsList.add("-nhnv");
@@ -399,9 +382,9 @@ public void testSecurityAdminReloadInvalidConfig() throws Exception {
399382

400383
List<String> argsAsList = new ArrayList<>();
401384
argsAsList.add("-ts");
402-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
385+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
403386
argsAsList.add("-ks");
404-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
387+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
405388
argsAsList.add("-p");
406389
argsAsList.add(String.valueOf(clusterInfo.httpPort));
407390
argsAsList.add("-cn");
@@ -503,9 +486,9 @@ public void testIsLegacySecurityIndexOnV7Index() throws Exception {
503486

504487
List<String> argsAsList = new ArrayList<>();
505488
argsAsList.add("-ts");
506-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "truststore")).toFile().getAbsolutePath());
489+
argsAsList.add(FileHelper.resolveStorePath(prefix + "truststore").toFile().getAbsolutePath());
507490
argsAsList.add("-ks");
508-
argsAsList.add(Objects.requireNonNull(FileHelper.resolveStorePath(prefix + "kirk-keystore")).toFile().getAbsolutePath());
491+
argsAsList.add(FileHelper.resolveStorePath(prefix + "kirk-keystore").toFile().getAbsolutePath());
509492
argsAsList.add("-p");
510493
argsAsList.add(String.valueOf(clusterInfo.httpPort));
511494
argsAsList.add("-cn");

‎src/test/java/org/opensearch/security/sanity/tests/SecurityRestTestCase.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.opensearch.common.io.PathUtils;
2727
import org.opensearch.common.settings.Settings;
2828
import org.opensearch.commons.rest.SecureRestClientBuilder;
29+
import org.opensearch.security.test.helper.file.FileHelper;
2930
import org.opensearch.test.rest.OpenSearchRestTestCase;
3031

3132
import static org.opensearch.security.ssl.SecureSSLSettings.SSLSetting.SECURITY_SSL_HTTP_KEYSTORE_KEYPASSWORD;
@@ -65,7 +66,7 @@ protected Settings restAdminSettings() {
6566
.put(SECURITY_SSL_HTTP_ENABLED, isHttps())
6667
// this is incorrect on common-utils side. It should be using `pemtrustedcas_filepath`
6768
.put(SECURITY_SSL_HTTP_PEMCERT_FILEPATH, CERT_FILE_DIRECTORY + "root-ca.pem")
68-
.put(SECURITY_SSL_HTTP_KEYSTORE_FILEPATH, CERT_FILE_DIRECTORY + "kirk-keystore.jks")
69+
.put(SECURITY_SSL_HTTP_KEYSTORE_FILEPATH, FileHelper.resolveStorePath(CERT_FILE_DIRECTORY + "kirk-keystore"))
6970
.put(SECURITY_SSL_HTTP_KEYSTORE_PASSWORD.insecurePropertyName, "changeit")
7071
.put(SECURITY_SSL_HTTP_KEYSTORE_KEYPASSWORD.insecurePropertyName, "changeit")
7172
.build();

‎src/test/java/org/opensearch/security/support/PemKeyReaderDetectStoreTypeTest.java‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*
8+
* Modifications Copyright OpenSearch Contributors. See
9+
* GitHub history for details.
10+
*/
11+
12+
package org.opensearch.security.support;
13+
14+
import java.io.File;
15+
import java.io.FileOutputStream;
16+
import java.io.UncheckedIOException;
17+
import java.security.KeyStore;
18+
import java.security.Security;
19+
20+
import org.junit.Rule;
21+
import org.junit.Test;
22+
import org.junit.rules.TemporaryFolder;
23+
import org.bouncycastle.crypto.CryptoServicesRegistrar;
24+
import org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider;
25+
26+
import static org.hamcrest.MatcherAssert.assertThat;
27+
import static org.hamcrest.Matchers.equalTo;
28+
import static org.opensearch.security.ssl.util.SSLConfigConstants.DEFAULT_STORE_TYPE;
29+
import static org.junit.Assert.assertNotNull;
30+
import static org.junit.Assert.assertThrows;
31+
import static org.junit.Assume.assumeFalse;
32+
33+
public class PemKeyReaderDetectStoreTypeTest {
34+
35+
static {
36+
if (Security.getProvider("BCFIPS") == null) {
37+
Security.addProvider(new BouncyCastleFipsProvider());
38+
}
39+
}
40+
41+
@Rule
42+
public TemporaryFolder tempDir = new TemporaryFolder();
43+
44+
@Test
45+
public void detectsJks() throws Exception {
46+
assumeFalse("JKS truststores are not supported in FIPS mode", CryptoServicesRegistrar.isInApprovedOnlyMode());
47+
File file = storeFile("JKS");
48+
assertThat(PemKeyReader.extractStoreType(file.getAbsolutePath(), null), equalTo(PemKeyReader.JKS));
49+
}
50+
51+
@Test
52+
public void detectsPkcs12() throws Exception {
53+
assumeFalse("PKCS12 truststores are not supported in FIPS mode", CryptoServicesRegistrar.isInApprovedOnlyMode());
54+
File file = storeFile("PKCS12");
55+
assertThat(PemKeyReader.extractStoreType(file.getAbsolutePath(), null), equalTo(PemKeyReader.PKCS12));
56+
}
57+
58+
@Test
59+
public void detectsBcfks() throws Exception {
60+
File file = storeFile("BCFKS");
61+
assertThat(PemKeyReader.extractStoreType(file.getAbsolutePath(), null), equalTo(PemKeyReader.BCFKS));
62+
}
63+
64+
@Test
65+
public void explicitTypeSkipsDetection() throws Exception {
66+
// file content is irrelevant when type is explicitly provided
67+
assumeFalse("PKCS12 truststores are not supported in FIPS mode", CryptoServicesRegistrar.isInApprovedOnlyMode());
68+
File file = tempDir.newFile("irrelevant.bin");
69+
try (FileOutputStream fos = new FileOutputStream(file)) {
70+
fos.write(new byte[] { 0x00, 0x01, 0x02, 0x03 });
71+
}
72+
assertThat(PemKeyReader.extractStoreType(file.getAbsolutePath(), PemKeyReader.PKCS12), equalTo(PemKeyReader.PKCS12));
73+
}
74+
75+
@Test
76+
public void throwsForFileTooShort() throws Exception {
77+
File file = tempDir.newFile("short.bin");
78+
try (FileOutputStream fos = new FileOutputStream(file)) {
79+
fos.write(new byte[] { 0x30, 0x00 });
80+
}
81+
assertThrows(IllegalArgumentException.class, () -> PemKeyReader.extractStoreType(file.getAbsolutePath(), null));
82+
}
83+
84+
@Test
85+
public void throwsForUnknownFormat() throws Exception {
86+
File file = tempDir.newFile("unknown.bin");
87+
try (FileOutputStream fos = new FileOutputStream(file)) {
88+
fos.write(new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 });
89+
}
90+
assertThrows(IllegalArgumentException.class, () -> PemKeyReader.extractStoreType(file.getAbsolutePath(), null));
91+
}
92+
93+
@Test
94+
public void throwsForNonExistentFile() {
95+
assertThrows(UncheckedIOException.class, () -> PemKeyReader.extractStoreType("/nonexistent/path/store.jks", null));
96+
}
97+
98+
@Test
99+
public void throwsForEmptyFile() throws Exception {
100+
File file = tempDir.newFile("empty.bin");
101+
assertThrows(IllegalArgumentException.class, () -> PemKeyReader.extractStoreType(file.getAbsolutePath(), null));
102+
}
103+
104+
@Test
105+
public void newEmptyStoreReturnsUsableStore() throws Exception {
106+
System.out.println("DEFAULT_STORE_TYPE:\t" + DEFAULT_STORE_TYPE);
107+
KeyStore ks = PemKeyReader.newEmptyStore();
108+
assertNotNull(ks);
109+
assertThat(ks.getType(), equalTo(DEFAULT_STORE_TYPE));
110+
assertThat(ks.size(), equalTo(0));
111+
}
112+
113+
private File storeFile(String type) throws Exception {
114+
File file = tempDir.newFile("store." + type.toLowerCase());
115+
KeyStore ks = KeyStore.getInstance(type);
116+
ks.load(null, null);
117+
try (FileOutputStream fos = new FileOutputStream(file)) {
118+
ks.store(fos, new char[0]);
119+
}
120+
return file;
121+
}
122+
}

‎src/test/resources/ssl/crl/revoked.bcfks‎

-3.94 KB
Binary file not shown.

‎src/test/resources/ssl/pem/node-4.bcfks‎

-3.94 KB
Binary file not shown.

0 commit comments

Comments
 (0)