Skip to content

Prevent UAF on DER objects, reject API in SNI callback #10905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
padelsbach wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Prevent UAF on DER objects, reject API in SNI callback #10905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7174,6 +7174,15 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
/* ctx still owns certificate, certChain, key, dh, and cm */
ssl->buffers.certificate = ctx->certificate;
ssl->buffers.certChain = ctx->certChain;
/* Inc refcount on the shared DER buffers */
if (!RefDer(ssl->buffers.certificate)) {
ssl->buffers.certificate = NULL;
return BAD_MUTEX_E;
}
if (!RefDer(ssl->buffers.certChain)) {
ssl->buffers.certChain = NULL;
return BAD_MUTEX_E;
}
#endif
ssl->buffers.certChainCnt = ctx->certChainCnt;
#ifndef WOLFSSL_BLIND_PRIVATE_KEY
Expand All @@ -7191,10 +7200,15 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
ssl->buffers.weOwnKey = 1;
}
else {
ssl->buffers.key = ctx->privateKey;
ssl->buffers.key = ctx->privateKey;
}
#else
ssl->buffers.key = ctx->privateKey;
ssl->buffers.key = ctx->privateKey;
/* Inc refcount on the shared DER buffer */
if (!RefDer(ssl->buffers.key)) {
ssl->buffers.key = NULL;
return BAD_MUTEX_E;
}
#endif
#else
if (ctx->privateKey != NULL) {
Expand Down Expand Up @@ -7225,6 +7239,10 @@ static int SetSSL_CTX_CertsAndKeys(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
#ifdef WOLFSSL_DUAL_ALG_CERTS
#ifndef WOLFSSL_BLIND_PRIVATE_KEY
ssl->buffers.altKey = ctx->altPrivateKey;
if (!RefDer(ssl->buffers.altKey)) {
ssl->buffers.altKey = NULL;
return BAD_MUTEX_E;
}
#else
if (ctx->altPrivateKey != NULL) {
ret = AllocCopyDer(&ssl->buffers.altKey, ctx->altPrivateKey->buffer,
Expand Down Expand Up @@ -9218,6 +9236,12 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl)
#ifndef NO_CERTS
ssl->keepCert = 0; /* make sure certificate is free'd */
wolfSSL_UnloadCertsKeys(ssl);
FreeSslDer(&ssl->buffers.certificate, ssl->buffers.weOwnCert);
FreeSslDer(&ssl->buffers.certChain, ssl->buffers.weOwnCertChain);
FreeSslDer(&ssl->buffers.key, ssl->buffers.weOwnKey);
#ifdef WOLFSSL_DUAL_ALG_CERTS
FreeSslDer(&ssl->buffers.altKey, ssl->buffers.weOwnAltKey);
#endif
#endif
#ifndef NO_RSA
FreeKey(ssl, DYNAMIC_TYPE_RSA, (void**)&ssl->peerRsaKey);
Expand Down Expand Up @@ -29047,6 +29071,9 @@ const char* wolfSSL_ERR_reason_error_string(unsigned long e)

case RPK_UNTRUSTED_E:
return "RFC 7250 Raw Public Key not trusted";

case CTX_BUSY_E:
return "Context in use by active sessions, operation not allowed";
}

return "unknown error number";
Expand Down Expand Up @@ -43830,8 +43857,13 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
/* Stunnel supports a custom sni callback to switch an SSL's ctx
* when SNI is received. Call it now if exists */
if(ssl && ssl->ctx && ssl->ctx->sniRecvCb) {
WOLFSSL_CTX* cbCtx = ssl->ctx;
byte inSniPrev = cbCtx->inSniCallback;

WOLFSSL_MSG("Calling custom sni callback");
sniRet = ssl->ctx->sniRecvCb(ssl, &ad, ssl->ctx->sniRecvCbArg);
cbCtx->inSniCallback = 1;
sniRet = cbCtx->sniRecvCb(ssl, &ad, cbCtx->sniRecvCbArg);
cbCtx->inSniCallback = inSniPrev;
switch (sniRet) {
case warning_return:
WOLFSSL_MSG("Error in custom sni callback. Warning alert");
Expand Down
39 changes: 10 additions & 29 deletions src/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12727,47 +12727,24 @@ void wolfSSL_certs_clear(WOLFSSL* ssl)
if (ssl == NULL)
return;

/* ctx still owns certificate, certChain, key, dh, and cm */
if (ssl->buffers.weOwnCert) {
FreeDer(&ssl->buffers.certificate);
ssl->buffers.weOwnCert = 0;
}
ssl->buffers.certificate = NULL;
if (ssl->buffers.weOwnCertChain) {
FreeDer(&ssl->buffers.certChain);
ssl->buffers.weOwnCertChain = 0;
}
ssl->buffers.certChain = NULL;
FreeSslDer(&ssl->buffers.certificate, ssl->buffers.weOwnCert);
FreeSslDer(&ssl->buffers.certChain, ssl->buffers.weOwnCertChain);
#ifdef WOLFSSL_TLS13
ssl->buffers.certChainCnt = 0;
#endif
if (ssl->buffers.weOwnKey) {
FreeDer(&ssl->buffers.key);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.keyMask);
#endif
ssl->buffers.weOwnKey = 0;
}
ssl->buffers.key = NULL;
FreeSslDer(&ssl->buffers.key, ssl->buffers.weOwnKey);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
ssl->buffers.keyMask = NULL;
FreeDer(&ssl->buffers.keyMask);
#endif
ssl->buffers.keyType = 0;
ssl->buffers.keyId = 0;
ssl->buffers.keyLabel = 0;
ssl->buffers.keySz = 0;
ssl->buffers.keyDevId = 0;
#ifdef WOLFSSL_DUAL_ALG_CERTS
if (ssl->buffers.weOwnAltKey) {
FreeDer(&ssl->buffers.altKey);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
FreeDer(&ssl->buffers.altKeyMask);
#endif
ssl->buffers.weOwnAltKey = 0;
}
ssl->buffers.altKey = NULL;
FreeSslDer(&ssl->buffers.altKey, ssl->buffers.weOwnAltKey);
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
ssl->buffers.altKeyMask = NULL;
FreeDer(&ssl->buffers.altKeyMask);
#endif
#endif /* WOLFSSL_DUAL_ALG_CERTS */
}
Expand Down Expand Up @@ -12830,6 +12807,10 @@ long wolfSSL_CTX_ctrl(WOLFSSL_CTX* ctx, int cmd, long opt, void* pt)
ret = WOLFSSL_FAILURE;
break;
}
if (wolfssl_ctx_certs_locked(ctx)) {
ret = CTX_BUSY_E;
break;
}
/* Clear certificate chain */
FreeDer(&ctx->certChain);
if (sk) {
Expand Down
Loading
Loading