Skip to content

Commit bbacc6b

Browse files
normanmaurernormanmaurer
authored
BouncyCastleAlpnSslUtils needs to use the correct SSLEngine class as … (#15630)
…otherwise it will fail to init static fields. Motivation: We recently did some changes related to how BouncyCastle is loaded and while doing so introduce a regression which would cause NPE's later on once BouncyCastle was detected and used and we tried to apply ALPN later on. This regression was introduced by #15533 The problem was that we used the wrong class to try to obtain the Method instances via reflection. This failed as the class itself is package-private and so did not allow to access the methods. To fix this we need to ensure we use the public interface that is used by BouncyCastle and not the implemention class itself. Beside this we also should be more defensive and only try to use ALPN via BouncyCastle if we can obtain the Methods in the first place. Modifications: - Fix refelective access - Add more guards - Add unit test Result: Fixes #15627
1 parent d28a0fc commit bbacc6b

5 files changed

Lines changed: 99 additions & 10 deletions

File tree

‎handler/src/main/java/io/netty/handler/ssl/BouncyCastleAlpnSslUtils.java‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.lang.reflect.Proxy;
2727
import java.security.AccessController;
2828
import java.security.PrivilegedExceptionAction;
29+
import java.security.SecureRandom;
2930
import java.util.List;
3031
import java.util.function.BiFunction;
3132
import javax.net.ssl.SSLContext;
@@ -43,6 +44,7 @@ final class BouncyCastleAlpnSslUtils {
4344
private static final Method GET_HANDSHAKE_APPLICATION_PROTOCOL_SELECTOR;
4445
private static final Class<?> BC_APPLICATION_PROTOCOL_SELECTOR;
4546
private static final Method BC_APPLICATION_PROTOCOL_SELECTOR_SELECT;
47+
private static final boolean SUPPORTED;
4648

4749
static {
4850
Method setApplicationProtocols;
@@ -52,18 +54,26 @@ final class BouncyCastleAlpnSslUtils {
5254
Method getHandshakeApplicationProtocolSelector;
5355
Method bcApplicationProtocolSelectorSelect;
5456
Class<?> bcApplicationProtocolSelector;
57+
boolean supported;
5558

5659
try {
5760
if (!BouncyCastleUtil.isBcTlsAvailable()) {
5861
throw new IllegalStateException(BouncyCastleUtil.unavailabilityCauseBcTls());
5962
}
60-
SSLContext context = getSSLContext(BouncyCastleUtil.getBcProviderJsse());
63+
SSLContext context = getSSLContext(BouncyCastleUtil.getBcProviderJsse(), new SecureRandom());
6164
SSLEngine engine = context.createSSLEngine();
62-
final Class<? extends SSLEngine> engineClass = engine.getClass();
65+
Class<? extends SSLEngine> engineClass = engine.getClass();
66+
// We need to use the class returned by BounceCastleUtil below to access the methods as the engine
67+
// returned by createSSLEngine might be package-private and so would not allow us to access the methods
68+
// even thought the interface itself that it implements is public and so the methods are public.
69+
// See https://github.com/netty/netty/issues/15627
70+
final Class<? extends SSLEngine> bcEngineClass = BouncyCastleUtil.getBcSSLEngineClass();
71+
if (bcEngineClass == null || !bcEngineClass.isAssignableFrom(engineClass)) {
72+
throw new IllegalStateException("Unexpected engine class: " + engineClass);
73+
}
6374

6475
final SSLParameters bcSslParameters = engine.getSSLParameters();
6576
final Class<?> bCSslParametersClass = bcSslParameters.getClass();
66-
6777
setApplicationProtocols = AccessController.doPrivileged(new PrivilegedExceptionAction<Method>() {
6878
@Override
6979
public Method run() throws Exception {
@@ -75,15 +85,15 @@ public Method run() throws Exception {
7585
getApplicationProtocol = AccessController.doPrivileged(new PrivilegedExceptionAction<Method>() {
7686
@Override
7787
public Method run() throws Exception {
78-
return engineClass.getMethod("getApplicationProtocol");
88+
return bcEngineClass.getMethod("getApplicationProtocol");
7989
}
8090
});
8191
getApplicationProtocol.invoke(engine);
8292

8393
getHandshakeApplicationProtocol = AccessController.doPrivileged(new PrivilegedExceptionAction<Method>() {
8494
@Override
8595
public Method run() throws Exception {
86-
return engineClass.getMethod("getHandshakeApplicationProtocol");
96+
return bcEngineClass.getMethod("getHandshakeApplicationProtocol");
8797
}
8898
});
8999
getHandshakeApplicationProtocol.invoke(engine);
@@ -104,7 +114,7 @@ public Method run() throws Exception {
104114
AccessController.doPrivileged(new PrivilegedExceptionAction<Method>() {
105115
@Override
106116
public Method run() throws Exception {
107-
return engineClass.getMethod("setBCHandshakeApplicationProtocolSelector",
117+
return bcEngineClass.getMethod("setBCHandshakeApplicationProtocolSelector",
108118
testBCApplicationProtocolSelector);
109119
}
110120
});
@@ -113,10 +123,11 @@ public Method run() throws Exception {
113123
AccessController.doPrivileged(new PrivilegedExceptionAction<Method>() {
114124
@Override
115125
public Method run() throws Exception {
116-
return engineClass.getMethod("getBCHandshakeApplicationProtocolSelector");
126+
return bcEngineClass.getMethod("getBCHandshakeApplicationProtocolSelector");
117127
}
118128
});
119129
getHandshakeApplicationProtocolSelector.invoke(engine);
130+
supported = true;
120131
} catch (Throwable t) {
121132
logger.error("Unable to initialize BouncyCastleAlpnSslUtils.", t);
122133
setApplicationProtocols = null;
@@ -126,6 +137,7 @@ public Method run() throws Exception {
126137
getHandshakeApplicationProtocolSelector = null;
127138
bcApplicationProtocolSelectorSelect = null;
128139
bcApplicationProtocolSelector = null;
140+
supported = false;
129141
}
130142
SET_APPLICATION_PROTOCOLS = setApplicationProtocols;
131143
GET_APPLICATION_PROTOCOL = getApplicationProtocol;
@@ -134,6 +146,7 @@ public Method run() throws Exception {
134146
GET_HANDSHAKE_APPLICATION_PROTOCOL_SELECTOR = getHandshakeApplicationProtocolSelector;
135147
BC_APPLICATION_PROTOCOL_SELECTOR_SELECT = bcApplicationProtocolSelectorSelect;
136148
BC_APPLICATION_PROTOCOL_SELECTOR = bcApplicationProtocolSelector;
149+
SUPPORTED = supported;
137150
}
138151

139152
private BouncyCastleAlpnSslUtils() {
@@ -222,4 +235,8 @@ static BiFunction<SSLEngine, List<String>, String> getHandshakeApplicationProtoc
222235
throw new IllegalStateException(ex);
223236
}
224237
}
238+
239+
static boolean isAlpnSupported() {
240+
return SUPPORTED;
241+
}
225242
}

‎handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
public final class JdkAlpnApplicationProtocolNegotiator extends JdkBaseApplicationProtocolNegotiator {
3030
private static final boolean AVAILABLE = Conscrypt.isAvailable() ||
3131
JdkAlpnSslUtils.supportsAlpn() ||
32-
BouncyCastleUtil.isBcTlsAvailable();
32+
(BouncyCastleUtil.isBcTlsAvailable() && BouncyCastleAlpnSslUtils.isAlpnSupported());
3333

3434
private static final SslEngineWrapperFactory ALPN_WRAPPER = AVAILABLE ? new AlpnWrapper() : new FailureWrapper();
3535

@@ -132,7 +132,7 @@ public SSLEngine wrapSslEngine(SSLEngine engine, ByteBufAllocator alloc,
132132
return isServer ? ConscryptAlpnSslEngine.newServerEngine(engine, alloc, applicationNegotiator)
133133
: ConscryptAlpnSslEngine.newClientEngine(engine, alloc, applicationNegotiator);
134134
}
135-
if (BouncyCastleUtil.isBcJsseInUse(engine)) {
135+
if (BouncyCastleUtil.isBcJsseInUse(engine) && BouncyCastleAlpnSslUtils.isAlpnSupported()) {
136136
return new BouncyCastleAlpnSslEngine(engine, applicationNegotiator, isServer);
137137
}
138138
// ALPN support was recently backported to Java8 as

‎handler/src/main/java/io/netty/handler/ssl/SslUtils.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.security.NoSuchAlgorithmException;
3535
import java.security.NoSuchProviderException;
3636
import java.security.Provider;
37+
import java.security.SecureRandom;
3738
import java.util.Collections;
3839
import java.util.LinkedHashSet;
3940
import java.util.List;
@@ -272,13 +273,18 @@ private static SSLContext newInitContext(Provider provider)
272273

273274
static SSLContext getSSLContext(Provider provider)
274275
throws NoSuchAlgorithmException, KeyManagementException, NoSuchProviderException {
276+
return getSSLContext(provider, null);
277+
}
278+
279+
static SSLContext getSSLContext(Provider provider, SecureRandom secureRandom)
280+
throws NoSuchAlgorithmException, KeyManagementException, NoSuchProviderException {
275281
final SSLContext context;
276282
if (provider == null) {
277283
context = SSLContext.getInstance(getTlsVersion());
278284
} else {
279285
context = SSLContext.getInstance(getTlsVersion(), provider);
280286
}
281-
context.init(null, new TrustManager[0], null);
287+
context.init(null, new TrustManager[0], secureRandom);
282288
return context;
283289
}
284290

‎handler/src/main/java/io/netty/handler/ssl/util/BouncyCastleUtil.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,17 @@ public static Provider getBcProviderJsse() {
132132
return provider;
133133
}
134134

135+
/**
136+
* Returns the public {@link SSLEngine} sub-class that is used by bouncy-castle or {@code null} if
137+
* it can't be loaded.
138+
*
139+
* @return engine class.
140+
*/
141+
public static Class<? extends SSLEngine> getBcSSLEngineClass() {
142+
ensureLoaded();
143+
return bcSSLEngineClass;
144+
}
145+
135146
/**
136147
* Reset the loaded providers. Useful for testing, to redo the loading under different conditions.
137148
*/

‎handler/src/test/java/io/netty/handler/ssl/BouncyCastleEngineAlpnTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2025 The Netty Project
3+
*
4+
* The Netty Project licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
package io.netty.handler.ssl;
17+
18+
import io.netty.handler.ssl.util.BouncyCastleUtil;
19+
import org.bouncycastle.jsse.provider.BouncyCastleJsseProvider;
20+
import org.junit.jupiter.api.Test;
21+
22+
import javax.net.ssl.SSLContext;
23+
import javax.net.ssl.SSLEngine;
24+
import java.security.Provider;
25+
import java.security.SecureRandom;
26+
import java.util.List;
27+
import java.util.function.BiFunction;
28+
import static org.junit.jupiter.api.Assertions.assertTrue;
29+
30+
public class BouncyCastleEngineAlpnTest {
31+
32+
@Test
33+
public void testBouncyCastleSSLEngineSupportsAlpn() throws Exception {
34+
Provider bouncyCastleProvider = new BouncyCastleJsseProvider();
35+
SSLContext context = SslUtils.getSSLContext(bouncyCastleProvider, new SecureRandom());
36+
SSLEngine engine = context.createSSLEngine();
37+
assertTrue(BouncyCastleUtil.isBcJsseInUse(engine));
38+
assertTrue(BouncyCastleAlpnSslUtils.isAlpnSupported());
39+
40+
BouncyCastleAlpnSslEngine alpnSslEngine = new BouncyCastleAlpnSslEngine(
41+
engine, new JdkAlpnApplicationProtocolNegotiator("fake"), true);
42+
43+
// Call methods to ensure these not throw.
44+
alpnSslEngine.setHandshakeApplicationProtocolSelector(new BiFunction<SSLEngine, List<String>, String>() {
45+
@Override
46+
public String apply(SSLEngine sslEngine, List<String> strings) {
47+
return "fake";
48+
}
49+
});
50+
// Check that none of the methods will throw.
51+
alpnSslEngine.getHandshakeApplicationProtocolSelector();
52+
alpnSslEngine.setNegotiatedApplicationProtocol("fake");
53+
alpnSslEngine.getNegotiatedApplicationProtocol();
54+
}
55+
}

0 commit comments

Comments
 (0)