bc-java icon indicating copy to clipboard operation
bc-java copied to clipboard

JSSE Provider: Handshake session data does not persist to final session (inconsistent with Oracle JSSE)

Open steffen-heil-secforge opened this issue 4 months ago • 3 comments

JSSE Provider: Handshake session data does not persist to final session

Description

The BC JSSE provider's handshake session allows storing application data via putValue(), but this data is not preserved in the final session after handshake completion. This behavior differs from Oracle/Sun JSSE and creates unexpected behavior for users.

Current Behavior:

  • Data stored in handshake session via putValue() is lost after handshake completion
  • finalSession.getValue() returns null for data stored during certificate verification

Expected Behavior:

  • Application data stored in handshake session should persist to final session
  • Consistent with Oracle/Sun JSSE implementation

Test Case

import javax.net.ssl.*;
import java.security.cert.X509Certificate;
import java.security.Security;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.bouncycastle.jsse.provider.BouncyCastleJsseProvider;

public class SessionPersistenceTest {
    public static void main(String[] args) throws Exception {
        Security.addProvider(new BouncyCastleProvider());
        Security.addProvider(new BouncyCastleJsseProvider()); 
        
        var ctx = args.length > 0 ? SSLContext.getInstance("TLS", args[0]) : SSLContext.getInstance("TLS");
        ctx.init(null, new TrustManager[]{new TM()}, null);
        
        var s = (SSLSocket) ctx.getSocketFactory().createSocket("www.google.com", 443);
        s.startHandshake();
        
        var r = (String) s.getSession().getValue("test");
        System.out.println(ctx.getProvider().getName() + ": value available: " + (r != null));
        s.close();
    }
    
    static class TM extends X509ExtendedTrustManager {
        public void checkServerTrusted(X509Certificate[] chain, String authType, java.net.Socket socket) {
            ((SSLSocket) socket).getHandshakeSession().putValue("test", "value");
        }
        public void checkServerTrusted(X509Certificate[] chain, String authType) {}
        public void checkClientTrusted(X509Certificate[] chain, String authType) {}
        public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
        public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
        public void checkClientTrusted(X509Certificate[] chain, String authType, java.net.Socket socket) {}
        public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[0]; }
    }
}

Test Results:

  • java SessionPersistenceTest BCJSSEBCJSSE: value available: false
  • java SessionPersistenceTestSunJSSE: value available: true

Root Cause

In ProvSSLEngine.notifyHandshakeComplete() (line 680), the handshake session is discarded without copying application data:

public synchronized void notifyHandshakeComplete(ProvSSLConnection connection) {
    if (null != handshakeSession) {
        // ... validation logic ...
        handshakeSession.getJsseSecurityParameters().clear();
    }
    this.handshakeSession = null; // ← Data lost here
    this.connection = connection;
}

Proposed Fix

public synchronized void notifyHandshakeComplete(ProvSSLConnection connection) {
    if (null != handshakeSession) {
        if (!handshakeSession.isValid()) {
            connection.getSession().invalidate();
        }

        // Copy application data from handshake session to final session
        String[] valueNames = handshakeSession.getValueNames();
        for (String name : valueNames) {
            connection.getSession().putValue(name, handshakeSession.getValue(name));
        }

        handshakeSession.getJsseSecurityParameters().clear();
    }
    this.handshakeSession = null;
    this.connection = connection;
}

Files Requiring Changes

  • tls/src/main/java/org/bouncycastle/jsse/provider/ProvSSLEngine.java:680
  • tls/src/main/java/org/bouncycastle/jsse/provider/ProvSSLSocketDirect.java:465
  • tls/src/main/java/org/bouncycastle/jsse/provider/ProvSSLSocketWrap.java:654

Impact

  • Backward Compatible: No breaking changes
  • Improves Compatibility: Aligns with Oracle JSSE behavior
  • Enables Common Use Cases: Certificate validation metadata storage
  • Minimal Code Change: 3-line addition per affected method

Use Case

Storing certificate verification results, extracted certificate attributes, or security context data during TLS handshake for later application use without requiring external storage mechanisms.

steffen-heil-secforge avatar Jul 30 '25 18:07 steffen-heil-secforge

While this behavior isn't explicitly specified, it has caused actual application failures when migrating from Oracle JSSE to BC JSSE. Our application relies on storing certificate validation metadata in the handshake session during X509TrustManager.checkServerTrusted() and accessing it later - this works with Oracle JSSE but fails with BC JSSE.

The inconsistency between implementations created an unexpected migration barrier.

steffen-heil-secforge avatar Jul 30 '25 18:07 steffen-heil-secforge

SunJSSE appears to use a single instance for the session (handshake and cached/final), although it's a lot of code to trace through to understand exactly what's going on there. In general we could probably stand to move closer to that for compatibility reasons.

If the "single instance" idea is right, then any value puts to the session, whether via the "handshake session", the "final session", or a "resumed session" should all be mutually visible, and it would be worth verifying this behaviour before deciding the approach in BC.

If it pans out, we could accomplish similar behaviour at least for the value bindings if we just make ProvSSLSessionBase#valueMap a ConcurrentHashMap and then propagate that single instance from ProvSSLSessionHandshake -> ProvSSLSession (note that #2137 only snapshots the state) -> ProvSSLSessionResumed (#2137 doesn't make values visible during resumption handshake).

peterdettman avatar Jul 31 '25 08:07 peterdettman

I have now confirmed the "single instance" behaviour testing with the SunJSSE provider, so I went ahead and updated BCJSSE to also share a single ConcurrentHashMap across all stages of the session lifecycle.

In particular, session bindings created using putValue on the handshake session will now be available on the connection session.

peterdettman avatar Aug 08 '25 12:08 peterdettman