nodemcu-firmware: HTTP module and SSL don't play nice on dev

I will put up a $500 bounty to get this fixed. This is really important to us. I have to believe that it is a serious issue for anyone who needs to send secure data.

I would contribute $$ to a bounty to get this fixed. I’m also stuck on the 1.5.4.1 branch for my project due to this issue.

Alright, I finally found some time to throw at this. Using @djphoenix’s experimental tlswrap branch and a few tweaks on top (see https://github.com/djphoenix/nodemcu-firmware/tree/tlswrap and https://github.com/nwf/nodemcu-firmware/tree/dev-ssl), I was able to make progress!

I was testing HTTP using function hg(url) http.get(url, nil, function(...) print(...) end) end and TLS with

function sg(...)
  srv = tls.createConnection()
  srv:on("receive", function(sck, c) print(c) end)
  srv:on("disconnection", function() print("DISCON") end)
  srv:on("connection", function(sck, c) print("CONN") sck:send("GET / HTTP/1.1\r\nConnection: keep-alive\r\nAccept: */*\r\n\r\n") end)
  srv:connect(...)
  return srv
end

I observed the following results:

Getting “https://raw.githubusercontent.com/espressif/esptool/master/MANIFEST.in” also works fine.

The error on hangup for google.com is really quite distressing:

> srv:close()
> HANDSHAKE ERROR: 7100
3fff8618 already freed
3fff2b48 already freed

It looks like TLS is behaving better, but not perfectly so. Despite my efforts to limit the maximum SSL segment size, it looks like that is not being respected by the HTTP module (via espconn’s wrapper of mbedtls). There are obviously some other problems lingering, too; it looks like tlswrap’s memory management is a little, uh, iffy.

@dtran123 is https working in 1.5.4.1?

//offtopic Would be ok if we create a Telegram group to share experiences with nodemcu, stay tuned with the last news, solve some problems, get interesting info about this project… Is anyone interested in? Just like this post and I will edit with some link.

Telegram NodeMcu Group: @NodeMcu

Just to let everyone know that unfortunately the upgrade to SDK 2.1.0 did not fix this secure tcp socket issue. So I am still stuck with 1.5.4.1 branch till this issue is resolved as secure tcp is important for me.

Hey guys, I will also contribute to the bounty with a 50$ USD for anyone who can solve this issue…though I am hoping that maybe the next SDK will address it. Refer to issue #1810.

@kirkryan If you need sunrise/sunset times why don’t you let the ESP calculate it itself? I have a module for this https://github.com/vsky279/nodemcu-firmware/blob/sun/app/modules/sun.c I did not try to pull it into dev but if there was interest I could prepare a PR.

I have encountered the same problem - http requests go through just fine, but https do not. Sorry it’s not helpful, I just want to bump this thread and to let everyone know it’s still a problem. I use the 2.1.0 build.

Wow @Jonathan411 that’s quite a generous bounty! I can’t afford that much right now, but I’d be willing to chip in 0.035 BTC (~$40 USD) to the developer that gets a PR merged that fixes this issue.

@djphoenix Have you found something interesting?

• NodeMCU 2.1.0 built with Docker provided by frightanic.com
• branch: dev
• SSL: true

If the browser accesses

https://api.telegram.org/bot446398269:AAF7jqBWZhj9oJlWT2f8mCS8_X5_LxJGLRA/sendMessage?chat_id=211698604&text=Hello!

then the response comes quickly.

If the request from NodeMCU

http.get(text_req, nil, function(code, data)
                if (code < 0) then 
                    print("telegram bot failed")
                    print(code, data) 
                else 
                    print(code, data) 
                end
            end) 

then

HTTP client: hostname=api.telegram.org HTTP client: port=443 HTTP client: method=GET HTTP client: path=/bot446398269:AAF7jqBWZhj9oJlWT2f8mCS8_X5_LxJGLRA/sendMessage?chat_id=211698604&text=Hello! HTTP client: DNS request HTTP client: DNS pending HTTP client: DNS found api.telegram.org 149.154.167.220 HTTP client: Connection timeout HTTP client: Calling disconnect HTTP client: manually Calling disconnect callback due to error -12 HTTP client: Disconnected

How to search for this error?

Hello everyone. Thanks for the tests of my dev-ssl branch; I’m glad to see things are better.

I’m currently away from my ESP8266s, but real soon now I’m going to push another version that bumps to mbedtls 2.6.1 and so should fix some CVEs in our TLS layer. I’ll carve out PRs for

• the mbedtls updates and some improvements to mbedtls debugging
• the bump of MBEDTLS_SSL_MAX_CONTENT_LEN to 5120
• general tidying around the tree

Those should be non-controversial and, as @dtran123 indicates, are likely to improve matters sufficiently for most people’s use cases. That leaves open

• the possibility of dynamically setting MBEDTLS_SSL_MAX_CONTENT_LEN
• the requisite work of integrating rtctime/sntp support into mbedtls to better support certificate verification.

and, of course, a set of scripts people can run to test TLS functionality in isolation. It’d be nice to have a list of services people are likely to care about (several of which are mentioned in this issue) and tests for each in various configurations (esp. varying values of MBEDTLS_SSL_MAX_CONTENT_LEN).

These last three are probably non-blocking.

• People can do their own testing as it stands.
• It should be OK to disable timestamp verification, as most people probably know what certs, or at least, root certs, they are expecting to see, if they turn on verification. (We should probably scream if they leave it off, tho’, as there’s minimal utility in such an unverified TLS connection.)
• 5120 is probably a more generally tolerable default than we have now and I don’t think I’d want it to be much bigger anyway.

Happy to report that setting MBEDTLS_SSL_MAX_CONTENT_LEN = 5120 did the trick !!! I now can connect to IFTTT as well as mnubo.com API. Awesome !

I feel like I will have more fun now with the ESP8266. I will definitely keep an eye on this type of error when secured connection fail. I agree with @nwf that there should be a log when we have busted the limit.

In my opinion, if there was a way to change this constant on the fly (or taking effect after a reboot), it will allow people to allocate enough buffer size as per their specific needs. A note in the documentation would indicate the inherent tradeoff by doing that with reduced memory for the lua app.

@kirkryan If you are equipped to build your own firmware images, you can try my experimental branch as mentioned in #2049 and peer into the TLS state machine. It is possible that the bumped version of mbedtls will solve the problem for you, but do note that since @djphoenix did his import of the later version, CVEs have been found in mbedtls which, I think, impact nodemcu’s usage of it. Nobody, so far as I know, has attempted further updates.

I have not had much time, but I spent a(n) (un)pleasant evening digging through debug messages and TLS packets. If anyone wants to follow along, my tree is up as commit nwf/nodemcu-firmware@d7f83d3305225675c5543c9a4055b165362b61d6 and the several proceeding. Some of those may be worth carving out as separate PRs, especially the “-Wall reduction” one. (It is merely a reduction; the tree is not -Wall clean afterwards.) Please expect this tree to be ephemeral and rewritten as I find time.

ETA: The reason for all the munging about with clocks is that I am now able to verify the timestamp on my test ECDSA self-signed cert. That’s a kind of progress, I guess. It’s still fragile, tho’.

@nwf Were you able to make progress this past week ? Also, would it be possible to send us a link to the latest custom build so we can try it out. thx.

@djphoenix Hi! Yeah, I see that. I think there is utility in keeping TLS’s utilization as small as possible, but perhaps the docs should note that the implementation in nodemcu is not fully spec conforming (by default, unless one raises the MBEDTLS_SSL_MAX_CONTENT_LEN) and should, therefore, not be assumed to be able to communicate with arbitrary servers. Fortunately, it doesn’t seem like nodemcu is often asked to fetch from servers unknown to the author, so perhaps this is an acceptable compromise. Perhaps a maximum fragment length overrun is also cause to scream a little louder on the console?

In any case, I will throw some more time at this next week; ideally testing will become automated and will grow to include MQTT (at least, against my own broker) and cert verification. 😃

Given the memory handling errors in tlswrap, I do not think it should be merged as is. I will attempt to understand and fix it, unless @djphoenix beats me to it. 😃

ETA: That said, the motion to the newer mbedtls might be sufficiently orthogonal that it could be merged independently. When I get a minute I’ll test with that configuration too.

I have had a reply from Espressif - the bug I hit is known, and they recommend using the mbedTLS library instead of SSL. Apparently it’s a drop in replacement for the ESP functions - remove libssl from your list and add mbedtls

http://www.espressif.com/en/support/download/sdks-demos

In all of that let’s not forget that actually verifying the server certificate is optional in establishing a connection to the server. It’s an important security aspect but optional. So, I suggest we stick with the old “make it run, then make it secure, make it fast, and make it pretty” mantra 😉 As for SNI, yes axTLS doesn’t support SNI but since Espressif switched to mbed TLS one should expect this to work fine.

So I have something working now (for tls module only). And I finally resolved why httpbin.org cannot be loaded - it doesn’t work at all without SNI. A new module have fix for this.

For early pre-alpha source see my tlswrap branch

@jmattsson there are debugging flags in user_mbedtls.h so you can enable each log level that you want. Can’t explain it carefully right now. So I suspect there are fatal architecture issues in http module because it’s “don’t play nice” for plain requests too. I can debug it carefully some later (maybe in this weekend).