Skip to content

Commit e9a2f27

Browse files
julek-wolfssljulek-wolfssl
committed
Address peer review
1 parent 38b52d8 commit e9a2f27

5 files changed

Lines changed: 56 additions & 52 deletions

File tree

‎.github/workflows/nginx.yml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,7 @@ jobs:
246246
./auto/configure --with-wolfssl=$GITHUB_WORKSPACE/build-dir --with-http_ssl_module \
247247
--with-stream --with-stream_ssl_module --with-stream_ssl_preread_module \
248248
--with-http_v2_module --with-mail --with-mail_ssl_module \
249-
--with-cc-opt='-fsanitize=address -DNGX_DEBUG_PALLOC=1 -g3 \
250-
${{ env.nginx_c_flags }}' \
249+
--with-cc-opt='-fsanitize=address -DNGX_DEBUG_PALLOC=1 -g3 ${{ env.nginx_c_flags }}' \
251250
--with-ld-opt='-fsanitize=address ${{ env.nginx_c_flags }}'
252251
make -j
253252

‎src/internal.c‎

Lines changed: 45 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -39034,12 +39034,10 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3903439034
}
3903539035
#endif
3903639036

39037-
/* Calculate actual internal ticket size */
39037+
internalTicketSz = (int)WOLFSSL_INTERNAL_TICKET_BASE_SZ;
3903839038
#if defined(OPENSSL_ALL) && defined(KEEP_PEER_CERT) && \
3903939039
!defined(NO_CERT_IN_TICKET)
39040-
internalTicketSz = (int)(WOLFSSL_INTERNAL_TICKET_BASE_SZ + peerCertSz);
39041-
#else
39042-
internalTicketSz = (int)WOLFSSL_INTERNAL_TICKET_BASE_SZ;
39040+
internalTicketSz += peerCertSz;
3904339041
#endif
3904439042
/* MAC is placed after the encrypted data */
3904539043
mac = et->enc_ticket + WOLFSSL_TICKET_ENC_SZ;
@@ -39326,6 +39324,48 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3932639324
}
3932739325
#endif /* WOLFSSL_SLT13 */
3932839326

