Skip to content

Commit

Permalink
Fix and unmute FIPS tests (elastic#119618)
Browse files Browse the repository at this point in the history
Fixes and un-mutes tests associated with FIPS.
Most of the fixes are due to differing expected exceptions or log messages when using BouncyCastle as the JCE/JSSE provider.
Only test code is changed with this commit.

fixes: elastic#49094
  • Loading branch information
jakelandis committed Jan 8, 2025
1 parent 9d8504f commit 94658d9
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,22 @@ public static void stopHttpServers() throws IOException {
}

public void testBuilderUsesDefaultSSLContext() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm());
final SSLContext defaultSSLContext = SSLContext.getDefault();
try {
try (RestClient client = buildRestClient()) {
try {
client.performRequest(new Request("GET", "/"));
fail("connection should have been rejected due to SSL handshake");
} catch (Exception e) {
assertThat(e, instanceOf(SSLHandshakeException.class));
if (inFipsJvm()) {
// Bouncy Castle throw a different exception
assertThat(e, instanceOf(IOException.class));
assertThat(e.getCause(), instanceOf(javax.net.ssl.SSLException.class));
} else {
assertThat(e, instanceOf(SSLHandshakeException.class));
}
}
}

SSLContext.setDefault(getSslContext());
try (RestClient client = buildRestClient()) {
Response response = client.performRequest(new Request("GET", "/"));
Expand All @@ -112,7 +116,6 @@ public void testBuilderUsesDefaultSSLContext() throws Exception {
}

public void testBuilderSetsThreadName() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm());
final SSLContext defaultSSLContext = SSLContext.getDefault();
try {
SSLContext.setDefault(getSslContext());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,20 @@ private static SSLContext buildServerSslContext() throws Exception {
}

public void testClientFailsWithUntrustedCertificate() throws IOException {
assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm());
final List<Thread> threads = new ArrayList<>();
final Settings.Builder builder = Settings.builder().put("path.home", createTempDir());
final Settings settings = builder.build();
final Environment environment = TestEnvironment.newEnvironment(settings);
final ReindexSslConfig ssl = new ReindexSslConfig(settings, environment, mock(ResourceWatcherService.class));
try (RestClient client = Reindexer.buildRestClient(getRemoteInfo(), ssl, 1L, threads)) {
expectThrows(SSLHandshakeException.class, () -> client.performRequest(new Request("GET", "/")));
if (inFipsJvm()) {
// Bouncy Castle throws a different exception
IOException exception = expectThrows(IOException.class, () -> client.performRequest(new Request("GET", "/")));
assertThat(exception.getCause(), Matchers.instanceOf(javax.net.ssl.SSLException.class));
} else {
expectThrows(SSLHandshakeException.class, () -> client.performRequest(new Request("GET", "/")));

}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ public void testReloadingKeyStore() throws Exception {
* Tests the reloading of SSLContext when a PEM key and certificate are used.
*/
public void testPEMKeyConfigReloading() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm());
Path tempDir = createTempDir();
Path keyPath = tempDir.resolve("testnode.pem");
Path certPath = tempDir.resolve("testnode.crt");
Expand Down Expand Up @@ -221,11 +220,19 @@ public void testPEMKeyConfigReloading() throws Exception {
try (MockWebServer server = new MockWebServer(updatedContext, false)) {
server.enqueue(new MockResponse().setResponseCode(200).setBody("body"));
server.start();
SSLHandshakeException sslException = expectThrows(
SSLHandshakeException.class,
() -> privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())
);
assertThat(sslException.getCause().getMessage(), containsString("PKIX path validation failed"));
if (inFipsJvm()) {
Exception sslException = expectThrows(
IOException.class,
() -> privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())
);
assertThat(sslException.getCause().getMessage(), containsString("Unable to construct a valid chain"));
} else {
SSLHandshakeException sslException = expectThrows(
SSLHandshakeException.class,
() -> privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())
);
assertThat(sslException.getCause().getMessage(), containsString("PKIX path validation failed"));
}
} catch (Exception e) {
throw new RuntimeException("Exception starting or connecting to the mock server", e);
}
Expand Down Expand Up @@ -290,7 +297,6 @@ public void testReloadingTrustStore() throws Exception {
* Test the reloading of SSLContext whose trust config is backed by PEM certificate files.
*/
public void testReloadingPEMTrustConfig() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm());
Path tempDir = createTempDir();
Path serverCertPath = tempDir.resolve("testnode.crt");
Path serverKeyPath = tempDir.resolve("testnode.pem");
Expand Down Expand Up @@ -324,11 +330,19 @@ public void testReloadingPEMTrustConfig() throws Exception {
// Client doesn't trust the Server certificate anymore so SSLHandshake should fail
final Consumer<SSLContext> trustMaterialPostChecks = (updatedContext) -> {
try (CloseableHttpClient client = createHttpClient(updatedContext)) {
SSLHandshakeException sslException = expectThrows(
SSLHandshakeException.class,
() -> privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())
);
assertThat(sslException.getCause().getMessage(), containsString("PKIX path validation failed"));
if (inFipsJvm()) {
Exception sslException = expectThrows(
IOException.class,
() -> privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())
);
assertThat(sslException.getCause().getMessage(), containsString("Unable to construct a valid chain"));
} else {
SSLHandshakeException sslException = expectThrows(
SSLHandshakeException.class,
() -> privilegedConnect(() -> client.execute(new HttpGet("https://localhost:" + server.getPort())).close())
);
assertThat(sslException.getCause().getMessage(), containsString("PKIX path validation failed"));
}
} catch (Exception e) {
throw new RuntimeException("Error closing CloseableHttpClient", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ protected boolean transportSSLEnabled() {
}

public void testThatSSLConfigurationReloadsOnModification() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm());
Path keyPath = createTempDir().resolve("testnode_updated.pem");
Path certPath = createTempDir().resolve("testnode_updated.crt");
Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode_updated.pem"), keyPath);
Expand Down Expand Up @@ -127,6 +126,9 @@ public void testThatSSLConfigurationReloadsOnModification() throws Exception {
fail("handshake should not have been successful!");
} catch (SSLException | SocketException expected) {
logger.trace("expected exception", expected);
} catch (IOException e) {
// BouncyCastle throws a different exception
assertThat(e.getClass().getName(), is("org.bouncycastle.tls.TlsFatalAlertReceived"));
}
// Copy testnode_updated.crt to the placeholder updateable.crt so that the nodes will start trusting it now
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import java.util.regex.Pattern;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.SSLSocketFactory;
Expand Down Expand Up @@ -104,7 +103,6 @@ public void testMessageForRestClientHostnameVerificationFailure() throws IOExcep
}

public void testDiagnosticTrustManagerForHostnameVerificationFailure() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm());
final Settings settings = getPemSSLSettings(
HTTP_SERVER_SSL,
"not-this-host.crt",
Expand Down Expand Up @@ -133,7 +131,7 @@ public void testDiagnosticTrustManagerForHostnameVerificationFailure() throws Ex
DiagnosticTrustManager.class.getName(),
Level.WARN,
"failed to establish trust with server at \\["
+ Pattern.quote(webServer.getHostName())
+ (inFipsJvm() ? "<unknown host>" : Pattern.quote(webServer.getHostName()))
+ "\\];"
+ " the server provided a certificate with subject name \\[CN=not-this-host\\],"
+ " fingerprint \\[[0-9a-f]{40}\\], no keyUsage and no extendedKeyUsage;"
Expand All @@ -154,13 +152,12 @@ public void testDiagnosticTrustManagerForHostnameVerificationFailure() throws Ex
enableHttpsHostnameChecking(clientSocket);
connect(clientSocket, webServer);
assertThat(clientSocket.isConnected(), is(true));
final SSLHandshakeException handshakeException = expectThrows(
SSLHandshakeException.class,
() -> clientSocket.getInputStream().read()
);
assertThat(handshakeException, throwableWithMessage(containsStringIgnoringCase("subject alternative names")));
assertThat(handshakeException, throwableWithMessage(containsString(webServer.getHostName())));

final Exception handshakeException = expectThrows(Exception.class, () -> clientSocket.getInputStream().read());
// Bouncy Castle throws a different exception message
if (inFipsJvm() == false) {
assertThat(handshakeException, throwableWithMessage(containsStringIgnoringCase("subject alternative names")));
assertThat(handshakeException, throwableWithMessage(containsString(webServer.getHostName())));
}
// Logging message failures are tricky to debug because you just get a "didn't find match" assertion failure.
// You should be able to check the log output for the text that was logged and compare to the regex above.
mockLog.assertAllExpectationsMatched();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import javax.mail.MessagingException;
import javax.mail.internet.MimeMessage;
Expand Down Expand Up @@ -74,7 +75,6 @@ public void stopSmtpServer() {
}

public void testFailureSendingMessageToSmtpServerWithUntrustedCertificateAuthority() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/49094", inFipsJvm());
final Settings.Builder settings = Settings.builder();
final MockSecureSettings secureSettings = new MockSecureSettings();
final ExecutableEmailAction emailAction = buildEmailAction(settings, secureSettings);
Expand All @@ -84,7 +84,14 @@ public void testFailureSendingMessageToSmtpServerWithUntrustedCertificateAuthori
() -> emailAction.execute("my_action_id", ctx, Payload.EMPTY)
);
final List<Throwable> allCauses = getAllCauses(exception);
assertThat(allCauses, Matchers.hasItem(Matchers.instanceOf(SSLException.class)));
if (inFipsJvm()) {
assertThat(
allCauses.stream().map(c -> c.getClass().getCanonicalName()).collect(Collectors.toSet()),
Matchers.hasItem("org.bouncycastle.tls.TlsFatalAlert")
);
} else {
assertThat(allCauses, Matchers.hasItem(Matchers.instanceOf(SSLException.class)));
}
}

public void testCanSendMessageToSmtpServerUsingTrustStore() throws Exception {
Expand Down Expand Up @@ -149,7 +156,6 @@ public void testCanSendMessageToSmtpServerUsingSmtpSslTrust() throws Exception {
* over the account level "smtp.ssl.trust" setting) but smtp.ssl.trust was ignored for a period of time (see #52153)
* so this is the least breaking way to resolve that.
*/
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/49094")
public void testNotificationSslSettingsOverrideSmtpSslTrust() throws Exception {
List<MimeMessage> messages = new ArrayList<>();
server.addListener(messages::add);
Expand All @@ -165,9 +171,16 @@ public void testNotificationSslSettingsOverrideSmtpSslTrust() throws Exception {
MessagingException.class,
() -> emailAction.execute("my_action_id", ctx, Payload.EMPTY)
);

final List<Throwable> allCauses = getAllCauses(exception);
assertThat(allCauses, Matchers.hasItem(Matchers.instanceOf(SSLException.class)));
if (inFipsJvm()) {
assertThat(
allCauses.stream().map(c -> c.getClass().getCanonicalName()).collect(Collectors.toSet()),
Matchers.hasItem("org.bouncycastle.tls.TlsFatalAlert")
);
} else {
assertThat(allCauses, Matchers.hasItem(Matchers.instanceOf(SSLException.class)));
}

} finally {
server.clearListeners();
}
Expand Down
8 changes: 8 additions & 0 deletions x-pack/qa/smoke-test-plugins-ssl/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ project(':plugins').getChildProjects().each { pluginName, pluginProject ->
testClusters.matching { it.name == "yamlRestTest" }.configureEach {
testDistribution = 'DEFAULT'
setting 'xpack.monitoring.collection.interval', '1s'
setting 'xpack.monitoring.collection.enabled', 'false' //will enable in test
setting 'xpack.monitoring.exporters._http.enabled', 'false'//will enable in test
setting 'xpack.monitoring.exporters._http.type', "http"
setting 'xpack.monitoring.exporters._http.host', "https://example.com" //will be replaced in test
setting 'xpack.monitoring.exporters._http.auth.username', 'monitoring_agent'
setting 'xpack.monitoring.exporters._http.ssl.verification_mode', 'full'
setting 'xpack.monitoring.exporters._http.ssl.certificate_authorities', 'testnode.crt'
keystore 'xpack.monitoring.exporters._http.auth.secure_password', 'x-pack-test-password'

setting 'xpack.license.self_generated.type', 'trial'
setting 'xpack.security.enabled', 'true'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.elasticsearch.client.Response;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
Expand Down Expand Up @@ -53,26 +52,25 @@ public class SmokeTestMonitoringWithSecurityIT extends ESRestTestCase {

private static final String USER = "test_user";
private static final SecureString PASS = new SecureString("x-pack-test-password".toCharArray());
private static final String KEYSTORE_PASS = "testnode";
private static final String MONITORING_PATTERN = ".monitoring-*";

static Path keyStore;
static Path trustStore;

@BeforeClass
public static void getKeyStore() {
try {
keyStore = PathUtils.get(SmokeTestMonitoringWithSecurityIT.class.getResource("/testnode.jks").toURI());
trustStore = PathUtils.get(SmokeTestMonitoringWithSecurityIT.class.getResource("/testnode.crt").toURI());
} catch (URISyntaxException e) {
throw new ElasticsearchException("exception while reading the store", e);
throw new ElasticsearchException("exception while reading the truststore", e);
}
if (Files.exists(keyStore) == false) {
throw new IllegalStateException("Keystore file [" + keyStore + "] does not exist.");
if (Files.exists(trustStore) == false) {
throw new IllegalStateException("Truststore file [" + trustStore + "] does not exist.");
}
}

@AfterClass
public static void clearKeyStore() {
keyStore = null;
trustStore = null;
}

@Override
Expand All @@ -85,24 +83,17 @@ protected Settings restClientSettings() {
String token = basicAuthHeaderValue(USER, PASS);
return Settings.builder()
.put(ThreadContext.PREFIX + ".Authorization", token)
.put(ESRestTestCase.TRUSTSTORE_PATH, keyStore)
.put(ESRestTestCase.TRUSTSTORE_PASSWORD, KEYSTORE_PASS)
.put(ESRestTestCase.CERTIFICATE_AUTHORITIES, trustStore)
.build();
}

@Before
public void enableExporter() throws Exception {
MockSecureSettings secureSettings = new MockSecureSettings();
secureSettings.setString("xpack.monitoring.exporters._http.auth.secure_password", "x-pack-test-password");
Settings exporterSettings = Settings.builder()
.put("xpack.monitoring.collection.enabled", true)
.put("xpack.monitoring.exporters._http.enabled", true)
.put("xpack.monitoring.exporters._http.type", "http")
.put("xpack.monitoring.exporters._http.host", "https://" + randomNodeHttpAddress())
.put("xpack.monitoring.exporters._http.auth.username", "monitoring_agent")
.put("xpack.monitoring.exporters._http.ssl.verification_mode", "full")
.put("xpack.monitoring.exporters._http.ssl.certificate_authorities", "testnode.crt")
.setSecureSettings(secureSettings)
.build();
updateClusterSettingsIgnoringWarnings(client(), exporterSettings);
}
Expand Down Expand Up @@ -140,7 +131,6 @@ private boolean getMonitoringUsageExportersDefined() throws Exception {
return exporters != null && exporters.isEmpty() == false;
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/49094")
public void testHTTPExporterWithSSL() throws Exception {
// Ensures that the exporter is actually on
assertBusy(() -> assertThat("[_http] exporter is not defined", getMonitoringUsageExportersDefined(), is(true)));
Expand Down Expand Up @@ -193,12 +183,11 @@ public void testHTTPExporterWithSSL() throws Exception {
});
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/49094")
public void testSettingsFilter() throws IOException {
final Request request = new Request("GET", "/_cluster/settings");
final Response response = client().performRequest(request);
final ObjectPath path = ObjectPath.createFromResponse(response);
final Map<String, Object> settings = path.evaluate("transient.xpack.monitoring.exporters._http");
final Map<String, Object> settings = path.evaluate("persistent.xpack.monitoring.exporters._http");
assertThat(settings, hasKey("type"));
assertThat(settings, not(hasKey("auth")));
assertThat(settings, not(hasKey("ssl")));
Expand Down

0 comments on commit 94658d9

Please sign in to comment.