Skip to content

Add an error message in case of invalid configured dependency mbedTLS. #3364

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 17, 2019
Merged

Add an error message in case of invalid configured dependency mbedTLS. #3364

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions libraries/WiFiClientSecure/src/ssl_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
#include "ssl_client.h"
#include "WiFi.h"

#ifndef MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would need to be prefixed with CONFIG_... But I don't see this key on IDF v3.2 or IDF v3.3.

Copy link
Collaborator

Choose a reason for hiding this comment

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

CONFIG_MBEDTLS_PSK_MODES and CONFIG_MBEDTLS_TLS_ENABLED would be better to check for here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not quite easy to handle with the CONFIG_* definitions because internally the library will use a lot of things together (the basic config must be set + at least one cipher must be selected) to enable this function. So I choose to use the library definition which really enables the function to not have duplicate logic in there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The problem is the define you have used will never exist, regardless of sdkconfig entries selected. If you switch to the CONFIG_XXX entries I posted above it will at least compile for test cases.

But you are right, there is still the requirement to have at least one key exchange format enabled which will require explicit checking for the various keys used by the code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually digging into this further it looks like MBEDTLS_KEY_EXCHANGE__SOME__PSK_ENABLED would be defined internally by mbedTLS. It is not clear though why the GitHub Actions failed. @me-no-dev can you retrigger them?

# error "Please configure IDF framework to include mbedTLS -> Enable pre-shared-key ciphersuites and activate at least one cipher"
#endif

const char *pers = "esp32-tls";

Expand Down