Handle empty cipher stack from wolfSSL_get_ciphers_compat(), filter anon ciphers by default#343
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a guard in the JNI layer to handle an empty cipher stack from wolfSSL_get_ciphers_compat(), returning NULL instead of proceeding with an empty list. Includes test coverage for all TLS protocol versions.
Changes:
- Added empty cipher stack check in native C code, returning NULL early with proper cleanup
- Added test iterating all TLS_VERSION values to validate
getCiphersAvailableIana()results
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| native/com_wolfssl_WolfSSL.c | Early return NULL when cipher stack is non-NULL but empty |
| src/test/com/wolfssl/test/WolfSSLTest.java | Test all protocol versions for valid cipher list results |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR adds two defensive fixes: handling empty cipher stacks from wolfSSL_get_ciphers_compat() by returning NULL, and filtering anonymous cipher suites from default enabled lists to match SunJSSE behavior.
Changes:
- Guard against empty cipher stack in native
getAvailableCipherSuitesIana()JNI code - Add
filterAnonparameter toWolfSSLUtil.sanitizeSuites()to filter_anon_suites from defaults - Update all callers and add comprehensive tests across Socket, ServerSocket, Engine, and Factory classes
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| native/com_wolfssl_WolfSSL.c | Return NULL when cipher stack is non-NULL but empty |
| src/java/com/wolfssl/provider/jsse/WolfSSLUtil.java | Add filterAnon param to sanitizeSuites() |
| src/java/com/wolfssl/provider/jsse/WolfSSLContext.java | Pass true for anon filtering in default context |
| src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java | Update all sanitizeSuites calls with false |
| src/java/com/wolfssl/provider/jsse/WolfSSLServerSocket.java | Update calls, false for explicit/supported |
| src/java/com/wolfssl/provider/jsse/WolfSSLSocketFactory.java | Filter anon in defaults, preserve in supported |
| src/java/com/wolfssl/provider/jsse/WolfSSLServerSocketFactory.java | Same default/supported split |
| src/test/.../WolfSSLTest.java | Test per-version cipher availability |
| src/test/.../WolfSSLContextTest.java | Test default params exclude anon |
| src/test/.../WolfSSLEngineTest.java | Test engine anon filtering |
| src/test/.../WolfSSLSocketTest.java | Test socket anon filtering |
| src/test/.../WolfSSLServerSocketTest.java | Test server socket anon filtering |
| src/test/.../WolfSSLSocketFactoryTest.java | Test socket factory anon filtering |
| src/test/.../WolfSSLServerSocketFactoryTest.java | Test server socket factory anon filtering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR includes two fixes:
Returns
NULLfrom nativegetAvailableCipherSuitesIana()whenwolfSSL_get_ciphers_compat()returns a non-NULL but empty cipher stack (sk_num == 0).Background:
wolfSSL_get_ciphers_compat() should return NULL on an empty cipher list, but a recent regression in native wolfSSL commit fb824962440c (PR #9831) caused a test failure here to do this. Native wolfSSL fix has been put up (wolfSSL/wolfssl#9972), but adding this guard here for future safety.
This PR also filters out anonymous cipher suites (
_anon_) from the default enabled cipher suite list inWolfSSLUtil.sanitizeSuites().Background
Native wolfSSL commit 3084305200d4 (in PR #9831) removed
WOLFSSL_QTfrom--enable-all. Previously, WOLFSSL_QT force-setctx->havePSK = 1on every context, which placed PSK cipher suites in the suite list at higher priority than anonymous suites. WithWOLFSSL_QTremoved, PSK suites are no longer in the default list, and anonymous suites likeTLS_DH_anon_WITH_AES_128_CBC_SHAcan now be negotiated whenHAVE_ANONis defined (which--enable-allsets viaOPENSSL_COMPATIBLE_DEFAULTS). This causedtestSetWantNeedClientAuth_ServerNoKeyManagerto fail. A server with no KeyManager (no certificate) was successfully completing a TLS 1.2 handshake using an anonymous suite, where previously the handshake would fail.Anonymous cipher suites provide no authentication and can be a security risk. SunJSSE also filters them from the default enabled list (via anon in jdk.tls.disabledAlgorithms). Applications that explicitly need anonymous suites can still enable them by calling
SSLEngine.setEnabledCipherSuites()orSSLSocket.setEnabledCipherSuites()directly.