Skip to content

Commit

Permalink
WFSSL-9 SSL Context does not handler intermediate certificates correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
stuartwdouglas committed Apr 11, 2018
1 parent fa1bcb5 commit 7563364
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 14 deletions.
12 changes: 9 additions & 3 deletions java/src/main/java/org/wildfly/openssl/OpenSSLContextSPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ private synchronized void init(KeyManager[] kms, TrustManager[] tms) throws KeyM
if (aliases != null && aliases.length != 0) {
for(String alias: aliases) {

X509Certificate certificate = keyManager.getCertificateChain(alias)[0];
X509Certificate[] certificateChain = keyManager.getCertificateChain(alias);
PrivateKey key = keyManager.getPrivateKey(alias);
if(key == null || certificate == null || key.getEncoded() == null) {
if(key == null || certificateChain == null || key.getEncoded() == null) {
continue;
}
if (LOG.isLoggable(Level.FINE)) {
Expand All @@ -216,7 +216,13 @@ private synchronized void init(KeyManager[] kms, TrustManager[] tms) throws KeyM
}
sb.append(Base64.getMimeEncoder(64, new byte[]{'\n'}).encodeToString(encodedPrivateKey));
sb.append(rsa ? END_RSA_CERT : END_DSA_CERT);
SSL.getInstance().setCertificate(ctx, certificate.getEncoded(), sb.toString().getBytes(StandardCharsets.US_ASCII), rsa ? SSL.SSL_AIDX_RSA : SSL.SSL_AIDX_DSA);

byte[][] encodedIntermediaries = new byte[certificateChain.length - 1][];
for(int i = 1; i < certificateChain.length; ++i) {
encodedIntermediaries[i - 1] = certificateChain[i].getEncoded();
}
X509Certificate certificate = certificateChain[0];
SSL.getInstance().setCertificate(ctx, certificate.getEncoded(), encodedIntermediaries, sb.toString().getBytes(StandardCharsets.US_ASCII), rsa ? SSL.SSL_AIDX_RSA : SSL.SSL_AIDX_DSA);
break;
}
}
Expand Down
6 changes: 3 additions & 3 deletions java/src/main/java/org/wildfly/openssl/SSL.java
Original file line number Diff line number Diff line change
Expand Up @@ -973,14 +973,14 @@ protected abstract boolean setCARevocation(long ctx, String file,
* to point at the key file. Keep in mind that if
* you've both a RSA and a DSA private key you can configure
* both in parallel (to also allow the use of DSA ciphers, etc.)
*
* @param ctx Server or Client context to use.
* @param ctx Server or Client context to use.
* @param cert Certificate file.
* @param encodedIntermediaries
* @param key Private Key file to use if not in cert.
* @param idx Certificate index SSL_AIDX_RSA or SSL_AIDX_DSA.
*/
protected abstract boolean setCertificate(long ctx, byte[] cert,
byte[] key,
byte[][] encodedIntermediaries, byte[] key,
int idx)
throws Exception;

Expand Down
12 changes: 6 additions & 6 deletions java/src/main/java/org/wildfly/openssl/SSLImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -587,8 +587,8 @@ protected boolean setCARevocation(long ctx, String file, String path) throws Exc
}

@Override
protected boolean setCertificate(long ctx, byte[] cert, byte[] key, int idx) throws Exception {
return setCertificate0(ctx, cert, key, idx);
protected boolean setCertificate(long ctx, byte[] cert, byte[][] encodedIntermediaries, byte[] key, int idx) throws Exception {
return setCertificate0(ctx, cert, encodedIntermediaries, key, idx);
}

/**
Expand Down Expand Up @@ -646,15 +646,15 @@ static native boolean setCARevocation0(long ctx, String file,
* to point at the key file. Keep in mind that if
* you've both a RSA and a DSA private key you can configure
* both in parallel (to also allow the use of DSA ciphers, etc.)
*
* @param ctx Server or Client context to use.
* @param ctx Server or Client context to use.
* @param cert Certificate file.
* @param encodedIntermediaries
* @param key Private Key file to use if not in cert.
* @param idx Certificate index SSL_AIDX_RSA or SSL_AIDX_DSA.
*/
static native boolean setCertificate0(long ctx, byte[] cert,
byte[] key,
int idx) throws Exception;
byte[][] encodedIntermediaries, byte[] key,
int idx) throws Exception;

/**
* Set the size of the internal session cache.
Expand Down
1 change: 1 addition & 0 deletions libwfssl/include/wfssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ typedef unsigned __int64 uint64_t;
#define SSL_SESS_CACHE_BOTH (SSL_SESS_CACHE_CLIENT|SSL_SESS_CACHE_SERVER)

#define SSL_CTRL_SET_TMP_DH 3
#define SSL_CTRL_EXTRA_CHAIN_CERT 14
#define SSL_CTRL_SESS_NUMBER 20
#define SSL_CTRL_SESS_CONNECT 21
#define SSL_CTRL_SESS_CONNECT_GOOD 22
Expand Down
45 changes: 43 additions & 2 deletions libwfssl/src/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ WF_OPENSSL(void, setSSLOptions)(JNIEnv *e, jobject o, jlong ssl, jint opt);
WF_OPENSSL(void, clearSSLOptions)(JNIEnv *e, jobject o, jlong ssl, jint opt);
WF_OPENSSL(jboolean, setCipherSuite)(JNIEnv *e, jobject o, jlong ctx, jstring ciphers);
WF_OPENSSL(jboolean, setCARevocation)(JNIEnv *e, jobject o, jlong ctx, jstring file, jstring path);
WF_OPENSSL(jboolean, setCertificate)(JNIEnv *e, jobject o, jlong ctx, jbyteArray javaCert, jbyteArray javaKey, jint idx);
WF_OPENSSL(jboolean, setCertificate)(JNIEnv *e, jobject o, jlong ctx, jbyteArray javaCert, jobjectArray intermediateCerts, jbyteArray javaKey, jint idx);
WF_OPENSSL(void, setCertVerifyCallback)(JNIEnv *e, jobject o, jlong ctx, jobject verifier);
WF_OPENSSL(jboolean, setSessionIdContext)(JNIEnv *e, jobject o, jlong ctx, jbyteArray sidCtx);
WF_OPENSSL(jlong, newSSL)(JNIEnv *e, jobject o, jlong ctx /* tcn_ssl_ctxt_t * */, jboolean server);
Expand Down Expand Up @@ -881,11 +881,12 @@ WF_OPENSSL(jboolean, setCARevocation)(JNIEnv *e, jobject o, jlong ctx, jstring f
return rv;
}

