Skip to content

Commit 5677a93

Browse files
JeremiahM37JeremiahM37
committed
OkHttp testing fixes for SSLEngine, verify callback, session handling, and trust manager
1 parent bc3352a commit 5677a93

11 files changed

Lines changed: 197 additions & 97 deletions

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ protected void updateTimeouts(int in, int side) {
765765
diff = (now - current.creation.getTime()) / 1000;
766766

767767
if (diff < 0) {
768-
/* session is from the future ... */ /* TODO */
768+
/* session is from the future ... TODO */
769769

770770
}
771771

‎src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,7 @@ protected synchronized void cacheRequestedServerNamesFromNetData() {
321321
List<SNIServerName> names;
322322
WolfSSLImplementSSLSession session;
323323

324-
if (this.engineHelper == null ||
325-
this.engineHelper.getUseClientMode()) {
324+
if (this.engineHelper == null || this.engineHelper.getUseClientMode()) {
326325
return;
327326
}
328327

@@ -354,7 +353,8 @@ private List<SNIServerName> parseRequestedServerNamesFromNetData() {
354353
List<SNIServerName> names;
355354

356355
synchronized (netDataLock) {
357-
if (this.netData == null || this.netData.remaining() < 5) {
356+
if (this.netData == null ||
357+
this.netData.remaining() < TLS_RECORD_HEADER_LEN) {
358358
return null;
359359
}
360360
in = this.netData.asReadOnlyBuffer();
@@ -1199,8 +1199,7 @@ private synchronized int RecvAppData(ByteBuffer[] out, int ofst, int length)
11991199
default:
12001200
/* Throw SSLHandshakeException if handshake not finished */
12011201
if (!this.handshakeFinished) {
1202-
SSLHandshakeException hse =
1203-
new SSLHandshakeException(
1202+
SSLHandshakeException hse = new SSLHandshakeException(
12041203
"SSL/TLS handshake error in read: " + ret +
12051204
" , err = " + err);
12061205
if (this.engineHelper != null) {
@@ -1235,11 +1234,13 @@ private synchronized int RecvAppData(ByteBuffer[] out, int ofst, int length)
12351234
/* Copy from intermediate buffer to output bufs */
12361235
for (i = 0; i < ret;) {
12371236
if (idx + ofst >= length) {
1237+
/* no more output buffers left */
12381238
break;
12391239
}
12401240

12411241
bufSpace = out[idx + ofst].remaining();
12421242
if (bufSpace == 0) {
1243+
/* no more space in current out buffer, advance */
12431244
idx++;
12441245
continue;
12451246
}
@@ -1450,13 +1451,15 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP &&
14501451
* report bytesConsumed() == 0. DTLS still
14511452
* relies on the native WANT_READ path. */
14521453
boolean bufferUnderflow = false;
1453-
if (inRemaining > 0 &&
1454-
(this.ssl.dtls() == 0)) {
1454+
if (inRemaining > 0 && (this.ssl.dtls() == 0)) {
14551455
synchronized (netDataLock) {
14561456
int pos = in.position();
14571457
if (inRemaining < TLS_RECORD_HEADER_LEN) {
1458+
/* Not enough for TLS record header */
14581459
bufferUnderflow = true;
14591460
} else {
1461+
/* Peek at record length from header
1462+
* bytes 3-4 (big-endian) */
14601463
int recLen =
14611464
((in.get(pos + TLS_RECORD_LEN_HI_OFF)
14621465
& 0xFF) << 8) |
@@ -1482,8 +1485,7 @@ else if (hs == SSLEngineResult.HandshakeStatus.NEED_WRAP &&
14821485
for (int pos = 0;
14831486
pos < this.pendingAppDataLen;) {
14841487
if (idx2 + ofst >= length) break;
1485-
int space =
1486-
out[idx2 + ofst].remaining();
1488+
int space = out[idx2 + ofst].remaining();
14871489
if (space == 0) { idx2++; continue; }
14881490
int sz2 = Math.min(space,
14891491
this.pendingAppDataLen - pos);
@@ -1537,8 +1539,7 @@ else if (inRemaining > 0 &&
15371539
in.position(inPosition);
15381540
}
15391541
produced = 0;
1540-
status =
1541-
SSLEngineResult.Status.BUFFER_OVERFLOW;
1542+
status = SSLEngineResult.Status.BUFFER_OVERFLOW;
15421543
}
15431544
else {
15441545
/* Check for BUFFER_OVERFLOW status. This can
@@ -1657,9 +1658,8 @@ else if (ret < 0 &&
16571658
"SSL/TLS handshake error, ret:err = " +
16581659
ret + " : " + err);
16591660
if (this.engineHelper != null) {
1660-
Exception verifyEx2 =
1661-
this.engineHelper
1662-
.getLastVerifyException();
1661+
Exception verifyEx2 = this.engineHelper
1662+
.getLastVerifyException();
16631663
if (verifyEx2 != null) {
16641664
hse2.initCause(verifyEx2);
16651665
}

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -866,8 +866,7 @@ private boolean isTls13CipherSuite(String suite) {
866866
}
867867

868868
return suite.startsWith("TLS_AES_") ||
869-
suite.startsWith("TLS_CHACHA20_") ||
870-
suite.startsWith("TLS_SM4_");
869+
suite.startsWith("TLS_CHACHA20_") || suite.startsWith("TLS_SM4_");
871870
}
872871

873872
private String[] getEffectiveProtocolsForCiphers(String[] protocols,
@@ -948,8 +947,7 @@ else if ("TLSv1.2".equals(proto) || "DTLSv1.2".equals(proto) ||
948947
return protocols;
949948
}
950949

951-
private void applyConfiguredCipherProtocolSettings()
952-
throws SSLException {
950+
private void applyConfiguredCipherProtocolSettings() throws SSLException {
953951

954952
String[] suites;
955953
String[] protocols;
@@ -1081,8 +1079,7 @@ private void setLocalServerNames(SSLEngine engine) {
10811079
/* SSLEngine(host, port) should send SNI by default if no explicit
10821080
* server names were configured and SNI extension is enabled. */
10831081
boolean isEngineConnectionWithHost = this.clientMode &&
1084-
engine != null &&
1085-
this.hostname != null &&
1082+
engine != null && this.hostname != null &&
10861083
this.params.getServerNames() == null;
10871084

10881085
/* Enable SNI if explicitly requested via property, if

‎src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,8 +1136,7 @@ public synchronized List<SNIServerName> getRequestedServerNames()
11361136
byte[] sniRequestArr = null;
11371137

11381138
if (this.ssl == null) {
1139-
if (this.sniServerNames != null &&
1140-
!this.sniServerNames.isEmpty()) {
1139+
if (this.sniServerNames != null && !this.sniServerNames.isEmpty()) {
11411140
return Collections.unmodifiableList(
11421141
new ArrayList<SNIServerName>(this.sniServerNames));
11431142
}

‎src/java/com/wolfssl/provider/jsse/WolfSSLInternalVerifyCb.java‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,7 @@ else if (sock != null) {
310310
}
311311
} catch (Exception e) {
312312
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
313-
() -> "Hostname verification error: " +
314-
e.getMessage());
313+
() -> "Hostname verification error: " + e.getMessage());
315314
this.verifyException = e;
316315
return 0;
317316
} finally {
@@ -545,29 +544,47 @@ public int verifyCallback(int preverify_ok, long x509StorePtr) {
545544
}
546545

547546
/* If server-side application has explicitly disabled client
548-
* authentication, return as success and skip X509TrustManager
549-
* verification */
547+
* authentication (neither needClientAuth nor wantClientAuth set),
548+
* return as success and skip X509TrustManager verification. */
550549
if ((!this.clientMode) && (this.params != null) &&
551-
(!this.params.getNeedClientAuth())) {
550+
(!this.params.getNeedClientAuth()) &&
551+
(!this.params.getWantClientAuth())) {
552552
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
553-
() -> "Application has disabled client verification with " +
554-
"setNeedClientAuth(false), skipping verification");
553+
() -> "Application has disabled client verification " +
554+
"(needClientAuth=false, wantClientAuth=false), " +
555+
"skipping verification");
555556
return 1;
556557
}
557-
else if ((preverify_ok == 1) && (x509certs.length == 0) &&
558+
else if ((x509certs.length == 0) &&
558559
(!this.clientMode) && (this.params != null) &&
559560
this.params.getWantClientAuth() &&
560561
(!this.params.getNeedClientAuth())) {
561-
/* If native wolfSSL verification has passed, and we have no peer
562-
* certificates, if application has set client authentication be
563-
* requested (wantClientAuth == true), but not fatal if no
564-
* certificate was sent (needClientAuth == false), don't call
565-
* TrustManager with empty certificate chain just consider
566-
* verification successful at this point */
562+
/* No peer certificates sent and client authentication is
563+
* optional (wantClientAuth == true, needClientAuth == false).
564+
* Don't call TrustManager with empty certificate chain,
565+
* just consider verification successful. Don't require
566+
* preverify_ok == 1 here since native wolfSSL may report
567+
* failure when no CAs are loaded (foreign TrustManager). */
567568
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
568569
() -> "No client cert sent and client auth marked optional, " +
569570
"not calling TrustManager for hostname verification");
570571
}
572+
else if ((!this.clientMode) && (this.params != null) &&
573+
this.params.getWantClientAuth() &&
574+
(!this.params.getNeedClientAuth())) {
575+
/* wantClientAuth is set and client sent a certificate.
576+
* Try to verify via TrustManager, but don't fail the
577+
* handshake if verification fails — wantClientAuth means
578+
* verification failure is non-fatal (JSSE contract). */
579+
if (VerifyCertChainWithTrustManager(x509certs, authType)) {
580+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
581+
() -> "wantClientAuth: client cert verified successfully");
582+
} else {
583+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
584+
() -> "wantClientAuth: client cert verification failed, " +
585+
"continuing handshake (non-fatal)");
586+
}
587+
}
571588
else {
572589
/* Poll X509TrustManager / X509ExtendedTrustManager for certificate
573590
* verification status. Returns 0 if certificates are rejected,

‎src/java/com/wolfssl/provider/jsse/WolfSSLSessionContext.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,7 @@ public SSLSession getSession(byte[] sessionId) {
9191
/* Native session may have been freed already */
9292
}
9393
}
94-
WolfSSLImplementSSLSession session =
95-
store.getSession(sessionId, side);
94+
WolfSSLImplementSSLSession session = store.getSession(sessionId, side);
9695
if (session == null) {
9796
return null;
9897
}

‎src/java/com/wolfssl/provider/jsse/WolfSSLTrustX509.java‎

Lines changed: 118 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,77 @@ private X509Certificate findRootCAFromKeyStoreForCert(X509Certificate cert,
335335
return null;
336336
}
337337

338+
/**
339+
* Find the actual issuer of a certificate from the KeyStore by checking
340+
* both subject DN match and signature verification. This handles the case
341+
* where multiple CA certificates share the same subject DN (e.g.,
342+
* cross-signed roots with the same CN but different key pairs).
343+
*
344+
* @param cert Certificate whose issuer we want to find
345+
* @param ks KeyStore containing trusted CA certificates
346+
*
347+
* @return The CA certificate that actually signed cert, or null if not
348+
* found
349+
*/
350+
private X509Certificate findIssuerBySignature(X509Certificate cert,
351+
KeyStore ks) {
352+
353+
if (cert == null || ks == null) {
354+
return null;
355+
}
356+
357+
X500Principal issuerDN = cert.getIssuerX500Principal();
358+
if (issuerDN == null) {
359+
return null;
360+
}
361+
362+
try {
363+
Enumeration<String> aliases = ks.aliases();
364+
while (aliases.hasMoreElements()) {
365+
String alias = aliases.nextElement();
366+
X509Certificate candidate = null;
367+
368+
if (ks.isKeyEntry(alias)) {
369+
java.security.cert.Certificate[] chain =
370+
ks.getCertificateChain(alias);
371+
if (chain != null && chain.length > 0) {
372+
candidate = (X509Certificate) chain[0];
373+
}
374+
} else {
375+
java.security.cert.Certificate c =
376+
ks.getCertificate(alias);
377+
if (c instanceof X509Certificate) {
378+
candidate = (X509Certificate) c;
379+
}
380+
}
381+
382+
if (candidate == null) {
383+
continue;
384+
}
385+
386+
/* Check subject DN matches cert's issuer DN */
387+
if (!candidate.getSubjectX500Principal().equals(issuerDN)) {
388+
continue;
389+
}
390+
391+
/* Verify the signature to confirm this is the actual issuer,
392+
* not just a CA with the same subject DN */
393+
try {
394+
cert.verify(candidate.getPublicKey());
395+
return candidate;
396+
} catch (Exception e) {
397+
/* Signature didn't match, try next candidate */
398+
}
399+
}
400+
} catch (KeyStoreException e) {
401+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
402+
() -> "KeyStoreException in findIssuerBySignature: " +
403+
e.getMessage());
404+
}
405+
406+
return null;
407+
}
408+
338409
/**
339410
* Verify cert chain using WolfSSLCertManager.
340411
* Do all loading and verification in one function to avoid holding native
@@ -404,7 +475,13 @@ private List<X509Certificate> certManagerVerify(
404475
* Similarly to native wolfSSL WOLFSSL_ALT_CERT_CHAINS behavior: if a CA
405476
* certificate cannot be verified, we skip it and continue building
406477
* the chain through other certificates. This allows handling of
407-
* cross-signed certificates and extra certificates in the chain. */
478+
* cross-signed certificates and extra certificates in the chain.
479+
*
480+
* When verification fails for a CA cert, we also try to find its
481+
* actual issuer in the KeyStore by signature verification. This
482+
* handles the case where multiple CAs share the same subject DN
483+
* (e.g., cross-signed roots) and the native CA lookup returns the
484+
* wrong one first. */
408485

409486
for (int i = sortedCerts.length-1; i > 0; i--) {
410487
final int tmpI = i;
@@ -418,22 +495,48 @@ private List<X509Certificate> certManagerVerify(
418495
ret = cm.CertManagerVerifyBuffer(encoded, encoded.length,
419496
WolfSSL.SSL_FILETYPE_ASN1);
420497
if (ret != WolfSSL.SSL_SUCCESS) {
421-
/* Failure here is ok if this is a CA cert and we can still
422-
* build and verify a complete chain. Some cert chains may
423-
* include extra CA certs. Similar to native wolfSSL
424-
* WOLFSSL_ALT_CERT_CHAINS. */
498+
/* Verification failed. If this is a CA cert, try to find
499+
* and load its actual issuer from the KeyStore. This handles
500+
* cross-signed certs where multiple CAs share the same
501+
* subject DN but have different keys. */
425502
if (sortedCerts[tmpI].getBasicConstraints() != -1) {
426-
/* This is a CA certificate, skip it and continue.
427-
* Do not add it to the certificate manager. */
428-
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
429-
() -> "Using alternate cert chain (skipping CA): " +
430-
sortedCerts[tmpI].getSubjectX500Principal().getName());
431-
continue;
503+
X509Certificate issuer = findIssuerBySignature(
504+
sortedCerts[tmpI], this.store);
505+
if (issuer != null) {
506+
/* Found the actual issuer, load it and retry */
507+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
508+
() -> "Found issuer by signature for: " +
509+
sortedCerts[tmpI].getSubjectX500Principal()
510+
.getName());
511+
try {
512+
byte[] issuerEnc = issuer.getEncoded();
513+
cm.CertManagerLoadCABuffer(issuerEnc,
514+
issuerEnc.length, WolfSSL.SSL_FILETYPE_ASN1);
515+
} catch (Exception e) {
516+
/* If loading issuer fails, fall through to
517+
* skip logic below */
518+
}
519+
/* Retry verification after loading issuer */
520+
ret = cm.CertManagerVerifyBuffer(encoded,
521+
encoded.length, WolfSSL.SSL_FILETYPE_ASN1);
522+
}
523+
}
524+
if (ret != WolfSSL.SSL_SUCCESS) {
525+
if (sortedCerts[tmpI].getBasicConstraints() != -1) {
526+
/* This is a CA certificate, skip it and continue.
527+
* Do not add it to the certificate manager. */
528+
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
529+
() -> "Using alternate cert chain " +
530+
"(skipping CA): " +
531+
sortedCerts[tmpI].getSubjectX500Principal()
532+
.getName());
533+
continue;
534+
}
535+
/* Non-CA certificates must verify successfully */
536+
cm.free();
537+
throw new CertificateException(
538+
"Failed to verify intermediate chain cert");
432539
}
433-
/* Non-CA certificates must verify successfully */
434-
cm.free();
435-
throw new CertificateException(
436-
"Failed to verify intermediate chain cert");
437540
}
438541

439542
/* Load chain cert as trusted CA */

0 commit comments

Comments
 (0)