Software-only support for recent esp32 targets

[dpalffy]

Description

This adds generic support for any target supported in recent esp-idf SDKs including RISC-V.

Partially fixes #5215, #6234

Testing

• all examples build for all supported targets on esp-idf v4.4 and v5.0.
• wolfssl_test passes all tests on an esp32c3 with both esp-idf v4.4 and v5.0
• eolfssl_benchmark gives reasonable results on an esp32c3 with both esp-idf v4.4 and v5.0

Checklist

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

[wolfSSL-Bot]

Can one of the admins verify this patch?

[gojimmypi]

Hello @dpalffy and thanks for your PR and your interest in wolfSSL on the Espressif ESP32!

I don't recognize your GitHub name. Have you contributed to wolfSSL before? If not, we'll need to have a contributor agreement on file for you in order to merge this PR. If you could, please: contact [email protected] to get started.

In the meantime, are there any specifics you can share about your project? If you'd like to discuss offline, you can reach me at jim (at wolfssl.com) or we can chat on discord.

You may be interested in my WIP branch. I have quite a few changes, including support for some of the chipsets in this PR. I plan to get many of those changes merged soon. I see you've found several helpful things that changed in the latest ESP-IDF. Your #define esp_cpu_get_cycle_count() cpu_hal_get_cycle_count() is quite handy, as I've been meaning to find the new function name.

Again, thank you for your proposed changes.

[dpalffy]

Hi @gojimmypi, thanks for looking at my changes.

This would be my first contribution, I started with the contributor agreement.

I have way too many experimental systems around my house running unsupported bootloaders, kernels at hardly accessible locations that may need console access, so I decided to start making a more "productionized" version of the wolfssh serial console example, with BLE Wifi provisioning, runtime configurable authentication, etc, and apparently the cheapest boards have the esp32c3. Still very much work in progress, but will be released when I have something working.

I was looking around for ongoing work, but I missed that branch of yours, probably the name didn't ring a bell that it may be c3 related... :) But I will check now. Do you maybe have something like #6018 for wolfssh in the works, too?

[kareem-wolfssl]

Contributor agreement approved.

[dgarske]

OK to test

[gojimmypi]

Hi @dpalffy

This would be my first contribution, I started with the contributor agreement.

Awesome! Thank you and welcome. :)

It looks like your contributor agreement is in progress. You should be hearing something soon. We appreciate you taking the time to do that.

Do you maybe have something like #6018 for wolfssh in the works, too?

Yes, absolutely. The "no install" wolfSSL has been really quite helpful for not only development of the wolfSSL libraries, but also to make it easy to get started. I have the same plans for wolfSSH.

experimental systems ...that may need console access, so I decided to start making a more "productionized" version of the wolfssh serial console

Have you seen my Espressif wolfSSH examples for SSH-to-UART? Are you building on that? It would be really cool to have the additional capabilities you mentioned.

I've taken a closer look at this PR. You should be aware that the automated build check is very particular and enforces wolfSSL standards; Consistent coding style is very important. For example this warning:

weird control chars, hard tabs, CRs, trailing whitespace:
./wolfcrypt/src/random.c:3396:»#include <esp_random.h>
./wolfcrypt/src/random.c:3398:»#include <esp_system.h>
./wolfcrypt/src/random.c:3403:»word32 rand;
./wolfcrypt/src/random.c:3404:»while (sz > 0) {
./wolfcrypt/src/random.c:3405:»    word32 len = sizeof(rand);
./wolfcrypt/src/random.c:3406:»    if (sz < len)
./wolfcrypt/src/random.c:3407:»»len = sz;
./wolfcrypt/src/random.c:3408:»    /* Get one random 32-bit word from hw RNG */
./wolfcrypt/src/random.c:3409:»    rand = esp_random( );
./wolfcrypt/src/random.c:3410:»    XMEMCPY(output, &rand, len);

Yes, a single trailing space on a line can cause this failure. I use Visual Studio with the VisualGDB extension. There's an option to "Remove trailing whitespace on save".

At some point the folks that merge code will also ask that individual commits be squashed, too. We might get away with not doing that for your first commit. Heads up for future reference. :)

