Skip to content

Commit

Permalink
write some additional tests.
Browse files Browse the repository at this point in the history
Signed-off-by: Iwan Igonin <iigonin@sternad.de>
  • Loading branch information
iigonin committed Oct 24, 2024
1 parent a63fe98 commit d5b496e
Show file tree
Hide file tree
Showing 17 changed files with 256 additions and 17 deletions.
11 changes: 10 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,8 @@ gradle.projectsEvaluated {
}
}

// test retry configuration
subprojects {
// test retry configuration
tasks.withType(Test).configureEach {
develocity.testRetry {
if (BuildParams.isCi()) {
Expand Down Expand Up @@ -552,6 +552,15 @@ subprojects {
}
}
}

// test with FIPS-140-2 enabled
plugins.withType(JavaPlugin).configureEach {
tasks.withType(Test).configureEach { testTask ->
if (System.getenv('OPENSEARCH_CRYPTO_STANDARD') == 'FIPS-140-2') {
testTask.jvmArgs += "-Dorg.bouncycastle.fips.approved_only=true"
}
}
}
}

// eclipse configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,10 @@ public void execute(Task t) {
test.systemProperty("tests.seed", BuildParams.getTestSeed());
}

var securityFile = BuildParams.isInFipsJvm() ? "fips_java.security" : "java.security";
test.systemProperty(
"java.security.properties",
project.getRootProject().getLayout().getProjectDirectory() + "/distribution/src/config/fips_java.security"
project.getRootProject().getLayout().getProjectDirectory() + "/distribution/src/config/" + securityFile
);

// don't track these as inputs since they contain absolute paths and break cache relocatability
Expand Down
12 changes: 11 additions & 1 deletion distribution/src/config/fips_java.security
Original file line number Diff line number Diff line change
@@ -1,27 +1,34 @@
# Security Properties for JDK 11 and higher, with BouncyCastle FIPS provider and BouncyCastleJsseProvider in FIPS mode
# Security Properties for JDK 11 and higher, with BouncyCastle FIPS provider and BouncyCastleJsseProvider in approved-only mode

security.provider.1=org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider C:HYBRID;ENABLE{All};
security.provider.2=org.bouncycastle.jsse.provider.BouncyCastleJsseProvider fips:BCFIPS
security.provider.3=SUN
security.provider.4=SunJGSS

securerandom.source=file:/dev/urandom
securerandom.strongAlgorithms=NativePRNGBlocking:SUN,DRBG:SUN
securerandom.drbg.config=

login.configuration.provider=sun.security.provider.ConfigFile
policy.provider=sun.security.provider.PolicyFile
policy.expandProperties=true
policy.allowSystemProperty=true
policy.ignoreIdentityScope=false
keystore.type.compat=true

package.access=sun.misc.,\
sun.reflect.
package.definition=sun.misc.,\
sun.reflect.

security.overridePropertiesFile=true

ssl.KeyManagerFactory.algorithm=PKIX
ssl.TrustManagerFactory.algorithm=PKIX

networkaddress.cache.negative.ttl=10
krb5.kdc.bad.policy = tryLast

jdk.certpath.disabledAlgorithms=MD2, MD5, SHA1, jdkCA&usageTLSServer, RSA keySize < 2048, DSA keySize < 2048, EC keySize < 224
jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 2048, DSA keySize < 2048
jdk.tls.disabledAlgorithms=SSLv3, TLSv1, TLSv1.1, RC4, MD5withRSA, DH keySize < 2048, EC keySize < 224, DES40_CBC, RC4_40, 3DES_EDE_CBC
Expand All @@ -31,7 +38,9 @@ jdk.tls.legacyAlgorithms= \
RC4_128, RC4_40, DES_CBC, DES40_CBC, \
3DES_EDE_CBC
jdk.tls.keyLimits=AES/GCM/NoPadding KeyUpdate 2^37

crypto.policy=unlimited

jdk.xml.dsig.secureValidationPolicy=\
disallowAlg http://www.w3.org/TR/1999/REC-xslt-19991116,\
disallowAlg http://www.w3.org/2001/04/xmldsig-more#rsa-md5,\
Expand All @@ -45,5 +54,6 @@ jdk.xml.dsig.secureValidationPolicy=\
minKeySize EC 224,\
noDuplicateIds,\
noRetrievalMethodLoops

jceks.key.serialFilter = java.base/java.lang.Enum;java.base/java.security.KeyRep;\
java.base/java.security.KeyRep$Type;java.base/javax.crypto.spec.SecretKeySpec;!*
58 changes: 58 additions & 0 deletions distribution/src/config/java.security
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Security Properties for JDK 11 and higher, with BouncyCastle FIPS provider and BouncyCastleJSSEProvider in non-approved mode

