Description
Basic Infos
- This issue complies with the issue POLICY doc.I have read the documentation at readthedocs and the issue is not addressed there.I have tested that the issue is present in current master branch (aka latest git).I have searched the issue tracker for a similar issue.If there is a stack dump, I have decoded it.I have filled out all fields below.
Platform
- Hardware: ESP-12
- Core Version: 2c36cfe
- Development Env: Arduino IDE
- Operating System: MacOS
Problem Description
The method probeMaxFragmentLength of BearSSL::WiFiClientSecure is not compatible with OpenSSL supporting Maximum Fragment Length Negotiation extension. I've tested on OpenSSL 1.1.1b1 (beta 1) and also the latest OpenSSL 1.1.1 build. The method probeMaxFragmentLength generates very simple ClientHello message which is rejected by the server with the following error:
Client connection from X.X.X.X failed: error:1417A0C1:SSL routines:tls_post_process_client_hello:no shared cipher.
The server drops connection immediately during processing of ClientHello message. OpenSSL 1.1.1 supports usage of Maximum Fragment Length Negotiation extension properly. If probing is not done but setBufferSizes is used before connection, server accepts extension sent in ClientHello and confirms selected max fragment length in ServerHello.
MCVE Sketch
Use example in libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino against a server backed by OpenSSL 1.1.1 (Mosquitto, Haproxy or Apache). The function fetchMaxFragmentLength in this example will always report that MFLN is not supported even it is working well. If you skip probing, setBufferSizes(512, 512) and examine ClientHello and ServerHello packets, you will find that server accepts MFLN and fragments are not bigger than 512.
Metadata
Metadata
Assignees
Type
Projects
Milestone
Relationships
Development
Participants
Activity
Jeroen88 commentedon Apr 17, 2019
I use the latest Node.js release as backend with the Standard OpenSSL and that works like a charm
earlephilhower commentedon Apr 17, 2019
@sislakd, you'll need to provide some example code. I've just tried with a locally built 1.1.1a on Ubuntu and MFLN negotiation reports success:
On the client I made setup:
And it always reports "OK" (so MLFN scanning worked).
earlephilhower commentedon Apr 17, 2019
The "user cancelled" bit of the protocol is expected, that's the way the probing works. Once it gets the header back saying the MFLN was accepted it stops the connection attempt.
sislakd commentedon Apr 18, 2019
Here is output from my test:
Does your probeMaxFragmentLength expecting usage of legacy cipher suites (not Forward Secrecy)? I'm enforcing TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 for its Forward Secrecy and good performance of CHACHA20 on ESP8266. Maybe in that case ClientHello should include some additional components which are not sent by probeMaxFragmentLength at the moment (including only TLS version, Random, SessionID, Cipher Suites, No compression and MFLN extension). The full connection made by BearSSL using this cipher suite includes beside these also Signature algorithms extension (rsa_pkcs1_sha256), Supported Groups extension (secp256r1, secp384r1, secp521r1) and EC Point formats extension (uncompressed) in Client Hello. I think that ECDHE requires these otherwise server don't know how to generate ECDH server params for Server Key Exchange response immediately following Server Hello.
earlephilhower commentedon Apr 18, 2019
Have you modifies the BearSSL /WiFiClientSecure code in any way? What you're showing the 8266 sending can't be possible otherwise.
0027,0028 == length of cipher list in bytes
In yours, it's showing "0002" meaning a single cipher. In mine, and in git head it shows "005a", meaning a bunch.
The code takes the size of a constant array and stuffs it into the packet followed by the contents of the constant array:
So there's no way for 2 to find its way into the handshake packet unless
sizeof(suites_P)!= 0x5a.sislakd commentedon Apr 18, 2019
As mentioned I'm enforcing TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 for its Forward Secrecy and good performance of CHACHA20 on ESP8266. Thus, the modification is in used default suites_P .
earlephilhower commentedon Apr 18, 2019
That's not the way to do it. Just set a custom list using the built in function
setCiphersand pass in your list before connecting. The server expects a list of possible ciphers, in order or preference by the client. Your modified the core code is sending only a single cipher that OpenSSL doesn't support and hence the issue. Probing can and should send all ciphers in the world, since it never gets that far in the handshake process to actually select or use one.Closing as not core related.
sislakd commentedon Apr 19, 2019
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 cipher is supported by OpenSSL as the connection made by BearSSL using this cipher suite runs well. The issue is the same if you sent all ciphers from probeMaxFragmentLength and target server is configured to accept only strong cipher suites on TLS1.2 (those which have been selected for TLS1.3). I've did experimental modification of probing function to include other necessary extensions and it works well now. Thus definitely, probing can be fixed.
The MFLN probing in the current form introduce security vulnerability. There is a security risk in having MFLN probing without verification of server identity. An attacker can accept usage of small fragment length being in the middle of the connection as a response to probeMaxFragmentLength Client Hello. Then an attacker will let real connection go to target server which is not supporting MFLN extension. This results into crash of client because server sends 16k fragments instead of requested small ones. The right way is to integrate MFLN verification directly into BearSSL where full server identity is verified (unless going with insecure mode). If configured receive buffer size is not accepted by the server with verified identity, connection should be terminated and calling code have to receive appropriate error code. Upon this error, client should enlarge receive buffer and try to make connection again. Optionally, the whole procedure can be done inside BearSSL transparently enlarging receive buffer size.
earlephilhower commentedon Apr 19, 2019
Interesting observation, I didn't think of that particular scenario. Thanks for describing it.
In any case, the proper thing is to have the probe use the current cipher list, whether its the default (which you should not change) or one you set via setCiphers which can contain a single entry.
What is your exact OpenSSL 1.1.1
openssl s_servercommand line, so I can give a like to like comparison?The SSL handshake protocol is rigid in the first few messages, and we only get as fas as the ClientHello/ServerHello response, so assuming the server accepts 0xcca8 as a cipher, I'm not sure where the trouble lies and need to see a live example to troubleshoot.
devyte commentedon Apr 19, 2019
Also, @sislakd said:
What exactly did you modify in the probing function?
earlephilhower commentedon Apr 19, 2019
On second thought, there's a more basic issue. If you don't go through a complete handshake and cert verification then the probing as-is is still subject to MITM attack. So there is no need to change the cipher list in the probe call, it doesn't buy you anything.
Anything less than the full, 5-second+ SSL negotiation where certs are examined seems to suffer the same issue, so it's a matter of adding a new probeDeep() to check this. Not an insignificant change and will require 1K RAM (512b each buffer) to run.
13 remaining items