Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ESP32 HW ED25519 test #5997

Closed
wants to merge 3 commits into from

Conversation

gojimmypi
Copy link
Contributor

Description

This PR reorders the items in the WC_ESP32SHA typedef struct in the esp32-crypt.h used by Espressif HW Acceleration as noted in #5948 (comment), addressing the byte-alignment problem that cause the ESP32 ED25519 test to fail with Stack Smashing disabled. (The test did not fail with Stack Smashing set to Normal)

Fixes zd# n/a

Testing

Confirm Error Condition

With a struct like this, having the lockDepth listed first, Stack Smashing set to None and Espressif Hardware Acceleration enabled:

    typedef struct {
        int lockDepth;     /* see ref_counts[periph] in periph_ctrl.c    */
        /* NOTE:
        **
        ** There's a known Espressif byte alignment issue. See:
        ** https://github.com/wolfSSL/wolfssl/issues/5948
        **
        ** To avoid problems, list the largest types first.
        */
        ESP32_MODE mode; /* typically 0 init, 1 HW, 2 SW */

        /* see esp_rom/include/esp32/rom/sha.h */
        enum SHA_TYPE sha_type; /* the Espressif type: SHA1, SHA256, etc.*/

        /* we'll keep track of our own locks.
        ** actual enable/disable only occurs for ref_counts[periph] == 0 */
        byte isfirstblock; /* 0 is not first block; 1 = is first block   */
    } WC_ESP32SHA;

Results in this error -229 (SIG_VERIFY_E = wolfcrypt signature verify error):

[ ... snip ... ]
ECC buffer test passed!
CURVE25519 test passed!
ED25519  test failed!
 error = -229
I (127180) wolfcrypt_test: Exiting main with return code: -1

E (127180) wolfssl_test: wolf_test_task FAIL result code = -1

Confirm Success

Load the VisualGDB project and confirm Stack Smashing is set to None:

image

Run the wolfSSL test as changed in this PR (be sure the latest wolfSSL component is installed) and confirm all tests pass:

ets Jun  8 2016 00:22:57

rst:0x3 (SW_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff0030,len:6612
load:0x40078000,len:14788
load:0x40080400,len:3792
entry 0x40080694
I (26) boot: ESP-IDF v4.4.1-dirty 2nd stage bootloader
I (26) boot: compile time 12:13:16
I (26) boot: chip revision: 1
I (29) boot_comm: chip revision: 1, min. bootloader chip revision: 0
I (36) boot.esp32: SPI Speed      : 40MHz
I (41) boot.esp32: SPI Mode       : DIO
I (46) boot.esp32: SPI Flash Size : 4MB
I (50) boot: Enabling RNG early entropy source...
I (56) boot: Partition Table:
I (59) boot: ## Label            Usage          Type ST Offset   Length
I (66) boot:  0 nvs              WiFi data        01 02 00009000 00006000
I (74) boot:  1 phy_init         RF data          01 01 0000f000 00001000
I (81) boot:  2 factory          factory app      00 00 00010000 00177000
I (89) boot: End of partition table
I (93) boot_comm: chip revision: 1, min. application chip revision: 0
I (100) esp_image: segment 0: paddr=00010020 vaddr=3f400020 size=16f68h ( 94056) map
I (143) esp_image: segment 1: paddr=00026f90 vaddr=3ffb0000 size=024d4h (  9428) load
I (147) esp_image: segment 2: paddr=0002946c vaddr=40080000 size=06bach ( 27564) load
I (160) esp_image: segment 3: paddr=00030020 vaddr=400d0020 size=3fc94h (261268) map
I (255) esp_image: segment 4: paddr=0006fcbc vaddr=40086bac size=047b0h ( 18352) load
I (263) esp_image: segment 5: paddr=00074474 vaddr=50000000 size=00010h (    16) load
I (268) boot: Loaded app from partition at offset 0x10000
I (268) boot: Disabling RNG early entropy source...
I (283) cpu_start: Pro cpu up.
I (284) cpu_start: Starting app cpu, entry point is 0x40081080
I (272) cpu_start: App cpu up.
I (298) cpu_start: Pro cpu start user code
I (298) cpu_start: cpu freq: 160000000
I (298) cpu_start: Application information:
I (302) cpu_start: Project name:     wolfssl_test
I (308) cpu_start: App version:      v5.5.4-stable-191-ga40da56f1-di
I (315) cpu_start: Compile time:     Jan 21 2023 12:12:59
I (321) cpu_start: ELF file SHA256:  7465217a78da03e9...
I (327) cpu_start: ESP-IDF:          v4.4.1-dirty
I (332) heap_init: Initializing. RAM available for dynamic allocation:
I (340) heap_init: At 3FFAE6E0 len 00001920 (6 KiB): DRAM
I (346) heap_init: At 3FFB2DE8 len 0002D218 (180 KiB): DRAM
I (352) heap_init: At 3FFE0440 len 00003AE0 (14 KiB): D/IRAM
I (358) heap_init: At 3FFE4350 len 0001BCB0 (111 KiB): D/IRAM
I (365) heap_init: At 4008B35C len 00014CA4 (83 KiB): IRAM
I (372) spi_flash: detected chip: generic
I (375) spi_flash: flash io: dio
I (381) cpu_start: Starting scheduler on PRO CPU.
I (0) cpu_start: Starting scheduler on APP CPU.
I (390) wolfssl_test: --------------------------------------------------------
I (390) wolfssl_test: --------------------------------------------------------
I (400) wolfssl_test: ---------------------- BEGIN MAIN ----------------------
I (400) wolfssl_test: --------------------------------------------------------
I (410) wolfssl_test: --------------------------------------------------------
I (420) wolfssl_test: CONFIG_IDF_TARGET = esp32
I (420) wolfssl_test: LIBWOLFSSL_VERSION_STRING = 5.5.4
I (430) wolfssl_test: CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ = 160 MHz
I (440) wolfssl_test: Xthal_have_ccount = 1
I (440) wolfssl_test: Stack HWM: 3800

I (450) wolfssl_test: ESP32WROOM32_CRYPT is enabled.
------------------------------------------------------------------------------
 wolfSSL version 5.5.4
------------------------------------------------------------------------------
error    test passed!
MEMORY   test passed!
base64   test passed!
asn      test passed!
RANDOM   test passed!
MD5      test passed!
MD4      test passed!
SHA      test passed!
SHA-256  test passed!
SHA-512  test passed!
Hash     test passed!
HMAC-MD5 test passed!
HMAC-SHA test passed!
HMAC-SHA256 test passed!
HMAC-SHA512 test passed!
HMAC-KDF    test passed!
TLSv1.3 KDF test passed!
GMAC     test passed!
DES      test passed!
DES3     test passed!
AES      test passed!
AES192   test passed!
AES256   test passed!
AES-GCM  test passed!
RSA      test passed!
PWDBASED test passed!
ECC      test passed!
ECC buffer test passed!
CURVE25519 test passed!
ED25519  test passed!
logging  test passed!
time test passed!
mutex    test passed!
Test complete
I (127630) wolfcrypt_test: Exiting main with return code:  0

I (127630) wolfssl_test: wolf_test_task complete success result code = 0

Thank you @dgarske for suggesting item order. I may have otherwise simply changed the struct item mode type from ESP32_MODE mode to byte mode as a working but still very fragile solution.

Note the ESP32_SHA_FAIL_NEED_UNROLL value, although changed in this PR, did not actually affect the success or failure of the ED25519 test as related to this byte alignment problem. It is changed only as related to a Best Practice advice.

This code is still somewhat fragile. It might be nice to have something detect such byte alignment problems in Espressif, and possibly other environments as noted in #5989.

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@gojimmypi gojimmypi added the bug label Jan 21, 2023
@gojimmypi gojimmypi requested review from dgarske and bandi13 January 21, 2023 21:16
@gojimmypi gojimmypi self-assigned this Jan 21, 2023
@dgarske dgarske self-assigned this Jan 23, 2023
Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed please implement fix using the memset we discussed. Bug here is use of uninitialized stack memory.

@@ -115,19 +115,26 @@ int esp_CryptHwMutexUnLock(wolfSSL_Mutex* mutex);
ESP32_SHA_INIT = 0,
ESP32_SHA_HW = 1,
ESP32_SHA_SW = 2,
ESP32_SHA_FAIL_NEED_UNROLL = -1
ESP32_SHA_FAIL_NEED_UNROLL = 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If -1 worked consider reverting this change.

@dgarske dgarske removed their assignment Jan 24, 2023
@gojimmypi gojimmypi marked this pull request as draft February 6, 2023 20:07
@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

@dgarske
Copy link
Contributor

dgarske commented Feb 9, 2023

Closing in favor of #6068.

@dgarske dgarske closed this Feb 9, 2023
@gojimmypi
Copy link
Contributor Author

gojimmypi commented Feb 9, 2023

@dgarske I do not believe the zeroize of buffer will fully resolve this issue.

The situation that caused the odd ED25519 failure here was the use of a copy of an already-initialized SHA ctx object.

@gojimmypi
Copy link
Contributor Author

For reference, see #5948 (comment) for notes on resolution of this issue.

@gojimmypi gojimmypi deleted the Espressif_ED25519_fix branch October 12, 2023 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants