Skip to content

Commit

Permalink
Merge pull request #7789 from SparkiDev/test_ssl_load
Browse files Browse the repository at this point in the history 
SSL loading of keys/certs: testing and fixes
  • Loading branch information
@dgarske
dgarske authored Jul 26, 2024
2 parents f7094ff + caab2c2 commit 5e58aff
Show file tree
Hide file tree
Showing 3 changed files with 739 additions and 88 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
88 changes: 35 additions & 53 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,22 +291,11 @@ 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 {
#ifndef WOLFSSL_SMALL_STACK
byte stackBuffer[FILE_BUFFER_SIZE];
#endif
Expand Down Expand Up @@ -884,17 +862,17 @@ static int ProcessBufferTryDecodeFalcon(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
ret = wc_falcon_init(key);
if (ret == 0) {
/* Set up key to parse the format specified. */
if (*keyFormat == FALCON_LEVEL1k) {
if ((*keyFormat == FALCON_LEVEL1k) || ((*keyFormat == 0) &&
((der->length == FALCON_LEVEL1_KEY_SIZE) ||
(der->length == FALCON_LEVEL1_PRV_KEY_SIZE)))) {
ret = wc_falcon_set_level(key, 1);
}
else if (*keyFormat == FALCON_LEVEL5k) {
else if ((*keyFormat == FALCON_LEVEL5k) || ((*keyFormat == 0) &&
((der->length == FALCON_LEVEL5_KEY_SIZE) ||
(der->length == FALCON_LEVEL5_PRV_KEY_SIZE)))) {
ret = wc_falcon_set_level(key, 5);
}
else {
/* What if *keyformat is 0? We might want to do something more
* graceful here. */
/* TODO: get the size of the private key for different formats and
* compare with DER length. */
wc_falcon_free(key);
ret = ALGO_ID_E;
}
Expand Down Expand Up @@ -935,6 +913,11 @@ static int ProcessBufferTryDecodeFalcon(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
/* Free dynamically allocated data in key. */
wc_falcon_free(key);
}
else if ((ret == ALGO_ID_E) && (*keyFormat == 0)) {
WOLFSSL_MSG("Not a Falcon key");
/* Format unknown so keep trying. */
ret = 0;
}

/* Dispose of allocated key. */
XFREE(key, heap, DYNAMIC_TYPE_FALCON);
Expand Down Expand Up @@ -977,20 +960,22 @@ static int ProcessBufferTryDecodeDilithium(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
ret = wc_dilithium_init(key);
if (ret == 0) {
/* Set up key to parse the format specified. */
if (*keyFormat == DILITHIUM_LEVEL2k) {
if ((*keyFormat == DILITHIUM_LEVEL2k) || ((*keyFormat == 0) &&
((der->length == DILITHIUM_LEVEL2_KEY_SIZE) ||
(der->length == DILITHIUM_LEVEL2_PRV_KEY_SIZE)))) {
ret = wc_dilithium_set_level(key, 2);
}
else if (*keyFormat == DILITHIUM_LEVEL3k) {
else if ((*keyFormat == DILITHIUM_LEVEL3k) || ((*keyFormat == 0) &&
((der->length == DILITHIUM_LEVEL3_KEY_SIZE) ||
(der->length == DILITHIUM_LEVEL3_PRV_KEY_SIZE)))) {
ret = wc_dilithium_set_level(key, 3);
}
else if (*keyFormat == DILITHIUM_LEVEL5k) {
else if ((*keyFormat == DILITHIUM_LEVEL5k) || ((*keyFormat == 0) &&
((der->length == DILITHIUM_LEVEL5_KEY_SIZE) ||
(der->length == DILITHIUM_LEVEL5_PRV_KEY_SIZE)))) {
ret = wc_dilithium_set_level(key, 5);
}
else {
/* What if *keyformat is 0? We might want to do something more
* graceful here. */
/* TODO: get the size of the private key for different formats and
* compare with DER length. */
wc_dilithium_free(key);
ret = ALGO_ID_E;
}
Expand Down Expand Up @@ -1036,6 +1021,11 @@ static int ProcessBufferTryDecodeDilithium(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
/* Free dynamically allocated data in key. */
wc_dilithium_free(key);
}
else if ((ret == ALGO_ID_E) && (*keyFormat == 0)) {
WOLFSSL_MSG("Not a Dilithium key");
/* Format unknown so keep trying. */
ret = 0;
}

/* Dispose of allocated key. */
XFREE(key, heap, DYNAMIC_TYPE_DILITHIUM);
Expand Down Expand Up @@ -4846,8 +4836,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 +4899,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 +5425,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 +5437,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 +5473,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 +5504,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 +5539,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 5e58aff

Please sign in to comment.