Merge pull request #139 from cconlon/enableAllFixGet1Session

JNI/JSSE: call wolfSSL_get1_session() for saving session in WolfSSLAuthStore

native/com_wolfssl_WolfSSLSession.c

@@ -1238,10 +1238,38 @@ JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_getSession
     (void)jenv;
     (void)jcl;
 
-    /* wolfSSL checks ssl for NULL */
+    /* wolfSSL checks ssl for NULL, returns pointer into WOLFSSL which is
+     * freed when wolfSSL_free() is called. */
     return (jlong)(uintptr_t)wolfSSL_get_session(ssl);
 }
 
+JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_get1Session
+  (JNIEnv* jenv, jobject jcl, jlong sslPtr)
+{
+    WOLFSSL* ssl = (WOLFSSL*)(uintptr_t)sslPtr;
+    (void)jenv;
+    (void)jcl;
+
+    /* wolfSSL checks ssl for NULL, returns pointer to new WOLFSSL_SESSION,
+     * instead of pointer into WOLFSSL like wolfSSL_get_session(). Needs to
+     * be freed with wolfSSL_SESSION_free() when finished with pointer. */
+    return (jlong)(uintptr_t)wolfSSL_get1_session(ssl);
+}
+
+JNIEXPORT void JNICALL Java_com_wolfssl_WolfSSLSession_freeNativeSession
+  (JNIEnv* jenv, jclass jcl, jlong sessionPtr)
+{
+    WOLFSSL_SESSION* session = (WOLFSSL_SESSION*)(uintptr_t)sessionPtr;
+    (void)jcl;
+
+    if (jenv == NULL) {
+        return;
+    }
+
+    /* checks session for NULL */
+    wolfSSL_SESSION_free(session);
+}
+
 JNIEXPORT jbyteArray JNICALL Java_com_wolfssl_WolfSSLSession_getSessionID
   (JNIEnv* jenv, jobject jcl, jlong sessionPtr)
 {

native/com_wolfssl_WolfSSLSession.h

@@ -151,6 +151,22 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_setSession
 JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_getSession
   (JNIEnv *, jobject, jlong);
 
+/*
+ * Class:     com_wolfssl_WolfSSLSession
+ * Method:    get1Session
+ * Signature: (J)J
+ */
+JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_get1Session
+  (JNIEnv *, jobject, jlong);
+
+/*
+ * Class:     com_wolfssl_WolfSSLSession
+ * Method:    freeNativeSession
+ * Signature: (J)V
+ */
+JNIEXPORT void JNICALL Java_com_wolfssl_WolfSSLSession_freeNativeSession
+  (JNIEnv *, jclass, jlong);
+
 /*
  * Class:     com_wolfssl_WolfSSLSession
  * Method:    getSessionID

src/java/com/wolfssl/WolfSSLSession.java

@@ -67,8 +67,8 @@ public class WolfSSLSession {
     private WolfSSLIORecvCallback internRecvSSLCb;
     private WolfSSLIOSendCallback internSendSSLCb;
 
-    /* have session tickets been enabled for this session? */
+    /* have session tickets been enabled for this session? Default to false. */
-    private boolean sessionTicketsEnabled = true;
+    private boolean sessionTicketsEnabled = false;
 
     /* is this context active, or has it been freed? */
     private boolean active = false;
@@ -207,6 +207,8 @@ public class WolfSSLSession {
     private native int getError(long ssl, int ret);
     private native int setSession(long ssl, long session);
     private native long getSession(long ssl);
+    private native long get1Session(long ssl);
+    private static native void freeNativeSession(long session);
     private native byte[] getSessionID(long session);
     private native int setTimeout(long ssl, long t);
     private native long getTimeout(long ssl);
@@ -973,6 +975,11 @@ public class WolfSSLSession {
      * At this point, the application may call <code>connect()</code> and
      * wolfSSL will try to resume the session.
      *
+     * The pointer (WOLFSSL_SESSION) returned by this method needs to be freed
+     * when the application is finished with it, by calling
+     * <code>freeSession(long)</code>. This will release the underlying
+     * native memory associated with this WOLFSSL_SESSION.
+     *
      * @throws IllegalStateException WolfSSLContext has been freed
      * @return      a pointer to the current SSL session object on success.
      *              <code>null</code> if <b>ssl</b> is <code>null</code>,
@@ -984,7 +991,17 @@ public class WolfSSLSession {
 
         confirmObjectIsActive();
 
-        return getSession(getSessionPtr());
+        return get1Session(getSessionPtr());
+    }
+
+    public static synchronized void freeSession(long session) {
+        /* No need to call confirmObjectIsActive() because the
+         * WOLFSSL_SESSION pointer being passed in here is not associated
+         * with this WOLFSSL object or WolfSSLSession. */
+
+        if (session != 0) {
+            freeNativeSession(session);
+        }
     }
 
     /**
@@ -999,9 +1016,10 @@ public class WolfSSLSession {
 
         confirmObjectIsActive();
 
-        long sess = getSession();
+        long sess = getSession(getSessionPtr());
         if (sess != 0) {
-            return getSessionID(sess);
+            /* returns new byte[] independent of sess ptr */
+             return getSessionID(sess);
         } else {
             return new byte[0];
         }
@@ -1061,7 +1079,7 @@ public class WolfSSLSession {
 
         confirmObjectIsActive();
 
-        return getSessTimeout(this.getSession());
+        return getSessTimeout(this.getSession(getSessionPtr()));
     }
 
     /**

src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java

@@ -57,10 +57,12 @@ public class WolfSSLAuthStore {
     private X509TrustManager tm = null;
     private SecureRandom sr = null;
     private String alias = null;
-    private SessionStore<Integer, WolfSSLImplementSSLSession> store;
     private WolfSSLSessionContext serverCtx = null;
     private WolfSSLSessionContext clientCtx = null;
 
+    private SessionStore<Integer, WolfSSLImplementSSLSession> store = null;
+    private final Object storeLock = new Object();
+
     /**
      * Protected constructor to create new WolfSSLAuthStore
      * @param keyman key manager to use
@@ -267,8 +269,10 @@ public class WolfSSLAuthStore {
                     new SessionStore<>(sz);
 
         //@TODO check for side server/client, currently a resize is for all
-        store.putAll(newStore);
+        synchronized (storeLock) {
-        store = newStore;
+            store.putAll(newStore);
+            store = newStore;
+        }
     }
 
     /** Returns either an existing session to use or creates a new session. Can
@@ -299,7 +303,10 @@ public class WolfSSLAuthStore {
 
         /* check if is in table */
         toHash = host.concat(Integer.toString(port));
-        ses = store.get(toHash.hashCode());
+
+        synchronized (storeLock) {
+            ses = store.get(toHash.hashCode());
+        }
         if (ses == null) {
             WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                     "session not found in cache table, creating new");
@@ -324,7 +331,8 @@ public class WolfSSLAuthStore {
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                 "creating new session");
 
-        WolfSSLImplementSSLSession ses = new WolfSSLImplementSSLSession(ssl, this);
+        WolfSSLImplementSSLSession ses =
+            new WolfSSLImplementSSLSession(ssl, this);
 
         ses.setValid(true);
         ses.setPseudoSessionId(Integer.toString(ssl.hashCode()).getBytes());
@@ -332,6 +340,29 @@ public class WolfSSLAuthStore {
         return ses;
     }
 
+    /**
+     * Internal helper function to check if session ID is all zeros.
+     * Used by addSession()
+     *
+     * @param id session ID
+     * @return true if array is all zeros (0x00), otherwise false
+     */
+    private boolean idAllZeros(byte[] id) {
+        boolean ret = true;
+
+        if (id == null) {
+            return true;
+        }
+
+        for (int i = 0; i < id.length; i++) {
+            if (id[i] != 0x00) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     /**
      * Add the session for possible resumption
      * @param session the session to add to stored session map
@@ -349,18 +380,25 @@ public class WolfSSLAuthStore {
             hashCode = toHash.hashCode();
         }
         else {
-                /* if no peer host is available then create hash key from
+            /* if no peer host is available then create hash key from
-                 * session id */
+             * 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();
+            }
         }
 
-        if (hashCode != 0 && !store.containsKey(hashCode)) {
+        synchronized (storeLock) {
-            WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
+            if (hashCode != 0 && !store.containsKey(hashCode)) {
-                    "stored session in cache table (host: " +
+                WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-                    session.getPeerHost() + ", port: " +
+                        "stored session in cache table (host: " +
-                    session.getPeerPort() + ") " +
+                        session.getPeerHost() + ", port: " +
-                    "hashCode = " + hashCode + " side = " + session.getSide());
+                        session.getPeerPort() + ") " +
-                store.put(hashCode, session);
+                        "hashCode = " + hashCode + " side = " +
+                        session.getSide());
+                    store.put(hashCode, session);
+            }
         }
         return WolfSSL.SSL_SUCCESS;
     }
@@ -374,10 +412,13 @@ public class WolfSSLAuthStore {
     protected Enumeration<byte[]> getAllIDs(int side) {
         List<byte[]> ret = new ArrayList<>();
 
-        for (Object obj : store.values()) {
+        synchronized (storeLock) {
-            WolfSSLImplementSSLSession current = (WolfSSLImplementSSLSession)obj;
+            for (Object obj : store.values()) {
-            if (current.getSide() == side) {
+                WolfSSLImplementSSLSession current =
-                ret.add(current.getId());
+                    (WolfSSLImplementSSLSession)obj;
+                if (current.getSide() == side) {
+                    ret.add(current.getId());
+                }
             }
         }
         return Collections.enumeration(ret);
@@ -393,12 +434,15 @@ public class WolfSSLAuthStore {
     protected WolfSSLImplementSSLSession getSession(byte[] ID, int side) {
         WolfSSLImplementSSLSession ret = null;
 
-        for (Object obj : store.values()) {
+        synchronized (storeLock) {
-            WolfSSLImplementSSLSession current = (WolfSSLImplementSSLSession)obj;
+            for (Object obj : store.values()) {
-            if (current.getSide() == side &&
+                WolfSSLImplementSSLSession current =
-                    java.util.Arrays.equals(ID, current.getId())) {
+                    (WolfSSLImplementSSLSession)obj;
-                ret = current;
+                if (current.getSide() == side &&
-                break;
+                        java.util.Arrays.equals(ID, current.getId())) {
+                    ret = current;
+                    break;
+                }
             }
         }
         return ret;
@@ -415,24 +459,26 @@ public class WolfSSLAuthStore {
         Date currentDate = new Date();
         long now = currentDate.getTime();
 
-        for (Object obj : store.values()) {
+        synchronized (storeLock) {
-            long diff;
+            for (Object obj : store.values()) {
-            WolfSSLImplementSSLSession current =
+                long diff;
-                (WolfSSLImplementSSLSession)obj;
+                WolfSSLImplementSSLSession current =
+                    (WolfSSLImplementSSLSession)obj;
 
-            if (current.getSide() == side) {
+                if (current.getSide() == side) {
-                /* difference in seconds */
+                    /* difference in seconds */
-                diff = (now - current.creation.getTime()) / 1000;
+                    diff = (now - current.creation.getTime()) / 1000;
 
-                if (diff < 0) {
+                    if (diff < 0) {
-                /* session is from the future ... */ //@TODO
+                    /* session is from the future ... */ //@TODO
 
+                    }
+
+                    if (in > 0 && diff > in) {
+                        current.invalidate();
+                    }
+                    current.setNativeTimeout(in);
                 }
-
-                if (in > 0 && diff > in) {
-                    current.invalidate();
-                }
-                current.setNativeTimeout(in);
             }
         }
     }

src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java

@@ -274,9 +274,9 @@ public class WolfSSLEngine extends SSLEngine {
     private synchronized int ClosingConnection() {
         int ret;
 
-        if (this.getUseClientMode()) {
+        /* Save session into WolfSSLAuthStore cache, saves session
-            EngineHelper.saveSession();
+         * pointer for resumption if on client side */
-        }
+        EngineHelper.saveSession();
 
         /* get current close_notify state */
         UpdateCloseNotifyStatus();
@@ -317,6 +317,13 @@ public class WolfSSLEngine extends SSLEngine {
                     "ssl.accept() ret:err = " + ret + " : " +
                     ssl.getError(ret));
             }
+
+            /* Once handshake is finished, save session for resumption in
+             * case caller does not explicitly close connection. Saves
+             * session in WolfSSLAuthStore cache, and gets/saves session
+             * pointer for resumption if on client side. */
+            EngineHelper.saveSession();
+
         } catch (SocketTimeoutException e) {
             throw new SSLException(e);
         }

src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java

@@ -260,7 +260,7 @@ public class WolfSSLEngineHelper {
     protected void setUseClientMode(boolean mode)
         throws IllegalArgumentException {
 
-        if (ssl.handshakeDone()) {
+        if (this.ssl.handshakeDone()) {
             throw new IllegalArgumentException("setUseClientMode() not " +
                 "allowed after handshake is completed");
         }
@@ -365,7 +365,7 @@ public class WolfSSLEngineHelper {
      *         handshake has not finished
      */
     protected byte[] getAlpnSelectedProtocol() {
-        if (ssl.handshakeDone()) {
+        if (this.ssl.handshakeDone()) {
             return ssl.getAlpnSelected();
         }
         return null;
@@ -378,7 +378,7 @@ public class WolfSSLEngineHelper {
      *         if handshake has not finished
      */
     protected String getAlpnSelectedProtocolString() {
-        if (ssl.handshakeDone()) {
+        if (this.ssl.handshakeDone()) {
             String proto = ssl.getAlpnSelectedString();
 
             WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
@@ -746,12 +746,6 @@ public class WolfSSLEngineHelper {
 
                 throw new SSLHandshakeException("Session creation not allowed");
             }
-
-            if (this.sessionCreation) {
-                /* can only add new sessions to the resumption table if session
-                 * creation is allowed */
-                this.authStore.addSession(this.session);
-            }
         }
 
         this.setLocalParams();
@@ -828,6 +822,17 @@ public class WolfSSLEngineHelper {
                  (err == WolfSSL.SSL_ERROR_WANT_READ ||
                   err == WolfSSL.SSL_ERROR_WANT_WRITE));
 
+        if (this.sessionCreation && ret == WolfSSL.SSL_SUCCESS) {
+            /* can only add new sessions to the resumption table if session
+             * creation is allowed */
+            if (this.clientMode) {
+                /* Only need to set resume on client side, server-side
+                 * maintains session cache at native level. */
+                this.session.setResume();
+            }
+            this.authStore.addSession(this.session);
+        }
+
         return ret;
     }
 
@@ -836,7 +841,12 @@ public class WolfSSLEngineHelper {
      */
     protected void saveSession() {
         if (this.session != null && this.session.isValid()) {
-            this.session.setResume();
+            if (this.clientMode) {
+                /* Only need to set resume on client side, server-side
+                 * maintains session cache at native level. */
+                this.session.setResume();
+            }
+            this.authStore.addSession(this.session);
         }
     }
 }

src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java

@@ -48,21 +48,23 @@ import javax.net.ssl.X509KeyManager;
  */
 @SuppressWarnings("deprecation")
 public class WolfSSLImplementSSLSession implements SSLSession {
-    private WolfSSLSession ssl;
+    private WolfSSLSession ssl = null;
     private final WolfSSLAuthStore authStore;
     private WolfSSLSessionContext ctx = null;
-    private boolean valid;
+    private boolean valid = false;
     private final HashMap<String, Object> binding;
     private final int port;
     private final String host;
-    Date creation;
+    Date creation = null;
-    Date accessed; /* when new connection was made using session */
+    Date accessed = null; /* when new connection was made using session */
     byte[] pseudoSessionID = null; /* used with TLS 1.3*/
     private int side = 0;
 
     /** Has this session been registered */
     protected boolean fromTable = false;
 
+    /** Native pointer to WOLFSSL_SESSION structure. Obtained via
+     * wolfSSL_get1_session(), so needs to be freed */
     private long sesPtr = 0;
     private String nullCipher = "SSL_NULL_WITH_NULL_NULL";
     private String nullProtocol = "NONE";
@@ -506,8 +508,10 @@ public class WolfSSLImplementSSLSession implements SSLSession {
      * @param in WOLFSSL session to set resume in
      */
     protected synchronized void resume(WolfSSLSession in) {
+        /* Set session (WOLFSSL_SESSION) into native WOLFSSL, makes
+         * a copy of the session so this object can free sesPtr when ready */
+        in.setSession(this.sesPtr);
         ssl = in;
-        ssl.setSession(this.sesPtr);
     }
 
 
@@ -516,6 +520,9 @@ public class WolfSSLImplementSSLSession implements SSLSession {
      */
     protected synchronized void setResume() {
         if (ssl != null) {
+            if (this.sesPtr != 0) {
+                WolfSSLSession.freeSession(this.sesPtr);
+            }
             this.sesPtr = ssl.getSession();
         }
     }
@@ -555,4 +562,16 @@ public class WolfSSLImplementSSLSession implements SSLSession {
     protected int getSide() {
         return this.side;
     }
+
+    @SuppressWarnings("deprecation")
+    @Override
+    protected void finalize() throws Throwable
+    {
+        if (this.sesPtr != 0) {
+            WolfSSLSession.freeSession(this.sesPtr);
+            this.sesPtr = 0;
+        }
+
+        super.finalize();
+    }
 }

src/test/com/wolfssl/test/WolfSSLContextTest.java

@@ -384,7 +384,11 @@ public class WolfSSLContextTest {
                 fail("setMinRSAKeySize did not pass as expected (1024 limit)");
             }
 
-            /* set min key size to something very large for next test */
+            /* set min key size to something very large for next test. Below
+             * we test ctx.useCertificateFile(), but that API will only fail
+             * based on key size limitations when peer verification is
+             * enabled, set SSL_VERIFY_PEER here. */
+            ctx.setVerify(WolfSSL.SSL_VERIFY_PEER, null);
             ret = ctx.setMinRSAKeySize(8192);
             if (ret != WolfSSL.SSL_SUCCESS) {
                 System.out.println("\t\t... failed");