security.provider.1=org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider
security.provider.2=org.bouncycastle.jsse.provider.BouncyCastleJsseProvider
security.provider.3=SUN
security.provider.4=SunJGSS

securerandom.source=file:/dev/urandom
securerandom.strongAlgorithms=NativePRNGBlocking:SUN,DRBG:SUN
securerandom.drbg.config=

login.configuration.provider=sun.security.provider.ConfigFile
policy.provider=sun.security.provider.PolicyFile
policy.expandProperties=true
policy.allowSystemProperty=true
policy.ignoreIdentityScope=false
keystore.type.compat=true

package.access=sun.misc.,\
sun.reflect.
package.definition=sun.misc.,\
sun.reflect.

security.overridePropertiesFile=true

ssl.KeyManagerFactory.algorithm=PKIX
ssl.TrustManagerFactory.algorithm=PKIX

networkaddress.cache.negative.ttl=10
krb5.kdc.bad.policy = tryLast

jdk.certpath.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < 1024
jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < 1024
jdk.tls.disabledAlgorithms=SSLv3, RC4, MD5withRSA, DH keySize < 1024, DES40_CBC, RC4_40
jdk.tls.legacyAlgorithms= \
K_NULL, C_NULL, M_NULL, \
DH_anon, ECDH_anon, \
RC4_128, RC4_40, DES_CBC, DES40_CBC, \
3DES_EDE_CBC
jdk.tls.keyLimits=AES/GCM/NoPadding KeyUpdate 2^37

crypto.policy=unlimited

jdk.xml.dsig.secureValidationPolicy=\
disallowAlg http://www.w3.org/2001/04/xmldsig-more#rsa-md5,\
disallowAlg http://www.w3.org/2001/04/xmldsig-more#hmac-md5,\
disallowAlg http://www.w3.org/2001/04/xmldsig-more#md5,\
maxTransforms 5,\
maxReferences 30,\
disallowReferenceUriSchemes file http https,\
minKeySize RSA 1024,\
minKeySize DSA 1024,\
minKeySize EC 224,\
noDuplicateIds,\
noRetrievalMethodLoops

jceks.key.serialFilter = java.base/java.lang.Enum;java.base/java.security.KeyRep;\
java.base/java.security.KeyRep$Type;java.base/javax.crypto.spec.SecretKeySpec;!*
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@

