Skip to content

Commit 8ca139f

Browse files
authored
Merge pull request #349 from rlm2002/sni_fix
JSSE: Update setLocalServerNames
2 parents e86fe01 + 5be8623 commit 8ca139f

4 files changed

Lines changed: 143 additions & 27 deletions

File tree

.github/workflows/windows-vs.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ on:
66
ant_version:
77
description: 'Apache Ant version to use'
88
required: false
9-
default: '1.10.15'
9+
default: '1.10.16'
1010
type: string
1111
platform_toolset:
1212
description: 'Visual Studio platform toolset (auto-detect if not specified)'

src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -411,27 +411,6 @@ else if (!sessionCipherSuiteAvailable(
411411

412412
ses.isFromTable = true;
413413

414-
/* Check if the session has stored SNI server names */
415-
List<SNIServerName> sniNames = ses.getSNIServerNames();
416-
if (sniNames != null && !sniNames.isEmpty()) {
417-
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
418-
() -> "Found SNI server names in cached session");
419-
420-
/* Apply SNI settings to the SSL connection */
421-
for (SNIServerName name : sniNames) {
422-
if (name instanceof SNIHostName) {
423-
String hostName = ((SNIHostName)name).getAsciiName();
424-
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
425-
() -> "Applying SNI hostname for resumption: " +
426-
hostName);
427-
428-
/* Set the SNI directly on the SSL object */
429-
ssl.useSNI((byte)WolfSSL.WOLFSSL_SNI_HOST_NAME,
430-
hostName.getBytes(StandardCharsets.UTF_8));
431-
}
432-
}
433-
}
434-
435414
if (ses.resume(ssl) != WolfSSL.SSL_SUCCESS) {
436415
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
437416
() -> "native wolfSSL_set_session() failed, " +

src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,25 +1122,38 @@ else if (this.clientMode) {
11221122

11231123
/* Explicitly set if user has set through SSLParameters. Check
11241124
* wolfSSL-specific server names first, then fall back to standard
1125-
* SSLParameters server names (set via parent setServerNames()). */
1125+
* SSLParameters server names (set via parent setServerNames()).
1126+
* If neither have been set and session is being resumed, check
1127+
* cached SNI */
1128+
boolean sniSet = false;
11261129
List<WolfSSLSNIServerName> names =
11271130
this.params.getWolfSSLServerNames();
11281131
List<SNIServerName> parentNames =
11291132
this.params.getServerNames();
1133+
List<SNIServerName> sniServerNames =
1134+
(this.session == null) ? null :
1135+
this.session.getSNIServerNames();
11301136
if (names != null && !names.isEmpty()) {
11311137
WolfSSLSNIServerName sni = names.get(0);
11321138
if (sni != null) {
11331139
this.ssl.useSNI((byte)sni.getType(), sni.getEncoded());
1140+
sniSet = true;
11341141
}
11351142
} else if (parentNames != null && !parentNames.isEmpty()) {
11361143
SNIServerName sni = parentNames.get(0);
11371144
if (sni != null) {
11381145
this.ssl.useSNI((byte)sni.getType(), sni.getEncoded());
1146+
sniSet = true;
1147+
}
1148+
} else if (sniServerNames != null && !sniServerNames.isEmpty()) {
1149+
SNIServerName sni = sniServerNames.get(0);
1150+
if (sni != null) {
1151+
this.ssl.useSNI((byte)sni.getType(), sni.getEncoded());
1152+
sniSet = true;
11391153
}
11401154
}
11411155

1142-
if ((names == null || names.isEmpty()) &&
1143-
(parentNames == null || parentNames.isEmpty()) && autoSNI) {
1156+
if (!sniSet && autoSNI) {
11441157
if (this.peerAddr != null && this.jdkTlsTrustNameService) {
11451158
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
11461159
() -> "setting SNI extension with " +
@@ -1173,10 +1186,14 @@ else if (this.clientMode) {
11731186
() -> "hostname and peerAddr are null, " +
11741187
"not setting SNI");
11751188
}
1189+
} else if (sniSet) {
1190+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
1191+
() -> "SNI configured through SSLParameters " +
1192+
"or session resumption");
11761193
} else {
11771194
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
1178-
() -> "No SNI configured through SSLParameters, " +
1179-
"not setting SNI");
1195+
() -> "No SNI configured through SSLParameters " +
1196+
"or session resumption, not setting SNI");
11801197
}
11811198
}
11821199
}

src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@
5252
import javax.net.ssl.SSLSocket;
5353
import javax.net.ssl.SSLServerSocket;
5454
import javax.net.ssl.TrustManager;
55+
import javax.net.ssl.SNIHostName;
56+
import javax.net.ssl.SNIMatcher;
57+
import javax.net.ssl.SSLParameters;
5558
import javax.net.ssl.X509TrustManager;
5659
import javax.net.ssl.X509ExtendedTrustManager;
5760
import javax.net.ssl.SSLEngineResult;
@@ -1212,6 +1215,123 @@ public void testReuseSession()
12121215
}
12131216
}
12141217

