Skip to content

Commit ece9926

Browse files
committed
Remove insecure TLS trust-all mode
1 parent 22b6c46 commit ece9926

3 files changed

Lines changed: 26 additions & 30 deletions

File tree

src/main/java/dev/krotname/networkchat/network/ChatSockets.java

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package dev.krotname.networkchat.network;
22

3-
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
43
import java.io.IOException;
54
import java.io.InputStream;
65
import java.net.ServerSocket;
@@ -15,7 +14,6 @@
1514
import javax.net.ssl.SSLContext;
1615
import javax.net.ssl.TrustManager;
1716
import javax.net.ssl.TrustManagerFactory;
18-
import javax.net.ssl.X509TrustManager;
1917

2018
/** Socket factory helpers for plain TCP and optional JSSE TLS mode. */
2119
public final class ChatSockets {
@@ -68,9 +66,6 @@ private static SSLContext clientSslContext(TlsClientConfig config)
6866

6967
private static TrustManager[] trustManagers(TlsClientConfig config)
7068
throws IOException, GeneralSecurityException {
71-
if (config.trustAllCertificates()) {
72-
return new TrustManager[] {new InsecureTrustAllManager()};
73-
}
7469
if (config.trustStoreFile() == null) {
7570
return null;
7671
}
@@ -89,20 +84,4 @@ private static KeyStore loadKeyStore(Path file, String password)
8984
}
9085
return keyStore;
9186
}
92-
93-
@SuppressFBWarnings(
94-
value = "WEAK_TRUST_MANAGER",
95-
justification = "Explicit opt-in development mode; production docs require a truststore.")
96-
private static final class InsecureTrustAllManager implements X509TrustManager {
97-
@Override
98-
public void checkClientTrusted(java.security.cert.X509Certificate[] chain, String authType) {}
99-
100-
@Override
101-
public void checkServerTrusted(java.security.cert.X509Certificate[] chain, String authType) {}
102-
103-
@Override
104-
public java.security.cert.X509Certificate[] getAcceptedIssuers() {
105-
return new java.security.cert.X509Certificate[0];
106-
}
107-
}
10887
}

src/main/java/dev/krotname/networkchat/network/TlsClientConfig.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,14 @@
55
import java.util.Map;
66

77
/** Client-side TLS settings resolved from environment variables or tests. */
8-
public record TlsClientConfig(
9-
boolean enabled, Path trustStoreFile, String trustStorePassword, boolean trustAllCertificates) {
8+
public record TlsClientConfig(boolean enabled, Path trustStoreFile, String trustStorePassword) {
109
public static final String ENV_TLS_ENABLED = "NETWORK_CHAT_TLS";
1110
public static final String ENV_TRUSTSTORE = "NETWORK_CHAT_TRUSTSTORE";
1211
public static final String ENV_TRUSTSTORE_PASSWORD = "NETWORK_CHAT_TRUSTSTORE_PASSWORD";
1312
public static final String ENV_TRUST_ALL = "NETWORK_CHAT_TLS_TRUST_ALL";
1413

1514
public static TlsClientConfig disabled() {
16-
return new TlsClientConfig(false, null, "", false);
15+
return new TlsClientConfig(false, null, "");
1716
}
1817

1918
public static TlsClientConfig fromEnvironment() {
@@ -22,11 +21,14 @@ public static TlsClientConfig fromEnvironment() {
2221

2322
public static TlsClientConfig fromEnvironment(Map<String, String> environment) {
2423
boolean enabled = booleanEnvironment(environment, ENV_TLS_ENABLED);
25-
boolean trustAll = booleanEnvironment(environment, ENV_TRUST_ALL);
24+
if (booleanEnvironment(environment, ENV_TRUST_ALL)) {
25+
throw new IllegalArgumentException(
26+
ENV_TRUST_ALL + " is not supported; configure " + ENV_TRUSTSTORE + " instead.");
27+
}
2628
String trustStore = environment.get(ENV_TRUSTSTORE);
2729
Path trustStoreFile = trustStore == null || trustStore.isBlank() ? null : Path.of(trustStore);
2830
return new TlsClientConfig(
29-
enabled, trustStoreFile, environment.getOrDefault(ENV_TRUSTSTORE_PASSWORD, ""), trustAll);
31+
enabled, trustStoreFile, environment.getOrDefault(ENV_TRUSTSTORE_PASSWORD, ""));
3032
}
3133

3234
private static boolean booleanEnvironment(Map<String, String> environment, String name) {

src/test/java/dev/krotname/networkchat/network/NetworkSecuritySupportTest.java

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertNotNull;
5+
import static org.junit.jupiter.api.Assertions.assertThrows;
56
import static org.junit.jupiter.api.Assertions.assertTrue;
67

78
import java.io.ByteArrayOutputStream;
@@ -29,14 +30,28 @@ void tlsClientConfigReadsEnvironmentMap() {
2930
TlsClientConfig.ENV_TRUSTSTORE,
3031
"chat.p12",
3132
TlsClientConfig.ENV_TRUSTSTORE_PASSWORD,
32-
"changeit",
33-
TlsClientConfig.ENV_TRUST_ALL,
34-
"1"));
33+
"changeit"));
3534

3635
assertTrue(config.enabled());
3736
assertEquals(Path.of("chat.p12"), config.trustStoreFile());
3837
assertEquals("changeit", config.trustStorePassword());
39-
assertTrue(config.trustAllCertificates());
38+
}
39+
40+
@Test
41+
void tlsClientConfigRejectsTrustAllMode() {
42+
IllegalArgumentException error =
43+
assertThrows(
44+
IllegalArgumentException.class,
45+
() ->
46+
TlsClientConfig.fromEnvironment(
47+
Map.of(
48+
TlsClientConfig.ENV_TLS_ENABLED,
49+
"true",
50+
TlsClientConfig.ENV_TRUST_ALL,
51+
"1")));
52+
53+
assertTrue(error.getMessage().contains(TlsClientConfig.ENV_TRUST_ALL));
54+
assertTrue(error.getMessage().contains(TlsClientConfig.ENV_TRUSTSTORE));
4055
}
4156

4257
@Test

0 commit comments

Comments
 (0)