diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java b/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java index b141007..9c9eb21 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java @@ -65,7 +65,7 @@ public class WolfSSLAuthStore { private WolfSSLSessionContext serverCtx = null; private WolfSSLSessionContext clientCtx = null; - private SessionStore store = null; + private SessionStore store = null; private static final Object storeLock = new Object(); /** @@ -282,7 +282,7 @@ public class WolfSSLAuthStore { * @param side server/client side for cache resize */ protected void resizeCache(int sz, int side) { - SessionStore newStore = + SessionStore newStore = new SessionStore<>(sz); /* @TODO check for side server/client, currently a resize is for all */ @@ -328,8 +328,7 @@ public class WolfSSLAuthStore { boolean needNewSession = false; WolfSSLImplementSSLSession ses = null; - String toHash = null; - int hashCode = 0; + String cacheKey = null; if (ssl == null) { return null; @@ -349,9 +348,8 @@ public class WolfSSLAuthStore { * Synchronizes on storeLock internally. */ printSessionStoreStatus(); - /* Generate cache key hash (host:port), outside lock */ - toHash = host.concat(Integer.toString(port)); - hashCode = toHash.hashCode(); + /* Generate cache key (host:port), outside lock */ + cacheKey = sessionCacheKey(host, port); /* Lock on static/global storeLock while getting session out of * store, since Java session cache table is shared between all @@ -359,14 +357,14 @@ public class WolfSSLAuthStore { synchronized (storeLock) { /* Try getting session out of Java store */ - ses = store.get(hashCode); + ses = store.get(cacheKey); /* Remove old entry from table. TLS 1.3 binder changes between * resumptions and stored session should only be used to * resume once. New session structure/object will be cached * after the resumed session completes the handshake, for * subsequent resumption attempts to use. */ - store.remove(hashCode); + store.remove(cacheKey); } /* Check conditions where we need to create a new new session: @@ -598,16 +596,31 @@ public class WolfSSLAuthStore { return true; } + /** + * Build the client session cache key from peer host and port. + * + * The key is a "host:port" String so the cache map compares hosts with + * String.equals() and distinct hosts never share a slot. The ':' separator + * keeps the port unambiguous even when the host ends in digits or is an + * IPv6 literal. + * + * @param host peer host name or IP literal, must not be null + * @param port peer port number + * @return session cache key String + */ + private String sessionCacheKey(String host, int port) { + return host + ":" + Integer.toString(port); + } + /** * Add SSLSession into wolfJSSE Java session cache table, to be used * for session resumption. * - * Session is stored into the session table using a hash code as the key. - * If the peer host is not null, the hash code is based on a concatenation - * of the peer host and port. If the peer host is null, the hash code - * is based on the session ID (if ID is not null, and non-zero length). - * Otherwise, no hash code is generated and the session is not stored into - * the session cache table. + * Session is stored into the session table using a String key. If the + * peer host is not null, the key is the "host:port" String. If the peer + * host is null, the key is based on the session ID (if ID is not null, + * and non-zero length). Otherwise, no key is generated and the session is + * not stored into the session cache table. * * This method synchronizes on the static/global storeLock object, since * the session cache is global and shared amongst all threads. @@ -617,8 +630,7 @@ public class WolfSSLAuthStore { */ protected int addSession(WolfSSLImplementSSLSession session) { - String toHash; - final int hashCode; + final String cacheKey; final boolean haveKey; /* Don't store session if invalid (or not complete with sesPtr @@ -640,40 +652,38 @@ public class WolfSSLAuthStore { if (session.getPeerHost() != null) { /* Generate key for storing into session table (host:port) */ - toHash = session.getPeerHost().concat(Integer.toString( - session.getPeerPort())); - hashCode = toHash.hashCode(); + cacheKey = sessionCacheKey(session.getPeerHost(), + session.getPeerPort()); haveKey = true; } else { - /* If no peer host is available then create hash key from + /* If no peer host is available then create key from * session ID if not null, not zero length, and not all zeros */ byte[] sessionId = session.getId(); if (sessionId != null && sessionId.length > 0 && (idAllZeros(sessionId) == false)) { - hashCode = Arrays.toString(session.getId()).hashCode(); + cacheKey = Arrays.toString(sessionId); haveKey = true; } else { - hashCode = 0; + cacheKey = null; haveKey = false; } } - /* Only store session into cache if we have a usable key. If session - * already exists for hashCode, it will be overwritten with the new - * version. Note that hashCode == 0 is a legitimate hash value, so - * we use haveKey rather than hashCode == 0 to gate caching. */ + /* Only store session into cache if we have a usable key. If a session + * already exists for cacheKey, it will be overwritten with the new + * version. */ if (haveKey) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "stored session in cache table (host: " + session.getPeerHost() + ", port: " + - session.getPeerPort() + ") " + "hashCode = " + hashCode + + session.getPeerPort() + ") " + "cacheKey = " + cacheKey + " side = " + session.getSideString()); /* Lock access to store while adding new session, store is global */ synchronized (storeLock) { session.isInTable = true; - store.put(hashCode, session); + store.put(cacheKey, session); } printSessionStoreStatus(); diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java index 30b4ba3..2fea1db 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java @@ -2572,6 +2572,144 @@ public class WolfSSLSocketTest { } } + /* Two host labels whose "host"+"port" String.hashCode() collides must not + * share a client session cache slot. Otherwise one host could resume or + * evict another host's cached session. */ + @Test + public void testSessionResumptionHostKeyedNoHashCollision() + throws Exception { + + byte[] sessionIdAa = null; + byte[] sessionIdColliding = null; + byte[] sessionIdAaResume = null; + String protocol = null; + + /* TLS 1.2/1.1/1.0 resume via session ID or ticket. TLS 1.3 is + * excluded here for the same reason as testSessionResumption(). */ + if (WolfSSL.TLSv12Enabled()) { + protocol = "TLSv1.2"; + } else if (WolfSSL.TLSv11Enabled()) { + protocol = "TLSv1.1"; + } else if (WolfSSL.TLSv1Enabled()) { + protocol = "TLSv1.0"; + } + Assume.assumeNotNull(protocol); + + /* "Aa" and "BB" both contribute 2112 to String.hashCode(), so + * "Aa.corp.internal"+port and "BB.corp.internal"+port collide in + * hashCode() while differing as Strings. Both resolve to loopback via + * getByAddress() so the TCP connection reaches the local test server, + * while the hostname label used for the cache key stays distinct. */ + byte[] loopback = new byte[]{127, 0, 0, 1}; + InetAddress hostAa = + InetAddress.getByAddress("Aa.corp.internal", loopback); + InetAddress hostBb = + InetAddress.getByAddress("BB.corp.internal", loopback); + + /* Make sure client session cache is enabled for this test. */ + String originalProp = Security.getProperty( + "wolfjsse.clientSessionCache.disabled"); + Security.setProperty("wolfjsse.clientSessionCache.disabled", "false"); + + SSLServerSocket ss = null; + ExecutorService es = null; + Future serverFuture = null; + + try { + this.ctx = tf.createSSLContext(protocol, ctxProvider); + + ss = (SSLServerSocket)ctx.getServerSocketFactory() + .createServerSocket(0); + final int port = ss.getLocalPort(); + final SSLServerSocket fss = ss; + + SSLSocketFactory cliFactory = ctx.getSocketFactory(); + + /* Server accepts three sequential handshakes. */ + es = Executors.newSingleThreadExecutor(); + serverFuture = es.submit(new Callable() { + @Override + public Void call() throws Exception { + try { + for (int i = 0; i < 3; i++) { + SSLSocket server = (SSLSocket)fss.accept(); + server.startHandshake(); + server.close(); + } + } catch (SSLException e) { + fail(); + } + return null; + } + }); + + try { + /* #1: host label "Aa...", full handshake, gets cached. */ + SSLSocket cs = (SSLSocket)cliFactory.createSocket(); + cs.connect(new InetSocketAddress(hostAa, port)); + cs.startHandshake(); + sessionIdAa = cs.getSession().getId(); + cs.close(); + + /* #2: colliding host label "BB...". Must not resume or evict + * Aa's cached session even though "BB..."+port and + * "Aa..."+port share a 32-bit hashCode. */ + cs = (SSLSocket)cliFactory.createSocket(); + cs.connect(new InetSocketAddress(hostBb, port)); + cs.startHandshake(); + sessionIdColliding = cs.getSession().getId(); + cs.close(); + + /* #3: reconnect "Aa...". Its cached session must still be + * present (not evicted by BB) and resume. */ + cs = (SSLSocket)cliFactory.createSocket(); + cs.connect(new InetSocketAddress(hostAa, port)); + cs.startHandshake(); + sessionIdAaResume = cs.getSession().getId(); + cs.close(); + + } catch (SSLHandshakeException e) { + fail(); + } + + /* Surface any server-thread failure, bounded so a stuck server + * cannot hang the suite. */ + serverFuture.get(10, TimeUnit.SECONDS); + + /* Colliding host must get its own session, not Aa's. */ + if (Arrays.equals(sessionIdAa, sessionIdColliding)) { + fail("colliding host resumed victim host's session"); + } + + /* Same host still resumes, so the String key does not break + * legitimate resumption and BB did not evict Aa's entry. */ + if (!Arrays.equals(sessionIdAa, sessionIdAaResume)) { + fail("same host failed to resume after colliding host connect"); + } + + } finally { + /* Close the server socket first so any pending accept() unblocks, + * then tear down the executor. Runs on every path so a failure + * cannot leave threads or sockets behind. */ + if (ss != null && !ss.isClosed()) { + try { + ss.close(); + } catch (IOException e) { + /* ignore */ + } + } + if (serverFuture != null) { + serverFuture.cancel(true); + } + if (es != null) { + es.shutdownNow(); + } + /* Restore the original property state. */ + Security.setProperty("wolfjsse.clientSessionCache.disabled", + originalProp == null ? "" : originalProp); + } + } + @Test public void testSessionResumptionSysPropDisabled() throws Exception {