Repository navigation
ports/esp32: Enable mbedtls cert time validation. - #13100
Conversation
There was a problem hiding this comment.
I think you want to include #include "mbedtls/platform.h" instead.
There was a problem hiding this comment.
mbedtls/platform.h includes mbedtls/platform_time.h, since this only requires mbedtls_platform_set_time I think it's simpler to include platform_time directly.
There was a problem hiding this comment.
I don't think this set of includes is needed??
There was a problem hiding this comment.
This line should not be needed.
There was a problem hiding this comment.
Can't you use #ifdef MBEDTLS_PLATFORM_TIME_ALT here instead?
8407d27 to
93aff62
Compare
There was a problem hiding this comment.
I see that this file is indeed included in the build. But that seems to be by accident: esp-idf/components/mbedtls/port/include/mbedtls/esp_config.h includes mbedtls/mbedtls_config.h which is intended to pick up the default mbedtls_config.h from the mbedtls component of the IDF.
So having this file here will completely override the default set of settings.
Why is this file needed? I don't think the MBEDTLS_HAVE_ASM option does anything, that seems to only be needed for x84 architectures.
There was a problem hiding this comment.
To keep things simple, you could just put this function in main.c, before the mp_task() function. Then this separate file is not needed, nor are any additional headers like mbedtls_config.h.
There was a problem hiding this comment.
I agree, I've just tested this and it is in fact the simplest option, thanks for the help again 🙏🏼 !!
93aff62 to
6de72bc
Compare
|
This looks much better now. Is it ready to be merged? |
|
Yes I've been testing this and it works as expected 👍🏼 . With this commit now all ports that use mbedtls have cert time validation enabled, so I think a note should be added to otherwise |
Signed-off-by: Carlos Gil <[email protected]>
6de72bc to
30b0ee3
Compare
|
Thanks for testing. Now merged. |
2 participants
WIP