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

Espressif ESP32 Benchmark, Test, TLS 1.3 Client & Server Updates #6844

Merged

Conversation

gojimmypi
Copy link
Contributor

@gojimmypi gojimmypi commented Oct 5, 2023

Description

This PR updates the Espressif ESP32 TLS Client and Server example apps.

Of particular interest is the new CMakeLists.txt that searches for wolfSSL source code and expects the user_settings.h to now be in the project directory: components/wolfssl/include. See #6118.

The update also adds support for the ESP-IDF v5 (specifically v5.1) as noted in #5909.

The Wi-Fi networking connection has been improved, using example ESP-IDF code when possible. There's also improved time-setting and NTP support, falling back to GitHub recent commit date, and then worst case to a fixed date time value.

SM Cipher support has been added. See #6671 and wolfSSL/wolfsm. The SM ciphers need to be manually installed as noted in the README.md. Enable SM in the local user_settings.h file. The wolfssl/certs_test_sm.h is also needed from #6681 / #6825 for these examples to work properly with SM ciphers.

Fixes zd# n/a

See #6234 for a roadmap of Espressif updates.

Testing

How did you test?

Manually testing in VisualGDB and using ESP-IDF from command prompt.

Checklist

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

@gojimmypi
Copy link
Contributor Author

Jenkins retest this please

@gojimmypi
Copy link
Contributor Author

Jenkins retest this please

@gojimmypi gojimmypi changed the title Espressif ESP32 TLS 1.3 Client & Server Updates Espressif ESP32 Benchmark, Test, TLS 1.3 Client & Server Updates Nov 22, 2023
@@ -0,0 +1,333 @@
/* time_helper.c
Copy link
Contributor

Choose a reason for hiding this comment

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

This (time_helper.c) and wifi_connect.c look to be duplicated with wolfssl_client and wolfssl_server. Not critical, and could be approved with out changing, but please take a look at combining them for ease of maintenance. Having the client and server use the same time_helper.c and wifi_connect.c files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @JacobBarthelmeh, you are correct: the code is duplicated between both the client and server applications. It was always duplicated in the main.c, but now it is more obvious in separate files.

I prefer to have the examples ready-to-run, fresh out of the source code, with no additional steps for install/setup (well, other than setting up the external ESP-IDF environment). These are also examples that are published as-is to the Espressif component registry. So it seems best to have the duplicate source code in order to have each example be stand-alone.

I do agree with the duplication and maintenance annoyance. @bandi13 also pointed that out in a recent PR of mine.

Other than moving it to something like esp_util.c in wolfCrypt (where it arguably may not belong), do you have a suggestion where "common-but-not-wolfssl" code like this should live? Perhaps just move both the client and server to a shared directory with two separate projects but otherwise common code? This would be different than all the other examples. Completely open to suggestions here.

@gojimmypi
Copy link
Contributor Author

The latest commit addresses code review, polishes examples, and updates for these SoC devices:

  • esp32
  • esp32s2
  • esp32c3
  • esp32s3
  • esp32c2
  • esp32c6
  • esp32h2

For ESP32-C2 support, a future separate PR will be needed to actually enable HW encryption. (coming soon)

@gojimmypi
Copy link
Contributor Author

Jenkins retest this please

@JacobBarthelmeh JacobBarthelmeh merged commit 7753e3d into wolfSSL:master Dec 5, 2023
108 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants