> From: Mark Thomas [mailto:ma...@apache.org] 
> Subject: Re: svn commit: r1737154 - in /tomcat/native/trunk: 
> native/src/sslcontext.c xdocs/miscellaneous/changelog.xml

> The implementation is essentially a copy/paste of setCertificateRaw with
> what looked to be the right changes to remove the unnecessary private
> key code and to call the right OpenSSL method to set the chain.

> It does work - in that SSL Labs sees the full chain - but the code may
> well be terrible. I wouldn't be surprised if it leaked memory.

I don't see any obvious leaks (although I'm unfamiliar with OpenSSL semantics), 
but using a goto is generally frowned upon.  Better code might be something 
like this:

+    certs = d2i_X509(NULL, &tmp, lengthOfCert);
+    if (certs == NULL) {
+        ERR_error_string(ERR_get_error(), err);
+        tcn_Throw(e, "Error reading certificate (%s)", err);
+        rv = JNI_FALSE;
+    } else if (SSL_CTX_add0_chain_cert(c->ctx, certs) <= 0) {
+        ERR_error_string(ERR_get_error(), err);
+        tcn_Throw(e, "Error setting certificate (%s)", err);
+        rv = JNI_FALSE;
+    }
+
+    free(cert);
+    return rv;

 - Chuck


THIS COMMUNICATION MAY CONTAIN CONFIDENTIAL AND/OR OTHERWISE PROPRIETARY 
MATERIAL and is thus for use only by the intended recipient. If you received 
this in error, please contact the sender and delete the e-mail and its 
attachments from all computers.


-----Original Message-----
From: Mark Thomas [mailto:ma...@apache.org] 
Sent: 2016 March 30, Wednesday 14:33
To: dev@tomcat.apache.org
Subject: Re: svn commit: r1737154 - in /tomcat/native/trunk: 
native/src/sslcontext.c xdocs/miscellaneous/changelog.xml

On 30/03/2016 20:27, ma...@apache.org wrote:
> Author: markt
> Date: Wed Mar 30 19:27:29 2016
> New Revision: 1737154
> 
> URL: http://svn.apache.org/viewvc?rev=1737154&view=rev
> Log:
> Add support for obtaining the certificate chain from a Java keystore

This needs a review by someone who knows C better than I do.

The implementation is essentially a copy/paste of setCertificateRaw with
what looked to be the right changes to remove the unnecessary private
key code and to call the right OpenSSL method to set the chain.

It does work - in that SSL Labs sees the full chain - but the code may
well be terrible. I wouldn't be surprised if it leaked memory.

Once this has been reviewed and fixed, I plan to do a tc-native release
so we can up the minimum required version in 9.0.x and 8.5.x and ship
the next releases with the necessary tc-native code to use this feature.

Mark


> 
> Modified:
>     tomcat/native/trunk/native/src/sslcontext.c
>     tomcat/native/trunk/xdocs/miscellaneous/changelog.xml
> 
> Modified: tomcat/native/trunk/native/src/sslcontext.c
> URL: 
> http://svn.apache.org/viewvc/tomcat/native/trunk/native/src/sslcontext.c?rev=1737154&r1=1737153&r2=1737154&view=diff
> ==============================================================================
> --- tomcat/native/trunk/native/src/sslcontext.c (original)
> +++ tomcat/native/trunk/native/src/sslcontext.c Wed Mar 30 19:27:29 2016
> @@ -1051,7 +1051,7 @@ TCN_IMPLEMENT_CALL(jboolean, SSLContext,
>      certs = d2i_X509(NULL, &tmp, lengthOfCert);
>      if (certs == NULL) {
>          ERR_error_string(ERR_get_error(), err);
> -        tcn_Throw(e, "Error reading certificat (%s)", err);
> +        tcn_Throw(e, "Error reading certificate (%s)", err);
>          rv = JNI_FALSE;
>          goto cleanup;
>      }
> @@ -1119,6 +1119,50 @@ cleanup:
>      free(cert);
>      return rv;
>  }
> +
> +TCN_IMPLEMENT_CALL(jboolean, SSLContext, 
> addChainCertificateRaw)(TCN_STDARGS, jlong ctx,
> +                                                                 jbyteArray 
> javaCert)
> +{
> +    jsize lengthOfCert;
> +    unsigned char* cert;
> +    X509 * certs;
> +    EVP_PKEY * evp;
> +    const unsigned char *tmp;
> +    BIO * bio;
> +
> +    tcn_ssl_ctxt_t *c = J2P(ctx, tcn_ssl_ctxt_t *);
> +    jboolean rv = JNI_TRUE;
> +    char err[256];
> +
> +    /* we get the cert contents into a byte array */
> +    jbyte* bufferPtr = (*e)->GetByteArrayElements(e, javaCert, NULL);
> +    lengthOfCert = (*e)->GetArrayLength(e, javaCert);
> +    cert = malloc(lengthOfCert);
> +    memcpy(cert, bufferPtr, lengthOfCert);
> +    (*e)->ReleaseByteArrayElements(e, javaCert, bufferPtr, 0);
> +
> +    UNREFERENCED(o);
> +    TCN_ASSERT(ctx != 0);
> +
> +    tmp = (const unsigned char *)cert;
> +    certs = d2i_X509(NULL, &tmp, lengthOfCert);
> +    if (certs == NULL) {
> +        ERR_error_string(ERR_get_error(), err);
> +        tcn_Throw(e, "Error reading certificate (%s)", err);
> +        rv = JNI_FALSE;
> +        goto cleanup;
> +    }
> +
> +    if (SSL_CTX_add0_chain_cert(c->ctx, certs) <= 0) {
> +        ERR_error_string(ERR_get_error(), err);
> +        tcn_Throw(e, "Error setting certificate (%s)", err);
> +        rv = JNI_FALSE;
> +    }
> +
> +cleanup:
> +    free(cert);
> +    return rv;
> +}
>  
>  static int ssl_array_index(apr_array_header_t *array,
>                             const char *s)
> 
> Modified: tomcat/native/trunk/xdocs/miscellaneous/changelog.xml
> URL: 
> http://svn.apache.org/viewvc/tomcat/native/trunk/xdocs/miscellaneous/changelog.xml?rev=1737154&r1=1737153&r2=1737154&view=diff
> ==============================================================================
> --- tomcat/native/trunk/xdocs/miscellaneous/changelog.xml (original)
> +++ tomcat/native/trunk/xdocs/miscellaneous/changelog.xml Wed Mar 30 19:27:29 
> 2016
> @@ -54,6 +54,9 @@
>      <fix>
>        Fix some compiler warnings in native ssl code. (rjung)
>      </fix>
> +    <add>
> +      Add support for using Java keystores for certificate chains. (markt)
> +    </add>
>    </changelog>
>  </section>
>  <section name="Changes in 1.2.5">
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to