But overall, there are some nice additions here. In particular, it's very cool you found the -C3 cycle counter! Otherwise, I was doing this wonky thing with a timer.

I've confirmed your changes work with the tests and benchmark apps. Both work fine on the ESP32-C3 and the Xtensa ESP32.

Heads up @PaulMartinsen has also been an active contributor for Espressif code. We've been focusing on the ESP32-S3 but it might be a good idea to coordinate updates so that we don't duplicate efforts.

Again, thank you so much for taking the time to contribute!

[gojimmypi]

Nice additions, thank you. See comments.

[gojimmypi]

Although I agree that the ambiguous WOLFSSL_ESPWROOM32 should be refined, it is probably not a good idea to remove it at this time. See wolfssl/wolfcrypt/settings.h. Let's leave HW acceleration enabled by default for the classic ESP32 and disable for architectures that don't support it at this time.

[dpalffy]

Ok, I'll look into adding some #ifdefs to enable it on the appropriate targets.

[dpalffy]

Please help me clarify a few things here if I read the code correctly:

• WOLFSSL_ESPWROOM32 has nothing to do with the WROOM module, it's the internal crypto accelerator of the ESP32 chip. and should essentially be CONFIG_IDF_TARGET_ESP32. The other targets have different crypto accelerators.
• the WROOM32-SE module has the ATECC608A in addition. WOLFSSL_ESPWROOM32SE enables the same features as WOLFSSL_ESPWROOM32 and adds the WOLFSSL_ATECC508A #define (although the code has 608A define, too).

[gojimmypi]

Yes, you understand correctly. Unfortunately that's true, and not very intuitive, I know. That was created when there was only the ESP32WROOM chip. At some point that needs to be cleaned up.

I agree the WOLFSSL_ESPWROOM32 and WOLFSSL_ESPWROOM32SE should probably be completely removed and replaced with something more appropriate.

The ESP32-S3 on the Xtensa architecture has very similar crypto accelerators. As you noted: the RISC-V chipsets are very different. I have a WIP for some HW acceleration on the -C3 and -C6.

Oh, and yes: the SE uses the ATECC508A. I'm not certain if it is 100% compatible with the ATECC608A.

[dpalffy]

According to the datasheet, the SE has a 608A. Do you maybe have the hardware to test if changing the define to 608 still works?

Otherwise how about this:

• Remove WROOM32 from the following macros, they all refer to the internal crypto in the ESP32:
  • NO_ESP32WROOM32_CRYPT
  • WOLFSSL_ESP32WROOM32_CRYPT
  • NO_WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI
  • NO_WOLFSSL_ESP32WROOM32_CRYPT_HASH
  • NO_WOLFSSL_ESP32WROOM32_CRYPT_AES
  • WOLFSSL_ESP32WROOM32_CRYPT_DEBUG
  • WOLFSSL_USE_ESP32WROOM32_CRYPT_HASH_HW
  • WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI
• Replace WOLFSSL_ESPWROOM32 with CONFIG_IDF_TARGET_ESP32. It is only used in settings.h/user_settings.h to set other macros. HW crypto can still be disabled from user_settings.h with the NO_ macros.
• Leave WOLFSSL_ESPWROOM32SE as is (maybe change 508A to 608A) to enable the Atmel chip from user_settings.h, by default disabled.
• If S3 hardware crypto is merged and shares a significant amount of code with the original ESP32, maybe also set some WOLFSSL_ESP32_XTENSA_CRYPT based on CONFIG_IDF_TARGET in settings.h for both to mark the shared code.

[gojimmypi]

My mistake on the SE, I thought it was the '608A. I don't have either one to test.

I agree on others: I like the idea of removing WROOM32. Good to keep WOLFSSL_ESPWROOM32SE as it is a very specific chipset. At some point, I'd like to have 508A/608A support on other chipsets that may have a manually added device.

[dpalffy]

Done, PTAL.

[gojimmypi]

@dpalffy thanks for your prompt reply & clarifications.

btw - you may be interested in my updated wolfssl component directory that allows for project-specific user_settings.h. I'll be working on getting a PR to get those merged soon.

Oh - and check out #6149 that will improve the way all examples show key info at startup time.

[dpalffy]

It looks like your contributor agreement is in progress. You should be hearing something soon. We appreciate you taking the time to do that.

It should be approved by now.

Yes, absolutely. The "no install" wolfSSL has been really quite helpful for not only development of the wolfSSL libraries, but also to make it easy to get started. I have the same plans for wolfSSH.

Plans only or anything WiP I could look at?

Have you seen my Espressif wolfSSH examples for SSH-to-UART? Are you building on that? It would be really cool to have the additional capabilities you mentioned.

Yes, I want to build on that.

I've taken a closer look at this PR. You should be aware that the automated build check is very particular and enforces wolfSSL standards; Consistent coding style is very important. For example this warning:

I know that, I used to work for Google.

Yes, a single trailing space on a line can cause this failure. I use Visual Studio with the VisualGDB extension. There's an option to "Remove trailing whitespace on save".

I use vim, time to make a vimrc for wolfssl.

Maybe add something to pre-commit.sh to catch these as early as possible? Where can I find the exact rules?

At some point the folks that merge code will also ask that individual commits be squashed, too. We might get away with not doing that for your first commit. Heads up for future reference. :)

No problem, just get something finished that you would approve first.

[gojimmypi]

@dpalffy I've been taking a look at your code. Thanks for the updates. It's going to be a bit challenging to merge with my my WIP branch, but those are changes that have been needed ever since Espressif started coming out with new chipsets.

One of the problems I've not been able to pinpoint, is there's a significant performance difference between my branch benchmark results and your benchmark updates to master.

Although, yes: I have many fixes and improvements in my branch, I don't think I can take credit for such a performance improvement. Perhaps one or more of the gated HW sections in your update is actually doing SW, instead?

As for your question:

I use vim, time to make a vimrc for wolfssl. Where can I find the exact [wolfssl style] rules?

I've reached out to the team to see if we have a list, or perhaps even a vimrc to share.

Plans only [for no-install wolfSSH] or anything WiP I could look at?

Plans only at this time. You can see my latest wolfssl component here.

edit: one of the engineers has kindly shared their vimrc file, attached: vimrc.txt

[dpalffy]

One of the problems I've not been able to pinpoint, is there's a significant performance difference between my branch benchmark results and your benchmark updates to master.

Although, yes: I have many fixes and improvements in my branch, I don't think I can take credit for such a performance improvement. Perhaps one or more of the gated HW sections in your update is actually doing SW, instead?

I'm pretty sure it doesn't. For the libraries, my branch should compile pretty much exactly the same code as 753ad4c (HEAD a few days ago). I'm sorry I don't have other hardware than c3 to test, can you maybe compare to that version, too? That should clarify where the difference comes from.

edit: one of the engineers has kindly shared their vimrc file, attached: vimrc.txt

Thanks!

[dpalffy]

I ordered an ESP32 board to test on xtensa too, if everything goes well I might get it on Saturday.

[gojimmypi]

I ordered an ESP32 board to test on xtensa too, if everything goes well I might get it on Saturday.

Awesome!

I received another suggestion regarding the 80-character line length limit for vimrc:

related, I have this in my vimrc to search for long lines with just shift + q

" search for columns wider than 79
" This replaces the Ex mode activation with Q
"   https://vim.fandom.com/wiki/Highlight_long_lines#Searching
noremap Q /\%>79v.\+

As for performance, there are multiple changing variables here. I see your branch is currently about 55 commits behind master (ya, this is a pretty active repo!). Of particular interest is this "math cleanup" recently merged PR: #6123. Still, so far, I've been unable to account for the difference.

I'd be super interested in the resultant benchmark performance difference (and thankful!) if could could apply the same proposed macro updates to my branch.

Thanks again for your assistance! Cheers.

[gojimmypi]

Hello @dpalffy - new answer to this question:

Plans only [for no-install wolfSSH] or anything WiP I could look at?

I have an initial wolfssh Espressif "no setup" cmake file here.

It's in pretty rough shape and needs some cleanup before creating a PR, but should be functional.

One of the wonky things I need to resolve is how & which wolfssl source directory to point to from wolfssh component project. At the moment, it is hard-coded to point to ../wolfssl-gojimmypi/