From 11133e578ddf2ff9942b00df2759e2a1dc4cb15e Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 12 Apr 2017 10:07:38 -0700 Subject: [PATCH] Fixes and cleanups based on feedback from Sean. Added ifdef checks around WC_PENDING_E code to reduce code size for non-async builds. Cleanup accumulative result code checking in SSL_hmac. Cleanup of the RSA async state advancement. --- src/internal.c | 45 ++++++++++++++++++++++++++-------------- wolfcrypt/src/rsa.c | 38 ++++++++++++++++++++++----------- wolfcrypt/src/wolfmath.c | 14 ++++++------- wolfcrypt/test/test.c | 8 +++---- 4 files changed, 66 insertions(+), 39 deletions(-) diff --git a/src/internal.c b/src/internal.c index 983e0eef3..b18bcbdca 100755 --- a/src/internal.c +++ b/src/internal.c @@ -8437,6 +8437,7 @@ static int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, ret = DECODE_E; } +#ifdef WOLFSSL_ASYNC_CRYPT /* if async, offset index so this msg will be processed again */ if (ret == WC_PENDING_E) { *inOutIdx -= HANDSHAKE_HEADER_SZ; @@ -8446,6 +8447,7 @@ static int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx, } #endif } +#endif WOLFSSL_LEAVE("DoHandShakeMsgType()", ret); return ret; @@ -9304,9 +9306,11 @@ static INLINE int Encrypt(WOLFSSL* ssl, byte* out, const byte* input, word16 sz, { int ret = 0; +#ifdef WOLFSSL_ASYNC_CRYPT if (asyncOkay && ssl->error == WC_PENDING_E) { ssl->error = 0; /* clear async */ } +#endif switch (ssl->encrypt.state) { case CIPHER_STATE_BEGIN: @@ -9350,10 +9354,12 @@ static INLINE int Encrypt(WOLFSSL* ssl, byte* out, const byte* input, word16 sz, /* Advance state */ ssl->encrypt.state = CIPHER_STATE_END; + #ifdef WOLFSSL_ASYNC_CRYPT /* If pending, then leave and return will resume below */ if (ret == WC_PENDING_E) { return ret; } + #endif } case CIPHER_STATE_END: @@ -9581,10 +9587,12 @@ static INLINE int Decrypt(WOLFSSL* ssl, byte* plain, const byte* input, /* Advance state */ ssl->decrypt.state = CIPHER_STATE_END; + #ifdef WOLFSSL_ASYNC_CRYPT /* If pending, leave and return below */ if (ret == WC_PENDING_E) { return ret; } + #endif } case CIPHER_STATE_END: @@ -10393,10 +10401,11 @@ int ProcessReply(WOLFSSL* ssl) ssl->buffers.inputBuffer.idx, ssl->curSize, ssl->curRL.type, &ssl->keys.padSz); + #ifdef WOLFSSL_ASYNC_CRYPT + if (ret == WC_PENDING_E) + return ret; + #endif if (ret < 0) { - if (ret == WC_PENDING_E) - return ret; - WOLFSSL_MSG("VerifyMac failed"); WOLFSSL_ERROR(ret); return DECRYPT_ERROR; @@ -10714,11 +10723,11 @@ static int SSL_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, /* inner */ ret = wc_Md5Update(&md5, macSecret, digestSz); - ret += wc_Md5Update(&md5, PAD1, padSz); - ret += wc_Md5Update(&md5, seq, SEQ_SZ); - ret += wc_Md5Update(&md5, conLen, sizeof(conLen)); + ret |= wc_Md5Update(&md5, PAD1, padSz); + ret |= wc_Md5Update(&md5, seq, SEQ_SZ); + ret |= wc_Md5Update(&md5, conLen, sizeof(conLen)); /* in buffer */ - ret += wc_Md5Update(&md5, in, sz); + ret |= wc_Md5Update(&md5, in, sz); if (ret != 0) return VERIFY_MAC_ERROR; ret = wc_Md5Final(&md5, result); @@ -10733,8 +10742,8 @@ static int SSL_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, /* outer */ ret = wc_Md5Update(&md5, macSecret, digestSz); - ret += wc_Md5Update(&md5, PAD2, padSz); - ret += wc_Md5Update(&md5, result, digestSz); + ret |= wc_Md5Update(&md5, PAD2, padSz); + ret |= wc_Md5Update(&md5, result, digestSz); if (ret != 0) return VERIFY_MAC_ERROR; ret = wc_Md5Final(&md5, digest); @@ -10756,11 +10765,11 @@ static int SSL_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, /* inner */ ret = wc_ShaUpdate(&sha, macSecret, digestSz); - ret += wc_ShaUpdate(&sha, PAD1, padSz); - ret += wc_ShaUpdate(&sha, seq, SEQ_SZ); - ret += wc_ShaUpdate(&sha, conLen, sizeof(conLen)); + ret |= wc_ShaUpdate(&sha, PAD1, padSz); + ret |= wc_ShaUpdate(&sha, seq, SEQ_SZ); + ret |= wc_ShaUpdate(&sha, conLen, sizeof(conLen)); /* in buffer */ - ret += wc_ShaUpdate(&sha, in, sz); + ret |= wc_ShaUpdate(&sha, in, sz); if (ret != 0) return VERIFY_MAC_ERROR; ret = wc_ShaFinal(&sha, result); @@ -10775,8 +10784,8 @@ static int SSL_hmac(WOLFSSL* ssl, byte* digest, const byte* in, word32 sz, /* outer */ ret = wc_ShaUpdate(&sha, macSecret, digestSz); - ret += wc_ShaUpdate(&sha, PAD2, padSz); - ret += wc_ShaUpdate(&sha, result, digestSz); + ret |= wc_ShaUpdate(&sha, PAD2, padSz); + ret |= wc_ShaUpdate(&sha, result, digestSz); if (ret != 0) return VERIFY_MAC_ERROR; ret = wc_ShaFinal(&sha, digest); @@ -11181,9 +11190,11 @@ exit_buildmsg: WOLFSSL_LEAVE("BuildMessage", ret); +#ifdef WOLFSSL_ASYNC_CRYPT if (ret == WC_PENDING_E) { return ret; } +#endif /* make sure build message state is reset */ ssl->options.buildMsgState = BUILD_MSG_BEGIN; @@ -12170,8 +12181,10 @@ int SendData(WOLFSSL* ssl, const void* data, int sz) sendSz = BuildMessage(ssl, out, outputSz, sendBuffer, buffSz, application_data, 0, 0, 1); if (sendSz < 0) { + #ifdef WOLFSSL_ASYNC_CRYPT if (sendSz == WC_PENDING_E) ssl->error = sendSz; + #endif return BUILD_MSG_ERROR; } @@ -12230,10 +12243,12 @@ int ReceiveData(WOLFSSL* ssl, byte* output, int sz, int peek) int err; WOLFSSL_MSG("Handshake not complete, trying to finish"); if ( (err = wolfSSL_negotiate(ssl)) != SSL_SUCCESS) { + #ifdef WOLFSSL_ASYNC_CRYPT /* if async would block return WANT_WRITE */ if (ssl->error == WC_PENDING_E) { return WOLFSSL_CBIO_ERR_WANT_READ; } + #endif return err; } } diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index ee3fb2799..158e3591e 100755 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -1116,7 +1116,7 @@ static int RsaPublicEncryptEx(const byte* in, word32 inLen, byte* out, enum wc_HashType hash, int mgf, byte* label, word32 labelSz, WC_RNG* rng) { - int ret = RSA_WRONG_TYPE_E, sz; + int ret, sz; if (in == NULL || inLen == 0 || out == NULL || key == NULL) { return BAD_FUNC_ARG; @@ -1166,28 +1166,35 @@ static int RsaPublicEncryptEx(const byte* in, word32 inLen, byte* out, if (ret < 0) { break; } - /* fall through */ - case RSA_STATE_ENCRYPT_EXPTMOD: + key->state = RSA_STATE_ENCRYPT_EXPTMOD; + /* fall through */ + + case RSA_STATE_ENCRYPT_EXPTMOD: key->dataLen = outLen; ret = wc_RsaFunction(out, sz, out, &key->dataLen, rsa_type, key, rng); + + if (ret >= 0 || ret == WC_PENDING_E) { + key->state = RSA_STATE_ENCRYPT_RES; + } if (ret < 0) { break; } + /* fall through */ + case RSA_STATE_ENCRYPT_RES: - key->state = RSA_STATE_ENCRYPT_RES; ret = key->dataLen; break; default: ret = BAD_STATE_E; + break; } /* if async pending then return and skip done cleanup below */ if (ret == WC_PENDING_E) { - key->state++; return ret; } @@ -1257,7 +1264,8 @@ static int RsaPrivateDecryptEx(byte* in, word32 inLen, byte* out, /* verify the tmp ptr is NULL, otherwise indicates bad state */ if (key->data != NULL) { - ERROR_OUT(BAD_STATE_E); + ret = BAD_STATE_E; + break; } /* if not doing this inline then allocate a buffer for it */ @@ -1266,7 +1274,8 @@ static int RsaPrivateDecryptEx(byte* in, word32 inLen, byte* out, key->data = (byte*)XMALLOC(inLen, key->heap, DYNAMIC_TYPE_WOLF_BIGINT); key->dataIsAlloc = 1; if (key->data == NULL) { - ERROR_OUT(MEMORY_E); + ret = MEMORY_E; + break; } XMEMCPY(key->data, in, inLen); } @@ -1275,14 +1284,19 @@ static int RsaPrivateDecryptEx(byte* in, word32 inLen, byte* out, } ret = wc_RsaFunction(key->data, inLen, key->data, &key->dataLen, rsa_type, key, rng); + + if (ret >= 0 || ret == WC_PENDING_E) { + key->state = RSA_STATE_DECRYPT_UNPAD; + } if (ret < 0) { break; } + /* fall through */ + case RSA_STATE_DECRYPT_UNPAD: { byte* pad = NULL; - key->state = RSA_STATE_DECRYPT_UNPAD; ret = wc_RsaUnPad_ex(key->data, key->dataLen, &pad, pad_value, pad_type, hash, mgf, label, labelSz, key->heap); if (ret > 0 && ret <= (int)outLen && pad != NULL) { @@ -1300,10 +1314,11 @@ static int RsaPrivateDecryptEx(byte* in, word32 inLen, byte* out, if (ret < 0) { break; } + + key->state = RSA_STATE_DECRYPT_RES; /* fall through */ } case RSA_STATE_DECRYPT_RES: - key->state = RSA_STATE_DECRYPT_RES; #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_RSA) && \ defined(HAVE_CAVIUM) if (key->asyncDev.marker == WOLFSSL_ASYNC_MARKER_RSA) { @@ -1312,15 +1327,14 @@ static int RsaPrivateDecryptEx(byte* in, word32 inLen, byte* out, } #endif break; + default: ret = BAD_STATE_E; + break; } -done: - /* if async pending then return and skip done cleanup below */ if (ret == WC_PENDING_E) { - key->state++; return ret; } diff --git a/wolfcrypt/src/wolfmath.c b/wolfcrypt/src/wolfmath.c index ae569e12e..2f368989d 100644 --- a/wolfcrypt/src/wolfmath.c +++ b/wolfcrypt/src/wolfmath.c @@ -122,7 +122,7 @@ int mp_rand(mp_int* a, int digits, WC_RNG* rng) #ifdef HAVE_WOLF_BIGINT void wc_bigint_init(WC_BIGINT* a) { - if (a) { + if (a != NULL) { a->buf = NULL; a->len = 0; a->heap = NULL; @@ -142,12 +142,12 @@ int wc_bigint_alloc(WC_BIGINT* a, word32 sz) } if (a->buf == NULL) { a->buf = (byte*)XMALLOC(sz, a->heap, DYNAMIC_TYPE_WOLF_BIGINT); - if (a->buf) { - XMEMSET(a->buf, 0, sz); - } - else { - err = MP_MEM; - } + } + if (a->buf == NULL) { + err = MP_MEM; + } + else { + XMEMSET(a->buf, 0, sz); } } a->len = sz; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 603656901..a0471b7f3 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -340,12 +340,10 @@ static void myFipsCb(int ok, int err, const char* hash) #ifdef WOLFSSL_STATIC_MEMORY #ifdef BENCH_EMBEDDED static byte gTestMemory[10000]; + #elif defined(USE_FAST_MATH) && !defined(ALT_ECC_SIZE) + static byte gTestMemory[130000]; #else - #if defined(USE_FAST_MATH) && !defined(ALT_ECC_SIZE) - static byte gTestMemory[130000]; - #else - static byte gTestMemory[80000]; - #endif + static byte gTestMemory[80000]; #endif #endif