1218+
/**
1219+
* Test that SNI is correctly preserved across TLS session resumption
1220+
* when using SSLEngine. Regression test for the bug where
1221+
* setLocalServerNames() would overwrite session-cached SNI with the
1222+
* autoSNI hostname on resumption.
1223+
*/
1224+
@Test
1225+
public void testEngineSessionResumptionSNI()
1226+
throws NoSuchProviderException, NoSuchAlgorithmException,
1227+
KeyManagementException, KeyStoreException, IOException,
1228+
CertificateException, UnrecoverableKeyException {
1229+
1230+
System.out.print("\tEngine session resumption SNI");
1231+
1232+
/* Requires WOLFSSL_ALWAYS_KEEP_SNI, enabled by default with
1233+
* --enable-jni in wolfSSL 5.7.6+ (PR 8283) */
1234+
if (WolfSSL.getLibVersionHex() < 0x05007006L) {
1235+
System.out.println("\t... skipped");
1236+
return;
1237+
}
1238+
1239+
if (!enabledProtocols.contains("TLSv1.3")) {
1240+
System.out.println("\t... skipped");
1241+
return;
1242+
}
1243+
1244+
/* wolfjsse.clientSessionCache.disabled could be set in users
1245+
* java.security file which would cause this test to not work
1246+
* properly. Save their setting here, and re-enable session
1247+
* cache for this test */
1248+
String originalProp = Security.getProperty(
1249+
"wolfjsse.clientSessionCache.disabled");
1250+
Security.setProperty("wolfjsse.clientSessionCache.disabled", "false");
1251+
1252+
/* SNI hostname intentionally differs from peer hostname.
1253+
* autoSNI would send "localhost"; the fix sends "www.example.com"
1254+
* from the session cache instead. The server SNI matcher only
1255+
* accepts "www.example.com", so the outcome distinguishes the two. */
1256+
final String sniHostname = "www.example.com";
1257+
final String peerHostname = "localhost";
1258+
final SNIMatcher matcher =
1259+
SNIHostName.createSNIMatcher("www\\.example\\.com");
1260+
1261+
try {
1262+
int ret;
1263+
SSLEngine client1 = null, client2 = null;
1264+
SSLEngine server1 = null, server2 = null;
1265+
SSLContext ctx13 = tf.createSSLContext("TLSv1.3", engineProvider);
1266+
1267+
/* --- Connection #1: explicit SNI, caches it in the session --- */
1268+
try {
1269+
client1 = ctx13.createSSLEngine(peerHostname, 8443);
1270+
client1.setUseClientMode(true);
1271+
SSLParameters cliParams1 = client1.getSSLParameters();
1272+
cliParams1.setServerNames(
1273+
Arrays.asList(new SNIHostName(sniHostname)));
1274+
client1.setSSLParameters(cliParams1);
1275+
1276+
server1 = ctx13.createSSLEngine();
1277+
server1.setUseClientMode(false);
1278+
SSLParameters srvParams1 = server1.getSSLParameters();
1279+
srvParams1.setSNIMatchers(Arrays.asList(matcher));
1280+
server1.setSSLParameters(srvParams1);
1281+
1282+
ret = tf.testConnection(server1, client1, null, null, "Hello");
1283+
if (ret != 0) {
1284+
error("\t... failed");
1285+
fail("Initial connection with explicit SNI failed");
1286+
}
1287+
} finally {
1288+
if (server1 != null && client1 != null) {
1289+
tf.CloseConnection(server1, client1, false);
1290+
}
1291+
}
1292+
1293+
/* Connection #2: SNI must come from session cache */
1294+
try {
1295+
client2 = ctx13.createSSLEngine(peerHostname, 8443);
1296+
client2.setUseClientMode(true);
1297+
/* Intentionally no setSSLParameters() with server names.
1298+
* peerHostname ("localhost") makes
1299+
* isEngineConnectionWithHost=true and autoSNI=true. Without
1300+
* the fix, autoSNI would send "localhost" and the server
1301+
* would reject it. With the fix, the session-cached
1302+
* "www.example.com" is used instead. */
1303+
1304+
server2 = ctx13.createSSLEngine();
1305+
server2.setUseClientMode(false);
1306+
SSLParameters srvParams2 = server2.getSSLParameters();
1307+
srvParams2.setSNIMatchers(Arrays.asList(matcher));
1308+
server2.setSSLParameters(srvParams2);
1309+
1310+
ret = tf.testConnection(server2, client2, null, null, "Hello");
1311+
} finally {
1312+
if (server2 != null && client2 != null) {
1313+
tf.CloseConnection(server2, client2, false);
1314+
}
1315+
}
1316+
if (ret != 0) {
1317+
error("\t... failed");
1318+
fail("Resumed connection did not carry session-cached SNI");
1319+
}
1320+
1321+
pass("\t... passed");
1322+
1323+
} catch (Exception e) {
1324+
error("\t... failed");
1325+
e.printStackTrace();
1326+
fail("testEngineSessionResumptionSNI failed with exception: " + e);
1327+
} finally {
1328+
if (originalProp != null && !originalProp.isEmpty()) {
1329+
Security.setProperty(
1330+
"wolfjsse.clientSessionCache.disabled", originalProp);
1331+
}
1332+
}
1333+
}
1334+
12151335
/**
12161336
* More extended threading test of SSLEngine class.
12171337
* Launches a simple multi-threaded SSLSocket-based server, which

0 commit comments

Comments
 (0)