Skip to content

Security and correctness fixes from internal scan#363

Merged
rlm2002 merged 15 commits intowolfSSL:masterfrom
cconlon:scanIssues
May 1, 2026
Merged

Security and correctness fixes from internal scan#363
rlm2002 merged 15 commits intowolfSSL:masterfrom
cconlon:scanIssues

Conversation

@cconlon
Copy link
Copy Markdown
Member

@cconlon cconlon commented Apr 30, 2026

This PR includes fixes from internal AI-based scan.

  • Skip the "loaded trusted root cert" log on CA load failure (no longer claims success after a parse error)
  • Make WolfSSLContext.getProtocolsMask() private (removed accidentally-public mutator)
  • Log KeyManager/TrustManager class names instead of toString() (avoid leaking caller-supplied debug output)
  • Zero plaintext byte[] and staticAppDataBuf after SendAppData(); defensive zero in engine cleanup
  • Replace printStackTrace in beginHandshake() catch with WolfSSLDebug.log()
  • Drop unnecessary class-wide synchronized from getTotalOutputSize(); hoist array element to local
  • Send shutdownSSL() before returning SSL_HANDSHAKE_FAILURE when sessionCreation==false (path mirroring)
  • Drop unnecessary class-wide synchronized from getAllCiphers() / getAllProtocols()
  • Fix initSecureRandom() overwriting the default with the original null arg (missing else)
  • Distinguish "no cache key" from hashCode==0 in addSession(); return SSL_FAILURE when no key
  • Invalidate sessions whose creation time is in the future (clock skew / tampering)
  • Switch SSLSession value bindings from HashMap to ConcurrentHashMap; reject null values per SunJSSE
  • Wrap remaining getRequestedServerNames() return path with Collections.unmodifiableList()
  • Throw SSLPeerUnverifiedException from getPeerCertificateChain() error path instead of returning null
  • Replace printStackTrace in getId() with WolfSSLDebug.log()

@cconlon cconlon self-assigned this Apr 30, 2026
Copilot AI review requested due to automatic review settings April 30, 2026 23:42
Copy link
Copy Markdown

Copilot AI left a comment

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 applies a set of JSSE-provider security/correctness hardening fixes identified by an internal scan, focusing on safer logging, stricter API behavior, improved session/cache correctness, and defensive plaintext cleanup.

Changes:

  • Harden logging and API behavior (avoid caller-controlled toString() output, replace printStackTrace, restrict accidentally-public APIs, tighten SSLSession value semantics).
  • Improve session/cache correctness (better cache-key handling, invalidate sessions with future creation times, ensure handshake failure paths close cleanly).
  • Add defensive plaintext cleanup (zero sensitive buffers after send and during engine cleanup) and reduce unnecessary synchronization in hot paths.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java Switch SSLSession value bindings to ConcurrentHashMap, tighten value handling, make SNI list immutable, and improve error-path behavior/logging.
src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java Remove unnecessary synchronization from static capability helpers and ensure CloseNotify is sent in a session-creation-disabled handshake failure path.
src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java Add plaintext wiping in send/cleanup paths, remove unnecessary synchronization from buffer sizing helper, and replace printStackTrace with structured logging.
src/java/com/wolfssl/provider/jsse/WolfSSLContext.java Fix CA-load logging correctness on failures, avoid logging caller-controlled toString() for KM/TM arrays, and make getProtocolsMask() private.
src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java Log manager class names instead of toString(), fix SecureRandom init logic, refine session-cache key handling/return codes, and invalidate “future” sessions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java Outdated
Comment thread src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java Outdated
Comment thread src/java/com/wolfssl/provider/jsse/WolfSSLContext.java
@cconlon cconlon assigned rlm2002 and unassigned cconlon May 1, 2026
@rlm2002 rlm2002 merged commit 43d6b42 into wolfSSL:master May 1, 2026
102 checks passed
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