WF_OPENSSL(jboolean, setCertificate)(JNIEnv *e, jobject o, jlong ctx, jbyteArray javaCert, jbyteArray javaKey, jint idx)
WF_OPENSSL(jboolean, setCertificate)(JNIEnv *e, jobject o, jlong ctx, jbyteArray javaCert, jobjectArray intermediateCerts, jbyteArray javaKey, jint idx)
{
#pragma comment(linker, "/EXPORT:"__FUNCTION__"="__FUNCDNAME__)
/* we get the key contents into a byte array */
unsigned char* cert;
unsigned char* intCert;
char err[256];
jboolean rv;
const unsigned char *tmp;
Expand All @@ -895,15 +896,21 @@ WF_OPENSSL(jboolean, setCertificate)(JNIEnv *e, jobject o, jlong ctx, jbyteArray
unsigned char* key = malloc(lengthOfKey);
tcn_ssl_ctxt_t *c;
BIO * bio;
X509* tmpIntCert;
int intermediateLength;
jbyteArray byteArray;

memmove(key, bufferPtr, lengthOfKey);
(*e)->ReleaseByteArrayElements(e, javaKey, bufferPtr, 0);


bufferPtr = (*e)->GetByteArrayElements(e, javaCert, NULL);
lengthOfCert = (*e)->GetArrayLength(e, javaCert);
cert = malloc(lengthOfCert);
memmove(cert, bufferPtr, lengthOfCert);
(*e)->ReleaseByteArrayElements(e, javaCert, bufferPtr, 0);


c = J2P(ctx, tcn_ssl_ctxt_t *);
rv = JNI_TRUE;

Expand Down Expand Up @@ -944,6 +951,40 @@ WF_OPENSSL(jboolean, setCertificate)(JNIEnv *e, jobject o, jlong ctx, jbyteArray
rv = JNI_FALSE;
goto cleanup;
}

/*intermediate certs */
intermediateLength = (*e)->GetArrayLength(e, intermediateCerts);
for(int i = 0; i < intermediateLength; ++i ) {

byteArray = (*e)->GetObjectArrayElement(e, intermediateCerts, i);
bufferPtr = (*e)->GetByteArrayElements(e, byteArray, NULL);
lengthOfCert = (*e)->GetArrayLength(e, byteArray);
intCert = malloc(lengthOfCert);
memmove(intCert, bufferPtr, lengthOfCert);
(*e)->ReleaseByteArrayElements(e, byteArray, bufferPtr, 0);

tmp = (const unsigned char *)intCert;


if ((tmpIntCert = crypto_methods.d2i_X509(NULL, &tmp, lengthOfCert)) == NULL) {
crypto_methods.ERR_error_string(crypto_methods.ERR_get_error(), err);
throwIllegalStateException(e, err);
rv = JNI_FALSE;
free(intCert);
goto cleanup;
}

if (ssl_methods.SSL_CTX_ctrl(c->ctx,SSL_CTRL_EXTRA_CHAIN_CERT,0,(char *)tmpIntCert) <= 0) {
crypto_methods.ERR_error_string(crypto_methods.ERR_get_error(), err);
tcn_Throw(e, "Error setting certificate (%s)", err);
rv = JNI_FALSE;
free(intCert);
goto cleanup;
}
free(intCert);
}


if (ssl_methods.SSL_CTX_use_PrivateKey(c->ctx, c->keys[idx]) <= 0) {
crypto_methods.ERR_error_string(crypto_methods.ERR_get_error(), err);
tcn_Throw(e, "Error setting private key (%s)", err);
Expand Down

1 comment on commit 7563364

@mchoma
Copy link

@mchoma mchoma commented on 7563364 Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes seems reasonable on first sight.

Please sign in to comment.