On 30/03/2016 21:31, Caldarale, Charles R wrote:
>> 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),
ACK. Thanks.
> but using a goto is generally frowned upon. Better code might be something
> like this:
My defence is that I was copying the style of the previous method. If we
fix one, we should fix both. I'll see what I can do.
Cheers,
Mark
> + 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
