-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Secured-Two-Way: Client certificate is not sent if used along with setTrustAnchors #84
Comments
Later testing: I've tested the I attach the code I used for making the test.
|
And I found the bug and the necessary fix. In websockets_client.cpp, between line However, the When the method is called in WiFiClientSecureBearSSL.h file of the ESP8266's SDK, it executes Fortunately, all we have to do is modify the websockets_client.cpp file and swap the last 2 "if" statements of the mentioned code block, as follows:
This way the client certificate (set using Looking forward for the fix to be included and merged! All the best, |
… not sent if used along with setTrustAnchors]
Awesome documentation and work @adelin-mcbsoft , as always! Hopefully the PR will be merged soon. Thanks. |
Solved by you :p |
Hi @gilmaimon ,
Long time no speak.
I came across a bug of the library, I'm laying out details below.
Hope you can have an eye on it,
Thanks,
Regards,
Adelin
Platform used to test:
ESP8266, SDK 2.7.2 on Arduino IDE
Description:
Consider your SecuredTwoWay-Esp8266-Client example .
Before moving on to the main issue, just want to point that the code misses a time-synchronization code block.
ESP's time has to be updated as to a NTP servers time, otherwise you will get this SSL Error:
since the library does not know what time currently is, to check if the certificate date is valid.
I'm using this setup (the entire code is below):
When trying to connect to a WSS server using
setTrustedAnchors
to verify the remote server (as it is in your example), the library apparently does not send the client certificate (which is set usingsetClientRSACert
).nginx in particular reports this: "client sent no required SSL certificate while reading client request headers".
On the other hand, in its logs I can see that it does make the server SSL handshake successfully and the library does send its headers, but nginx replies with HTTP 400 Bad Request, since it receives no client certificate.
The WebSocket error (generated by the library) is 1002 (an endpoint is terminating the connection due to a protocol error).
Using the client certificates together with
setFingerprint
instead ofsetTrustAnchors
works without any issue, howeversetFingerprint
has the downside of not being valid after the remote server certificate is renewed. (consider Let's Encrypt for example, which renews it every 3 months).Using
setTrustedAnchors
alone (by disabling client certificate requirement both on server and client side) also works.Since on ESP8266 the extended SSL class is BearSSL, I implemented locally the setKnownKey method, which allows to specify the public key of a certificate (which does not change at each renewal), however, it fails in the same way as above (but works when used alone), which makes me think there is a problem during the handshake computing.
It could also be a bug in
WiFiClientSecure
as well, however I found no issue with it so far, which makes me think the problem still lies with the WebSockets library.The fact that it works separately it proves that the certificates are okay and there's no issue with them.
They are RSA 1024bit and the common and subject name are both set on my local testing IP (192.168.0.123).
AFAIK, BearSSL only fails when using certificates lower (or equal to) 512bit.
To Reproduce
Implement a SSL server that asks for client certificates (I used nginx), take the SecuredTwoWay-Esp8266-Client example, add the time-sync code block along with your own certificates, enable debug and check the behavior I described above.
Expected behavior
To have it connected and working while using both client certificate (
setClientRSACert
) and trusted anchors (setTrustAnchors
), in the same fashion it works using it with thesetFingerprint
method.Code
Additional context
After this issue is resolved, would be nice to have:
The text was updated successfully, but these errors were encountered: