From 0bf29a6b1966e85bd4bc8f83e6327011f97c79d4 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Mon, 19 Aug 2019 14:12:09 +1000 Subject: [PATCH 1/7] Reject misconfigured/ambiguous SSL server config This commit makes it an error to start a node where either of the server contexts (xpack.security.transport.ssl and xpack.security.http.ssl) meet either of these conditions: 1. The server lacks a certificate/key pair (i.e. neither ssl.keystore.path not ssl.certificate are configured) 2. The server has some ssl configuration, but ssl.enabled is not specified. This new validation does not care whether ssl.enabled is true or false (though other validation might), it simply makes it an error to configure server SSL without being explicit about whether to enable that configuration. --- .../migration/migrate_8_0/security.asciidoc | 68 +++++++++++++++++++ .../xpack/core/ssl/SSLService.java | 31 ++++++++- .../transport/ProfileConfigurationsTests.java | 22 +++++- .../ssl/SSLConfigurationReloaderTests.java | 32 +++++++-- .../xpack/core/ssl/SSLServiceTests.java | 55 +++++++++++---- .../test/SettingsFilterTests.java | 1 + .../security/PkiRealmBootstrapCheckTests.java | 14 ++++ .../tool/CommandLineHttpClientTests.java | 24 ++++--- .../LdapUserSearchSessionFactoryTests.java | 1 + .../security/authc/saml/SamlRealmTests.java | 1 + ...ecurityNetty4HttpServerTransportTests.java | 17 +---- .../SecurityNioHttpServerTransportTests.java | 24 +------ .../transport/ssl/SslIntegrationTests.java | 2 +- .../xpack/ssl/SSLErrorMessageTests.java | 58 ++++++++++++++++ .../xpack/ssl/SSLReloadIntegTests.java | 1 + .../xpack/ssl/SSLTrustRestrictionsTests.java | 1 + 16 files changed, 283 insertions(+), 69 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/security.asciidoc b/docs/reference/migration/migrate_8_0/security.asciidoc index f3ee4fd97ef85..5a6e77987ce0e 100644 --- a/docs/reference/migration/migrate_8_0/security.asciidoc +++ b/docs/reference/migration/migrate_8_0/security.asciidoc @@ -41,3 +41,71 @@ realm directly. The `transport.profiles.*.xpack.security.type` setting has been removed since the Transport Client has been removed and therefore all client traffic now uses the HTTP transport. Transport profiles using this setting should be removed. + +[float] +[[ssl-validation-changes]] +==== SSL/TLS configuration validation + +[float] +===== The `xpack.security.transport.ssl.enabled` setting may be required + +It is now an error to configure any SSL settings for +`xpack.security.transport.ssl` without also configuring +`xpack.security.transport.ssl.enabled`. + +For example, the following configuration is invalid: +[source,yaml] +-------------------------------------------------- +xpack.security.transport.ssl.keystore.path: elastic-certificates.p12 +xpack.security.transport.ssl.truststore.path: elastic-certificates.p12 +-------------------------------------------------- + +And must be configured as: +[source,yaml] +-------------------------------------------------- +xpack.security.transport.ssl.enabled: true <1> +xpack.security.transport.ssl.keystore.path: elastic-certificates.p12 +xpack.security.transport.ssl.truststore.path: elastic-certificates.p12 +-------------------------------------------------- +<1> or `false`. + +[float] +===== The `xpack.security.http.ssl.enabled` setting may be required + +It is now an error to configure any SSL settings for +`xpack.security.http.ssl` without also configuring +`xpack.security.http.ssl.enabled`. + +For example, the following configuration is invalid: +[source,yaml] +-------------------------------------------------- +xpack.security.http.ssl.keystore.path: elastic-certificates.p12 +xpack.security.http.ssl.truststore.path: elastic-certificates.p12 +-------------------------------------------------- + +And must be configured as either: +[source,yaml] +-------------------------------------------------- +xpack.security.http.ssl.enabled: true <1> +xpack.security.http.ssl.keystore.path: elastic-certificates.p12 +xpack.security.http.ssl.truststore.path: elastic-certificates.p12 +-------------------------------------------------- +<1> or `false`. + +[float] +===== The `xpack.security.transport.ssl` Certificate and Key may be required + +It is now an error to enable SSL for the transport interface without also configuring +a certificate and key through use of the `xpack.security.transport.ssl.keystore.path` +setting or the `xpack.security.transport.ssl.certificate` and +`xpack.security.transport.ssl.key` settings. + +[float] +===== The `xpack.security.http.ssl` Certificate and Key may be required + +It is now an error to enable SSL for the HTTP (Rest) server without also configuring +a certificate and key through use of the `xpack.security.http.ssl.keystore.path` +setting or the `xpack.security.http.ssl.certificate` and +`xpack.security.http.ssl.key` settings. + + diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 618daf1a76235..b53ccd14fbf1f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -33,7 +33,6 @@ import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedKeyManager; import javax.net.ssl.X509ExtendedTrustManager; - import java.io.IOException; import java.net.InetAddress; import java.net.Socket; @@ -428,6 +427,10 @@ Map loadSSLConfigurations() { Map profileSettings = getTransportProfileSSLSettings(settings); profileSettings.forEach((key, profileSetting) -> loadConfiguration(key, profileSetting, sslContextHolders)); + for (String context : List.of("xpack.security.transport.ssl", "xpack.security.http.ssl")) { + validateServerConfiguration(context); + } + return Collections.unmodifiableMap(sslContextHolders); } @@ -446,6 +449,32 @@ private SSLConfiguration loadConfiguration(String key, Settings settings, Map sslKeys = settings.keySet().stream() + .filter(s -> s.startsWith(prefix)) + .sorted() + .collect(Collectors.toUnmodifiableList()); + if (sslKeys.isEmpty() == false) { + throw new ElasticsearchSecurityException("invalid configuration for " + prefix + " - [" + enabledSetting + + "] is not set, but the following settings have been configured in elasticsearch.yml : [" + + Strings.collectionToCommaDelimitedString(sslKeys) + "]"); + } + } + } + private void storeSslConfiguration(String key, SSLConfiguration configuration) { if (key.endsWith(".")) { key = key.substring(0, key.length() - 1); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java index 02cb1d70c7f62..fd7315d7457c2 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/transport/ProfileConfigurationsTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.core.security.transport; +import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; @@ -15,14 +16,16 @@ import org.elasticsearch.xpack.core.ssl.VerificationMode; import org.hamcrest.Matchers; +import java.nio.file.Path; import java.util.Map; public class ProfileConfigurationsTests extends ESTestCase { public void testGetSecureTransportProfileConfigurations() { - final Settings settings = Settings.builder() + final Settings settings = getBaseSettings() .put("path.home", createTempDir()) .put("xpack.security.transport.ssl.verification_mode", VerificationMode.CERTIFICATE.name()) + .put("xpack.security.transport.ssl.verification_mode", VerificationMode.CERTIFICATE.name()) .put("transport.profiles.full.xpack.security.ssl.verification_mode", VerificationMode.FULL.name()) .put("transport.profiles.cert.xpack.security.ssl.verification_mode", VerificationMode.CERTIFICATE.name()) .build(); @@ -39,7 +42,7 @@ public void testGetSecureTransportProfileConfigurations() { public void testGetInsecureTransportProfileConfigurations() { assumeFalse("Can't run in a FIPS JVM with verification mode None", inFipsJvm()); - final Settings settings = Settings.builder() + final Settings settings = getBaseSettings() .put("path.home", createTempDir()) .put("xpack.security.transport.ssl.verification_mode", VerificationMode.CERTIFICATE.name()) .put("transport.profiles.none.xpack.security.ssl.verification_mode", VerificationMode.NONE.name()) @@ -53,4 +56,19 @@ public void testGetInsecureTransportProfileConfigurations() { assertThat(profileConfigurations.get("none").verificationMode(), Matchers.equalTo(VerificationMode.NONE)); assertThat(profileConfigurations.get("default"), Matchers.sameInstance(defaultConfig)); } + + private Settings.Builder getBaseSettings() { + final Path keystore = randomBoolean() + ? getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks") + : getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.p12"); + + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); + + return Settings.builder() + .setSecureSettings(secureSettings) + .put("xpack.security.transport.ssl.enabled", true) + .put("xpack.security.transport.ssl.keystore.path", keystore.toString()); + } + } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java index 3b1a7431f629f..16b668080a545 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java @@ -110,6 +110,7 @@ public void testReloadingKeyStore() throws Exception { secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); final Settings settings = Settings.builder() .put("path.home", createTempDir()) + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.keystore.path", keystorePath) .setSecureSettings(secureSettings) .build(); @@ -166,6 +167,7 @@ public void testPEMKeyConfigReloading() throws Exception { secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); final Settings settings = Settings.builder() .put("path.home", createTempDir()) + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.key", keyPath) .put("xpack.security.transport.ssl.certificate", certPath) .putList("xpack.security.transport.ssl.certificate_authorities", certPath.toString()) @@ -222,10 +224,10 @@ public void testReloadingTrustStore() throws Exception { updatedTruststorePath); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.truststore.secure_password", "testnode"); - Settings settings = Settings.builder() + final Settings settings = baseKeystoreSettings(tempDir, secureSettings) + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.truststore.path", trustStorePath) .put("path.home", createTempDir()) - .setSecureSettings(secureSettings) .build(); Environment env = TestEnvironment.newEnvironment(settings); // Create the MockWebServer once for both pre and post checks @@ -273,7 +275,8 @@ public void testReloadingPEMTrustConfig() throws Exception { Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"), serverCertPath); Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem"), serverKeyPath); Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode_updated.crt"), updatedCert); - Settings settings = Settings.builder() + Settings settings = baseKeystoreSettings(tempDir, null) + .put("xpack.security.transport.ssl.enabled", true) .putList("xpack.security.transport.ssl.certificate_authorities", serverCertPath.toString()) .put("path.home", createTempDir()) .build(); @@ -322,6 +325,7 @@ public void testReloadingKeyStoreException() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.keystore.path", keystorePath) .setSecureSettings(secureSettings) .put("path.home", createTempDir()) @@ -372,6 +376,7 @@ public void testReloadingPEMKeyConfigException() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.key", keyPath) .put("xpack.security.transport.ssl.certificate", certPath) .putList("xpack.security.transport.ssl.certificate_authorities", certPath.toString(), clientCertPath.toString()) @@ -419,10 +424,10 @@ public void testTrustStoreReloadException() throws Exception { Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"), trustStorePath); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.truststore.secure_password", "testnode"); - Settings settings = Settings.builder() + Settings settings = baseKeystoreSettings(tempDir, secureSettings) + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.truststore.path", trustStorePath) .put("path.home", createTempDir()) - .setSecureSettings(secureSettings) .build(); Environment env = TestEnvironment.newEnvironment(settings); final SSLService sslService = new SSLService(settings, env); @@ -463,7 +468,8 @@ public void testPEMTrustReloadException() throws Exception { Path tempDir = createTempDir(); Path clientCertPath = tempDir.resolve("testclient.crt"); Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testclient.crt"), clientCertPath); - Settings settings = Settings.builder() + Settings settings = baseKeystoreSettings(tempDir, null) + .put("xpack.security.transport.ssl.enabled", true) .putList("xpack.security.transport.ssl.certificate_authorities", clientCertPath.toString()) .put("path.home", createTempDir()) .build(); @@ -501,6 +507,20 @@ void reloadSSLContext(SSLConfiguration configuration) { assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context)); } + private Settings.Builder baseKeystoreSettings(Path tempDir, MockSecureSettings secureSettings) throws IOException { + final Path keystorePath = tempDir.resolve("testclient.jks"); + Files.copy(getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"), keystorePath); + + if (secureSettings == null) { + secureSettings = new MockSecureSettings(); + } + secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); + + return Settings.builder() + .put("xpack.security.transport.ssl.keystore.path", keystorePath.toString()) + .setSecureSettings(secureSettings); + } + private void validateSSLConfigurationIsReloaded(Settings settings, Environment env, Consumer preChecks, Runnable modificationFunction, Consumer postChecks) throws Exception { final CountDownLatch reloadLatch = new CountDownLatch(1); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java index ce043b4597d80..c68985954ff87 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java @@ -111,8 +111,11 @@ public void testThatCustomTruststoreCanBeSpecified() throws Exception { Path testClientStore = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testclient.jks"); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.truststore.secure_password", "testnode"); + secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); secureSettings.setString("transport.profiles.foo.xpack.security.ssl.truststore.secure_password", "testclient"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) + .put("xpack.security.transport.ssl.keystore.path", testnodeStore) .put("xpack.security.transport.ssl.truststore.path", testnodeStore) .put("xpack.security.transport.ssl.truststore.type", testnodeStoreType) .setSecureSettings(secureSettings) @@ -145,6 +148,7 @@ public void testThatSslContextCachingWorks() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.certificate", testnodeCert) .put("xpack.security.transport.ssl.key", testnodeKey) .setSecureSettings(secureSettings) @@ -170,6 +174,7 @@ public void testThatKeyStoreAndKeyCanHaveDifferentPasswords() throws Exception { secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); secureSettings.setString("xpack.security.transport.ssl.keystore.secure_key_password", "testnode1"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.keystore.path", differentPasswordsStore) .setSecureSettings(secureSettings) .build(); @@ -204,6 +209,7 @@ public void testThatSSLv3IsNotEnabled() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.certificate", testnodeCert) .put("xpack.security.transport.ssl.key", testnodeKey) .setSecureSettings(secureSettings) @@ -223,13 +229,14 @@ public void testThatCreateClientSSLEngineWithoutAnySettingsWorks() throws Except public void testThatCreateSSLEngineWithOnlyTruststoreWorks() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString("xpack.security.transport.ssl.truststore.secure_password", "testclient"); + secureSettings.setString("xpack.http.ssl.truststore.secure_password", "testclient"); Settings settings = Settings.builder() - .put("xpack.security.transport.ssl.truststore.path", testclientStore) + .put("xpack.http.ssl.enabled", true) + .put("xpack.http.ssl.truststore.path", testclientStore) .setSecureSettings(secureSettings) .build(); SSLService sslService = new SSLService(settings, env); - SSLConfiguration configuration = sslService.getSSLConfiguration("xpack.security.transport.ssl"); + SSLConfiguration configuration = sslService.getSSLConfiguration("xpack.security.http.ssl"); SSLEngine sslEngine = sslService.createSSLEngine(configuration, null, -1); assertThat(sslEngine, notNullValue()); } @@ -240,6 +247,7 @@ public void testCreateWithKeystoreIsValidForServer() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.keystore.path", testnodeStore) .put("xpack.security.transport.ssl.keystore.type", testnodeStoreType) .setSecureSettings(secureSettings) @@ -252,25 +260,27 @@ public void testCreateWithKeystoreIsValidForServer() throws Exception { public void testValidForServer() throws Exception { assumeFalse("Can't run in a FIPS JVM, JKS keystores can't be used", inFipsJvm()); MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString("xpack.security.transport.ssl.truststore.secure_password", "testnode"); + secureSettings.setString("xpack.http.ssl.truststore.secure_password", "testnode"); Settings settings = Settings.builder() - .put("xpack.security.transport.ssl.truststore.path", testnodeStore) - .put("xpack.security.transport.ssl.truststore.type", testnodeStoreType) + .put("xpack.http.ssl.truststore.path", testnodeStore) + .put("xpack.http.ssl.truststore.type", testnodeStoreType) .setSecureSettings(secureSettings) .build(); SSLService sslService = new SSLService(settings, env); - assertFalse(sslService.isConfigurationValidForServerUsage(sslService.getSSLConfiguration("xpack.security.transport.ssl"))); + // Technically, we don't care whether xpack.http.ssl is valid for server - it's a client context, but we validate both of the + // server contexts (http & transport) during construction, so this is the only way to make a non-server-valid context. + assertFalse(sslService.isConfigurationValidForServerUsage(sslService.getSSLConfiguration("xpack.http.ssl"))); - secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); + secureSettings.setString("xpack.http.ssl.keystore.secure_password", "testnode"); settings = Settings.builder() - .put("xpack.security.transport.ssl.truststore.path", testnodeStore) - .put("xpack.security.transport.ssl.truststore.type", testnodeStoreType) + .put("xpack.http.ssl.truststore.path", testnodeStore) + .put("xpack.http.ssl.truststore.type", testnodeStoreType) .setSecureSettings(secureSettings) - .put("xpack.security.transport.ssl.keystore.path", testnodeStore) - .put("xpack.security.transport.ssl.keystore.type", testnodeStoreType) + .put("xpack.http.ssl.keystore.path", testnodeStore) + .put("xpack.http.ssl.keystore.type", testnodeStoreType) .build(); sslService = new SSLService(settings, env); - assertTrue(sslService.isConfigurationValidForServerUsage(sslService.getSSLConfiguration("xpack.security.transport.ssl"))); + assertTrue(sslService.isConfigurationValidForServerUsage(sslService.getSSLConfiguration("xpack.http.ssl"))); } public void testGetVerificationMode() throws Exception { @@ -280,6 +290,7 @@ public void testGetVerificationMode() throws Exception { is(XPackSettings.VERIFICATION_MODE_DEFAULT)); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", false) .put("xpack.security.transport.ssl.verification_mode", "certificate") .put("transport.profiles.foo.xpack.security.ssl.verification_mode", "full") .build(); @@ -294,6 +305,7 @@ public void testIsSSLClientAuthEnabled() throws Exception { assertTrue(sslService.getSSLConfiguration("xpack.security.transport.ssl").sslClientAuth().enabled()); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", false) .put("xpack.security.transport.ssl.client_authentication", "optional") .put("transport.profiles.foo.port", "9400-9410") .build(); @@ -303,9 +315,18 @@ public void testIsSSLClientAuthEnabled() throws Exception { } public void testThatHttpClientAuthDefaultsToNone() throws Exception { + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); + secureSettings.setString("xpack.security.http.ssl.keystore.secure_password", "testnode"); final Settings globalSettings = Settings.builder() .put("xpack.security.http.ssl.enabled", true) + .put("xpack.security.http.ssl.keystore.path", testnodeStore) + .put("xpack.security.http.ssl.keystore.type", testnodeStoreType) + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.client_authentication", SSLClientAuth.OPTIONAL.name()) + .put("xpack.security.transport.ssl.keystore.path", testnodeStore) + .put("xpack.security.transport.ssl.keystore.type", testnodeStoreType) + .setSecureSettings(secureSettings) .build(); final SSLService sslService = new SSLService(globalSettings, env); @@ -350,6 +371,7 @@ public void testCiphersAndInvalidCiphersWork() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.certificate", testnodeCert) .put("xpack.security.transport.ssl.key", testnodeKey) .setSecureSettings(secureSettings) @@ -383,6 +405,7 @@ public void testThatSSLEngineHasCipherSuitesOrderSet() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.certificate", testnodeCert) .put("xpack.security.transport.ssl.key", testnodeKey) .setSecureSettings(secureSettings) @@ -398,6 +421,7 @@ public void testThatSSLSocketFactoryHasProperCiphersAndProtocols() throws Except MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.certificate", testnodeCert) .put("xpack.security.transport.ssl.key", testnodeKey) .setSecureSettings(secureSettings) @@ -423,6 +447,7 @@ public void testThatSSLEngineHasProperCiphersAndProtocols() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.certificate", testnodeCert) .put("xpack.security.transport.ssl.key", testnodeKey) .setSecureSettings(secureSettings) @@ -514,6 +539,9 @@ public void testGetConfigurationByContextName() throws Exception { final MockSecureSettings secureSettings = new MockSecureSettings(); final Settings.Builder builder = Settings.builder(); for (String prefix : contextNames) { + if (prefix.startsWith("xpack.security.transport") || prefix.startsWith("xpack.security.http")) { + builder.put(prefix + ".enabled", true); + } secureSettings.setString(prefix + ".keystore.secure_password", "testnode"); builder.put(prefix + ".keystore.path", testnodeStore) .putList(prefix + ".cipher_suites", cipher.next()); @@ -548,6 +576,7 @@ public void testReadCertificateInformation() throws Exception { secureSettings.setString("xpack.http.ssl.keystore.secure_password", "testnode"); final Settings settings = Settings.builder() + .put("xpack.security.transport.ssl.enabled", randomBoolean()) .put("xpack.security.transport.ssl.keystore.path", jksPath) .put("xpack.security.transport.ssl.truststore.path", jksPath) .put("xpack.http.ssl.keystore.path", p12Path) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java index 014ebc72a982b..e669d41eab5be 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SettingsFilterTests.java @@ -71,6 +71,7 @@ public void testFiltering() throws Exception { configureFilteredSetting("xpack.security.authc.realms.pki.pki1.truststore.algorithm", "SunX509"); + configureUnfilteredSetting("xpack.security.transport.ssl.enabled", "true"); configureFilteredSetting("xpack.security.transport.ssl.cipher_suites", Strings.arrayToCommaDelimitedString(XPackSettings.DEFAULT_CIPHERS.toArray())); configureFilteredSetting("xpack.security.transport.ssl.supported_protocols", randomFrom("TLSv1", "TLSv1.1", "TLSv1.2")); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java index ec3e0a5132dfd..b9fc4047fc172 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java @@ -14,6 +14,8 @@ import org.elasticsearch.xpack.core.ssl.SSLService; import org.hamcrest.Matchers; +import java.nio.file.Path; + public class PkiRealmBootstrapCheckTests extends AbstractBootstrapCheckTestCase { public void testPkiRealmBootstrapDefault() throws Exception { @@ -23,23 +25,34 @@ public void testPkiRealmBootstrapDefault() throws Exception { } public void testBootstrapCheckWithPkiRealm() throws Exception { + final Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"); + final Path keyPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem"); + + MockSecureSettings secureSettings = new MockSecureSettings(); Settings settings = Settings.builder() .put("xpack.security.authc.realms.pki.test_pki.order", 0) .put("path.home", createTempDir()) + .setSecureSettings(secureSettings) .build(); Environment env = TestEnvironment.newEnvironment(settings); assertTrue(runCheck(settings, env).isFailure()); // enable transport tls + secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); settings = Settings.builder().put(settings) .put("xpack.security.transport.ssl.enabled", true) + .put("xpack.security.transport.ssl.certificate", certPath) + .put("xpack.security.transport.ssl.key", keyPath) .build(); assertFalse(runCheck(settings, env).isFailure()); // enable ssl for http + secureSettings.setString("xpack.security.http.ssl.secure_key_passphrase", "testnode"); settings = Settings.builder().put(settings) .put("xpack.security.transport.ssl.enabled", false) .put("xpack.security.http.ssl.enabled", true) + .put("xpack.security.http.ssl.certificate", certPath) + .put("xpack.security.http.ssl.key", keyPath) .build(); env = TestEnvironment.newEnvironment(settings); assertTrue(runCheck(settings, env).isFailure()); @@ -82,6 +95,7 @@ private BootstrapCheck.BootstrapCheckResult runCheck(Settings settings, Environm public void testBootstrapCheckWithDisabledRealm() throws Exception { Settings settings = Settings.builder() .put("xpack.security.authc.realms.pki.test_pki.enabled", false) + .put("xpack.security.transport.ssl.enabled", false) .put("xpack.security.transport.ssl.client_authentication", "none") .put("path.home", createTempDir()) .build(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/CommandLineHttpClientTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/CommandLineHttpClientTests.java index 1e77dfc6b16e4..4f2484d193116 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/CommandLineHttpClientTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/CommandLineHttpClientTests.java @@ -37,9 +37,14 @@ public class CommandLineHttpClientTests extends ESTestCase { private MockWebServer webServer; private Environment environment = TestEnvironment.newEnvironment(Settings.builder().put("path.home", createTempDir()).build()); + private Path certPath; + private Path keyPath; @Before public void setup() throws Exception { + certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"); + keyPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem"); + webServer = createMockWebServer(); webServer.enqueue(new MockResponse().setResponseCode(200).setBody("{\"test\": \"complete\"}")); webServer.start(); @@ -51,8 +56,7 @@ public void shutdown() { } public void testCommandLineHttpClientCanExecuteAndReturnCorrectResultUsingSSLSettings() throws Exception { - Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"); - Settings settings = Settings.builder() + Settings settings = getHttpSslSettings() .put("xpack.security.http.ssl.certificate_authorities", certPath.toString()) .put("xpack.security.http.ssl.verification_mode", VerificationMode.CERTIFICATE) .build(); @@ -75,17 +79,19 @@ public void testGetDefaultURLFailsWithHelpfulMessage() { } private MockWebServer createMockWebServer() { - Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"); - Path keyPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem"); + Settings settings = getHttpSslSettings().build(); + TestsSSLService sslService = new TestsSSLService(settings, environment); + return new MockWebServer(sslService.sslContext("xpack.security.http.ssl."), false); + } + + private Settings.Builder getHttpSslSettings() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.http.ssl.secure_key_passphrase", "testnode"); - Settings settings = Settings.builder() + return Settings.builder() + .put("xpack.security.http.ssl.enabled", true) .put("xpack.security.http.ssl.key", keyPath.toString()) .put("xpack.security.http.ssl.certificate", certPath.toString()) - .setSecureSettings(secureSettings) - .build(); - TestsSSLService sslService = new TestsSSLService(settings, environment); - return new MockWebServer(sslService.sslContext("xpack.security.http.ssl."), false); + .setSecureSettings(secureSettings); } private HttpResponseBuilder responseBuilder(final InputStream is) throws IOException { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java index 2598b9da5507f..cb8892d6c8c42 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactoryTests.java @@ -65,6 +65,7 @@ public void init() throws Exception { globalSettings = Settings.builder() .put("path.home", createTempDir()) + .put("xpack.security.transport.ssl.enabled", false) .put("xpack.security.transport.ssl.certificate_authorities", certPath) .build(); sslService = new SSLService(globalSettings, env); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java index 824655d59c7e5..43567d17a4a98 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlRealmTests.java @@ -130,6 +130,7 @@ public void testReadIdpMetadataFromHttps() throws Exception { final MockSecureSettings mockSecureSettings = new MockSecureSettings(); mockSecureSettings.setString("xpack.security.http.ssl.secure_key_passphrase", "testnode"); final Settings settings = Settings.builder() + .put("xpack.security.http.ssl.enabled", true) .put("xpack.security.http.ssl.key", getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem")) .put("xpack.security.http.ssl.certificate", diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java index 20ceee5d52e92..8efa61ebe40b6 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java @@ -29,7 +29,6 @@ import java.util.Locale; import static org.hamcrest.Matchers.arrayContaining; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -49,6 +48,7 @@ public void createSSLService() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.http.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.http.ssl.enabled", true) .put("xpack.security.http.ssl.key", testnodeKey) .put("xpack.security.http.ssl.certificate", testnodeCert) .put("path.home", createTempDir()) @@ -147,24 +147,11 @@ public void testCustomSSLConfiguration() throws Exception { assertThat(customEngine.getEnabledProtocols(), not(equalTo(defaultEngine.getEnabledProtocols()))); } - public void testThatExceptionIsThrownWhenConfiguredWithoutSslKey() throws Exception { - Settings settings = Settings.builder() - .put("xpack.security.http.ssl.certificate_authorities", testnodeCert) - .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) - .put("path.home", createTempDir()) - .build(); - env = TestEnvironment.newEnvironment(settings); - sslService = new SSLService(settings, env); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> new SecurityNetty4HttpServerTransport(settings, new NetworkService(Collections.emptyList()), mock(BigArrays.class), - mock(IPFilter.class), sslService, mock(ThreadPool.class), xContentRegistry(), new NullDispatcher())); - assertThat(e.getMessage(), containsString("key must be provided")); - } - public void testNoExceptionWhenConfiguredWithoutSslKeySSLDisabled() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.http.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.http.ssl.enabled", false) .put("xpack.security.http.ssl.key", testnodeKey) .put("xpack.security.http.ssl.certificate", testnodeCert) .setSecureSettings(secureSettings) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java index 7acef50ed4eb8..14addd0620b43 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransportTests.java @@ -36,7 +36,6 @@ import java.util.Locale; import static org.hamcrest.Matchers.arrayContaining; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -57,6 +56,7 @@ public void createSSLService() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.http.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.http.ssl.enabled", true) .put("xpack.security.http.ssl.key", testNodeKey) .put("xpack.security.http.ssl.certificate", testNodeCert) .put("path.home", createTempDir()) @@ -180,31 +180,11 @@ public void testCustomSSLConfiguration() throws IOException { assertThat(customEngine.getEnabledProtocols(), not(equalTo(defaultEngine.getEnabledProtocols()))); } - public void testThatExceptionIsThrownWhenConfiguredWithoutSslKey() { - MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString("xpack.security.http.ssl.truststore.secure_password", "testnode"); - Settings settings = Settings.builder() - .put("xpack.security.http.ssl.truststore.path", - getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks")) - .setSecureSettings(secureSettings) - .put(XPackSettings.HTTP_SSL_ENABLED.getKey(), true) - .put("path.home", createTempDir()) - .build(); - env = TestEnvironment.newEnvironment(settings); - sslService = new SSLService(settings, env); - nioGroupFactory = new NioGroupFactory(settings, logger); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> new SecurityNioHttpServerTransport(settings, - new NetworkService(Collections.emptyList()), mock(BigArrays.class), mock(PageCacheRecycler.class), mock(ThreadPool.class), - xContentRegistry(), new NullDispatcher(), mock(IPFilter.class), sslService, nioGroupFactory)); - assertThat(e.getMessage(), containsString("key must be provided")); - } - public void testNoExceptionWhenConfiguredWithoutSslKeySSLDisabled() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.http.ssl.truststore.secure_password", "testnode"); Settings settings = Settings.builder() + .put("xpack.security.http.ssl.enabled", false) .put("xpack.security.http.ssl.truststore.path", getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks")) .setSecureSettings(secureSettings) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ssl/SslIntegrationTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ssl/SslIntegrationTests.java index 01f8c70fc7b36..741b70c2258c3 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ssl/SslIntegrationTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/ssl/SslIntegrationTests.java @@ -61,7 +61,7 @@ protected boolean transportSSLEnabled() { } public void testThatConnectionToHTTPWorks() throws Exception { - Settings.Builder builder = Settings.builder(); + Settings.Builder builder = Settings.builder().put("xpack.security.http.ssl.enabled", true); addSSLSettingsForPEMFiles( builder, "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testclient.pem", "testclient", diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java index 2bc9fd922c5e9..f78e329d7289d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java @@ -131,6 +131,51 @@ public void testMessageForCertificateAuthoritiesOutsideConfigDir() throws Except checkBlockedTrustManagerResource("certificate_authorities", "certificate_authorities"); } + public void testMessageForTransportSslEnabledWithoutKeys() throws Exception { + final String prefix = "xpack.security.transport.ssl"; + final Settings.Builder settings = Settings.builder(); + settings.put(prefix + ".enabled", true); + configureWorkingTruststore(prefix, settings); + + Throwable exception = expectFailure(settings); + assertThat(exception, throwableWithMessage("invalid configuration for " + prefix + + " - a server context requires a key and certificate, but these have not been configured" + + "; either [" + prefix + ".keystore.path] or [" + prefix + ".certificate] should be set")); + assertThat(exception, instanceOf(ElasticsearchException.class)); + } + + public void testNoErrorIfTransportSslDisabledWithoutKeys() throws Exception { + final String prefix = "xpack.security.transport.ssl"; + final Settings.Builder settings = Settings.builder(); + settings.put(prefix + ".enabled", false); + configureWorkingTruststore(prefix, settings); + expectSuccess(settings); + } + + public void testMessageForTransportNotEnabledButKeystoreConfigured() throws Exception { + final String prefix = "xpack.security.transport.ssl"; + checkUnusedConfiguration(prefix, prefix + ".keystore.path," + prefix + ".keystore.secure_password", + this::configureWorkingKeystore); + } + + public void testMessageForTransportNotEnabledButTruststoreConfigured() throws Exception { + final String prefix = "xpack.security.transport.ssl"; + checkUnusedConfiguration(prefix, prefix + ".truststore.path," + prefix + ".truststore.secure_password", + this::configureWorkingTruststore); + } + + public void testMessageForHttpsNotEnabledButKeystoreConfigured() throws Exception { + final String prefix = "xpack.security.http.ssl"; + checkUnusedConfiguration(prefix, prefix + ".keystore.path," + prefix + ".keystore.secure_password", + this::configureWorkingKeystore); + } + + public void testMessageForHttpsNotEnabledButTruststoreConfigured() throws Exception { + final String prefix = "xpack.security.http.ssl"; + checkUnusedConfiguration(prefix, prefix + ".truststore.path," + prefix + ".truststore.secure_password", + this::configureWorkingTruststore); + } + private void checkMissingKeyManagerResource(String fileType, String configKey, @Nullable Settings.Builder additionalSettings) { checkMissingResource("KeyManager", fileType, configKey, (prefix, builder) -> buildKeyConfigSettings(additionalSettings, prefix, builder)); @@ -239,6 +284,16 @@ private void checkBlockedResource(String sslManagerType, String fileType, String assertThat(exception, throwableWithMessage(containsString(fileName))); } + private void checkUnusedConfiguration(String prefix, String settingsConfigured, BiConsumer configure) { + final Settings.Builder settings = Settings.builder(); + configure.accept(prefix, settings); + + Throwable exception = expectFailure(settings); + assertThat(exception, throwableWithMessage("invalid configuration for " + prefix + " - [" + prefix + ".enabled] is not set," + + " but the following settings have been configured in elasticsearch.yml : [" + settingsConfigured + "]")); + assertThat(exception, instanceOf(ElasticsearchException.class)); + } + private String missingFile() { return resource("cert1a.p12").replace("cert1a.p12", "file.dne"); } @@ -296,6 +351,9 @@ private Settings.Builder configureWorkingKeystore(String prefix, Settings.Builde private ElasticsearchException expectFailure(Settings.Builder settings) { return expectThrows(ElasticsearchException.class, () -> new SSLService(settings.build(), env)); } + private SSLService expectSuccess(Settings.Builder settings) { + return new SSLService(settings.build(), env); + } private String resource(String fileName) { final Path path = this.paths.get(fileName); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadIntegTests.java index 3eeaca1a3f114..25f3215a4841b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadIntegTests.java @@ -99,6 +99,7 @@ public void testThatSSLConfigurationReloadsOnModification() throws Exception { secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() .put("path.home", createTempDir()) + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.key", keyPath) .put("xpack.security.transport.ssl.certificate", certPath) .putList("xpack.security.transport.ssl.certificate_authorities", diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLTrustRestrictionsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLTrustRestrictionsTests.java index c4b75f1e73d38..2f097480f928d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLTrustRestrictionsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLTrustRestrictionsTests.java @@ -213,6 +213,7 @@ private void runResourceWatcher() { private void tryConnect(CertificateInfo certificate, boolean shouldFail) throws Exception { Settings settings = Settings.builder() .put("path.home", createTempDir()) + .put("xpack.security.transport.ssl.enabled", true) .put("xpack.security.transport.ssl.key", certificate.getKeyPath()) .put("xpack.security.transport.ssl.certificate", certificate.getCertPath()) .putList("xpack.security.transport.ssl.certificate_authorities", ca.getCertPath().toString()) From d9b73a6f6c3691b6881cfba073f48f9b32be14b8 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 15 Oct 2019 14:53:13 +1100 Subject: [PATCH 2/7] Address feedback --- .../migration/migrate_8_0/security.asciidoc | 10 ++++++---- .../elasticsearch/xpack/core/ssl/SSLService.java | 15 ++++++++------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/security.asciidoc b/docs/reference/migration/migrate_8_0/security.asciidoc index 5a6e77987ce0e..7433250a6cbad 100644 --- a/docs/reference/migration/migrate_8_0/security.asciidoc +++ b/docs/reference/migration/migrate_8_0/security.asciidoc @@ -79,16 +79,18 @@ It is now an error to configure any SSL settings for For example, the following configuration is invalid: [source,yaml] -------------------------------------------------- -xpack.security.http.ssl.keystore.path: elastic-certificates.p12 -xpack.security.http.ssl.truststore.path: elastic-certificates.p12 +xpack.security.http.ssl.certificate: elasticsearch.crt +xpack.security.http.ssl.key: elasticsearch.key +xpack.security.http.ssl.certificate_authorities: [ "corporate-ca.crt" ] -------------------------------------------------- And must be configured as either: [source,yaml] -------------------------------------------------- xpack.security.http.ssl.enabled: true <1> -xpack.security.http.ssl.keystore.path: elastic-certificates.p12 -xpack.security.http.ssl.truststore.path: elastic-certificates.p12 +xpack.security.http.ssl.certificate: elasticsearch.crt +xpack.security.http.ssl.key: elasticsearch.key +xpack.security.http.ssl.certificate_authorities: [ "corporate-ca.crt" ] -------------------------------------------------- <1> or `false`. diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 14045afea57b7..0eb46cb9fea1b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -457,20 +457,21 @@ private void validateServerConfiguration(String prefix) { // Client Authentication _should_ be required, but if someone turns it off, then this check is no longer relevant final SSLConfigurationSettings configurationSettings = SSLConfigurationSettings.withPrefix(prefix + "."); if (isConfigurationValidForServerUsage(configuration) == false) { - throw new ElasticsearchSecurityException("invalid configuration for " + prefix + - " - a server context requires a key and certificate, but these have not been configured; either [" + - configurationSettings.x509KeyPair.keystorePath.getKey() + "] or [" + - configurationSettings.x509KeyPair.certificatePath.getKey() + "] should be set"); + throw new ElasticsearchSecurityException("invalid SSL configuration for " + prefix + + " - server ssl configuration requires a key and certificate, but these have not been configured; you must set either " + + "[" + configurationSettings.x509KeyPair.keystorePath.getKey() + "] or [" + + configurationSettings.x509KeyPair.keyPath.getKey() + "] and [" + + configurationSettings.x509KeyPair.certificatePath.getKey() + "]"); } } else if (settings.hasValue(enabledSetting) == false) { - final List sslKeys = settings.keySet().stream() + final List sslSettingNames = settings.keySet().stream() .filter(s -> s.startsWith(prefix)) .sorted() .collect(Collectors.toUnmodifiableList()); - if (sslKeys.isEmpty() == false) { + if (sslSettingNames.isEmpty() == false) { throw new ElasticsearchSecurityException("invalid configuration for " + prefix + " - [" + enabledSetting + "] is not set, but the following settings have been configured in elasticsearch.yml : [" + - Strings.collectionToCommaDelimitedString(sslKeys) + "]"); + Strings.collectionToCommaDelimitedString(sslSettingNames) + "]"); } } } From 535094fa65fc55b37808b3f3d31ae7416691a506 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 15 Oct 2019 15:40:56 +1100 Subject: [PATCH 3/7] Fix broken tests We added a few tests with invalid config while this PR was stagnant --- .../xpack/security/PkiRealmBootstrapCheckTests.java | 9 +++++++++ .../xpack/watcher/common/http/HttpClientTests.java | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java index 625cff9cf65b1..62aece1f4fdf2 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/PkiRealmBootstrapCheckTests.java @@ -104,11 +104,20 @@ public void testBootstrapCheckWithDisabledRealm() throws Exception { } public void testBootstrapCheckWithDelegationEnabled() throws Exception { + final Path certPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.crt"); + final Path keyPath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem"); + MockSecureSettings secureSettings = new MockSecureSettings(); + // enable transport tls + secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() .put("xpack.security.authc.realms.pki.test_pki.enabled", true) .put("xpack.security.authc.realms.pki.test_pki.delegation.enabled", true) + .put("xpack.security.transport.ssl.enabled", randomBoolean()) .put("xpack.security.transport.ssl.client_authentication", "none") + .put("xpack.security.transport.ssl.certificate", certPath.toString()) + .put("xpack.security.transport.ssl.key", keyPath.toString()) .put("path.home", createTempDir()) + .setSecureSettings(secureSettings) .build(); Environment env = TestEnvironment.newEnvironment(settings); assertFalse(runCheck(settings, env).isFailure()); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java index 4d93e58924491..e83f9154c5459 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java @@ -197,6 +197,7 @@ public void testHttps() throws Exception { // We can't use the client created above for the server since it is only a truststore secureSettings.setString("xpack.security.http.ssl.secure_key_passphrase", "testnode"); Settings settings2 = Settings.builder() + .put("xpack.security.http.ssl.enabled", true) .put("xpack.security.http.ssl.key", keyPath) .put("xpack.security.http.ssl.certificate", certPath) .putList("xpack.security.http.ssl.supported_protocols", getProtocols()) @@ -226,6 +227,7 @@ public void testHttpsDisableHostnameVerification() throws Exception { // We can't use the client created above for the server since it only defines a truststore secureSettings.setString("xpack.security.http.ssl.secure_key_passphrase", "testnode-no-subjaltname"); Settings settings2 = Settings.builder() + .put("xpack.security.http.ssl.enabled", true) .put("xpack.security.http.ssl.key", keyPath) .put("xpack.security.http.ssl.certificate", certPath) .putList("xpack.security.http.ssl.supported_protocols", getProtocols()) @@ -382,6 +384,7 @@ public void testProxyCanHaveDifferentSchemeThanRequest() throws Exception { Settings serverSettings = Settings.builder() .put("xpack.http.ssl.key", keyPath) .put("xpack.http.ssl.certificate", certPath) + .put("xpack.security.http.ssl.enabled", false) .putList("xpack.security.http.ssl.supported_protocols", getProtocols()) .setSecureSettings(serverSecureSettings) .build(); @@ -397,6 +400,7 @@ public void testProxyCanHaveDifferentSchemeThanRequest() throws Exception { .put(HttpSettings.PROXY_SCHEME.getKey(), "https") .put("xpack.http.ssl.certificate_authorities", trustedCertPath) .putList("xpack.security.http.ssl.supported_protocols", getProtocols()) + .put("xpack.security.http.ssl.enabled", false) .build(); HttpRequest.Builder requestBuilder = HttpRequest.builder("localhost", webServer.getPort()) From 7077b90086f6cc62db37a4040eaded5c0f105cfa Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Fri, 18 Oct 2019 17:19:35 +1100 Subject: [PATCH 4/7] Update error message --- .../java/org/elasticsearch/xpack/core/ssl/SSLService.java | 2 +- .../org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 0eb46cb9fea1b..ec9f7a2d5a8f3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -459,7 +459,7 @@ private void validateServerConfiguration(String prefix) { if (isConfigurationValidForServerUsage(configuration) == false) { throw new ElasticsearchSecurityException("invalid SSL configuration for " + prefix + " - server ssl configuration requires a key and certificate, but these have not been configured; you must set either " + - "[" + configurationSettings.x509KeyPair.keystorePath.getKey() + "] or [" + + "[" + configurationSettings.x509KeyPair.keystorePath.getKey() + "], or both [" + configurationSettings.x509KeyPair.keyPath.getKey() + "] and [" + configurationSettings.x509KeyPair.certificatePath.getKey() + "]"); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java index bc89cb0007c68..3f88e6ea84809 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java @@ -134,9 +134,9 @@ public void testMessageForTransportSslEnabledWithoutKeys() throws Exception { configureWorkingTruststore(prefix, settings); Throwable exception = expectFailure(settings); - assertThat(exception, throwableWithMessage("invalid configuration for " + prefix + - " - a server context requires a key and certificate, but these have not been configured" + - "; either [" + prefix + ".keystore.path] or [" + prefix + ".certificate] should be set")); + assertThat(exception, throwableWithMessage("invalid SSL configuration for " + prefix + + " - server ssl configuration requires a key and certificate, but these have not been configured;" + + " you must set either [" + prefix + ".keystore.path], or both [" + prefix + ".key] and [" + prefix + ".certificate]")); assertThat(exception, instanceOf(ElasticsearchException.class)); } From fced860a41c9eba90b4b543cdfa1c4b7f7929ec7 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Fri, 18 Oct 2019 17:27:26 +1100 Subject: [PATCH 5/7] Fix HLRC tests --- client/rest-high-level/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/client/rest-high-level/build.gradle b/client/rest-high-level/build.gradle index e8c5f4d53f1a8..5f0c0f22cf187 100644 --- a/client/rest-high-level/build.gradle +++ b/client/rest-high-level/build.gradle @@ -118,6 +118,7 @@ testClusters.integTest { setting 'xpack.security.enabled', 'true' setting 'xpack.security.authc.token.enabled', 'true' setting 'xpack.security.authc.api_key.enabled', 'true' + setting 'xpack.security.http.ssl.enabled', false // Truststore settings are not used since TLS is not enabled. Included for testing the get certificates API setting 'xpack.security.http.ssl.certificate_authorities', 'testnode.crt' setting 'xpack.security.transport.ssl.truststore.path', 'testnode.jks' From 5b74feba21bd94fd9eaeb05c27411a6eef482843 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 5 Nov 2019 00:46:42 -0500 Subject: [PATCH 6/7] Switch boolean setting to string --- client/rest-high-level/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/rest-high-level/build.gradle b/client/rest-high-level/build.gradle index 8c05efd23aa3c..0cdcf414f730f 100644 --- a/client/rest-high-level/build.gradle +++ b/client/rest-high-level/build.gradle @@ -126,7 +126,7 @@ testClusters.all { setting 'xpack.security.enabled', 'true' setting 'xpack.security.authc.token.enabled', 'true' setting 'xpack.security.authc.api_key.enabled', 'true' - setting 'xpack.security.http.ssl.enabled', false + setting 'xpack.security.http.ssl.enabled', 'false' // Truststore settings are not used since TLS is not enabled. Included for testing the get certificates API setting 'xpack.security.http.ssl.certificate_authorities', 'testnode.crt' setting 'xpack.security.transport.ssl.truststore.path', 'testnode.jks' From fd7a95e2eff64c4f713344775049a6289e8ce01c Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 5 Nov 2019 01:44:38 -0500 Subject: [PATCH 7/7] Add transport.ssl.enabled to HLRC build --- client/rest-high-level/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/client/rest-high-level/build.gradle b/client/rest-high-level/build.gradle index 0cdcf414f730f..9b4a3454ba31c 100644 --- a/client/rest-high-level/build.gradle +++ b/client/rest-high-level/build.gradle @@ -127,6 +127,7 @@ testClusters.all { setting 'xpack.security.authc.token.enabled', 'true' setting 'xpack.security.authc.api_key.enabled', 'true' setting 'xpack.security.http.ssl.enabled', 'false' + setting 'xpack.security.transport.ssl.enabled', 'false' // Truststore settings are not used since TLS is not enabled. Included for testing the get certificates API setting 'xpack.security.http.ssl.certificate_authorities', 'testnode.crt' setting 'xpack.security.transport.ssl.truststore.path', 'testnode.jks'