Skip to content

Commit

Permalink
Properly omit self signed CA from untrusted intermediates, handle mem…
Browse files Browse the repository at this point in the history 
…ory leak for SSL case with proper flow
  • Loading branch information
@ColtonWilley
ColtonWilley committed Oct 23, 2024
1 parent 3e3ae01 commit c6492c9
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 38 deletions.
90 changes: 52 additions & 38 deletions src/x509_str.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ WOLFSSL_X509_STORE_CTX* wolfSSL_X509_STORE_CTX_new_ex(void* heap)
XMEMSET(ctx, 0, sizeof(WOLFSSL_X509_STORE_CTX));
ctx->heap = heap;
#ifdef OPENSSL_EXTRA
if (wolfSSL_X509_STORE_CTX_init(ctx, NULL, NULL, NULL) !=
if ((ctx->owned = wolfSSL_sk_X509_new_null()) == NULL) {
XFREE(ctx, heap, DYNAMIC_TYPE_X509_CTX);
ctx = NULL;
}
if (ctx != NULL &&
wolfSSL_X509_STORE_CTX_init(ctx, NULL, NULL, NULL) !=
WOLFSSL_SUCCESS) {
XFREE(ctx, heap, DYNAMIC_TYPE_X509_CTX);
ctx = NULL;
Expand Down Expand Up @@ -94,6 +99,9 @@ void wolfSSL_X509_STORE_CTX_free(WOLFSSL_X509_STORE_CTX* ctx)
if (ctx->chain != NULL) {
wolfSSL_sk_X509_free(ctx->chain);
}
if (ctx->owned != NULL) {
wolfSSL_sk_X509_pop_free(ctx->owned, NULL);
}

if (ctx->current_issuer != NULL) {
wolfSSL_X509_free(ctx->current_issuer);
Expand Down Expand Up @@ -292,6 +300,32 @@ static int X509StoreVerifyCert(WOLFSSL_X509_STORE_CTX* ctx)
return ret;
}

static int addAllButSelfSigned(WOLF_STACK_OF(WOLFSSL_X509)*to,
WOLF_STACK_OF(WOLFSSL_X509)*from, int *numAdded)
{
int ret = WOLFSSL_SUCCESS;
int i = 0;
int cnt = 0;
WOLFSSL_X509 *x = NULL;

for (i = 0; i < wolfSSL_sk_X509_num(from); i++) {
x = wolfSSL_sk_X509_value(from, i);
if (wolfSSL_X509_NAME_cmp(&x->issuer, &x->subject) != 0) {
if (wolfSSL_sk_X509_push(to, x) <= 0) {
ret = WOLFSSL_FAILURE;
goto exit;
}
cnt++;
}
}

exit:
if (numAdded != NULL) {
*numAdded = cnt;
}
return ret;
}

/* Verifies certificate chain using WOLFSSL_X509_STORE_CTX
* returns 0 on success or < 0 on failure.
*/
Expand All @@ -305,8 +339,8 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
int depth = 0;
WOLFSSL_X509 *issuer = NULL;
WOLFSSL_X509 *orig = NULL;
WOLFSSL_X509 *tmp = NULL;
WOLF_STACK_OF(WOLFSSL_X509)* certs = NULL;
WOLF_STACK_OF(WOLFSSL_X509)* certsToUse = NULL;
WOLFSSL_ENTER("wolfSSL_X509_verify_cert");

if (ctx == NULL || ctx->store == NULL || ctx->store->cm == NULL
Expand All @@ -315,32 +349,28 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
}

certs = ctx->store->certs;
if (ctx->chain != NULL) {
wolfSSL_sk_X509_free(ctx->chain);
}
ctx->chain = wolfSSL_sk_X509_new_null();

if (ctx->setTrustedSk != NULL) {
certs = ctx->setTrustedSk;
}

if (certs == NULL &&
wolfSSL_sk_X509_num(ctx->ctxIntermediates) > 0) {
certs = ctx->ctxIntermediates;
certsToUse = wolfSSL_sk_X509_new_null();
ret = addAllButSelfSigned(certsToUse, ctx->ctxIntermediates, NULL);
}
else {
/* Add the intermediates provided on init to the list of untrusted
* intermediates to be used */
for (i = 0; i < wolfSSL_sk_X509_num(ctx->ctxIntermediates); i++) {
ret = wolfSSL_sk_X509_push(certs,
wolfSSL_sk_X509_value(ctx->ctxIntermediates, i));
if (ret <= 0) {
goto exit;
}
ret = addAllButSelfSigned(certs, ctx->ctxIntermediates, &numInterAdd);
}
if (ret != WOLFSSL_SUCCESS) {
goto exit;
}

numInterAdd++;
}
if (ctx->chain != NULL) {
wolfSSL_sk_X509_free(ctx->chain);
}
ctx->chain = wolfSSL_sk_X509_new_null();

if (ctx->depth > 0) {
depth = ctx->depth + 1;
Expand All @@ -356,26 +386,6 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
/* Try to find an untrusted issuer first */
ret = X509StoreGetIssuerEx(&issuer, certs,
ctx->current_cert);
if (issuer != NULL &&
wolfSSL_X509_NAME_cmp(&issuer->issuer, &issuer->subject) == 0) {
ret = WOLFSSL_FAILURE;
/* Self signed allowed if in set trusted stack, otherwise
* ignore it and fall back to see if its in CM */
if ((certs == ctx->setTrustedSk) &&
(wolfSSL_sk_X509_num(certs) > numInterAdd)) {
for (i = wolfSSL_sk_X509_num(certs) - 1;
i > (numInterAdd > 0 ? numInterAdd - 1 : 0);
i--) {
tmp = wolfSSL_sk_X509_value(certs, i);
if (tmp != NULL && wolfSSL_X509_NAME_cmp(
&issuer->subject, &tmp->subject) == 0) {
ret = WOLFSSL_SUCCESS;
break;
}
tmp = NULL;
}
}
}
if (ret == WOLFSSL_SUCCESS) {
if (ctx->current_cert == issuer) {
wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert);
Expand Down Expand Up @@ -417,10 +427,11 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)

/* Cert verified, finish building the chain */
wolfSSL_sk_X509_push(ctx->chain, ctx->current_cert);
issuer = NULL;
#ifdef WOLFSSL_SIGNER_DER_CERT
x509GetIssuerFromCM(&issuer, ctx->store->cm, ctx->current_cert);
if (issuer != NULL && ctx->store->owned != NULL) {
wolfSSL_sk_X509_push(ctx->store->owned, issuer);
if (issuer != NULL && ctx->owned != NULL) {
wolfSSL_sk_X509_push(ctx->owned, issuer);
}
#else
if (ctx->setTrustedSk == NULL) {
Expand Down Expand Up @@ -463,6 +474,9 @@ int wolfSSL_X509_verify_cert(WOLFSSL_X509_STORE_CTX* ctx)
ctx->current_cert = orig;
}
}
if (certsToUse != NULL) {
wolfSSL_sk_X509_free(certsToUse);
}

return ret == WOLFSSL_SUCCESS ? WOLFSSL_SUCCESS : WOLFSSL_FAILURE;
}
Expand Down
1 change: 1 addition & 0 deletions wolfssl/ssl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,7 @@ struct WOLFSSL_X509_STORE_CTX {
WOLFSSL_X509_STORE_CTX_verify_cb verify_cb; /* verify callback */
void* heap;
int flags;
WOLF_STACK_OF(WOLFSSL_X509)* owned; /* Certs owned by this CTX */
WOLF_STACK_OF(WOLFSSL_X509)* ctxIntermediates; /* Intermediates specified
* on store ctx init */
WOLF_STACK_OF(WOLFSSL_X509)* setTrustedSk;/* A trusted stack override
Expand Down

0 comments on commit c6492c9

Please sign in to comment.