Skip to content

Commit fe3e14a

Browse files
Bennett-LynchBennett-Lynch
authored
[Apache HTTP Client] Stop auto-enabling TLS protocol versions (#2934)
* [Apache HTTP Client] Stop auto-enabling TLS protocol versions Since 2014, in the Java SDK v1, the SDK's Apache HTTP Client has auto-enabled certain TLS protocol versions if they are supported by the local system. The versions auto-enabled are: `TLSv1.2`, `TLSv1.1`, `TLSv1`, and `TLS`. The original motivation was to always try to use the latest and most secure version. This made sense at the time, especially when both Java 6 and Java 7 defaulted to only `TLSv1.0`. It wasn't until Java 8 that the default became `TLSv1.2`. So there was some apparent value in the SDK nudging clients towards the latest version that they may otherwise be slow to adopt. Fast-forward 7+ years and most JVM vendors now enable newer TLS versions (`TLSv1.3`) by default and many of the above versions are now considered older and less secure, yet the SDK continues to auto-enable them. Furthermore, the Java SDK v2 requires Java 8 or higher, which has an implicit guarantee of defaulting to `TLSv1.2` or higher, meaning these SDK-enabled versions are never newer than default version. While the TLS negotiation will resolve to the highest mutually supported version, some users may still wish to explicitly disable older versions as an extra precaution. The recommended and conventional way to do this is via the `jdk.tls.client.protocols` system property, but the SDK's implementation will continue to append its own list of enabled versions to the user's declared list. The only way for users to ensure they disable those versions is via the `jdk.tls.disabledAlgorithms` system property. https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/security-java-tls.html https://java.com/en/configure_crypto.html Since the SDK's list of preferred TLS versions is no longer up to date, and any new preferences would be similarly subject to becoming stale in the future, the SDK should stop trying to offer opinions on the preferred TLS version to use and instead allow the JVM defaults and user-declared properties to fully control this behavior.
1 parent 3cd391d commit fe3e14a

File tree

5 files changed

+66
-306
lines changed

5 files changed

+66
-306
lines changed

‎.changes/next-release/bugfix-ApacheHTTPClient-631fb0d.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "Apache HTTP Client",
3+
"contributor": "",
4+
"type": "bugfix",
5+
"description": "Stop auto-enabling TLS protocol versions"
6+
}

‎http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/conn/SdkTlsSocketFactory.java

Lines changed: 2 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
import java.io.IOException;
1919
import java.net.InetSocketAddress;
2020
import java.net.Socket;
21-
import java.util.ArrayList;
2221
import java.util.Arrays;
23-
import java.util.List;
2422
import javax.net.ssl.HostnameVerifier;
2523
import javax.net.ssl.SSLContext;
2624
import javax.net.ssl.SSLSocket;
@@ -32,9 +30,6 @@
3230
import software.amazon.awssdk.http.apache.internal.net.SdkSslSocket;
3331
import software.amazon.awssdk.utils.Logger;
3432

35-
/**
36-
* Used to enforce the preferred TLS protocol during SSL handshake.
37-
*/
3833
@SdkInternalApi
3934
public class SdkTlsSocketFactory extends SSLConnectionSocketFactory {
4035

@@ -50,54 +45,11 @@ public SdkTlsSocketFactory(final SSLContext sslContext, final HostnameVerifier h
5045
this.sslContext = sslContext;
5146
}
5247

53-
/**
54-
* {@inheritDoc} Used to enforce the preferred TLS protocol during SSL handshake.
55-
*/
5648
@Override
5749
protected final void prepareSocket(final SSLSocket socket) {
58-
String[] supported = socket.getSupportedProtocols();
59-
String[] enabled = socket.getEnabledProtocols();
6050
log.debug(() -> String.format("socket.getSupportedProtocols(): %s, socket.getEnabledProtocols(): %s",
61-
Arrays.toString(supported),
62-
Arrays.toString(enabled)));
63-
List<String> target = new ArrayList<>();
64-
if (supported != null) {
65-
// Append the preferred protocols in descending order of preference
66-
// but only do so if the protocols are supported
67-
TlsProtocol[] values = TlsProtocol.values();
68-
for (TlsProtocol value : values) {
69-
String pname = value.getProtocolName();
70-
if (existsIn(pname, supported)) {
71-
target.add(pname);
72-
}
73-
}
74-
}
75-
if (enabled != null) {
76-
// Append the rest of the already enabled protocols to the end
77-
// if not already included in the list
78-
for (String pname : enabled) {
79-
if (!target.contains(pname)) {
80-
target.add(pname);
81-
}
82-
}
83-
}
84-
if (target.size() > 0) {
85-
String[] enabling = target.toArray(new String[0]);
86-
socket.setEnabledProtocols(enabling);
87-
log.debug(() -> "TLS protocol enabled for SSL handshake: " + Arrays.toString(enabling));
88-
}
89-
}
90-
91-
/**
92-
* Returns true if the given element exists in the given array; false otherwise.
93-
*/
94-
private boolean existsIn(String element, String[] a) {
95-
for (String s : a) {
96-
if (element.equals(s)) {
97-
return true;
98-
}
99-
}
100-
return false;
51+
Arrays.toString(socket.getSupportedProtocols()),
52+
Arrays.toString(socket.getEnabledProtocols())));
10153
}
10254

