- Notifications
You must be signed in to change notification settings - Fork 7.8k
feature: create a Trust on First Use example#9103
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
…ly common copy & paste of the insecure approach making it to production
github-actionsbot commented Jan 14, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
👋 Hello dirkx, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
me-no-dev commented Jan 17, 2024
you need to add |
dirkx commented Jan 17, 2024 via email
Ah thanks - I was already wondering. Will fix. |
VojtechBartoska commented Jan 17, 2024
@lucasssvaz & @P-R-O-C-H-Y Please help with review when this is ready. |
libraries/WiFiClientSecure/examples/WiFiClientInsecure/WiFiClientInsecure.inoShow resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
libraries/WiFiClientSecure/examples/WiFiClientTrustOnFirstUse/WiFiClientTrustOnFirstUse.ino Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
libraries/WiFiClientSecure/examples/WiFiClientTrustOnFirstUse/WiFiClientTrustOnFirstUse.ino Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
libraries/WiFiClientSecure/examples/WiFiClientTrustOnFirstUse/WiFiClientTrustOnFirstUse.ino Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
libraries/WiFiClientSecure/examples/WiFiClientTrustOnFirstUse/WiFiClientTrustOnFirstUse.inoShow resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
libraries/WiFiClientSecure/examples/WiFiClientTrustOnFirstUse/WiFiClientTrustOnFirstUse.ino Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
libraries/WiFiClientSecure/examples/WiFiClientTrustOnFirstUse/WiFiClientTrustOnFirstUse.ino Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
libraries/WiFiClientSecure/examples/WiFiClientTrustOnFirstUse/WiFiClientTrustOnFirstUse.ino Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
…entInsecure.ino typo/improvement to text Co-authored-by: Lucas Saavedra Vaz <[email protected]>
…WiFiClientTrustOnFirstUse.ino typo/improvement to text Co-authored-by: Lucas Saavedra Vaz <[email protected]>
…WiFiClientTrustOnFirstUse.ino Fix formatting Co-authored-by: Lucas Saavedra Vaz <[email protected]>
…WiFiClientTrustOnFirstUse.ino typo/improvement to text Co-authored-by: Lucas Saavedra Vaz <[email protected]>
…WiFiClientTrustOnFirstUse.ino typo/improvement to text Co-authored-by: Lucas Saavedra Vaz <[email protected]>
…WiFiClientTrustOnFirstUse.ino typo/improvement to text Co-authored-by: Lucas Saavedra Vaz <[email protected]>
…WiFiClientTrustOnFirstUse.ino typo/improvement to text Co-authored-by: Lucas Saavedra Vaz <[email protected]>
…WiFiClientTrustOnFirstUse.ino typo/improvement to text Co-authored-by: Lucas Saavedra Vaz <[email protected]>
libraries/WiFiClientSecure/examples/WiFiClientTrustOnFirstUse/WiFiClientTrustOnFirstUse.ino Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
lucasssvaz left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also causing a ton of setSocketOption(): fail on 0, errno: 9, "Bad file number" errors, could you please check it ?
[ 1227][V][esp32-hal-uart.c:396] uartBegin(): UART0 baud(115200) Mode(800001c) rxPin(3) txPin(1) [ 1236][V][esp32-hal-uart.c:482] uartBegin(): UART0 not installed. Starting installation [ 1246][V][esp32-hal-uart.c:527] uartBegin(): UART0 initialization done. [ 1362][V][esp32-hal-periman.c:229] perimanSetBusDeinit(): Deinit function for type GPIO (1) successfully set to 0x4016b9d8 [ 1374][V][esp32-hal-periman.c:154] perimanSetPinBus(): Pin 0 successfully set to type GPIO (1) with bus 0x1 TOFU pegged to fingerprint: SHA256=d8:d4:da:06:9f:44:48:53:f1:32:0c:8d:80:d0:94:9f:f6:38:f1:28:a4:63:a4:0e:df:ec:4b:3d:10:2b:9f:75 Note: You can check this fingerprint by going to the URL <https://www.howsmyssl.com> and then click on the lock icon. Attempting to connect to SSID: Vaz_2.4GHz [ 1421][D][WiFiGeneric.cpp:1055] _eventCallback(): Arduino Event: 0 - WIFI_READY [ 1492][V][WiFiGeneric.cpp:345] _arduino_event_cb(): STA Started [ 1494][V][WiFiGeneric.cpp:98] set_esp_interface_ip(): Configuring Station static IP: 0.0.0.0, MASK: 0.0.0.0, GW: 0.0.0.0 [ 1511][D][WiFiGeneric.cpp:1055] _eventCallback(): Arduino Event: 2 - STA_START .[ 1689][V][WiFiGeneric.cpp:360] _arduino_event_cb(): STA Connected: SSID: Vaz_2.4GHz, BSSID: 84:0b:bb:29:c2:20, Channel: 1, Auth: WPA2_PSK [ 1702][D][WiFiGeneric.cpp:1055] _eventCallback(): Arduino Event: 4 - STA_CONNECTED [ 1750][V][WiFiGeneric.cpp:374] _arduino_event_cb(): STA Got New IP:192.168.15.39 [ 1757][D][WiFiGeneric.cpp:1055] _eventCallback(): Arduino Event: 7 - STA_GOT_IP [ 1765][D][WiFiGeneric.cpp:1119] _eventCallback(): STA IP: 192.168.15.39, MASK: 255.255.255.0, GW: 192.168.15.1 Connected to Vaz_2.4GHz Trying to connect to a server; using TOFU details from the eeprom [ 2561][V][ssl_client.cpp:61] start_ssl_client(): Free internal heap before TLS 252864 [ 2569][V][ssl_client.cpp:67] start_ssl_client(): Starting socket [ 3925][V][ssl_client.cpp:145] start_ssl_client(): Seeding the random number generator [ 3934][V][ssl_client.cpp:154] start_ssl_client(): Setting up the SSL/TLS structure... [ 3942][D][ssl_client.cpp:175] start_ssl_client(): WARNING: Skipping SSL Verification. INSECURE! [ 3951][V][ssl_client.cpp:256] start_ssl_client(): Setting hostname for TLS session... [ 3959][V][ssl_client.cpp:271] start_ssl_client(): Performing the SSL/TLS handshake... [ 4744][V][ssl_client.cpp:292] start_ssl_client(): Verifying peer X.509 certificate... [ 4752][V][ssl_client.cpp:300] start_ssl_client(): Certificate verified. [ 4758][V][ssl_client.cpp:315] start_ssl_client(): Free internal heap after TLS 209024 All well - you are talking to the same server as when you set up TOFU. So we can now do a GET. [ 4766][V][ssl_client.cpp:371] send_ssl_data(): Writing HTTP request with 21 bytes... [ 4784][V][ssl_client.cpp:371] send_ssl_data(): Writing HTTP request with 2 bytes... [ 4792][V][ssl_client.cpp:371] send_ssl_data(): Writing HTTP request with 6 bytes... [ 4800][V][ssl_client.cpp:371] send_ssl_data(): Writing HTTP request with 17 bytes... [ 4809][V][ssl_client.cpp:371] send_ssl_data(): Writing HTTP request with 2 bytes... [ 4817][V][ssl_client.cpp:371] send_ssl_data(): Writing HTTP request with 17 bytes... [ 4826][V][ssl_client.cpp:371] send_ssl_data(): Writing HTTP request with 2 bytes... [ 4834][V][ssl_client.cpp:371] send_ssl_data(): Writing HTTP request with 2 bytes... HTTP/1.0 200 OK Access-Control-Allow-Origin: * Content-Length: 3501 Content-Type: application/json Strict-Transport-Security: max-age=631138519; includeSubdomains; preload Vary: Accept-Encoding Date: Thu, 18 Jan 2024 15:17:16 GMT -- headers received. Payload follows [ 5105][V][ssl_client.cpp:323] stop_ssl_socket(): Cleaning SSL connection. [ 5240][E][WiFiClient.cpp:329] setSocketOption(): fail on 0, errno: 9, "Bad file number" [ 5248][E][WiFiClient.cpp:329] setSocketOption(): fail on 0, errno: 9, "Bad file number" [ 5256][E][WiFiClient.cpp:329] setSocketOption(): fail on 0, errno: 9, "Bad file number" [ 5264][E][WiFiClient.cpp:329] setSocketOption(): fail on 0, errno: 9, "Bad file number" ... [ 6094][E][WiFiClient.cpp:329] setSocketOption(): fail on 0, errno: 9, "Bad file number" [ 6102][E][WiFiClient.cpp:329] setSocketOption(): fail on 0, errno: 9, "Bad file number"{"given_cipher_suites":["TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384","TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384","TLS_ECDHE_ECDSA_WITH_AES_256_CCM","TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384","TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384","TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA","TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA","TLS_ECDHE_ECDSA_WITH_AES_256_CCM_8","TLS_ECDHE_ECDSA_WITH_CAMELLIA_256_GCM_SHA384","TLS_ECDHE_RSA_WITH_CAMELLIA_256_GCM_SHA384","TLS_ECDHE_ECDSA_WITH_CAMELLIA_256_CBC_SHA384","TLS_ECDHE_RSA_WITH_CAMELLIA_256_CBC_SHA384","TLS_ECDHE_ECDSA_WITH_ARIA_256_GCM_SHA384","TLS_ECDHE_RSA_WITH_ARIA_256_GCM_SHA384","TLS_ECDHE_ECDSA_WITH_ARIA_256_CBC_SHA384","TLS_ECDHE_RSA_WITH_ARIA_256_CBC_SHA384","TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256","TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256","TLS_ECDHE_ECDSA_WITH_AES_128_CCM","TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256","TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256","TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA","TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA","TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8","TLS_ECDHE_ECDSA_WITH_CAMELLIA_128_GCM_SHA256","TLS_ECDHE_RSA_WITH_CAMELLIA_128_GCM_SHA256","TLS_ECDHE_ECDSA_WITH_CAMELLIA_128_CBC_SHA256","TLS_ECDHE_RSA_WITH_CAMELLIA_128_CBC_SHA256","TLS_ECDHE_ECDSA_WITH_ARIA_128_GCM_SHA256","TLS_ECDHE_RSA_WITH_ARIA_128_GCM_SHA256","TLS_ECDHE_ECDSA_WITH_ARIA_128_CBC_SHA256","TLS_ECDHE_RSA_WITH_ARIA_128_CBC_SHA256","TLS_RSA_WITH_AES_256_GCM_SHA384","TLS_RSA_WITH_AES_256_CCM","TLS_RSA_WITH_AES_256_CBC_SHA256","TLS_RSA_WITH_AES_256_CBC_SHA","TLS_ECDH_RSA_WITH_AES_256_GCM_SHA384","TLS_ECDH_RSA_WITH_AES_256_CBC_SHA384","TLS_ECDH_RSA_WITH_AES_256_CBC_SHA","TLS_ECDH_ECDSA_WITH_AES_256_GCM_SHA384","TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA384","TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA","TLS_RSA_WITH_AES_256_CCM_8","TLS_RSA_WITH_CAMELLIA_256_GCM_SHA384","TLS_RSA_WITH_CAMELLIA_256_CBC_SHA256","TLS_RSA_WITH_CAMELLIA_256_CBC_SHA","TLS_ECDH_RSA_WITH_CAMELLIA_256_GCM_SHA384","TLS_ECDH_RSA_WITH_CAMELLIA_256_CBC_SHA384","TLS_ECDH_ECDSA_WITH_CAMELLIA_256_GCM_SHA384","TLS_ECDH_ECDSA_WITH_CAMELLIA_256_CBC_SHA384","TLS_ECDH_ECDSA_WITH_ARIA_256_GCM_SHA384","TLS_ECDH_RSA_WITH_ARIA_256_GCM_SHA384","TLS_RSA_WITH_ARIA_256_GCM_SHA384","TLS_ECDH_ECDSA_WITH_ARIA_256_CBC_SHA384","TLS_ECDH_RSA_WITH_ARIA_256_CBC_SHA384","TLS_RSA_WITH_ARIA_256_CBC_SHA384","TLS_RSA_WITH_AES_128_GCM_SHA256","TLS_RSA_WITH_AES_128_CCM","TLS_RSA_WITH_AES_128_CBC_SHA256","TLS_RSA_WITH_AES_128_CBC_SHA","TLS_ECDH_RSA_WITH_AES_128_GCM_SHA256","TLS_ECDH_RSA_WITH_AES_128_CBC_SHA256","TLS_ECDH_RSA_WITH_AES_128_CBC_SHA","TLS_ECDH_ECDSA_WITH_AES_128_GCM_SHA256","TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA256","TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA","TLS_RSA_WITH_AES_128_CCM_8","TLS_RSA_WITH_CAMELLIA_128_GCM_SHA256","TLS_RSA_WITH_CAMELLIA_128_CBC_SHA256","TLS_RSA_WITH_CAMELLIA_128_CBC_SHA","TLS_ECDH_RSA_WITH_CAMELLIA_128_GCM_SHA256","TLS_ECDH_RSA_WITH_CAMELLIA_128_CBC_SHA256","TLS_ECDH_ECDSA_WITH_CAMELLIA_128_GCM_SHA256","TLS_ECDH_ECDSA_WITH_CAMELLIA_128_CBC_SHA256","TLS_ECDH_ECDSA_WITH_ARIA_128_GCM_SHA256","TLS_ECDH_RSA_WITH_ARIA_128_GCM_SHA256","TLS_RSA_WITH_ARIA_128_GCM_SHA256","TLS_ECDH_ECDSA_WITH_ARIA_128_CBC_SHA256","TLS_ECDH_RSA_WITH_ARIA_128_CBC_SHA256","TLS_RSA_WITH_ARIA_128_CBC_SHA256","TLS_EMPTY_RENEGOTIATION_INFO_SCSV"],"ephemeral_keys_supported":true,"session_ticket_supported":true,"tls_compression_supported":false,"unknown_cipher_suite_supported":false,"beast_vuln":false,"able_to_detect_n_minus_one_splitting":false,"insecure_cipher_suites":{},"tls_version":"TLS 1.2","rating":"Probably Okay"}[ 6407][E][WiFiClient.cpp:329] setSocketOption(): fail on 0, errno: 9, "Bad file number" -- Payload ended. [ 6422][V][ssl_client.cpp:323] stop_ssl_socket(): Cleaning SSL connection. ALL OK dirkx commented Jan 18, 2024 via email
I am not quite seeing that on an ESP32-S2 and DEBUG set to VERBOSE. Anything extra I need to set ? My first guess is that the server closes the connection: 18:08:59.617 -> [ 2947][E][WiFiClientSecure.cpp:316] available(): Closing connection on failed avail check 18:08:59.617 -> [ 2949][V][ssl_client.cpp:337] stop_ssl_socket(): Cleaning SSL connection. And that 'we' then close the connection again: 18:09:00.913 -> [ 4232][V][ssl_client.cpp:337] stop_ssl_socket(): Cleaning SSL connection. That could be solved in void WiFiClientSecure::stop(){if (sslclient->socket >= 0){close(sslclient->socket); _lastWriteTimeout = 0; ..... } stop_ssl_socket(sslclient, _CA_cert, _cert, _private_key)} by moving that stop_ssl_socket() into the IF statement. But then we're changing the API a little bit. Not sure if that is wise. Dw 8:08:58.506 -> Connected to Linate 18:08:58.506 -> Trying to connect to a server; using TOFU details from the eeprom 18:08:58.576 -> [ 1890][V][ssl_client.cpp:60] start_ssl_client(): Free internal heap before TLS 146408 18:08:58.576 -> [ 1890][V][ssl_client.cpp:66] start_ssl_client(): Starting socket 18:08:58.714 -> [ 2044][V][ssl_client.cpp:144] start_ssl_client(): Seeding the random number generator 18:08:58.714 -> [ 2046][V][ssl_client.cpp:153] start_ssl_client(): Setting up the SSL/TLS structure... 18:08:58.714 -> [ 2049][D][ssl_client.cpp:174] start_ssl_client(): WARNING: Skipping SSL Verification. INSECURE! 18:08:58.748 -> [ 2057][V][ssl_client.cpp:263] start_ssl_client(): Setting hostname for TLS session... 18:08:58.748 -> [ 2065][V][ssl_client.cpp:285] ssl_starttls_handshake(): Performing the SSL/TLS handshake... 18:08:59.380 -> [ 2713][V][ssl_client.cpp:306] ssl_starttls_handshake(): Verifying peer X.509 certificate... 18:08:59.380 -> [ 2713][V][ssl_client.cpp:314] ssl_starttls_handshake(): Certificate verified. 18:08:59.415 -> [ 2717][V][ssl_client.cpp:330] ssl_starttls_handshake(): Free internal heap after TLS 102744 18:08:59.415 -> All well - you are talking to the same server as 18:08:59.415 -> when you set up TOFU. So we can now do a GET. 18:08:59.415 -> 18:08:59.415 -> 18:08:59.583 -> HTTP/1.0 200 OK Access-Control-Allow-Origin: * Content-Length: 3299 Content-Type: application/json Strict-Transport-Security: max-age=631138519; includeSubdomains; preload Vary: Accept-Encoding Date: Thu, 18 Jan 2024 17:08:59 GMT …-- headers received. Payload follows 18:08:59.583 -> 18:08:59.583 -> 18:08:59.617 -> [ 2947][E][WiFiClientSecure.cpp:316] available(): Closing connection on failed avail check 18:08:59.617 -> [ 2949][V][ssl_client.cpp:337] stop_ssl_socket(): Cleaning SSL connection. 18:09:00.611 ->{"given_cipher_suites":["TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384","TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384","TLS_DHE_RSA_WITH_AES_256_GCM_SHA384","TLS_ECDHE_ECDSA_WITH_AES_256_CCM","TLS_DHE_RSA_WITH_AES_25....splitting":false,"insecure_cipher_suites":{},"tls_version":"TLS 1.2","rating":"Probably Okay"} 18:09:00.913 -> 18:09:00.913 -> -- Payload ended. 18:09:00.913 -> ALL OK |
lucasssvaz commented Jan 18, 2024
I tested on a ESP32 in the verbose mode. Idk if that should make a difference |
dirkx commented Jan 18, 2024 via email
Ok - will try an ESP32 rather than an ESP32-S2. Meanwhile - I did observe that, with the exception of setting the value of ssl_client->socket; al other socket work is delegated to the ssl_client.c world. So below diff does make things a bit cleaner; and at least surpresses the double closing when a socket normally closes and the user does a stop later on (again). If we assume that nothing calls stuff in ssl_client - then the API is not changing. <code> diff --git a/libraries/WiFiClientSecure/src/WiFiClientSecure.cpp b/libraries/WiFiClientSecure/src/WiFiClientSecure.cpp index ff2dedf5..0339e642 100644 --- a/libraries/WiFiClientSecure/src/WiFiClientSecure.cpp +++ b/libraries/WiFiClientSecure/src/WiFiClientSecure.cpp @@ -20,8 +20,6 @@ #include "WiFiClientSecure.h" #include "esp_crt_bundle.h" -#include <lwip/sockets.h> -#include <lwip/netdb.h> #include <errno.h> #undef connect @@ -91,15 +89,11 @@ WiFiClientSecure &WiFiClientSecure::operator=(const WiFiClientSecure &other) void WiFiClientSecure::stop(){- if (sslclient->socket >= 0){- close(sslclient->socket); - sslclient->socket = -1; - _connected = false; - _peek = -1; - _lastReadTimeout = 0; - _lastWriteTimeout = 0; - } stop_ssl_socket(sslclient, _CA_cert, _cert, _private_key); + _connected = false; + _peek = -1; + _lastReadTimeout = 0; + _lastWriteTimeout = 0} int WiFiClientSecure::connect(IPAddress ip, uint16_t port) </code> |
dirkx commented Jan 18, 2024 via email
Actually - looking at this a bit longer - I think you unearthed a much older bug due to that logic (I removed in above patch). If there is an error and ssl_client level class close the connection; they set ->socket to -1. But the WiFiSecureClient layer above it checks on _connected mostly (e.g. in read, write, etc). But _connected may not get unset - as WiFiSecureClient gate the set to false that on socket not being -1. Which can have happened in the layer below of ssl_client. So I guess my patch may actually solve that bug - unrelated to any of the TOFU / etc work. With kind regards, Dw. |
… but the WiFiClientSecure checks for _connected. So we want to make sure the latter is always set. And thus have moved the state handling around *ssl_client down into the C code; below WiFiClientSecure.
…nd print the LF/CR for the header lines.
me-no-dev commented Jan 23, 2024
@lucasssvaz PTAL |
dirkx commented Jan 23, 2024 via email
Apologies - will do - but paid customers intruding :) |
…WiFiClientTrustOnFirstUse.ino
lucasssvaz left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug found is unrelated to the example added. I'll open a issue to track it
create a Trust on First Use example to quell the increasingly ommon copy & paste of the insecure approach making it to production
This addresses #9102
Description of Change
Add an example. No impact. Put a warning in the existing NonSecure example (little impact)
Tests scenarios
ESP32
Related links
Would close#9102