final class SystemJvmOptions {

protected static final String OPENSEARCH_CRYPTO_STANDARD = "OPENSEARCH_CRYPTO_STANDARD";
protected static final String FIPS_140_2 = "FIPS-140-2";

static List<String> systemJvmOptions(final Path config) {
return Collections.unmodifiableList(
Arrays.asList(
Expand Down Expand Up @@ -88,16 +91,22 @@ static List<String> systemJvmOptions(final Path config) {
}

private static String enableFips() {
var cryptoStandard = System.getenv("OPENSEARCH_CRYPTO_STANDARD");
if (cryptoStandard != null && cryptoStandard.equals("FIPS-140-2")) {
var cryptoStandard = System.getenv(OPENSEARCH_CRYPTO_STANDARD);
if (cryptoStandard != null && cryptoStandard.equals(FIPS_140_2)) {
return "-Dorg.bouncycastle.fips.approved_only=true";
}
return "";
}

private static String loadJavaSecurityProperties(final Path config) {
var securityFile = config.resolve("fips_java.security");
return "-Djava.security.properties=" + securityFile.toAbsolutePath();
String securityFile;
var cryptoStandard = System.getenv(OPENSEARCH_CRYPTO_STANDARD);
if (cryptoStandard != null && cryptoStandard.equals(FIPS_140_2)) {
securityFile = "fips_java.security";
} else {
securityFile = "java.security";
}
return "-Djava.security.properties=" + config.resolve(securityFile).toAbsolutePath();
}

private static String allowSecurityManagerOption() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,13 @@ public enum KeyStoreType {
JKS("JKS"),
PKCS_12("PKCS12"),
PKCS_11("PKCS11"),
BKS("BKS"),
BCFKS("BCFKS");

private static final Map<KeyStoreType, List<String>> TYPE_TO_EXTENSION_MAP = new HashMap<>();
static final Map<KeyStoreType, List<String>> TYPE_TO_EXTENSION_MAP = new HashMap<>();

static {
TYPE_TO_EXTENSION_MAP.put(JKS, List.of(".jks", ".ks"));
TYPE_TO_EXTENSION_MAP.put(PKCS_12, List.of(".p12", ".pkcs12", ".pfx"));
TYPE_TO_EXTENSION_MAP.put(BKS, List.of(".bks")); // Bouncy Castle Keystore
TYPE_TO_EXTENSION_MAP.put(BCFKS, List.of(".bcfks")); // Bouncy Castle FIPS Keystore
}

Expand All @@ -54,7 +52,7 @@ public String getJcaName() {
public static KeyStoreType inferStoreType(String filePath) {
return TYPE_TO_EXTENSION_MAP.entrySet()
.stream()
.filter(entry -> entry.getValue().stream().anyMatch(filePath::endsWith))
.filter(entry -> entry.getValue().stream().anyMatch(it -> filePath.endsWith("." + it)))
.map(Map.Entry::getKey)
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("Unknown keystore type for file path: " + filePath));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.common.crypto;

import com.carrotsearch.randomizedtesting.generators.RandomStrings;

import org.opensearch.test.OpenSearchTestCase;

import static org.hamcrest.Matchers.equalTo;

public class KeyStoreFactoryTests extends OpenSearchTestCase {

public void testPKCS12KeyStore() {
assumeFalse("Can't run in a FIPS JVM as PBE is not available", inFipsJvm());
assertThat(KeyStoreFactory.getInstance(KeyStoreType.PKCS_12).getType(), equalTo("PKCS12"));
assertThat(KeyStoreFactory.getInstance(KeyStoreType.PKCS_12).getProvider().getName(), equalTo("BCFIPS"));

assertThat(KeyStoreFactory.getInstance(KeyStoreType.PKCS_12, "BCFIPS").getType(), equalTo("PKCS12"));
assertThat(KeyStoreFactory.getInstance(KeyStoreType.PKCS_12, "BCFIPS").getProvider().getName(), equalTo("BCFIPS"));

assertThat(KeyStoreFactory.getInstance(KeyStoreType.PKCS_12, "SUN").getType(), equalTo("PKCS12"));
assertThat(KeyStoreFactory.getInstance(KeyStoreType.PKCS_12, "SUN").getProvider().getName(), equalTo("SUN"));

KeyStoreType.TYPE_TO_EXTENSION_MAP.get(KeyStoreType.PKCS_12).forEach(extension -> {
var keyStore = KeyStoreFactory.getInstanceBasedOnFileExtension(createRandomFileName(extension));
assertThat(keyStore.getType(), equalTo(KeyStoreType.PKCS_12.getJcaName()));
});
}

public void testJKSKeyStore() {
assertThat(KeyStoreFactory.getInstance(KeyStoreType.JKS).getType(), equalTo("JKS"));
assertThat(KeyStoreFactory.getInstance(KeyStoreType.JKS).getProvider().getName(), equalTo("SUN"));

assertThat(KeyStoreFactory.getInstance(KeyStoreType.JKS, "SUN").getType(), equalTo("JKS"));
assertThat(KeyStoreFactory.getInstance(KeyStoreType.JKS, "SUN").getProvider().getName(), equalTo("SUN"));

assertThrows("BCFKS not found", SecurityException.class, () -> KeyStoreFactory.getInstance(KeyStoreType.JKS, "BCFIPS"));

KeyStoreType.TYPE_TO_EXTENSION_MAP.get(KeyStoreType.JKS).forEach(extension -> {
var keyStore = KeyStoreFactory.getInstanceBasedOnFileExtension(createRandomFileName(extension));
assertThat(keyStore.getType(), equalTo(KeyStoreType.JKS.getJcaName()));
});
}

public void testBCFIPSKeyStore() {
assertThat(KeyStoreFactory.getInstance(KeyStoreType.BCFKS).getType(), equalTo("BCFKS"));
assertThat(KeyStoreFactory.getInstance(KeyStoreType.BCFKS).getProvider().getName(), equalTo("BCFIPS"));

assertThat(KeyStoreFactory.getInstance(KeyStoreType.BCFKS, "BCFIPS").getType(), equalTo("BCFKS"));
assertThat(KeyStoreFactory.getInstance(KeyStoreType.BCFKS, "BCFIPS").getProvider().getName(), equalTo("BCFIPS"));

assertThrows("BCFKS not found", SecurityException.class, () -> KeyStoreFactory.getInstance(KeyStoreType.BCFKS, "SUN"));

KeyStoreType.TYPE_TO_EXTENSION_MAP.get(KeyStoreType.BCFKS).forEach(extension -> {
var keyStore = KeyStoreFactory.getInstanceBasedOnFileExtension(createRandomFileName(extension));
assertThat(keyStore.getType(), equalTo(KeyStoreType.BCFKS.getJcaName()));
});
}

public void testUnknownKeyStoreType() {
assertThrows(
"Unknown keystore type for file path: keystore.unknown",
IllegalArgumentException.class,
() -> KeyStoreFactory.getInstanceBasedOnFileExtension(createRandomFileName("unknown"))
);
}

private String createRandomFileName(String extension) {
return RandomStrings.randomAsciiAlphanumOfLengthBetween(random(), 0, 10) + "." + extension;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ final class PemUtils {

private static final String BCFIPS = "BCFIPS";

private PemUtils() {
PemUtils() {
throw new IllegalStateException("Utility class should not be instantiated");
}

Expand All @@ -87,7 +87,7 @@ static List<Certificate> readCertificates(Collection<Path> certPaths) throws Cer
try (InputStream input = Files.newInputStream(path)) {
final Collection<? extends Certificate> parsed = certFactory.generateCertificates(input);
if (parsed.isEmpty()) {
throw new SslConfigException("failed to parse any certificates from [" + path.toAbsolutePath() + "]");
throw new SslConfigException("Failed to parse any certificate from [" + path.toAbsolutePath() + "]");
}
certificates.addAll(parsed);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,6 @@ public SSLContext createSslContext() {
* {@link #getSupportedProtocols() configured protocols}.
*/
private String contextProtocol() {
if (supportedProtocols.isEmpty()) {
throw new SslConfigException("no SSL/TLS protocols have been configured");
}
if (CryptoServicesRegistrar.isInApprovedOnlyMode()) {
if (!new HashSet<>(SslConfigurationLoader.FIPS_APPROVED_PROTOCOLS).containsAll(supportedProtocols)) {
throw new SslConfigException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ public class PemUtilsTests extends OpenSearchTestCase {
private static final Supplier<char[]> TESTNODE_PASSWORD = "testnode"::toCharArray;
private static final Supplier<char[]> STRONG_PRIVATE_SECRET = "6!6428DQXwPpi7@$ggeg/="::toCharArray; // has to be at least 112 bit long.

public void testInstantiateWithDefaultConstructor() {
assertThrows("Utility class should not be instantiated", IllegalStateException.class, PemUtils::new);
}

public void testReadPKCS8RsaKey() throws Exception {
assumeFalse("Can't use JKS/PKCS12 keystores in a FIPS JVM", inFipsJvm());
Key key = getKeyFromKeystore("RSA", KeyStoreType.JKS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ public void testBasicConfigurationOptions() {
if (verificationMode == SslVerificationMode.NONE) {
final SslTrustConfig trustConfig = configuration.getTrustConfig();
assertThat(trustConfig, instanceOf(TrustEverythingConfig.class));

if (inFipsJvm()) {
assertThrows(
"The use of TrustEverythingConfig is not permitted in FIPS mode. This issue may be caused by the 'ssl.verification_mode=NONE' setting.",
IllegalStateException.class,
trustConfig::createTrustManager
);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.mockito.Mockito;

import static org.opensearch.common.ssl.SslConfigurationLoader.DEFAULT_CIPHERS;
import static org.opensearch.common.ssl.SslConfigurationLoader.FIPS_APPROVED_PROTOCOLS;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -204,4 +205,24 @@ public void testBuildSslContext() {
Mockito.verifyNoMoreInteractions(trustConfig, keyConfig);
}

public void testNotSupportedProtocolsInFipsJvm() {
assumeTrue("Test in FIPS JVM", inFipsJvm());
final SslTrustConfig trustConfig = Mockito.mock(SslTrustConfig.class);
final SslKeyConfig keyConfig = Mockito.mock(SslKeyConfig.class);
final String protocol = randomFrom(List.of("TLSv1.1", "TLSv1", "SSLv3", "SSLv2Hello", "SSLv2"));
final SslConfiguration configuration = new SslConfiguration(
trustConfig,
keyConfig,
randomFrom(SslVerificationMode.values()),
randomFrom(SslClientAuthenticationMode.values()),
DEFAULT_CIPHERS,
Collections.singletonList(protocol)
);

Mockito.when(trustConfig.createTrustManager()).thenReturn(null);
Mockito.when(keyConfig.createKeyManager()).thenReturn(null);
var message = String.format("in FIPS mode only the following SSL/TLS protocols are allowed: [%s]", FIPS_APPROVED_PROTOCOLS);
assertThrows(message, SslConfigException.class, configuration::createSslContext);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ public class SslDiagnosticsTests extends OpenSearchTestCase {
private static final byte[] MOCK_ENCODING_4 = { 0x64, 0x65, 0x66, 0x67, 0x68, 0x69 };
private static final String MOCK_FINGERPRINT_4 = "5d96965bfae50bf2be0d6259eb87a6cc9f5d0b26";

public void testTrustEmptyStore() {
var fileName = "cert-all/empty.jks";
var message = String.format("Failed to parse any certificate from [%s]", getDataPath("/certs/" + fileName).toAbsolutePath());
assertThrows(message, SslConfigException.class, () -> loadCertificate(fileName));
}

public void testDiagnosticMessageWhenServerProvidesAFullCertChainThatIsTrusted() throws Exception {
X509Certificate[] chain = loadCertChain("cert1/cert1.crt", "ca1/ca.crt");
final SSLSession session = session("192.168.1.1");
Expand Down
Loading

0 comments on commit d5b496e

Please sign in to comment.