Skip to content

Fixes for callbacks, CRL dates, DN parsing, PSK getters - #407

Open
cconlon wants to merge 4 commits into
wolfSSL:masterfrom
cconlon:fenrirAug24
Open

Fixes for callbacks, CRL dates, DN parsing, PSK getters#407
cconlon wants to merge 4 commits into
wolfSSL:masterfrom
cconlon:fenrirAug24

Conversation

@cconlon

@cconlon cconlon commented Aug 26, 2026

Copy link
Copy Markdown
Member

This PR includes 5 Fenrir fixes:

  • F-11870: Reformat WolfSSLX509 DNs by splitting on TAG= boundaries so any RDN type (ex: DC) is handled.
  • F-11875: Honor the revocation date in serial-based addRevoked(), and reject a non-null date in addRevokedCert().
  • F-11876: Return null from the PSK identity getters when wolfSSL returns a NULL string.
  • F-11882 / F-11883: Native verify callbacks return 0 on any JNI/callback error.

@cconlon cconlon self-assigned this Aug 26, 2026
Copilot AI lite review requested due to automatic review settings August 26, 2026 21:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens wolfSSL JNI/JSSE behavior around certificate verification callbacks, CRL revocation date handling, DN reformatting, and PSK identity getters, and adds regression tests to validate the new fail-closed and parsing behaviors.

Changes:

  • Make native verify callbacks fail closed by returning 0 on JNI/callback errors and only accepting an explicit 1 from Java callbacks.
  • Honor revocation dates for serial-based CRL revocations, and reject non-null revocation dates for cert-based revocation APIs (which always record current time).
  • Improve DN reformatting to split on TAG= boundaries so unrecognized RDN types (e.g., DC) terminate and round-trip correctly; add tests for the edge case.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/test/com/wolfssl/test/WolfSSLSessionTest.java Adds tests ensuring verify callback exceptions/negative returns cause handshake failure (fail closed).
src/test/com/wolfssl/test/WolfSSLCRLTest.java Adds/updates CRL tests to verify revocation date is honored for serial revocations and rejected for cert-based revocations.
src/test/com/wolfssl/provider/jsse/test/WolfSSLX509Test.java Adds regression test ensuring DN reformatting terminates and preserves unrecognized RDN types (DC).
src/java/com/wolfssl/WolfSSLVerifyCallback.java Updates callback contract documentation to clarify that only 1 continues the handshake.
src/java/com/wolfssl/WolfSSLCRL.java Fixes revocation-date encoding for addRevoked() and rejects non-null dates for addRevokedCert() overloads.
src/java/com/wolfssl/provider/jsse/WolfSSLX509.java Reworks DN reformatting to split on attribute boundaries and map known tags while passing unknown tags through.
native/com_wolfssl_WolfSSLSession.c Normalizes verify callback return handling to accept only 1; PSK getters now return null when wolfSSL returns NULL.
native/com_wolfssl_WolfSSLCRL.c Implements revocation date propagation for serial-based revocations when supported; rejects non-null dates for cert-based revocation.
native/com_wolfssl_WolfSSLContext.c Normalizes context verify callback return handling to accept only 1 and fail closed otherwise.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/test/com/wolfssl/test/WolfSSLCRLTest.java
@cconlon cconlon assigned rlm2002 and unassigned cconlon Sep 3, 2026
ExecutorService es = Executors.newSingleThreadExecutor();
Future<Void> srvFuture = runOneShotTlsServer(srvSocket, srvCtx, es);

Socket cliSock = new Socket("localhost", port);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

srvCtx,cliCtx, es and srvSocket can be leaked if new Socket("localhost", port) or new WolfSSLSession(cliCtx) throws

* @throws IllegalArgumentException if certDer is null or empty, or if
* revocationDate is non-null.
*/
public int addRevokedCert(byte[] certDer, Date revocationDate) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude: Both addRevokedCert(byte[], Date) and addRevokedCert(WolfSSLCertificate, Date) throw IllegalArgumentException on a non-null date they used to accept (and that this repo's own tests passed). It's a deliberate, documented change, but there's no deprecation path. The alternative that keeps the API: pull the serial off the cert and delegate to the now-working addRevoked(serial, date)

@rlm2002 rlm2002 assigned cconlon and unassigned rlm2002 Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants