Merge pull request #188 from cconlon/getPeerCertificate

Free WOLFSSL_X509 from wolfSSL_get_peer_certificate() with wolfSSL >= 5.3.0

examples/Client.java

@@ -29,6 +29,7 @@
 import com.wolfssl.WolfSSL;
 import com.wolfssl.WolfSSLSession;
 import com.wolfssl.WolfSSLContext;
+import com.wolfssl.WolfSSLCertificate;
 import com.wolfssl.WolfSSLException;
 import com.wolfssl.WolfSSLJNIException;
 import com.wolfssl.WolfSSLIOSendCallback;
@@ -722,11 +723,11 @@ public void run(String[] args) {
 
     void showPeer(WolfSSLSession ssl) {
 
+        long peerCrtPtr = 0;
         String altname;
 
         try {
-
-            long peerCrtPtr = ssl.getPeerCertificate();
+            peerCrtPtr = ssl.getPeerCertificate();
 
             if (peerCrtPtr != 0) {
                 System.out.println("issuer : " +
@@ -743,6 +744,13 @@ void showPeer(WolfSSLSession ssl) {
 
         } catch (WolfSSLJNIException e) {
             e.printStackTrace();
+
+        } finally {
+            if (WolfSSL.getLibVersionHex() >= 0x05003000) {
+                if (peerCrtPtr != 0) {
+                    WolfSSLCertificate.freeX509(peerCrtPtr);
+                }
+            }
         }
     }
 

examples/Server.java

@@ -29,6 +29,7 @@
 import com.wolfssl.WolfSSL;
 import com.wolfssl.WolfSSLSession;
 import com.wolfssl.WolfSSLContext;
+import com.wolfssl.WolfSSLCertificate;
 import com.wolfssl.WolfSSLException;
 import com.wolfssl.WolfSSLJNIException;
 import com.wolfssl.WolfSSLIOSendCallback;
@@ -618,7 +619,7 @@ public void run(String[] args) {
     void showPeer(WolfSSLSession ssl) {
 
         String altname;
-        long peerCrtPtr;
+        long peerCrtPtr = 0;
 
         try {
 
@@ -643,6 +644,13 @@ void showPeer(WolfSSLSession ssl) {
 
         } catch (WolfSSLJNIException e) {
             e.printStackTrace();
+
+        } finally {
+            if (WolfSSL.getLibVersionHex() >= 0x05003000) {
+                if (peerCrtPtr != 0) {
+                    WolfSSLCertificate.freeX509(peerCrtPtr);
+                }
+            }
         }
     }
 

src/java/com/wolfssl/WolfSSLCertificate.java

@@ -277,19 +277,29 @@ public WolfSSLCertificate(String fileName, int format)
      * Create WolfSSLCertificate from pre existing native pointer.
      *
      * @param x509 pre existing native pointer to WOLFSSL_X509 structure.
+     * @param doFree should this WOLFSSL_X509 structure be freed when free()
+     *        is called? true to free memory, false to skip free. Free
+     *        should be skipped if caller is controlling memory for this
+     *        WOLFSSL_X509 struct pointer.
      *
      * @throws WolfSSLException if input pointer is invalid
      */
-    public WolfSSLCertificate(long x509) throws WolfSSLException {
+    public WolfSSLCertificate(long x509, boolean doFree)
+        throws WolfSSLException {
 
         if (x509 == 0) {
             throw new WolfSSLException("Input pointer may not be 0/NULL");
         }
         x509Ptr = x509;
 
-        /* x509Ptr has NOT been allocated natively, do not mark as owned.
-         * Original owner is responsible for freeing. */
-        this.weOwnX509Ptr = false;
+        if (!doFree) {
+            /* x509Ptr has NOT been allocated natively, do not mark as owned.
+             * Original owner is responsible for freeing. */
+            this.weOwnX509Ptr = false;
+        }
+        else {
+            this.weOwnX509Ptr = true;
+        }
 
         synchronized (stateLock) {
             this.active = true;
@@ -1498,6 +1508,30 @@ public X509Certificate getX509Certificate()
         return cert;
     }
 
+    /**
+     * Free native WOLFSSL_X509 structure pointer.
+     * This method should be called with caution, so that double free
+     * issues do not happen. Calling code should take extra caution to ensure
+     * control is maintained around this native pointer and synchronization
+     * is used correctly to ensure multiple threads will not free the same
+     * pointer simultaneously.
+     *
+     * This method is needed when WolfSSLSession.getPeerCertificate()
+     * is called with wolfSSL versions later than or equal to 5.3.0.
+     * See wolfSSL PR 4807.
+     *
+     * @param x509 pointer to initialized WOLFSSL_X509 structure to be freed
+     *        by calling wolfSSL_X509_free() at the JNI level.
+     */
+    public static void freeX509(long x509) {
+
+        /* No object active check or synchronization needed, since this
+         * method is working on a standalone pointer, not related to this
+         * object. Method placed here since it relates to common
+         * WOLFSSL_X509 / WolfSSLCertificate functionality */
+        X509_free(x509);
+    }
+
     @Override
     public String toString() {
 

src/java/com/wolfssl/WolfSSLSession.java

@@ -1796,6 +1796,17 @@ public int sessionReused()
      * This can be used to retrieve further information about the peer's
      * certificate (issuer, subject, alt name, etc.)
      *
+     * wolfSSL versions 5.3.0 or later return a newly-allocated
+     * WOLFSSL_X509 structure poiner from the native
+     * wolfSSL_get_peer_certificate() API called by this wrapper. If using
+     * wolfSSL greater than or equal to 5.3.0, the pointer (long) returned
+     * from this method must be freed by the caller. Versions of wolfSSL
+     * less than 5.3.0 should not free the pointer returned since it points
+     * to internal memory that is freed by native wolfSSL.
+     *
+     * Pointer should be freed by calling:
+     *     WolfSSLCertificate.freeX509(long x509);
+     *
      * @return (long) WOLFSSL_X509 pointer to the peer's certificate.
      * @throws IllegalStateException WolfSSLContext has been freed
      * @throws WolfSSLJNIException Internal JNI error

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

@@ -480,7 +480,17 @@ public synchronized Certificate[] getPeerCertificates()
         }
 
         try {
-            cert = new WolfSSLX509(x509);
+            /* wolfSSL starting with 5.3.0 returns a new WOLFSSL_X509
+             * structure from wolfSSL_get_peer_certificate(). In that case,
+             * we need to free the pointer when finished. Prior to 5.3.0,
+             * this memory was freed internally by wolfSSL since the API
+             * only returned a pointer to internal memory */
+            if (WolfSSL.getLibVersionHex() >= 0x05003000) {
+                cert = new WolfSSLX509(x509, true);
+            }
+            else {
+                cert = new WolfSSLX509(x509, false);
+            }
         } catch (WolfSSLException ex) {
             throw new SSLPeerUnverifiedException("Error creating certificate");
         }
@@ -529,14 +539,32 @@ public Certificate[] getLocalCertificates() {
     @Override
     public synchronized javax.security.cert.X509Certificate[] getPeerCertificateChain()
         throws SSLPeerUnverifiedException {
+
+        long peerX509 = 0;
         WolfSSLX509X x509;
 
         if (ssl == null) {
             throw new SSLPeerUnverifiedException("handshake not done");
         }
 
         try {
-            x509 = new WolfSSLX509X(this.ssl.getPeerCertificate());
+            peerX509 = this.ssl.getPeerCertificate();
+            if (peerX509 == 0) {
+                return null;
+            }
+
+            /* wolfSSL starting with 5.3.0 returns a new WOLFSSL_X509
+             * structure from wolfSSL_get_peer_certificate(). In that case,
+             * we need to free the pointer when finished. Prior to 5.3.0,
+             * this memory was freed internally by wolfSSL since the API
+             * only returned a pointer to internal memory */
+            if (WolfSSL.getLibVersionHex() >= 0x05003000) {
+                x509 = new WolfSSLX509X(peerX509, true);
+            }
+            else {
+                x509 = new WolfSSLX509X(peerX509, false);
+            }
+
             return new javax.security.cert.X509Certificate[] {
                 (javax.security.cert.X509Certificate)x509 };
 
@@ -552,15 +580,37 @@ public synchronized javax.security.cert.X509Certificate[] getPeerCertificateChai
     @Override
     public synchronized Principal getPeerPrincipal()
         throws SSLPeerUnverifiedException {
+
+        long peerX509 = 0;
+        Principal peerPrincipal = null;
+        WolfSSLX509 x509 = null;
+
         if (ssl == null) {
             throw new SSLPeerUnverifiedException("handshake not done");
         }
 
         try {
-            Principal peerPrincipal = null;
-            WolfSSLX509 x509 = new WolfSSLX509(this.ssl.getPeerCertificate());
-            peerPrincipal = x509.getSubjectDN();
-            x509.free();
+            peerX509 = this.ssl.getPeerCertificate();
+            if (peerX509 == 0) {
+                return null;
+            }
+
+            /* wolfSSL starting with 5.3.0 returns a new WOLFSSL_X509
+             * structure from wolfSSL_get_peer_certificate(). In that case,
+             * we need to free the pointer when finished. Prior to 5.3.0,
+             * this memory was freed internally by wolfSSL since the API
+             * only returned a pointer to internal memory */
+            if (WolfSSL.getLibVersionHex() >= 0x05003000) {
+                x509 = new WolfSSLX509(peerX509, true);
+            }
+            else {
+                x509 = new WolfSSLX509(peerX509, false);
+            }
+
+            if (x509 != null) {
+                peerPrincipal = x509.getSubjectDN();
+                x509.free();
+            }
 
             return peerPrincipal;
 

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

@@ -103,15 +103,19 @@ public WolfSSLX509(String derName) throws WolfSSLException {
      * Create new WolfSSLX509 object
      *
      * @param x509 initialized pointer to native WOLFSSL_X509 struct
+     * @param doFree should this WOLFSSL_X509 structure be freed when free()
+     *        is called? true to free memory, false to skip free. Free
+     *        should be skipped if caller is controlling memory for this
+     *        WOLFSSL_X509 struct pointer.
      *
      * @throws WolfSSLException if certificate parsing fails
      */
-    public WolfSSLX509(long x509) throws WolfSSLException {
+    public WolfSSLX509(long x509, boolean doFree) throws WolfSSLException {
         super();
-        this.cert = new WolfSSLCertificate(x509);
+        this.cert = new WolfSSLCertificate(x509, doFree);
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-            "created new WolfSSLX509(long x509)");
+            "created new WolfSSLX509(long x509, boolean doFree)");
     }
 
     @Override

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

@@ -77,15 +77,19 @@ public WolfSSLX509X(String derName) throws WolfSSLException {
      * Create new WolfSSLX509X object
      *
      * @param x509 initialized pointer to native WOLFSSL_X509 struct
+     * @param doFree should this WOLFSSL_X509 structure be freed when free()
+     *        is called? true to free memory, false to skip free. Free
+     *        should be skipped if caller is controlling memory for this
+     *        WOLFSSL_X509 struct pointer.
      *
      * @throws WolfSSLException if certificate parsing fails
      */
-    public WolfSSLX509X(long x509) throws WolfSSLException {
+    public WolfSSLX509X(long x509, boolean doFree) throws WolfSSLException {
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-            "created new WolfSSLX509X(long x509)");
+            "created new WolfSSLX509X(long x509, boolean doFree)");
 
-        this.cert = new WolfSSLX509(x509);
+        this.cert = new WolfSSLX509(x509, doFree);
     }
 
     @Override