probeMaxFragmentLength of BearSSL::WiFiClientSecure is not compatible with OpenSSL supporting Maximum Fragment Length Negotiation extension #5996
Comments
|
I use the latest Node.js release as backend with the Standard OpenSSL and that works like a charm |
|
@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). |
|
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. |
earlephilhower
added
the
waiting for feedback
|
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. |
|
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 |
|
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 . |
|
That's not the way to do it. Just set a custom list using the built in function Closing as not core related. |
|
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. |
|
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 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. |
|
Also, @sislakd said:
What exactly did you modify in the probing function? |
|
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. |
probeMFLN was failing on some connection attempts to servers which only supported EC based ciphers because it did not include the proper TLS handshake extensions to list what kinds of ECs it supported. Add those to the probeMFLN ClientHello message to make probes pass. Partially fixes esp8266#5996
|
Did some wireshark checking and the probe packet is good, even with the truncated |
|
One more thing. If a MITM does fake a MFLN response that the server does not really support BearSSL and the 8266 will NOT crash. What will happen is no data will be able to be received because the buffer isn't big enough and you'll basically get "0" for all |
|
It would be better to to somehow verify MFLN response from real connection (not during probing). Do you know whether BearSSL allows reading of ServerHello extensions from BearSSL::WiFiClientSecure during TLS handshake? Or we need to submit feature request to BearSSL? |
|
I've been adding things like that to the port myself. BearSSL is a 1-man project (literally, he won't accept patches from others but wants to recode them himself for security) and this 8266 port is a unique beast. I can probably hack in something to stash away what MFLN(if-any) size was negotiated and add a method you can call after-the-fact. You'd probably not want a separate "probeMLFNSecure" because the handshake and cert checking can take 5+ seconds(!!) at 160MHz on the chip and you'd effectively have to do it twice, once to probe and once to connect. |
|
Well, this has lead to an interesting discovery about BearSSL. It auto-computes the MFLN to request using min(inbuff, outbuff)...and while inbuff = 16K (i.e. compatible with everything), we make obuff 512b. So BSSL requests a 512b buffer size via MFLN negotiation on all connections. On TLS 1.2 this request is ignored, so no problem. On TLS 1.3 this will succeed, though, and you'll end up wasting 15.5KB of receive space in the default case (no setBufferSizes() call). Looks like a BSSL bug to me... |
|
Yes, I meant to do MFLN check as a part of the final connection. Thus not wasting another TCP/IP and TLS handshakes just for probing. According to your finding, it seems that it has no sense to set different send and receive buffers. They should be the same. Maybe, we can check OpenSSL implementation how it behaves when it receives MFLN extension. Maybe, it is reducing also receive buffer as well. Thus, both send and receive buffers are the same. BTW: What are specific changes of your port compared to the original BearSSL? |
|
It definitely still makes sense for different buffers since TLS1.2 doesn't support MFLN so we have to always be able to receive 16K fragments. It's a small change in the BSSL code to only look at receive size as long as the xmit buffer is smaller than recv. I've pinged the author, we'll see what his thoughts are. The RFCs are pretty clear. If a MFLN is accepted then both sides may only send up to the negotiated size or smaller fragments. However it is always legal to send smaller fragments which is how we can use a 512b xmit buffer and a 16k recv buffer in normal operation (and which actually works that way for most servers since OpenSSL1.1.1 is only now percolating to distros release channels). Check the BSSL submodule. I have the whole repo online and you can diff vs. the upstream version. Lots of memory optimizations and some add'l changes and functions needed for the Ardunio object. |
|
I'm interested in the differences because I expect to have TLS1.3 support in BearSSL soon. This brings additional security improvements into TLS connections. For my purpose, I've done some additional changes in BearSSL::WiFiClientSecure leading into large decrease of program code size where BearSSL is used. The original version supports all various authentication, encryption, and message authentication code algorithms. The resulting code is much bigger than with legacy AxTLS. This brings issues with OTA updates if you have just 1 MB flash and you would like to do OTA over TLS as well (thus first upgrading to some minimal code version and then upgrade to final version doesn't help). It is good to have such large interoperability supporting all cipher suites accross TLS1.0 - TLS1.2 for applications where code based on Arduino connects to unknown servers (or it acts as a server for old browsers not supporting latest cipher suites). But if an application is connecting only to known servers which are regularly updated, it is enough to support only TLS1.2 and selected algorithms (e.g. TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 having good performance on ESP8266). This results into code smaller than legacy AxTLS. Thus, there is still sufficient space for application code build on top of Arduino and having OTA upgrade working perfectly. BTW: MFLN extension has been added to TLS1.2 within RFC6066. It was not mandatory. Thus, OpenSSL was not implementing it at all on the server side. But if MFLN is accepted, there is specified that both sides must fragment messages at max to the negotiated length. You can send always fragments which are smaller than negotiated length but then you are not getting maximum throughput and optionally waste RAM at one end the connection if there is some preallocated buffer. Thus, there are two cases: For TLS1.3, there is the new Record Size Limit extension (RFC8449) replacing MFLN from TLS1.2. |
|
Given how long MFLN took to get out, I'd not hold my breath on RFC8449. I've just added a push to the above mentioned PR which allows you to check the MFLN negotiation status after connection. It should give you what you need. Also, heard back from Tomas and he needs to think about some corner cases before changing the MFLN size requested, but at first glance it looks reasonable. |
…r a connection. (#6000) Fixes #5996 * Add extensions to probe message for EC, others probeMFLN was failing on some connection attempts to servers which only supported EC based ciphers because it did not include the proper TLS handshake extensions to list what kinds of ECs it supported. Add those to the probeMFLN ClientHello message to make probes pass. * Add client.getMFLNStatus method, returns MFLN state After a connection it is useful to check whether MFLN negotiation succeeded. getMFLNStatus returns a bool (valid only after client.connect() succeeds, of course) indicating whether the requested buffer sizes were negotiated successfully.
Basic Infos
Platform
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.
The text was updated successfully, but these errors were encountered: