Skip to content

Commit

Permalink
apply code review updates for wolfssl/wolfcrypt
Browse files Browse the repository at this point in the history
  • Loading branch information
gojimmypi committed Feb 9, 2024
1 parent 97ae90b commit 9b98879
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 116 deletions.
13 changes: 6 additions & 7 deletions src/internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -7466,15 +7466,14 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)

#if defined(WOLFSSL_DTLS) && !defined(NO_WOLFSSL_SERVER)
if (ssl->options.dtls && ssl->options.side == WOLFSSL_SERVER_END) {
if (!IsAtLeastTLSv1_3(ssl->version)) {
ret = wolfSSL_DTLS_SetCookieSecret(ssl, NULL, 0);
if (ret != 0) {
WOLFSSL_MSG("DTLS Cookie Secret error");
return ret;
}
/* Initialize both in case we allow downgrading. */
ret = wolfSSL_DTLS_SetCookieSecret(ssl, NULL, 0);
if (ret != 0) {
WOLFSSL_MSG("DTLS Cookie Secret error");
return ret;
}
#if defined(WOLFSSL_DTLS13) && defined(WOLFSSL_SEND_HRR_COOKIE)
else {
if (IsAtLeastTLSv1_3(ssl->version)) {
ret = wolfSSL_send_hrr_cookie(ssl, NULL, 0);
if (ret != WOLFSSL_SUCCESS) {
WOLFSSL_MSG("DTLS1.3 Cookie secret error");
Expand Down
70 changes: 35 additions & 35 deletions src/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1465,6 +1465,7 @@ int wolfSSL_CTX_new_rng(WOLFSSL_CTX* ctx)
}
#endif


WOLFSSL_ABI
WOLFSSL* wolfSSL_new(WOLFSSL_CTX* ctx)
{
Expand All @@ -1475,34 +1476,33 @@ WOLFSSL* wolfSSL_new(WOLFSSL_CTX* ctx)

if (ctx == NULL) {
WOLFSSL_MSG("wolfSSL_new ctx is null");
return ssl;
return NULL;
}

ssl = (WOLFSSL*) XMALLOC(sizeof(WOLFSSL), ctx->heap, DYNAMIC_TYPE_SSL);
if (ssl) {

if (ssl == NULL) {
WOLFSSL_MSG_EX("ssl xmalloc failed to allocate %d bytes",
(int)sizeof(WOLFSSL));
}
else {
ret = InitSSL(ssl, ctx, 0);
if (ret < 0) {
WOLFSSL_MSG_EX("wolfSSL_new failed during InitSSL. ret = %d", ret);
WOLFSSL_MSG_EX("wolfSSL_new failed during InitSSL. err = %d", ret);
FreeSSL(ssl, ctx->heap);
ssl = 0;
ssl = NULL;
}
else if (ret == 0) {
WOLFSSL_MSG("wolfSSL_new InitSSL success");
}
else {
if (ret == 0) {
WOLFSSL_MSG("wolfSSL_new InitSSL success");
}
else {
/* Only success (0) or negative values should ever be seen. */
WOLFSSL_MSG_EX("WARNING: wolfSSL_new unexpected InitSSL return"
" value = %d", ret);
}
} /* if InitSSL check */
/* Only success (0) or negative values should ever be seen. */
WOLFSSL_MSG_EX("WARNING: wolfSSL_new unexpected InitSSL return"
" value = %d", ret);
} /* InitSSL check */
} /* ssl XMALLOC success */
else {
WOLFSSL_MSG_EX("ssl xmalloc failed to allocate %d bytes",
(int)sizeof(WOLFSSL));
} /* if ssl check */

WOLFSSL_LEAVE("wolfSSL_new", ret);
WOLFSSL_LEAVE("wolfSSL_new InitSSL =", ret);
(void)ret;

return ssl;
Expand All @@ -1515,7 +1515,7 @@ void wolfSSL_free(WOLFSSL* ssl)
WOLFSSL_ENTER("wolfSSL_free");

if (ssl) {
WOLFSSL_MSG_EX("Free SSL: %0x\n", (int)ssl);
WOLFSSL_MSG_EX("Free SSL: %p", (uintptr_t)ssl);
FreeSSL(ssl, ssl->ctx->heap);
}
else {
Expand Down Expand Up @@ -11930,7 +11930,7 @@ static int wolfSSL_parse_cipher_list(WOLFSSL_CTX* ctx, WOLFSSL* ssl,

/* list contains ciphers either only for TLS 1.3 or <= TLS 1.2 */
if (suites->suiteSz == 0) {
WOLFSSL_MSG("\n\nWarning suites->suiteSz = 0 set to WOLFSSL_MAX_SUITE_SZ");
WOLFSSL_MSG("Warning suites->suiteSz = 0 set to WOLFSSL_MAX_SUITE_SZ");
suites->suiteSz = WOLFSSL_MAX_SUITE_SZ;
}
#ifdef WOLFSSL_SMALL_STACK
Expand Down Expand Up @@ -12625,12 +12625,12 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl,
#else
#ifdef WOLFSSL_TLS13
if (ssl->options.tls1_3) {
WOLFSSL_MSG("TLS 1.3!\n");
WOLFSSL_MSG("TLS 1.3");
return wolfSSL_connect_TLSv13(ssl);
}
#endif

WOLFSSL_MSG(">> NOT TLS 1.3!\n");
WOLFSSL_MSG("TLS 1.2 or lower");
WOLFSSL_ENTER("wolfSSL_connect");

/* make sure this wolfSSL object has arrays and rng setup. Protects
Expand Down Expand Up @@ -12748,9 +12748,9 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl,
neededState = SERVER_HELLOVERIFYREQUEST_COMPLETE;
#endif
/* get response */
WOLFSSL_MSG("ssl.c ssl->options.serverState < neededState");
WOLFSSL_MSG("Server state up to needed state.");
while (ssl->options.serverState < neededState) {
WOLFSSL_MSG("while (ssl->options.serverState < neededState)...");
WOLFSSL_MSG("Progressing server state...");
#ifdef WOLFSSL_TLS13
if (ssl->options.tls1_3)
return wolfSSL_connect_TLSv13(ssl);
Expand Down Expand Up @@ -21448,20 +21448,20 @@ int wolfSSL_ERR_GET_LIB(unsigned long err)

value = (err & 0xFFFFFFL);
switch (value) {
case -SSL_R_HTTP_REQUEST: /* -306 */
case -SSL_R_HTTP_REQUEST:
return ERR_LIB_SSL;
case -ASN_NO_PEM_HEADER: /* -162 */
case PEM_R_NO_START_LINE: /* (-MIN_CODE_E + 1) */
case PEM_R_PROBLEMS_GETTING_PASSWORD: /* (-MIN_CODE_E + 2) */
case PEM_R_BAD_PASSWORD_READ: /* duplicate (-MIN_CODE_E + 3) = 303 */
case PEM_R_BAD_DECRYPT: /* (-MIN_CODE_E + 4) */
case -ASN_NO_PEM_HEADER:
case PEM_R_NO_START_LINE:
case PEM_R_PROBLEMS_GETTING_PASSWORD:
case PEM_R_BAD_PASSWORD_READ:
case PEM_R_BAD_DECRYPT:
return ERR_LIB_PEM;
case EVP_R_BAD_DECRYPT: /* (-MIN_CODE_E + 100 + 1) */
case EVP_R_BN_DECODE_ERROR: /* (-MIN_CODE_E + 100 + 2) */
case EVP_R_DECODE_ERROR: /* (-MIN_CODE_E + 100 + 3) */
case EVP_R_PRIVATE_KEY_DECODE_ERROR: /* (-MIN_CODE_E + 100 + 4) */
case EVP_R_BAD_DECRYPT:
case EVP_R_BN_DECODE_ERROR:
case EVP_R_DECODE_ERROR:
case EVP_R_PRIVATE_KEY_DECODE_ERROR:
return ERR_LIB_EVP;
case ASN1_R_HEADER_TOO_LONG: /* (-MIN_CODE_E + 5) */
case ASN1_R_HEADER_TOO_LONG:
return ERR_LIB_ASN1;
default:
return 0;
Expand Down
25 changes: 23 additions & 2 deletions src/ssl_certman.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ WOLFSSL_CERT_MANAGER* wolfSSL_CertManagerNew_ex(void* heap)
WOLFSSL_MSG("heap param is null");
}
else {
WOLFSSL_MSG_EX("heap param = %p", heap);
/* Some systems may have heap in unexpected segments. (IRAM vs DRAM) */
WOLFSSL_MSG_EX("heap param = %p", heap);
}
WOLFSSL_MSG_EX("DYNAMIC_TYPE_CERT_MANAGER Allocating = %d bytes",
(word32)sizeof(WOLFSSL_CERT_MANAGER));
Expand All @@ -102,7 +103,8 @@ WOLFSSL_CERT_MANAGER* wolfSSL_CertManagerNew_ex(void* heap)
cm = (WOLFSSL_CERT_MANAGER*)XMALLOC(sizeof(WOLFSSL_CERT_MANAGER), heap,
DYNAMIC_TYPE_CERT_MANAGER);
if (cm == NULL) {
WOLFSSL_MSG("XMALLOC failed");
WOLFSSL_MSG_EX("XMALLOC failed to allocate WOLFSSL_CERT_MANAGER %d "
"bytes.", (int)sizeof(WOLFSSL_CERT_MANAGER));
err = 1;
}
if (!err) {
Expand Down Expand Up @@ -584,6 +586,19 @@ void wolfSSL_CertManagerSetVerify(WOLFSSL_CERT_MANAGER* cm, VerifyCallback vc)
}
#endif /* NO_WOLFSSL_CM_VERIFY */

#if defined(WOLFSSL_CUSTOM_OID) && defined(WOLFSSL_ASN_TEMPLATE) \
&& defined(HAVE_OID_DECODING)
void wolfSSL_CertManagerSetUnknownExtCallback(WOLFSSL_CERT_MANAGER* cm,
wc_UnknownExtCallback cb)
{
WOLFSSL_ENTER("wolfSSL_CertManagerSetUnknownExtCallback");
if (cm != NULL) {
cm->unknownExtCallback = cb;
}

}
#endif /* WOLFSSL_CUSTOM_OID && WOLFSSL_ASN_TEMPLATE && HAVE_OID_DECODING */

#if !defined(NO_WOLFSSL_CLIENT) || !defined(WOLFSSL_NO_CLIENT_AUTH)
/* Verify the certificate.
*
Expand Down Expand Up @@ -652,6 +667,12 @@ int CM_VerifyBuffer_ex(WOLFSSL_CERT_MANAGER* cm, const unsigned char* buff,
/* Create a decoded certificate with DER buffer. */
InitDecodedCert(cert, buff, (word32)sz, cm->heap);

#if defined(WOLFSSL_CUSTOM_OID) && defined(WOLFSSL_ASN_TEMPLATE) \
&& defined(HAVE_OID_DECODING)
if (cm->unknownExtCallback != NULL)
wc_SetUnknownExtCallback(cert, cm->unknownExtCallback);
#endif

/* Parse DER into decoded certificate fields and verify signature
* against a known CA. */
ret = ParseCertRelative(cert, CERT_TYPE, VERIFY, cm);
Expand Down
4 changes: 2 additions & 2 deletions src/tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -7459,8 +7459,8 @@ static int TLSX_KeyShare_GenEccKey(WOLFSSL *ssl, KeyShareEntry* kse)
/* Allocate an ECC key to hold private key. */
kse->key = (byte*)XMALLOC(sizeof(ecc_key), ssl->heap, DYNAMIC_TYPE_ECC);
if (kse->key == NULL) {
WOLFSSL_MSG_EX("Failed to allocate %d bytes, ssl->heap: %x",
(int)sizeof(ecc_key), (word32)ssl->heap);
WOLFSSL_MSG_EX("Failed to allocate %d bytes, ssl->heap: %p",
(int)sizeof(ecc_key), (uintptr_t)ssl->heap);
WOLFSSL_MSG("EccTempKey Memory error!");
return MEMORY_E;
}
Expand Down
27 changes: 18 additions & 9 deletions src/tls13.c
Original file line number Diff line number Diff line change
Expand Up @@ -3546,6 +3546,12 @@ int CreateCookieExt(const WOLFSSL* ssl, byte* hash, word16 hashSz,
return BAD_FUNC_ARG;
}

if (ssl->buffers.tls13CookieSecret.buffer == NULL ||
ssl->buffers.tls13CookieSecret.length == 0) {
WOLFSSL_MSG("Missing DTLS 1.3 cookie secret");
return COOKIE_ERROR;
}

/* Cookie Data = Hash Len | Hash | CS | KeyShare Group */
cookie[cookieSz++] = (byte)hashSz;
XMEMCPY(cookie + cookieSz, hash, hashSz);
Expand Down Expand Up @@ -4628,14 +4634,10 @@ int SendTls13ClientHello(WOLFSSL* ssl)
ret = EchHashHelloInner(ssl, args->ech);
}
#endif
/* TODO remove */
printf("Client hello %d\n", __LINE__);

/* compute the outer hash */
if (ret == 0)
ret = HashOutput(ssl, args->output, args->idx, 0);
/* TODO remove */
printf("Client hello %d\n", __LINE__);
}
}
if (ret != 0)
Expand Down Expand Up @@ -4697,7 +4699,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
}

#if defined(WOLFSSL_DTLS13) && !defined(WOLFSSL_NO_CLIENT)
static int Dtls13DoDowngrade(WOLFSSL* ssl)
static int Dtls13ClientDoDowngrade(WOLFSSL* ssl)
{
int ret;
if (ssl->dtls13ClientHello == NULL)
Expand Down Expand Up @@ -5103,7 +5105,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
if (ssl->options.dtls) {
ssl->chVersion.minor = DTLSv1_2_MINOR;
ssl->version.minor = DTLSv1_2_MINOR;
ret = Dtls13DoDowngrade(ssl);
ret = Dtls13ClientDoDowngrade(ssl);
if (ret != 0)
return ret;
}
Expand Down Expand Up @@ -5197,7 +5199,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
if (ssl->options.dtls) {
ssl->chVersion.minor = DTLSv1_2_MINOR;
ssl->version.minor = DTLSv1_2_MINOR;
ret = Dtls13DoDowngrade(ssl);
ret = Dtls13ClientDoDowngrade(ssl);
if (ret != 0)
return ret;
}
Expand Down Expand Up @@ -5270,7 +5272,7 @@ int DoTls13ServerHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,

#ifdef WOLFSSL_DTLS13
if (ssl->options.dtls) {
ret = Dtls13DoDowngrade(ssl);
ret = Dtls13ClientDoDowngrade(ssl);
if (ret != 0)
return ret;
}
Expand Down Expand Up @@ -6325,6 +6327,12 @@ int TlsCheckCookie(const WOLFSSL* ssl, const byte* cookie, word16 cookieSz)
byte cookieType = 0;
byte macSz = 0;

if (ssl->buffers.tls13CookieSecret.buffer == NULL ||
ssl->buffers.tls13CookieSecret.length == 0) {
WOLFSSL_MSG("Missing DTLS 1.3 cookie secret");
return COOKIE_ERROR;
}

#if !defined(NO_SHA) && defined(NO_SHA256)
cookieType = SHA;
macSz = WC_SHA_DIGEST_SIZE;
Expand Down Expand Up @@ -6699,6 +6707,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
* wolfSSL_accept_TLSv13 when changing this one. */
if (IsDtlsNotSctpMode(ssl) && ssl->options.sendCookie &&
!ssl->options.dtlsStateful) {
DtlsSetSeqNumForReply(ssl);
ret = DoClientHelloStateless(ssl, input + *inOutIdx, helloSz, 0, NULL);
if (ret != 0 || !ssl->options.dtlsStateful) {
*inOutIdx += helloSz;
Expand Down Expand Up @@ -12624,7 +12633,7 @@ int wolfSSL_connect_TLSv13(WOLFSSL* ssl)
}

ssl->options.connectState = CLIENT_HELLO_SENT;
WOLFSSL_MSG("tls13.c connect state: CLIENT_HELLO_SENT");
WOLFSSL_MSG("TLSv13 connect state: CLIENT_HELLO_SENT");
#ifdef WOLFSSL_EARLY_DATA
if (ssl->earlyData != no_early_data) {
#if defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT)
Expand Down
7 changes: 4 additions & 3 deletions wolfcrypt/src/aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -527,10 +527,10 @@ block cipher mechanism that uses n-bit binary string parameter key with 128-bits
#endif /* HAVE_AES_DECRYPT */

#elif defined(WOLFSSL_ESP32_CRYPT) && \
!defined(NO_WOLFSSL_ESP32_CRYPT_AES)
!defined(NO_WOLFSSL_ESP32_CRYPT_AES)
#include <esp_log.h>
#include <wolfssl/wolfcrypt/port/Espressif/esp32-crypt.h>
static const char* TAG = "aes";
#define TAG "aes"

/* We'll use SW for fallback:
* unsupported key lengths. (e.g. ESP32-S3)
Expand Down Expand Up @@ -2757,7 +2757,8 @@ static void AesEncryptBlocks_C(Aes* aes, const byte* in, byte* out, word32 sz)

#endif /* !WC_AES_BITSLICED */

/* Reminder: This section disabled with NO_AES_192. */
/* this section disabled with NO_AES_192 */
/* calling this one when missing NO_AES_192 */
static WARN_UNUSED_RESULT int wc_AesEncrypt(
Aes* aes, const byte* inBlock, byte* outBlock)
{
Expand Down
23 changes: 15 additions & 8 deletions wolfcrypt/src/ecc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3828,7 +3828,7 @@ int wc_ecc_mulmod_ex2(const mp_int* k, ecc_point* G, ecc_point* R, mp_int* a,
#endif
/* k can't have more bits than order */
if (mp_count_bits(k) > mp_count_bits(order)) {
WOLFSSL_MSG("WARNING: mp_count_bits(k) > mp_count_bits(order)");
WOLFSSL_MSG("Private key length is greater than order in bits.");
return ECC_OUT_OF_RANGE_E;
}

Expand Down Expand Up @@ -5802,26 +5802,34 @@ static int _ecc_make_key_ex(WC_RNG* rng, int keysize, ecc_key* key,
/* load curve info */
if (err == MP_OKAY) {
ALLOC_CURVE_SPECS(ECC_CURVE_FIELD_COUNT, err);
if (err != MP_OKAY) {
WOLFSSL_MSG("ALLOC_CURVE_SPECS failed");
}
}

if (err == MP_OKAY) {
err = wc_ecc_curve_load(key->dp, &curve, ECC_CURVE_FIELD_ALL);
}
else {
WOLFSSL_MSG("ALLOC_CURVE_SPECS failed");
if (err != MP_OKAY) {
WOLFSSL_MSG("wc_ecc_curve_load failed");
}
}

/* generate k */
if (err == MP_OKAY) {
err = wc_ecc_gen_k(rng, key->dp->size, key->k, curve->order);
if (err != MP_OKAY) {
WOLFSSL_MSG("wc_ecc_gen_k failed");
}
}

/* generate public key from k */
if (err == MP_OKAY) {
err = ecc_make_pub_ex(key, curve, NULL, rng);
if (err != MP_OKAY) {
WOLFSSL_MSG("ecc_make_pub_ex failed");
}
}
else {
WOLFSSL_MSG("wc_ecc_gen_k failed");
}

if (err == MP_OKAY
#ifdef WOLFSSL_ASYNC_CRYPT
|| err == WC_PENDING_E
Expand All @@ -5831,7 +5839,6 @@ static int _ecc_make_key_ex(WC_RNG* rng, int keysize, ecc_key* key,
}
else {
/* cleanup these on failure case only */
WOLFSSL_MSG("ecc_make_pub_ex failed");
mp_forcezero(key->k);
}

Expand Down
Loading

0 comments on commit 9b98879

Please sign in to comment.