Skip to content

Commit 7ad34ea

Browse files
committed
The SslChannelProvider class maintains a map of server name to Netty SslContext that is filled when a client provides a server name. When a server name does not resolve to a KeyManagerFactory or TrustManagerFactory, the default factories are used and the entry is stored in the map. Instead no specific factory is resolved the default Netty SslContext is used, since this can lead to a a memory leak when a client specifies spurious SNI server names. This affects only a TCP server when SNI is set in the HttpServerOptions.
1 parent ea749f6 commit 7ad34ea

File tree

5 files changed

+39
-40
lines changed

5 files changed

+39
-40
lines changed

src/main/java/io/vertx/core/net/impl/SSLHelper.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ public SSLHelper(TCPSSLOptions options, List<String> applicationProtocols) {
126126
this.applicationProtocols = applicationProtocols;
127127
}
128128

129+
public synchronized int sniEntrySize() {
130+
CachedProvider res = cachedProvider.result();
131+
if (res != null) {
132+
return res.sslChannelProvider.sniEntrySize();
133+
}
134+
return 0;
135+
}
136+
129137
private static class CachedProvider {
130138
final SSLOptions options;
131139
final long id;

src/main/java/io/vertx/core/net/impl/SslChannelProvider.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ public SslChannelProvider(SslContextProvider sslContextProvider,
6565
this.sslContextProvider = sslContextProvider;
6666
}
6767

68+
public int sniEntrySize() {
69+
return sslContextMaps[0].size() + sslContextMaps[1].size();
70+
}
71+
6872
public SslContextProvider sslContextProvider() {
6973
return sslContextProvider;
7074
}
@@ -83,17 +87,18 @@ public SslContext sslClientContext(String serverName, boolean useAlpn, boolean t
8387

8488
public SslContext sslContext(String serverName, boolean useAlpn, boolean server, boolean trustAll) throws Exception {
8589
int idx = idx(useAlpn);
86-
if (serverName == null) {
87-
if (sslContexts[idx] == null) {
88-
SslContext context = sslContextProvider.createContext(server, null, null, null, useAlpn, trustAll);
89-
sslContexts[idx] = context;
90-
}
91-
return sslContexts[idx];
92-
} else {
90+
if (serverName != null) {
9391
KeyManagerFactory kmf = sslContextProvider.resolveKeyManagerFactory(serverName);
9492
TrustManager[] trustManagers = trustAll ? null : sslContextProvider.resolveTrustManagers(serverName);
95-
return sslContextMaps[idx].computeIfAbsent(serverName, s -> sslContextProvider.createContext(server, kmf, trustManagers, s, useAlpn, trustAll));
93+
if (kmf != null || trustManagers != null || !server) {
94+
return sslContextMaps[idx].computeIfAbsent(serverName, s -> sslContextProvider.createContext(server, kmf, trustManagers, s, useAlpn, trustAll));
95+
}
96+
}
97+
if (sslContexts[idx] == null) {
98+
SslContext context = sslContextProvider.createContext(server, null, null, serverName, useAlpn, trustAll);
99+
sslContexts[idx] = context;
96100
}
101+
return sslContexts[idx];
97102
}
98103

99104
public SslContext sslServerContext(boolean useAlpn) {

src/main/java/io/vertx/core/net/impl/SslContextProvider.java

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,6 @@ protected void initEngine(SSLEngine engine) {
154154
}
155155
}
156156

157-
public KeyManagerFactory loadKeyManagerFactory(String serverName) throws Exception {
158-
if (keyManagerFactoryMapper != null) {
159-
return keyManagerFactoryMapper.apply(serverName);
160-
}
161-
return null;
162-
}
163-
164157
public TrustManager[] defaultTrustManagers() {
165158
return trustManagerFactory != null ? trustManagerFactory.getTrustManagers() : null;
166159
}
@@ -174,8 +167,7 @@ public KeyManagerFactory defaultKeyManagerFactory() {
174167
}
175168

176169
/**
177-
* Resolve the {@link KeyManagerFactory} for the {@code serverName}, when a factory cannot be resolved, the default
178-
* factory is returned.
170+
* Resolve the {@link KeyManagerFactory} for the {@code serverName}, when a factory cannot be resolved, {@code null} is returned.
179171
* <br/>
180172
* This can block and should be executed on the appropriate thread.
181173
*
@@ -184,23 +176,14 @@ public KeyManagerFactory defaultKeyManagerFactory() {
184176
* @throws Exception anything that would prevent loading the factory
185177
*/
186178
public KeyManagerFactory resolveKeyManagerFactory(String serverName) throws Exception {
187-
KeyManagerFactory kmf = loadKeyManagerFactory(serverName);
188-
if (kmf == null) {
189-
kmf = keyManagerFactory;
190-
}
191-
return kmf;
192-
}
193-
194-
public TrustManager[] loadTrustManagers(String serverName) throws Exception {
195-
if (trustManagerMapper != null) {
196-
return trustManagerMapper.apply(serverName);
179+
if (keyManagerFactoryMapper != null) {
180+
return keyManagerFactoryMapper.apply(serverName);
197181
}
198182
return null;
199183
}
200184

201185
/**
202-
* Resolve the {@link TrustManager}[] for the {@code serverName}, when managers cannot be resolved, the default
203-
* managers are returned.
186+
* Resolve the {@link TrustManager}[] for the {@code serverName}, when managers cannot be resolved, {@code null} is returned.
204187
* <br/>
205188
* This can block and should be executed on the appropriate thread.
206189
*
@@ -209,11 +192,10 @@ public TrustManager[] loadTrustManagers(String serverName) throws Exception {
209192
* @throws Exception anything that would prevent loading the managers
210193
*/
211194
public TrustManager[] resolveTrustManagers(String serverName) throws Exception {
212-
TrustManager[] trustManagers = loadTrustManagers(serverName);
213-
if (trustManagers == null && trustManagerFactory != null) {
214-
trustManagers = trustManagerFactory.getTrustManagers();
195+
if (trustManagerMapper != null) {
196+
return trustManagerMapper.apply(serverName);
215197
}
216-
return trustManagers;
198+
return null;
217199
}
218200

219201
private VertxTrustManagerFactory buildVertxTrustManagerFactory(TrustManager[] mgrs) {

src/main/java/io/vertx/core/net/impl/TCPServerBase.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ private GlobalTrafficShapingHandler createTrafficShapingHandler(EventLoopGroup e
125125
return trafficShapingHandler;
126126
}
127127

128+
public int sniEntrySize() {
129+
return sslHelper.sniEntrySize();
130+
}
131+
128132
public Future<Boolean> updateSSLOptions(SSLOptions options, boolean force) {
129133
TCPServerBase server = actualServer;
130134
if (server != null && server != this) {

src/test/java/io/vertx/core/net/NetTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,7 @@
5858
import io.vertx.core.impl.logging.LoggerFactory;
5959
import io.vertx.core.json.JsonArray;
6060
import io.vertx.core.json.JsonObject;
61-
import io.vertx.core.net.impl.HAProxyMessageCompletionHandler;
62-
import io.vertx.core.net.impl.NetServerImpl;
63-
import io.vertx.core.net.impl.NetSocketInternal;
64-
import io.vertx.core.net.impl.VertxHandler;
61+
import io.vertx.core.net.impl.*;
6562
import io.vertx.core.spi.tls.SslContextFactory;
6663
import io.vertx.core.streams.ReadStream;
6764
import io.vertx.test.core.CheckingSender;
@@ -1538,14 +1535,17 @@ public void testClientSniMultipleServerName() throws Exception {
15381535
receivedServerNames.add(so.indicatedServerName());
15391536
});
15401537
startServer();
1541-
List<String> serverNames = Arrays.asList("host1", "host2.com");
1538+
List<String> serverNames = Arrays.asList("host1", "host2.com", "fake");
1539+
List<String> cns = new ArrayList<>();
15421540
client = vertx.createNetClient(new NetClientOptions().setSsl(true).setTrustAll(true));
15431541
for (String serverName : serverNames) {
15441542
NetSocket so = client.connect(testAddress, serverName).toCompletionStage().toCompletableFuture().get();
15451543
String host = cnOf(so.peerCertificates().get(0));
1546-
assertEquals(serverName, host);
1544+
cns.add(host);
15471545
}
1548-
assertWaitUntil(() -> receivedServerNames.size() == 2);
1546+
assertEquals(Arrays.asList("host1", "host2.com", "localhost"), cns);
1547+
assertEquals(2, ((TCPServerBase)server).sniEntrySize());
1548+
assertWaitUntil(() -> receivedServerNames.size() == 3);
15491549
assertEquals(receivedServerNames, serverNames);
15501550
}
15511551

0 commit comments

Comments
 (0)