Skip to content

Commit

Permalink
SSL loading of keys/certs: testing and fixes
Browse files Browse the repository at this point in the history 
Added tests to cover ssl_load.c functions.
Fixes from testing.
pk.c: renamed wolfssl_dh_load_key to wolfssl_dh_load_params as it
doesn't handle keys - just parameters.
  • Loading branch information
@SparkiDev
SparkiDev committed Jul 25, 2024
1 parent 7c6eb7c commit 08c5562
Show file tree
Hide file tree
Showing 3 changed files with 710 additions and 85 deletions.
8 changes: 4 additions & 4 deletions src/pk.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7283,7 +7283,7 @@ WOLFSSL_BIGNUM* wolfSSL_DH_8192_prime(WOLFSSL_BIGNUM* bn)

#ifndef NO_CERTS

/* Load the DER encoded DH parameters/key into DH key.
/* Load the DER encoded DH parameters into DH key.
*
* @param [in, out] dh DH key to load parameters into.
* @param [in] der Buffer holding DER encoded parameters data.
Expand All @@ -7294,7 +7294,7 @@ WOLFSSL_BIGNUM* wolfSSL_DH_8192_prime(WOLFSSL_BIGNUM* bn)
* @return 0 on success.
* @return 1 when decoding DER or setting the external key fails.
*/
static int wolfssl_dh_load_key(WOLFSSL_DH* dh, const unsigned char* der,
static int wolfssl_dh_load_params(WOLFSSL_DH* dh, const unsigned char* der,
word32* idx, word32 derSz)
{
int err = 0;
Expand Down Expand Up @@ -7407,7 +7407,7 @@ WOLFSSL_DH *wolfSSL_d2i_DHparams(WOLFSSL_DH** dh, const unsigned char** pp,
WOLFSSL_ERROR_MSG("wolfSSL_DH_new() failed");
err = 1;
}
if ((!err) && (wolfssl_dh_load_key(newDh, *pp, &idx,
if ((!err) && (wolfssl_dh_load_params(newDh, *pp, &idx,
(word32)length) != 0)) {
WOLFSSL_ERROR_MSG("Loading DH parameters failed");
err = 1;
Expand Down Expand Up @@ -7567,7 +7567,7 @@ int wolfSSL_DH_LoadDer(WOLFSSL_DH* dh, const unsigned char* derBuf, int derSz)
ret = -1;
}

if ((ret == 1) && (wolfssl_dh_load_key(dh, derBuf, &idx,
if ((ret == 1) && (wolfssl_dh_load_params(dh, derBuf, &idx,
(word32)derSz) != 0)) {
WOLFSSL_ERROR_MSG("DH key decode failed");
ret = -1;
Expand Down
62 changes: 13 additions & 49 deletions src/ssl_load.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,21 +142,10 @@ static int DataToDerBuffer(const unsigned char* buff, word32 len, int format,
}
/* Data in buffer is ASN.1 format - get first SEQ or OCT into der. */
else {
int length;
word32 inOutIdx = 0;

/* Get length of SEQ including header. */
if ((info->consumed = wolfssl_der_length(buff, (int)len)) > 0) {
ret = 0;
}
/* Private keys may be wrapped in OCT when PKCS#8 wrapper removed.
* TODO: is this really needed? */
else if ((type == PRIVATEKEY_TYPE) &&
(GetOctetString(buff, &inOutIdx, &length, len) >= 0)) {
/* Include octet string DER header. */
info->consumed = length + inOutIdx;
ret = 0;
}
else {
ret = ASN_PARSE_E;
}
Expand Down Expand Up @@ -302,29 +291,20 @@ static int ProcessUserChain(WOLFSSL_CTX* ctx, WOLFSSL* ssl,

WOLFSSL_ENTER("ProcessUserChain");

/* Validate parameters. */
if ((type == CA_TYPE) && (ctx == NULL)) {
WOLFSSL_MSG("Need context for CA load");
ret = BAD_FUNC_ARG;
}

/* Ignore non-certificate types. */
if ((ret == 0) && (type != CERT_TYPE) && (type != CHAIN_CERT_TYPE) &&
(type != CA_TYPE)) {
WOLFSSL_MSG("File type not a certificate");
}
/* Check we haven't consumed all the data. */
else if ((ret == 0) && (info->consumed >= sz)) {
if (info->consumed >= sz) {
WOLFSSL_MSG("Already consumed data");
}
else if (ret == 0) {
else if (type == WOLFSSL_FILETYPE_PEM) {
ret = BAD_FUNC_ARG;
}
else {
#ifndef WOLFSSL_SMALL_STACK
byte stackBuffer[FILE_BUFFER_SIZE];
#endif
StaticBuffer chain;
long consumed = info->consumed;
word32 idx = 0;
int gotOne = 0;
int cnt = 0;
/* Calculate max possible size, including max headers */
long maxSz = (sz - consumed) + (CERT_HEADER_SZ * MAX_CHAIN_DEPTH);
Expand Down Expand Up @@ -352,21 +332,13 @@ static int ProcessUserChain(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
ret = ProcessUserCert(ctx->cm, &part, type, verify,
chain.buffer, &idx, (word32)maxSz);
}
/* PEM may have trailing data that can be ignored. */
if ((ret == WC_NO_ERR_TRACE(ASN_NO_PEM_HEADER)) && gotOne) {
WOLFSSL_MSG("We got one good cert, so stuff at end ok");
ret = 0;
break;
}
/* Certificate data handled. */
FreeDer(&part);

if (ret == 0) {
/* Update consumed length. */
consumed += info->consumed;
WOLFSSL_MSG(" Consumed another Cert in Chain");
/* Update whether we got a user certificate. */
gotOne |= (type != CA_TYPE);
/* Update count of certificates added to chain. */
cnt++;
}
Expand Down Expand Up @@ -4846,8 +4818,7 @@ int wolfSSL_add0_chain_cert(WOLFSSL* ssl, WOLFSSL_X509* x509)
WOLFSSL_ENTER("wolfSSL_add0_chain_cert");

/* Validate parameters. */
if ((ssl == NULL) || (ssl->ctx == NULL) || (x509 == NULL) ||
(x509->derCert == NULL)) {
if ((ssl == NULL) || (x509 == NULL) || (x509->derCert == NULL)) {
ret = 0;
}

Expand Down Expand Up @@ -4910,8 +4881,7 @@ int wolfSSL_add1_chain_cert(WOLFSSL* ssl, WOLFSSL_X509* x509)
WOLFSSL_ENTER("wolfSSL_add1_chain_cert");

/* Validate parameters. */
if ((ssl == NULL) || (ssl->ctx == NULL) || (x509 == NULL) ||
(x509->derCert == NULL)) {
if ((ssl == NULL) || (x509 == NULL) || (x509->derCert == NULL)) {
ret = 0;
}

Expand Down Expand Up @@ -5437,10 +5407,6 @@ int wolfSSL_CTX_SetTmpDH(WOLFSSL_CTX* ctx, const unsigned char* p, int pSz,
pAlloc = (byte*)XMALLOC(pSz, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
gAlloc = (byte*)XMALLOC(gSz, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
if ((pAlloc == NULL) || (gAlloc == NULL)) {
XFREE(pAlloc, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
pAlloc = NULL;
XFREE(gAlloc, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
gAlloc = NULL;
ret = MEMORY_E;
}
}
Expand All @@ -5453,12 +5419,10 @@ int wolfSSL_CTX_SetTmpDH(WOLFSSL_CTX* ctx, const unsigned char* p, int pSz,
ret = wolfssl_ctx_set_tmp_dh(ctx, pAlloc, pSz, gAlloc, gSz);
}

if (ret != 1) {
if ((ret != 1) && (ctx != NULL)) {
/* Free the allocated buffers if not assigned into SSL context. */
if (pAlloc)
XFREE(pAlloc, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
if (gAlloc)
XFREE(gAlloc, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
XFREE(pAlloc, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
XFREE(gAlloc, ctx->heap, DYNAMIC_TYPE_PUBLIC_KEY);
}
return ret;
}
Expand Down Expand Up @@ -5491,7 +5455,7 @@ long wolfSSL_set_tmp_dh(WOLFSSL *ssl, WOLFSSL_DH *dh)
}

if (ret == 1) {
/* Get needed size for p and g. */
/* Get sizes of p and g. */
pSz = wolfSSL_BN_bn2bin(dh->p, NULL);
gSz = wolfSSL_BN_bn2bin(dh->g, NULL);
/* Validate p and g size. */
Expand Down Expand Up @@ -5522,7 +5486,7 @@ long wolfSSL_set_tmp_dh(WOLFSSL *ssl, WOLFSSL_DH *dh)
ret = wolfssl_set_tmp_dh(ssl, p, pSz, g, gSz);
}

if (ret != 1 && ssl != NULL) {
if ((ret != 1) && (ssl != NULL)) {
/* Free the allocated buffers if not assigned into SSL. */
XFREE(p, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY);
XFREE(g, ssl->heap, DYNAMIC_TYPE_PUBLIC_KEY);
Expand Down Expand Up @@ -5557,7 +5521,7 @@ long wolfSSL_CTX_set_tmp_dh(WOLFSSL_CTX* ctx, WOLFSSL_DH* dh)
}

if (ret == 1) {
/* Get needed size for p and g. */
/* Get sizes of p and g. */
pSz = wolfSSL_BN_bn2bin(dh->p, NULL);
gSz = wolfSSL_BN_bn2bin(dh->g, NULL);
/* Validate p and g size. */
Expand Down
Loading

0 comments on commit 08c5562

Please sign in to comment.