security: use SSLParameters to set ciphers/protocols/client auth
This change moves to using SSLParameters as the configuration source for SSLEngine and SSLSocket objects that are configured by the SSLService. Previously we used a mix of specific methods and SSLParameters, which resulted in issues where ordering of calls is important. For example, if configuring client authentication directly on the engine prior to setting the SSLParameters resulted in the client authentication configuration being reset to the default. Additionally, this change also sets use cipher suite order to true to ensure preferred ciphers will be used. Original commit: elastic/x-pack-elasticsearch@8ddecdc20c
This commit is contained in:
parent
288f682fee
commit
6c587330fd
|
@ -5,7 +5,7 @@
|
|||
*/
|
||||
package org.elasticsearch.xpack.ssl;
|
||||
|
||||
import javax.net.ssl.SSLEngine;
|
||||
import javax.net.ssl.SSLParameters;
|
||||
import java.util.Locale;
|
||||
|
||||
/**
|
||||
|
@ -18,10 +18,10 @@ public enum SSLClientAuth {
|
|||
return false;
|
||||
}
|
||||
|
||||
public void configure(SSLEngine engine) {
|
||||
public void configure(SSLParameters sslParameters) {
|
||||
// nothing to do here
|
||||
assert !engine.getWantClientAuth();
|
||||
assert !engine.getNeedClientAuth();
|
||||
assert !sslParameters.getWantClientAuth();
|
||||
assert !sslParameters.getNeedClientAuth();
|
||||
}
|
||||
},
|
||||
OPTIONAL() {
|
||||
|
@ -29,8 +29,8 @@ public enum SSLClientAuth {
|
|||
return true;
|
||||
}
|
||||
|
||||
public void configure(SSLEngine engine) {
|
||||
engine.setWantClientAuth(true);
|
||||
public void configure(SSLParameters sslParameters) {
|
||||
sslParameters.setWantClientAuth(true);
|
||||
}
|
||||
},
|
||||
REQUIRED() {
|
||||
|
@ -38,8 +38,8 @@ public enum SSLClientAuth {
|
|||
return true;
|
||||
}
|
||||
|
||||
public void configure(SSLEngine engine) {
|
||||
engine.setNeedClientAuth(true);
|
||||
public void configure(SSLParameters sslParameters) {
|
||||
sslParameters.setNeedClientAuth(true);
|
||||
}
|
||||
};
|
||||
|
||||
|
@ -49,9 +49,9 @@ public enum SSLClientAuth {
|
|||
public abstract boolean enabled();
|
||||
|
||||
/**
|
||||
* Configure client authentication of the provided {@link SSLEngine}
|
||||
* Configure client authentication of the provided {@link SSLParameters}
|
||||
*/
|
||||
public abstract void configure(SSLEngine engine);
|
||||
public abstract void configure(SSLParameters sslParameters);
|
||||
|
||||
public static SSLClientAuth parse(String value) {
|
||||
assert value != null;
|
||||
|
|
|
@ -193,32 +193,22 @@ public class SSLService extends AbstractComponent {
|
|||
SSLContext sslContext = sslContext(configuration);
|
||||
SSLEngine sslEngine = sslContext.createSSLEngine(host, port);
|
||||
String[] ciphers = supportedCiphers(sslEngine.getSupportedCipherSuites(), configuration.cipherSuites(), false);
|
||||
try {
|
||||
sslEngine.setEnabledCipherSuites(ciphers);
|
||||
} catch (ElasticsearchException e) {
|
||||
throw e;
|
||||
} catch (Exception e) {
|
||||
throw new IllegalArgumentException("failed loading cipher suites " + Arrays.toString(ciphers), e);
|
||||
}
|
||||
|
||||
String[] supportedProtocols = configuration.supportedProtocols().toArray(Strings.EMPTY_ARRAY);
|
||||
try {
|
||||
sslEngine.setEnabledProtocols(supportedProtocols);
|
||||
} catch (IllegalArgumentException e) {
|
||||
throw new IllegalArgumentException("failed setting supported protocols " + Arrays.toString(supportedProtocols), e);
|
||||
}
|
||||
|
||||
SSLParameters parameters = new SSLParameters(ciphers, supportedProtocols);
|
||||
if (configuration.verificationMode().isHostnameVerificationEnabled() && host != null) {
|
||||
// By default, a SSLEngine will not perform hostname verification. In order to perform hostname verification
|
||||
// we need to specify a EndpointIdentificationAlgorithm. We use the HTTPS algorithm to prevent against
|
||||
// man in the middle attacks for all of our connections.
|
||||
SSLParameters parameters = new SSLParameters();
|
||||
parameters.setEndpointIdentificationAlgorithm("HTTPS");
|
||||
sslEngine.setSSLParameters(parameters);
|
||||
}
|
||||
// we use the cipher suite order so that we can prefer the ciphers we set first in the list
|
||||
parameters.setUseCipherSuitesOrder(true);
|
||||
configuration.sslClientAuth().configure(parameters);
|
||||
|
||||
// TODO configure using SSLParameters
|
||||
configuration.sslClientAuth().configure(sslEngine);
|
||||
// many SSLEngine options can be configured using either SSLParameters or direct methods on the engine itself, but there is one
|
||||
// tricky aspect; if you set a value directly on the engine and then later set the SSLParameters the value set directly on the
|
||||
// engine will be overwritten by the value in the SSLParameters
|
||||
sslEngine.setSSLParameters(parameters);
|
||||
return sslEngine;
|
||||
}
|
||||
|
||||
|
@ -493,8 +483,10 @@ public class SSLService extends AbstractComponent {
|
|||
}
|
||||
|
||||
private void configureSSLSocket(SSLSocket socket) {
|
||||
socket.setEnabledProtocols(supportedProtocols);
|
||||
socket.setEnabledCipherSuites(ciphers);
|
||||
SSLParameters parameters = new SSLParameters(ciphers, supportedProtocols);
|
||||
// we use the cipher suite order so that we can prefer the ciphers we set first in the list
|
||||
parameters.setUseCipherSuitesOrder(true);
|
||||
socket.setSSLParameters(parameters);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -283,6 +283,17 @@ public class SSLServiceTests extends ESTestCase {
|
|||
assertThat(e.getMessage(), is("none of the ciphers [foo, bar] are supported by this JVM"));
|
||||
}
|
||||
|
||||
public void testThatSSLEngineHasCipherSuitesOrderSet() throws Exception {
|
||||
Settings settings = Settings.builder()
|
||||
.put("xpack.ssl.keystore.path", testnodeStore)
|
||||
.put("xpack.ssl.keystore.password", "testnode")
|
||||
.build();
|
||||
SSLService sslService = new SSLService(settings, env);
|
||||
SSLEngine engine = sslService.createSSLEngine(Settings.EMPTY, Settings.EMPTY);
|
||||
assertThat(engine, is(notNullValue()));
|
||||
assertTrue(engine.getSSLParameters().getUseCipherSuitesOrder());
|
||||
}
|
||||
|
||||
public void testThatSSLSocketFactoryHasProperCiphersAndProtocols() throws Exception {
|
||||
Settings settings = Settings.builder()
|
||||
.put("xpack.ssl.keystore.path", testnodeStore)
|
||||
|
@ -294,13 +305,34 @@ public class SSLServiceTests extends ESTestCase {
|
|||
final String[] ciphers = sslService.supportedCiphers(factory.getSupportedCipherSuites(), config.cipherSuites(), false);
|
||||
assertThat(factory.getDefaultCipherSuites(), is(ciphers));
|
||||
|
||||
final String[] supportedProtocols = config.supportedProtocols().toArray(Strings.EMPTY_ARRAY);
|
||||
try (SSLSocket socket = (SSLSocket) factory.createSocket()) {
|
||||
assertThat(socket.getEnabledCipherSuites(), is(ciphers));
|
||||
// the order we set the protocols in is not going to be what is returned as internally the JDK may sort the versions
|
||||
assertThat(socket.getEnabledProtocols(), arrayContainingInAnyOrder(config.supportedProtocols().toArray(Strings.EMPTY_ARRAY)));
|
||||
assertThat(socket.getEnabledProtocols(), arrayContainingInAnyOrder(supportedProtocols));
|
||||
assertArrayEquals(ciphers, socket.getSSLParameters().getCipherSuites());
|
||||
assertThat(socket.getSSLParameters().getProtocols(), arrayContainingInAnyOrder(supportedProtocols));
|
||||
assertTrue(socket.getSSLParameters().getUseCipherSuitesOrder());
|
||||
}
|
||||
}
|
||||
|
||||
public void testThatSSLEngineHasProperCiphersAndProtocols() throws Exception {
|
||||
Settings settings = Settings.builder()
|
||||
.put("xpack.ssl.keystore.path", testnodeStore)
|
||||
.put("xpack.ssl.keystore.password", "testnode")
|
||||
.build();
|
||||
SSLService sslService = new SSLService(settings, env);
|
||||
SSLEngine engine = sslService.createSSLEngine(Settings.EMPTY, Settings.EMPTY);
|
||||
SSLConfiguration config = sslService.sslConfiguration(Settings.EMPTY);
|
||||
final String[] ciphers = sslService.supportedCiphers(engine.getSupportedCipherSuites(), config.cipherSuites(), false);
|
||||
final String[] supportedProtocols = config.supportedProtocols().toArray(Strings.EMPTY_ARRAY);
|
||||
assertThat(engine.getEnabledCipherSuites(), is(ciphers));
|
||||
assertArrayEquals(ciphers, engine.getSSLParameters().getCipherSuites());
|
||||
// the order we set the protocols in is not going to be what is returned as internally the JDK may sort the versions
|
||||
assertThat(engine.getEnabledProtocols(), arrayContainingInAnyOrder(supportedProtocols));
|
||||
assertThat(engine.getSSLParameters().getProtocols(), arrayContainingInAnyOrder(supportedProtocols));
|
||||
}
|
||||
|
||||
public void testSSLStrategy() {
|
||||
// this just exhaustively verifies that the right things are called and that it uses the right parameters
|
||||
Settings settings = Settings.builder().build();
|
||||
|
|
Loading…
Reference in New Issue