10355
@Override

‎http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/conn/TlsProtocol.java

Lines changed: 0 additions & 49 deletions
This file was deleted.

‎http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/conn/SdkTlsSocketFactoryTest.java

Lines changed: 58 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -15,119 +15,77 @@
1515

1616
package software.amazon.awssdk.http.apache.internal.conn;
1717

18-
import static org.junit.Assert.assertTrue;
19-
import static org.junit.Assert.fail;
20-
21-
import java.io.IOException;
22-
import java.security.NoSuchAlgorithmException;
23-
import java.util.ArrayList;
24-
import java.util.Arrays;
25-
import java.util.Collections;
26-
import java.util.List;
18+
import static org.mockito.Matchers.any;
19+
import static org.mockito.Mockito.never;
20+
import static org.mockito.Mockito.verify;
21+
import static org.mockito.Mockito.when;
22+
2723
import javax.net.ssl.SSLContext;
2824
import javax.net.ssl.SSLSocket;
29-
import org.junit.Test;
25+
import org.junit.jupiter.api.BeforeEach;
26+
import org.junit.jupiter.api.Test;
27+
import org.mockito.Mockito;
3028

31-
public class SdkTlsSocketFactoryTest {
32-
/**
33-
* Test when the edge case when the both supported and enabled protocols are null.
34-
*/
35-
@Test
36-
public void preparedSocket_NullProtocols() throws NoSuchAlgorithmException, IOException {
37-
SdkTlsSocketFactory f = new SdkTlsSocketFactory(SSLContext.getDefault(), null);
38-
try (SSLSocket socket = new TestSSLSocket() {
39-
@Override
40-
public String[] getSupportedProtocols() {
41-
return null;
42-
}
43-
44-
@Override
45-
public String[] getEnabledProtocols() {
46-
return null;
47-
}
48-
49-
@Override
50-
public void setEnabledProtocols(String[] protocols) {
51-
fail();
52-
}
53-
}) {
54-
f.prepareSocket(socket);
55-
}
29+
class SdkTlsSocketFactoryTest {
30+
31+
SdkTlsSocketFactory factory;
32+
SSLSocket socket;
33+
34+
@BeforeEach
35+
public void before() throws Exception {
36+
factory = new SdkTlsSocketFactory(SSLContext.getDefault(), null);
37+
socket = Mockito.mock(SSLSocket.class);
5638
}
5739

5840
@Test
59-
public void typical() throws NoSuchAlgorithmException, IOException {
60-
SdkTlsSocketFactory f = new SdkTlsSocketFactory(SSLContext.getDefault(), null);
61-
try (SSLSocket socket = new TestSSLSocket() {
62-
@Override
63-
public String[] getSupportedProtocols() {
64-
return shuffle(new String[] {"SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1", "TLSv1.2"});
65-
}
66-
67-
@Override
68-
public String[] getEnabledProtocols() {
69-
return shuffle(new String[] {"SSLv3", "TLSv1"});
70-
}
71-
72-
@Override
73-
public void setEnabledProtocols(String[] protocols) {
74-
assertTrue(Arrays.equals(protocols, new String[] {"TLSv1.2", "TLSv1.1", "TLSv1", "SSLv3"}));
75-
}
76-
}) {
77-
f.prepareSocket(socket);
78-
}
41+
void nullProtocols() {
42+
when(socket.getSupportedProtocols()).thenReturn(null);
43+
when(socket.getEnabledProtocols()).thenReturn(null);
44+
45+
factory.prepareSocket(socket);
46+
47+
verify(socket, never()).setEnabledProtocols(any());
7948
}
8049

8150
@Test
82-
public void noTLS() throws NoSuchAlgorithmException, IOException {
83-
SdkTlsSocketFactory f = new SdkTlsSocketFactory(SSLContext.getDefault(), null);
84-
try (SSLSocket socket = new TestSSLSocket() {
85-
@Override
86-
public String[] getSupportedProtocols() {
87-
return shuffle(new String[] {"SSLv2Hello", "SSLv3"});
88-
}
89-
90-
@Override
91-
public String[] getEnabledProtocols() {
92-
return new String[] {"SSLv3"};
93-
}
94-
95-
@Override
96-
public void setEnabledProtocols(String[] protocols) {
97-
// For backward compatibility
98-
assertTrue(Arrays.equals(protocols, new String[] {"SSLv3"}));
99-
}
100-
}) {
101-
f.prepareSocket(socket);
102-
}
51+
void amazonCorretto_8_0_292_defaultEnabledProtocols() {
52+
when(socket.getSupportedProtocols()).thenReturn(new String[] {
53+
"TLSv1.3", "TLSv1.2", "TLSv1.1", "TLSv1", "SSLv3", "SSLv2Hello"
54+
});
55+
when(socket.getEnabledProtocols()).thenReturn(new String[] {
56+
"TLSv1.2", "TLSv1.1", "TLSv1"
57+
});
58+
59+
factory.prepareSocket(socket);
60+
61+
verify(socket, never()).setEnabledProtocols(any());
10362
}
10463

10564
@Test
106-
public void notIdeal() throws NoSuchAlgorithmException, IOException {
107-
SdkTlsSocketFactory f = new SdkTlsSocketFactory(SSLContext.getDefault(), null);
108-
try (SSLSocket socket = new TestSSLSocket() {
109-
@Override
110-
public String[] getSupportedProtocols() {
111-
return shuffle(new String[] {"SSLv2Hello", "SSLv3", "TLSv1", "TLSv1.1"});
112-
}
113-
114-
@Override
115-
public String[] getEnabledProtocols() {
116-
return shuffle(new String[] {"SSLv3", "TLSv1"});
117-
}
118-
119-
@Override
120-
public void setEnabledProtocols(String[] protocols) {
121-
assertTrue(Arrays.equals(protocols, new String[] {"TLSv1.1", "TLSv1", "SSLv3"}));
122-
}
123-
}) {
124-
f.prepareSocket(socket);
125-
}
65+
void amazonCorretto_11_0_08_defaultEnabledProtocols() {
66+
when(socket.getSupportedProtocols()).thenReturn(new String[] {
67+
"TLSv1.3", "TLSv1.2", "TLSv1.1", "TLSv1", "SSLv3", "SSLv2Hello"
68+
});
69+
when(socket.getEnabledProtocols()).thenReturn(new String[] {
70+
"TLSv1.3", "TLSv1.2", "TLSv1.1", "TLSv1"
71+
});
72+
73+
factory.prepareSocket(socket);
74+
75+
verify(socket, never()).setEnabledProtocols(any());
12676
}
12777

128-
private String[] shuffle(String[] in) {
129-
List<String> list = new ArrayList<String>(Arrays.asList(in));
130-
Collections.shuffle(list);
131-
return list.toArray(new String[0]);
78+
@Test
79+
void amazonCorretto_17_0_1_defaultEnabledProtocols() {
80+
when(socket.getSupportedProtocols()).thenReturn(new String[] {
81+
"TLSv1.3", "TLSv1.2", "TLSv1.1", "TLSv1", "SSLv3", "SSLv2Hello"
82+
});
83+
when(socket.getEnabledProtocols()).thenReturn(new String[] {
84+
"TLSv1.3", "TLSv1.2"
85+
});
86+
87+
factory.prepareSocket(socket);
88+
89+
verify(socket, never()).setEnabledProtocols(any());
13290
}
13391
}

0 commit comments

Comments
 (0)