39327+
#if defined(OPENSSL_ALL) && defined(KEEP_PEER_CERT) && \
39328+
!defined(NO_CERT_IN_TICKET)
39329+
static void RestorePeerCertFromTicket(WOLFSSL* ssl, InternalTicket* it)
39330+
{
39331+
word16 peerCertLen = 0;
39332+
ato16(it->peerCertLen, &peerCertLen);
39333+
39334+
if (peerCertLen > 0 && peerCertLen <= MAX_TICKET_PEER_CERT_SZ) {
39335+
#ifdef SESSION_CERTS
39336+
/* Clear existing chain and add the peer certificate */
39337+
ssl->session->chain.count = 0;
39338+
AddSessionCertToChain(&ssl->session->chain,
39339+
it->peerCert, peerCertLen);
39340+
#endif
39341+
/* Also decode into ssl->peerCert for direct access */
39342+
{
39343+
int ret;
39344+
DecodedCert* dCert;
39345+
39346+
dCert = (DecodedCert*)XMALLOC(sizeof(DecodedCert), ssl->heap,
39347+
DYNAMIC_TYPE_DCERT);
39348+
if (dCert != NULL) {
39349+
InitDecodedCert(dCert, it->peerCert, peerCertLen, ssl->heap);
39350+
ret = ParseCertRelative(dCert, CERT_TYPE, 0, NULL, NULL);
39351+
if (ret == 0) {
39352+
FreeX509(&ssl->peerCert);
39353+
InitX509(&ssl->peerCert, 0, ssl->heap);
39354+
ret = CopyDecodedToX509(&ssl->peerCert, dCert);
39355+
if (ret != 0) {
39356+
/* Failed to copy - clear peerCert */
39357+
FreeX509(&ssl->peerCert);
39358+
InitX509(&ssl->peerCert, 0, ssl->heap);
39359+
}
39360+
}
39361+
FreeDecodedCert(dCert);
39362+
XFREE(dCert, ssl->heap, DYNAMIC_TYPE_DCERT);
39363+
}
39364+
}
39365+
}
39366+
}
39367+
#endif /* OPENSSL_ALL && KEEP_PEER_CERT && !NO_CERT_IN_TICKET */
39368+
3932939369
void DoClientTicketFinalize(WOLFSSL* ssl, InternalTicket* it,
3933039370
const WOLFSSL_SESSION* sess)
3933139371
{
@@ -39416,44 +39456,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3941639456

3941739457
#if defined(OPENSSL_ALL) && defined(KEEP_PEER_CERT) && \
3941839458
!defined(NO_CERT_IN_TICKET)
39419-
/* Restore peer certificate from ticket to session chain and peerCert */
39420-
{
39421-
word16 peerCertLen = 0;
39422-
ato16(it->peerCertLen, &peerCertLen);
39423-
39424-
if (peerCertLen > 0 && peerCertLen <= MAX_TICKET_PEER_CERT_SZ) {
39425-
#ifdef SESSION_CERTS
39426-
/* Clear existing chain and add the peer certificate */
39427-
ssl->session->chain.count = 0;
39428-
AddSessionCertToChain(&ssl->session->chain,
39429-
it->peerCert, peerCertLen);
39430-
#endif
39431-
/* Also decode into ssl->peerCert for direct access */
39432-
{
39433-
int ret;
39434-
DecodedCert* dCert;
39435-
39436-
dCert = (DecodedCert*)XMALLOC(sizeof(DecodedCert), ssl->heap,
39437-
DYNAMIC_TYPE_DCERT);
39438-
if (dCert != NULL) {
39439-
InitDecodedCert(dCert, it->peerCert, peerCertLen, ssl->heap);
39440-
ret = ParseCertRelative(dCert, CERT_TYPE, 0, NULL, NULL);
39441-
if (ret == 0) {
39442-
FreeX509(&ssl->peerCert);
39443-
InitX509(&ssl->peerCert, 0, ssl->heap);
39444-
ret = CopyDecodedToX509(&ssl->peerCert, dCert);
39445-
if (ret != 0) {
39446-
/* Failed to copy - clear peerCert */
39447-
FreeX509(&ssl->peerCert);
39448-
InitX509(&ssl->peerCert, 0, ssl->heap);
39449-
}
39450-
}
39451-
FreeDecodedCert(dCert);
39452-
XFREE(dCert, ssl->heap, DYNAMIC_TYPE_DCERT);
39453-
}
39454-
}
39455-
}
39456-
}
39459+
RestorePeerCertFromTicket(ssl, it);
3945739460
#endif
3945839461

3945939462
ssl->version.minor = it->pv.minor;

‎src/ssl.c‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15508,11 +15508,10 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
1550815508
#endif
1550915509
#ifndef WOLFSSL_BLIND_PRIVATE_KEY
1551015510
#ifdef WOLFSSL_COPY_KEY
15511+
if (ssl->buffers.key != NULL && ssl->buffers.weOwnKey) {
15512+
FreeDer(&ssl->buffers.key);
15513+
}
1551115514
if (ctx->privateKey != NULL) {
15512-
if (ssl->buffers.key != NULL) {
15513-
FreeDer(&ssl->buffers.key);
15514-
ssl->buffers.key = NULL;
15515-
}
1551615515
ret = AllocCopyDer(&ssl->buffers.key, ctx->privateKey->buffer,
1551715516
ctx->privateKey->length, ctx->privateKey->type,
1551815517
ctx->privateKey->heap);
@@ -15523,8 +15522,6 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
1552315522
ssl->buffers.weOwnKey = 1;
1552415523
}
1552515524
else {
15526-
if (ssl->buffers.key != NULL && ssl->buffers.weOwnKey)
15527-
FreeDer(&ssl->buffers.key);
1552815525
ssl->buffers.key = ctx->privateKey;
1552915526
}
1553015527
#else

‎src/ssl_api_cert.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -881,8 +881,11 @@ int wolfSSL_UnloadCertsKeys(WOLFSSL* ssl)
881881
#ifdef WOLFSSL_DUAL_ALG_CERTS
882882
if (ssl->buffers.weOwnAltKey) {
883883
WOLFSSL_MSG("Unloading alt key");
884-
if (ssl->buffers.altKey != NULL && ssl->buffers.altKey->buffer != NULL)
885-
ForceZero(ssl->buffers.altKey->buffer, ssl->buffers.altKey->length);
884+
if (ssl->buffers.altKey != NULL &&
885+
ssl->buffers.altKey->buffer != NULL) {
886+
ForceZero(ssl->buffers.altKey->buffer,
887+
ssl->buffers.altKey->length);
888+
}
886889
FreeDer(&ssl->buffers.altKey);
887890
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
888891
FreeDer(&ssl->buffers.altKeyMask);

‎src/x509.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
#include <wolfssl/wolfio.h>
3939
#endif
4040

41+
/* 16 times MAX_X509_SIZE should be more than enough to read any X509
42+
* certificate file */
4143
#define MAX_BIO_READ_BUFFER (MAX_X509_SIZE * 16)
4244

4345
#if defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)

0 commit comments

Comments
 (0)