From 1a0a83c2e966bc17b5cfd443ed532acf788b4a5b Mon Sep 17 00:00:00 2001 From: gojimmypi Date: Wed, 3 May 2023 09:48:47 -0700 Subject: [PATCH 1/4] WOLFSSL_SP_INT_NEGATIVE declaration for all Espressif chipsets --- wolfcrypt/src/port/Espressif/esp32_mp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/wolfcrypt/src/port/Espressif/esp32_mp.c b/wolfcrypt/src/port/Espressif/esp32_mp.c index 7e57eff4cac..ae93e2453de 100644 --- a/wolfcrypt/src/port/Espressif/esp32_mp.c +++ b/wolfcrypt/src/port/Espressif/esp32_mp.c @@ -332,6 +332,10 @@ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) { int ret = 0; +#ifdef WOLFSSL_SP_INT_NEGATIVE + int neg = (X->sign == Y->sign) ? MP_ZPOS : MP_NEG; +#endif + #if CONFIG_IDF_TARGET_ESP32S3 int BitsInX = mp_count_bits(X); @@ -345,10 +349,6 @@ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) int WordsForOperand = bits2words(MinXYBits); int WordsForResult = bits2words(BitsInX + BitsInY); -#ifdef WOLFSSL_SP_INT_NEGATIVE - int neg; - neg = (X->sign == Y->sign) ? MP_ZPOS : MP_NEG; -#endif /* Make sure we are within capabilities of hardware. */ if ( (WordsForOperand * BITS_IN_ONE_WORD) > ESP_HW_MULTI_RSAMAX_BITS ) { ESP_LOGW(TAG, "exceeds max bit length(2048)"); From 19961ae88899fac19a9473e5e367d31d91af6629 Mon Sep 17 00:00:00 2001 From: gojimmypi Date: Wed, 3 May 2023 14:52:59 -0700 Subject: [PATCH 2/4] correct naming for WOLFSSL_SHA384 on ESP32-C3 --- wolfcrypt/src/port/Espressif/esp32_sha.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wolfcrypt/src/port/Espressif/esp32_sha.c b/wolfcrypt/src/port/Espressif/esp32_sha.c index 3f5535ba35a..39f4ad8ab16 100644 --- a/wolfcrypt/src/port/Espressif/esp32_sha.c +++ b/wolfcrypt/src/port/Espressif/esp32_sha.c @@ -855,7 +855,7 @@ static int esp_sha_start_process(WC_ESP32SHA* sha) break; #if defined(WOLFSSL_SHA384) case SHA2_384: - uHardwareAlgorithm = 3; + HardwareAlgorithm = 3; break; #endif #if defined(WOLFSSL_SHA512) From 2ec486ed6f46f49951092612667681d5caaba96f Mon Sep 17 00:00:00 2001 From: gojimmypi Date: Wed, 3 May 2023 14:59:06 -0700 Subject: [PATCH 3/4] code review changes; common handling of negative MATH_INT_T --- wolfcrypt/src/port/Espressif/esp32_mp.c | 26 ++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/wolfcrypt/src/port/Espressif/esp32_mp.c b/wolfcrypt/src/port/Espressif/esp32_mp.c index ae93e2453de..b74983e01fc 100644 --- a/wolfcrypt/src/port/Espressif/esp32_mp.c +++ b/wolfcrypt/src/port/Espressif/esp32_mp.c @@ -330,10 +330,17 @@ static int esp_get_rinv(MATH_INT_T *rinv, MATH_INT_T *M, word32 exp) /* Z = X * Y; */ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) { - int ret = 0; + int ret = MP_OKAY; /* assume success until proven wrong */ #ifdef WOLFSSL_SP_INT_NEGATIVE - int neg = (X->sign == Y->sign) ? MP_ZPOS : MP_NEG; + /* neg check: X*Y becomes negative */ + int neg = (mp_isneg(X) == mp_isneg(Y)) ? MP_ZPOS : MP_NEG; // (X->sign == Y->sign) ? MP_ZPOS : MP_NEG; + if (neg) { + /* Negative numbers are relatively infrequent. + * May be interesting during verbose debugging: */ + ESP_LOGV(TAG, "mp_isneg(X) = %d; mp_isneg(Y) = %d; neg = %d ", + mp_isneg(X), mp_isneg(Y), neg); + } #endif #if CONFIG_IDF_TARGET_ESP32S3 @@ -407,11 +414,6 @@ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) /* 7. clear and release HW */ esp_mp_hw_unlock(); -#ifdef WOLFSSL_SP_INT_NEGATIVE - Z->sign = (Z->used > 0) ? neg : MP_ZPOS; -#endif - - return ret; /* end if CONFIG_IDF_TARGET_ESP32S3 */ #else /* not CONFIG_IDF_TARGET_ESP32S3 */ @@ -422,11 +424,6 @@ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) word32 maxWords_sz; word32 hwWords_sz; - /* neg check - X*Y becomes negative */ -#ifdef WOLFSSL_SP_INT_NEGATIVE - neg = mp_isneg(X) != mp_isneg(Y) ? 1 : 0; -#endif - /* ask bits number */ Xs = mp_count_bits(X); Ys = mp_count_bits(Y); @@ -492,15 +489,18 @@ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) /* step.7 clear and release HW */ esp_mp_hw_unlock(); +#endif /* CONFIG_IDF_TARGET_ESP32S3 or not */ + /* common exit for all chipset types */ #ifdef WOLFSSL_SP_INT_NEGATIVE if (!mp_iszero(Z) && neg) { + /* for non-zero negative numbers, set negative flag for our result: + * Z->sign = FP_NEG */ mp_setneg(Z); } #endif return ret; -#endif /* CONFIG_IDF_TARGET_ESP32S3 or not */ } /* Z = X * Y (mod M) */ From 7aa07f0ad0b7cbf251eb4771d69b8b6764b1b0b9 Mon Sep 17 00:00:00 2001 From: gojimmypi Date: Wed, 3 May 2023 15:28:11 -0700 Subject: [PATCH 4/4] code review update: address mixed declaration with code. --- wolfcrypt/src/port/Espressif/esp32_mp.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/wolfcrypt/src/port/Espressif/esp32_mp.c b/wolfcrypt/src/port/Espressif/esp32_mp.c index b74983e01fc..982991ac247 100644 --- a/wolfcrypt/src/port/Espressif/esp32_mp.c +++ b/wolfcrypt/src/port/Espressif/esp32_mp.c @@ -330,11 +330,14 @@ static int esp_get_rinv(MATH_INT_T *rinv, MATH_INT_T *M, word32 exp) /* Z = X * Y; */ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) { - int ret = MP_OKAY; /* assume success until proven wrong */ + int ret; #ifdef WOLFSSL_SP_INT_NEGATIVE /* neg check: X*Y becomes negative */ - int neg = (mp_isneg(X) == mp_isneg(Y)) ? MP_ZPOS : MP_NEG; // (X->sign == Y->sign) ? MP_ZPOS : MP_NEG; + int neg; + + /* aka (X->sign == Y->sign) ? MP_ZPOS : MP_NEG; , but with mp_isneg(): */ + neg = (mp_isneg(X) == mp_isneg(Y)) ? MP_ZPOS : MP_NEG; if (neg) { /* Negative numbers are relatively infrequent. * May be interesting during verbose debugging: */ @@ -342,6 +345,7 @@ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) mp_isneg(X), mp_isneg(Y), neg); } #endif + ret = MP_OKAY; /* assume success until proven wrong */ #if CONFIG_IDF_TARGET_ESP32S3 @@ -384,8 +388,9 @@ int esp_mp_mul(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* Z) return ret; } - /* 2. Disable completion interrupt signal; we don't use. */ - DPORT_REG_WRITE(RSA_INTERRUPT_REG, 0); // 0 => no interrupt; 1 => interrupt on completion. + /* 2. Disable completion interrupt signal; we don't use. + ** 0 => no interrupt; 1 => interrupt on completion. */ + DPORT_REG_WRITE(RSA_INTERRUPT_REG, 0); /* 3. Write number of words required for result. */ if ( (WordsForOperand * BITS_IN_ONE_WORD * 2) > ESP_HW_RSAMAX_BIT) { @@ -596,8 +601,9 @@ int esp_mp_mulmod(MATH_INT_T* X, MATH_INT_T* Y, MATH_INT_T* M, MATH_INT_T* Z) return ret; } - /* 2. Disable completion interrupt signal; we don't use. */ - DPORT_REG_WRITE(RSA_INTERRUPT_REG, 0); // 0 => no interrupt; 1 => interrupt on completion. + /* 2. Disable completion interrupt signal; we don't use. + ** 0 => no interrupt; 1 => interrupt on completion. */ + DPORT_REG_WRITE(RSA_INTERRUPT_REG, 0); /* 3. Write (N_result_bits/32 - 1) to the RSA_MODE_REG. */ OperandBits = max(max(Xs, Ys), Ms); @@ -815,8 +821,9 @@ int esp_mp_exptmod(MATH_INT_T* X, MATH_INT_T* Y, word32 Ys, MATH_INT_T* M, MATH_ return ret; } - /* 2. Disable completion interrupt signal; we don't use. */ - DPORT_REG_WRITE(RSA_INTERRUPT_REG, 0); // 0 => no interrupt; 1 => interrupt on completion. + /* 2. Disable completion interrupt signal; we don't use. + ** 0 => no interrupt; 1 => interrupt on completion. */ + DPORT_REG_WRITE(RSA_INTERRUPT_REG, 0); /* 3. Write (N_result_bits/32 - 1) to the RSA_MODE_REG. */ OperandBits = max(max(Xs, Ys), Ms);