Mark, On 3/30/16 3:33 PM, Mark Thomas wrote: > 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.
See below. > 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); Since bufferPtr is the byte array you want to use, you can probably just use that directly for the call to d2i_x509(). I think the malloc/memcpy/free is not necessary. Obviously, don't call ReleaseByteArray until after calling d2i_509. >> + >> + 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; >> +} You could probably avoid the label/goto with a slight more complicated conditional structure, but I don't see a particular reason to do so. -chris
signature.asc
Description: OpenPGP digital signature
