AsyncTCP
AsyncTCP copied to clipboard
me-no-dev
mbed TLS client support, try 2
This is a tentative PR to replace #43 to account for the significant restructuring that was made on master since that PR. This still only has client-side TLS support. More testing is required before this can be considered for merging, I wanted to get this queued-up in case someone else is also working on this to avoid duplication of effort.
@fremouw left a few comments. Also! Very important. You will notice that all calls to LwIP's tcp_* api are done through callbacks executed on LwIP's thread. In the ssl C code I notice that calls to LwIP are being made directly. This will cause a slew of issues and can not be merged as is.
I dealt with the threading issue and added a pile of debug printfs in the process (they're all compiled out by default). The TLS connections now work, but I'd like to test it some more before you consider merging. However, I would appreciate a review first.
The main change that is not purely in #if ASYNC_TCP_SSL_ENABLED sections is the while loop you flagged earlier: https://github.com/me-no-dev/AsyncTCP/pull/48/files#diff-5c977e380a3eb2cac7b8881fd6fd9dcbR1012-R1054
I moved the _tcp_recvd call from inside tcp_mbedtls.c here and refactored a little the pb->next stuff. Should not change the semantics, but I did move a little code around.
FYI, this has been working well for me. I see some crashes when closing/disconnecting but I believe this has to do with #59.
@tve please rebase and also add an example or two so that code get's tested :)
@tve please rebase and also add an example or two so that code get's tested :)
I did the rebase. Where do I find an example I can modify to use TLS?
I fixed the Codacy issues, but now travis is throwing a fit and doesn't download some libraries....
@tve I had a quick look at your PR. My suggestion is that you change the file extension of your C file from c to cpp. That way it will become a c++ file and you will be able to call the functions without having to add an extra proxy function.
C++ is a super set of C so the code should just compile with few changes.
Checking in, how are things going? @tve did the merger of #59 help? I'm looking to use tls for an async client connection.
@robert-alfaro my fork is working great for me. WRT getting it merged, I'm stuck on:
@tve please rebase and also add an example or two so that code get's tested :)
I did the rebase. Where do I find an example I can modify to use TLS?
OK, now there are conflicts, but I don't want to spend the time rebasing and fixing again unless there's a path for getting it merged...
I think there no doubt we want this merged...@me-no-dev is busy, but I think can agree. If there is anything I can help with, please let me know.
I'm all for examples, but there are none in this repo... Why the requirement before merging?
Yup, that's where I stopped... Also, the codacity issues are b.s. They complain about any use of strlen and offer zero alternatives. It doesn't make sense to convert all the code to use String...
Have you tried strnlen()? I think it's considered safe because you pass in the size (compared to strlen running until it finds a \0)
Hey, great stuff @tve . I used this branch to test out TLS on https://github.com/marvinroger/async-mqtt-client and works great. I am also here to offer any help needed to merge this. I have one question to ask. I am getting constantly the following message when using this PR :
[E][AsyncTCP.cpp:1130] _poll(): 0x3ffcc15c != 0x3ffd1ccc
Besides the E, the stack behaves fine, so I am not sure why I get that.
Tried to look at the code but I am not understanding what's up. https://github.com/tve/AsyncTCP/blob/mbed-tls-try2/src/AsyncTCP.cpp#L1130
Any ideas ? thanks.
I've been busy elsewhere, need to get back to this... If you create a PR against my repo I'd be happy to pull it in. Just create a PR in github with tve/AsyncTCP : mbed-tls-try2 as base.
I was trying to push straight to this PR branch..avoiding forking your forked repo.
You should not need to create a new fork, you simply select my fork & branch in the github dialog to create a PR.
Is this assuming I've forked the upstream repo? What I've done thus far is:
git clone [email protected]:me-no-dev/AsyncTCP.git
git fetch origin pull/48/head:pr/48
git checkout pr/48
<work>
<commit>
git push [email protected]:tve/AsyncTCP pr/48:mbed-tls-try2
and obviously the push will fail because permissions:
ERROR: Permission to tve/AsyncTCP.git denied to robert-alfaro.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
fwiw, changes so far are just change strlen to sizeof for pers[]. Then I could do the rebase/merge. The strlen for psk_ vars is the way it's done everywhere.
I conflated this project with another..I do have a fork under my work email on same github account. I'll have something for you.
What's the best way for a github noob to get this into my download of ESPAsyncWebServer and make it work in arduino code? I'm currently using a combination of AsyncWebServer and AsyncWiFiManager, is it going to be a direct file overwrite? Is there a readme on how to implement? Cheers! @tve @me-no-dev
What's the best way for a github noob to get this into my download of ESPAsyncWebServer and make it work in arduino code? I'm currently using a combination of AsyncWebServer and AsyncWiFiManager, is it going to be a direct file overwrite? Is there a readme on how to implement? Cheers! @tve @me-no-dev
Adding this into platformio.ini worked for me
lib_deps = AsyncTCP=https://github.com/tve/AsyncTCP/archive/mbed-tls-try2.zip async-mqtt-client=https://github.com/tve/async-mqtt-client/archive/master.zip
What is there actually missing to merge these changes to add TLS client support?
if we wait on this, it will never go through. It has been years now. There are not enough users on those platforms with the same interests!
