Skip to content

Commit 499b1e8

Browse files
tvernumtvernum
committed
Move SSLIOSessionStrategy creation to a builder
1 parent 4ce6ee8 commit 499b1e8

File tree

4 files changed

+186
-114
lines changed

4 files changed

+186
-114
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLIOSessionStrategyBuilder.java‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.core.ssl;
9+
10+
import org.apache.http.HttpHost;
11+
import org.apache.http.conn.ssl.NoopHostnameVerifier;
12+
import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy;
13+
import org.apache.http.nio.reactor.IOSession;
14+
import org.elasticsearch.common.Strings;
15+
import org.elasticsearch.common.logging.LoggerMessageFormat;
16+
import org.elasticsearch.common.settings.Settings;
17+
import org.elasticsearch.common.ssl.SslConfiguration;
18+
import org.elasticsearch.common.ssl.SslDiagnostics;
19+
20+
import javax.net.ssl.HostnameVerifier;
21+
import javax.net.ssl.SSLContext;
22+
import javax.net.ssl.SSLException;
23+
import javax.net.ssl.SSLParameters;
24+
import javax.net.ssl.SSLPeerUnverifiedException;
25+
import javax.net.ssl.SSLSession;
26+
import javax.security.auth.x500.X500Principal;
27+
28+
import java.security.cert.Certificate;
29+
import java.security.cert.X509Certificate;
30+
import java.util.List;
31+
32+
public class SSLIOSessionStrategyBuilder {
33+
34+
public static final SSLIOSessionStrategyBuilder INSTANCE = new SSLIOSessionStrategyBuilder();
35+
36+
public SSLIOSessionStrategy sslIOSessionStrategy(SslConfiguration config, SSLContext sslContext) {
37+
String[] ciphers = supportedCiphers(sslParameters(sslContext).getCipherSuites(), config.getCipherSuites(), false);
38+
String[] supportedProtocols = config.supportedProtocols().toArray(Strings.EMPTY_ARRAY);
39+
HostnameVerifier verifier;
40+
41+
if (config.verificationMode().isHostnameVerificationEnabled()) {
42+
verifier = SSLIOSessionStrategy.getDefaultHostnameVerifier();
43+
} else {
44+
verifier = NoopHostnameVerifier.INSTANCE;
45+
}
46+
47+
return sslIOSessionStrategy(sslContext, supportedProtocols, ciphers, verifier);
48+
}
49+
50+
/**
51+
* This method exists to simplify testing
52+
*/
53+
String[] supportedCiphers(String[] supportedCiphers, List<String> requestedCiphers, boolean log) {
54+
return SSLService.supportedCiphers(supportedCiphers, requestedCiphers, log);
55+
}
56+
57+
/**
58+
* The {@link SSLParameters} that are associated with the {@code sslContext}.
59+
* <p>
60+
* This method exists to simplify testing since {@link SSLContext#getSupportedSSLParameters()} is {@code final}.
61+
*
62+
* @param sslContext The SSL context for the current SSL settings
63+
* @return Never {@code null}.
64+
*/
65+
SSLParameters sslParameters(SSLContext sslContext) {
66+
return sslContext.getSupportedSSLParameters();
67+
}
68+
69+
/**
70+
* This method only exists to simplify testing because {@link SSLIOSessionStrategy} does
71+
* not expose any of the parameters that you give it.
72+
*/
73+
SSLIOSessionStrategy sslIOSessionStrategy(SSLContext sslContext, String[] protocols, String[] ciphers, HostnameVerifier verifier) {
74+
return new SSLIOSessionStrategy(sslContext, protocols, ciphers, verifier) {
75+
@Override
76+
protected void verifySession(HttpHost host, IOSession iosession, SSLSession session) throws SSLException {
77+
if (verifier.verify(host.getHostName(), session) == false) {
78+
final Certificate[] certs = session.getPeerCertificates();
79+
final X509Certificate x509 = (X509Certificate) certs[0];
80+
final X500Principal x500Principal = x509.getSubjectX500Principal();
81+
final String altNames = Strings.collectionToCommaDelimitedString(SslDiagnostics.describeValidHostnames(x509));
82+
throw new SSLPeerUnverifiedException(
83+
LoggerMessageFormat.format(
84+
"Expected SSL certificate to be valid for host [{}],"
85+
+ " but it is only valid for subject alternative names [{}] and subject [{}]",
86+
new Object[] { host.getHostName(), altNames, x500Principal.toString() }
87+
)
88+
);
89+
}
90+
}
91+
};
92+
}
93+
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java‎

Lines changed: 3 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -240,64 +240,7 @@ public SslProfile profile(String profileName) {
240240
@Deprecated
241241
public SSLIOSessionStrategy sslIOSessionStrategy(Settings settingsToUse) {
242242
SslConfiguration config = sslConfiguration(settingsToUse);
243-
return sslIOSessionStrategy(config, sslContext(config));
244-
}
245-
246-
SSLIOSessionStrategy sslIOSessionStrategy(SslConfiguration config, SSLContext sslContext) {
247-
String[] ciphers = supportedCiphers(sslParameters(sslContext).getCipherSuites(), config.getCipherSuites(), false);
248-
String[] supportedProtocols = config.supportedProtocols().toArray(Strings.EMPTY_ARRAY);
249-
HostnameVerifier verifier;
250-
251-
if (config.verificationMode().isHostnameVerificationEnabled()) {
252-
verifier = SSLIOSessionStrategy.getDefaultHostnameVerifier();
253-
} else {
254-
verifier = NoopHostnameVerifier.INSTANCE;
255-
}
256-
257-
return sslIOSessionStrategy(sslContext, supportedProtocols, ciphers, verifier);
258-
}
259-
260-
/**
261-
* The {@link SSLParameters} that are associated with the {@code sslContext}.
262-
* <p>
263-
* This method exists to simplify testing since {@link SSLContext#getSupportedSSLParameters()} is {@code final}.
264-
*
265-
* @param sslContext The SSL context for the current SSL settings
266-
* @return Never {@code null}.
267-
*/
268-
SSLParameters sslParameters(SSLContext sslContext) {
269-
return sslContext.getSupportedSSLParameters();
270-
}
271-
272-
/**
273-
* This method only exists to simplify testing of {@link #sslIOSessionStrategy(Settings)} because {@link SSLIOSessionStrategy} does
274-
* not expose any of the parameters that you give it.
275-
*
276-
* @param sslContext SSL Context used to handle SSL / TCP requests
277-
* @param protocols Supported protocols
278-
* @param ciphers Supported ciphers
279-
* @param verifier Hostname verifier
280-
* @return Never {@code null}.
281-
*/
282-
SSLIOSessionStrategy sslIOSessionStrategy(SSLContext sslContext, String[] protocols, String[] ciphers, HostnameVerifier verifier) {
283-
return new SSLIOSessionStrategy(sslContext, protocols, ciphers, verifier) {
284-
@Override
285-
protected void verifySession(HttpHost host, IOSession iosession, SSLSession session) throws SSLException {
286-
if (verifier.verify(host.getHostName(), session) == false) {
287-
final Certificate[] certs = session.getPeerCertificates();
288-
final X509Certificate x509 = (X509Certificate) certs[0];
289-
final X500Principal x500Principal = x509.getSubjectX500Principal();
290-
final String altNames = Strings.collectionToCommaDelimitedString(SslDiagnostics.describeValidHostnames(x509));
291-
throw new SSLPeerUnverifiedException(
292-
LoggerMessageFormat.format(
293-
"Expected SSL certificate to be valid for host [{}],"
294-
+ " but it is only valid for subject alternative names [{}] and subject [{}]",
295-
new Object[] { host.getHostName(), altNames, x500Principal.toString() }
296-
)
297-
);
298-
}
299-
}
300-
};
243+
return SSLIOSessionStrategyBuilder.INSTANCE.sslIOSessionStrategy(config, sslContext(config));
301244
}
302245

303246
/**
@@ -374,7 +317,7 @@ Collection<SslConfiguration> getLoadedSslConfigurations() {
374317
*
375318
* @throws IllegalArgumentException if no supported ciphers are in the requested ciphers
376319
*/
377-
String[] supportedCiphers(String[] supportedCiphers, List<String> requestedCiphers, boolean log) {
320+
static String[] supportedCiphers(String[] supportedCiphers, List<String> requestedCiphers, boolean log) {
378321
List<String> supportedCiphersList = new ArrayList<>(requestedCiphers.size());
379322
List<String> unsupportedCiphers = new LinkedList<>();
380323
boolean found;
@@ -795,7 +738,7 @@ public SSLConnectionSocketFactory connectionSocketFactory() {
795738

796739
@Override
797740
public SSLIOSessionStrategy ioSessionStrategy4() {
798-
return sslIOSessionStrategy(this.sslConfiguration, context);
741+
return SSLIOSessionStrategyBuilder.INSTANCE.sslIOSessionStrategy(this.sslConfiguration, context);
799742
}
800743

801744
@Override

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLIOSessionStrategyBuilderTests.java‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.core.ssl;
9+
10+
import junit.framework.TestCase;
11+
12+
import org.apache.http.conn.ssl.DefaultHostnameVerifier;
13+
import org.apache.http.conn.ssl.NoopHostnameVerifier;
14+
import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy;
15+
import org.elasticsearch.common.settings.Settings;
16+
import org.elasticsearch.common.ssl.SslConfiguration;
17+
import org.elasticsearch.common.ssl.SslVerificationMode;
18+
import org.elasticsearch.env.TestEnvironment;
19+
import org.elasticsearch.test.ESTestCase;
20+
import org.mockito.Mockito;
21+
22+
import javax.net.ssl.HostnameVerifier;
23+
import javax.net.ssl.SSLContext;
24+
import javax.net.ssl.SSLParameters;
25+
26+
import java.util.List;
27+
28+
import static org.hamcrest.Matchers.instanceOf;
29+
import static org.hamcrest.Matchers.is;
30+
import static org.hamcrest.Matchers.sameInstance;
31+
import static org.mockito.ArgumentMatchers.any;
32+
import static org.mockito.ArgumentMatchers.anyList;
33+
import static org.mockito.Mockito.mock;
34+
import static org.mockito.Mockito.when;
35+
36+
public class SSLIOSessionStrategyBuilderTests extends ESTestCase {
37+
38+
public void testBuildSSLStrategy() {
39+
var env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", createTempDir()).build());
40+
// this just exhaustively verifies that the right things are called and that it uses the right parameters
41+
SslVerificationMode mode = randomFrom(SslVerificationMode.values());
42+
Settings settings = Settings.builder()
43+
.put("supported_protocols", "protocols")
44+
.put("cipher_suites", "INVALID_CIPHER")
45+
.put("verification_mode", mode.name())
46+
.build();
47+
SSLIOSessionStrategyBuilder builder = mock(SSLIOSessionStrategyBuilder.class);
48+
SslConfiguration sslConfig = SslSettingsLoader.load(settings, null, env);
49+
SSLParameters sslParameters = mock(SSLParameters.class);
50+
SSLContext sslContext = mock(SSLContext.class);
51+
String[] protocols = new String[] { "protocols" };
52+
String[] ciphers = new String[] { "ciphers!!!" };
53+
String[] supportedCiphers = new String[] { "supported ciphers" };
54+
List<String> requestedCiphers = List.of("INVALID_CIPHER");
55+
SSLIOSessionStrategy sslStrategy = mock(SSLIOSessionStrategy.class);
56+
57+
when(builder.supportedCiphers(any(String[].class), anyList(), any(Boolean.TYPE))).thenAnswer(inv -> {
58+
final Object[] args = inv.getArguments();
59+
assertThat(args[0], is(supportedCiphers));
60+
assertThat(args[1], is(requestedCiphers));
61+
assertThat(args[2], is(false));
62+
return ciphers;
63+
});
64+
when(builder.sslParameters(sslContext)).thenReturn(sslParameters);
65+
when(sslParameters.getCipherSuites()).thenReturn(supportedCiphers);
66+
67+
when(builder.sslIOSessionStrategy(any(SSLContext.class), any(String[].class), any(String[].class), any(HostnameVerifier.class)))
68+
.thenAnswer(inv -> {
69+
final Object[] args = inv.getArguments();
70+
assertThat(args[0], is(sslContext));
71+
assertThat(args[1], is(protocols));
72+
assertThat(args[2], is(ciphers));
73+
if (mode.isHostnameVerificationEnabled()) {
74+
assertThat(args[3], instanceOf(DefaultHostnameVerifier.class));
75+
} else {
76+
assertThat(args[3], sameInstance(NoopHostnameVerifier.INSTANCE));
77+
}
78+
return sslStrategy;
79+
});
80+
81+
when(builder.sslIOSessionStrategy(Mockito.eq(sslConfig), Mockito.any(SSLContext.class))).thenCallRealMethod();
82+
83+
final SslConfiguration config = new SSLService(env).sslConfiguration(settings);
84+
final SSLIOSessionStrategy actual = builder.sslIOSessionStrategy(config, sslContext);
85+
assertThat(actual, sameInstance(sslStrategy));
86+
}
87+
88+
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java‎

Lines changed: 2 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ public void testThatSSLSocketFactoryHasProperCiphersAndProtocols() throws Except
539539

540540
final SSLSocketFactory factory = profile.socketFactory();
541541
final SslConfiguration config = profile.configuration();
542-
final String[] ciphers = sslService.supportedCiphers(factory.getSupportedCipherSuites(), config.getCipherSuites(), false);
542+
final String[] ciphers = SSLService.supportedCiphers(factory.getSupportedCipherSuites(), config.getCipherSuites(), false);
543543
assertThat(factory.getDefaultCipherSuites(), is(ciphers));
544544

545545
final String[] getSupportedProtocols = config.supportedProtocols().toArray(Strings.EMPTY_ARRAY);
@@ -566,7 +566,7 @@ public void testThatSSLEngineHasProperCiphersAndProtocols() throws Exception {
566566
final SslProfile profile = sslService.profile("xpack.security.transport.ssl");
567567
final SSLEngine engine = profile.engine(null, -1);
568568
final SslConfiguration configuration = profile.configuration();
569-
final String[] ciphers = sslService.supportedCiphers(engine.getSupportedCipherSuites(), configuration.getCipherSuites(), false);
569+
final String[] ciphers = SSLService.supportedCiphers(engine.getSupportedCipherSuites(), configuration.getCipherSuites(), false);
570570
final String[] getSupportedProtocols = configuration.supportedProtocols().toArray(Strings.EMPTY_ARRAY);
571571
assertThat(engine.getEnabledCipherSuites(), is(ciphers));
572572
assertArrayEquals(ciphers, engine.getSSLParameters().getCipherSuites());
@@ -575,58 +575,6 @@ public void testThatSSLEngineHasProperCiphersAndProtocols() throws Exception {
575575
assertThat(engine.getSSLParameters().getProtocols(), arrayContainingInAnyOrder(getSupportedProtocols));
576576
}
577577

578-
public void testSSLStrategy() {
579-
// this just exhaustively verifies that the right things are called and that it uses the right parameters
580-
SslVerificationMode mode = randomFrom(SslVerificationMode.values());
581-
Settings settings = Settings.builder()
582-
.put("supported_protocols", "protocols")
583-
.put("cipher_suites", "INVALID_CIPHER")
584-
.put("verification_mode", mode.name())
585-
.build();
586-
SSLService sslService = mock(SSLService.class);
587-
SslConfiguration sslConfig = SslSettingsLoader.load(settings, null, env);
588-
SSLParameters sslParameters = mock(SSLParameters.class);
589-
SSLContext sslContext = mock(SSLContext.class);
590-
String[] protocols = new String[] { "protocols" };
591-
String[] ciphers = new String[] { "ciphers!!!" };
592-
String[] supportedCiphers = new String[] { "supported ciphers" };
593-
List<String> requestedCiphers = List.of("INVALID_CIPHER");
594-
SSLIOSessionStrategy sslStrategy = mock(SSLIOSessionStrategy.class);
595-
596-
when(sslService.sslConfiguration(settings)).thenReturn(sslConfig);
597-
when(sslService.sslContext(sslConfig)).thenReturn(sslContext);
598-
when(sslService.supportedCiphers(any(String[].class), anyList(), any(Boolean.TYPE))).thenAnswer(inv -> {
599-
final Object[] args = inv.getArguments();
600-
assertThat(args[0], is(supportedCiphers));
601-
assertThat(args[1], is(requestedCiphers));
602-
assertThat(args[2], is(false));
603-
return ciphers;
604-
});
605-
when(sslService.sslParameters(sslContext)).thenReturn(sslParameters);
606-
when(sslParameters.getCipherSuites()).thenReturn(supportedCiphers);
607-
608-
when(sslService.sslIOSessionStrategy(any(SSLContext.class), any(String[].class), any(String[].class), any(HostnameVerifier.class)))
609-
.thenAnswer(inv -> {
610-
final Object[] args = inv.getArguments();
611-
assertThat(args[0], is(sslContext));
612-
assertThat(args[1], is(protocols));
613-
assertThat(args[2], is(ciphers));
614-
if (mode.isHostnameVerificationEnabled()) {
615-
assertThat(args[3], instanceOf(DefaultHostnameVerifier.class));
616-
} else {
617-
assertThat(args[3], sameInstance(NoopHostnameVerifier.INSTANCE));
618-
}
619-
return sslStrategy;
620-
});
621-
622-
// ensure it actually goes through and calls the real method
623-
when(sslService.sslIOSessionStrategy(settings)).thenCallRealMethod();
624-
when(sslService.sslIOSessionStrategy(Mockito.eq(sslConfig), Mockito.any(SSLContext.class))).thenCallRealMethod();
625-
626-
final SSLIOSessionStrategy actual = sslService.sslIOSessionStrategy(settings);
627-
assertThat(actual, sameInstance(sslStrategy));
628-
}
629-
630578
public void testGetConfigurationByContextName() throws Exception {
631579
assumeFalse("Can't run in a FIPS JVM, JKS keystores can't be used", inFipsJvm());
632580
final SSLContext sslContext = SSLContext.getInstance("TLSv1.2");

0 commit comments

Comments
 (0)