Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/http update wi fi client parameter #4980

Merged
merged 49 commits into from Oct 6, 2018

Conversation

@Jeroen88
Copy link
Contributor

commented Jul 29, 2018

Pass Client parameter to httpUpdate to use any BearSSL security method for a firmware update (with example). This PR replaces #4832.

This is a combination with PR #4979, because The ESP8266HTTPUpdate class uses the ESP8266HTTPClient class from this PR to pass the BearSLL WiFiClient.

Added two new update() and one updateSpiffs() function that uses the ESP8266HTTPClient adapted by me in PR #4825. With these adaptations, it is possible to do an encrypted and authenticated server firmware or SPIFFS update, using any of the supported security options of the WiFiClientSecureBearSSL library. httpUpdateSecure.ino is a complete example.

To try this, you need either a sketch with enough free memory to allocate the full TLS/SSL-buffer (16k), or a, currently not very widely supported Maximum Fragment Length Negotiation enabled, server (or do the same trick as I do, using a nodeJS server and set maxSendFragmentLength) that sends the correct response headers and serves the binary. You need a certificate store in SPIFFs that includes your root CA, or a self signed certificate, or a certificate fingerprint. In the latter two cases you need to edit the example to use allowSelfSignedCerts() or setFingerprint(). It is also possible to use encryption without server authentication. In this case use setInsecure(). Of course any of the other supported WiFiClientSecureBearSSL functions may be used as well.

Jeroen88 added 13 commits Aug 3, 2018
add basicHttpsClient example, ensured in the backward compatible func…
…tion that only one connection is made, updated version to 1.2 deprecating present begin() functions, several minor updates
Adding #pragma's to ESP8266httpUpdate.cpp to ignore use of deprecated…
… functions. Adaptations to work with new ESP8266hhtpClient are made in PR #4980
@Jeroen88

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2018

@d-a-v

Could you by any chance try to reproduce that bug using lwip2 in #5126 ?

I would like to try it, but I don't know how. The clue 'git fetch origin pull/5126/head:lwip210' in the PR gives an error: 'fatal: Couldn't find remote ref pull/5126/head'

@d-a-v

This comment has been minimized.

Copy link
Collaborator

commented Oct 5, 2018

I would like to try it, but I don't know how. The clue 'git fetch origin pull/5126/head:lwip210' in the PR gives an error: 'fatal: Couldn't find remote ref pull/5126/head'

Can you try with this ?

git checkout master
git pull
git fetch origin pull/5126/head:lwip210
git checkout -b testonlybranch
git merge feature/HTTPUpdate_WiFiClient_parameter
git merge feature/Updater/flashLED
git merge lwip210
@Jeroen88

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2018

@d-a-v I had to use git fetch upstream pull/5126/head:lwip210 to find the PR. After restarting the Arduino IDE I have 6 lwIP options now: lwIP v2 Lower Memory, v2 Higher Bandwidth, v2 Lower Memory (no features), v2 higher Bandwidth (no features), v1.4 Higher Bandwidth, v1.4 Compile from source. If I use v2 High Bandwidth, so without (no features), it seams "impossible" to fully disturb the signal. I am able to slow it down, but it finishes and reboots with the new firmware.
As you already guessed, I am using the LED blink feature of the Updater to assess the speed of the download. Without disturbance it blinks slightly longer on than off, with disturbance clearly longer on than off

@d-a-v

This comment has been minimized.

Copy link
Collaborator

commented Oct 5, 2018

Thanks very much for your time with this confirmation !
We can thank lwIP devs for SACK :)
I indeed should have told you to restart the IDE.
(about upstream remote name, I assumed that origin was the default but I may be wrong?)

@Jeroen88

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2018

@earlephilhower

LGTM, but maybe you could update the example code to use this new feature? W/o an example, it's not exceptionally clear from the header what's going on.

Sure! All 3 examples updated in this PR with the usage of setLedPin() and a comment on how to use it

@Jeroen88

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2018

@d-a-v You're welcome, happy to help! I am glad that the lwIP devs SACKed us ;)
Maybe origin is the default, but I am a rather newbie with github and I do not comprehend it fully yet. My 'own' fork is origin and upstream is the original repo I cloned from. Before doing any changes I pull from upstream and push to origin to sync them

@earlephilhower

This comment has been minimized.

Copy link
Collaborator

commented Oct 5, 2018

You had a trailing space in a comment, causing the CI style_check to fail. Fixed it for you.

@CWempe CWempe referenced this pull request Oct 5, 2018
@earlephilhower

This comment has been minimized.

Copy link
Collaborator

commented Oct 5, 2018

I just tried this code with the original example code (simulating someone running their existing app) and it looks like the _tcpDeprecated is not quite there yet:

[HTTP] begin...
[HTTP-Client][end] tcp is closed
[HTTP-Client][begin] url: http://192.168.1.12/test.html
[HTTP-Client][begin] host: 192.168.1.12 port: 80 url: /test.html
[HTTP] GET...
[HTTP-Client] connect: HTTPClient::begin was not called or returned error
[HTTP-Client][returnError] error(-1): connection refused
[HTTP] GET... failed, error: connection refused
[HTTP-Client][end] tcp is closed
@earlephilhower

This comment has been minimized.

Copy link
Collaborator

commented Oct 6, 2018

Actually, I see the old sample code was silly and required a local 192.168.x.x server.

Changing the server to one that exists doesn't change the results, unfortunately:

[HTTP] begin...
[HTTP-Client][end] tcp is closed
[HTTP-Client][begin] url: http://jigsaw.w3.org/HTTP/connection.html
[HTTP-Client][begin] host: jigsaw.w3.org port: 80 url: /HTTP/connection.html
[HTTP] GET...
[HTTP-Client] connect: HTTPClient::begin was not called or returned error
[HTTP-Client][returnError] error(-1): connection refused
[HTTP] GET... failed, error: connection refused
[HTTP-Client][end] tcp is closed
@earlephilhower

This comment has been minimized.

Copy link
Collaborator

commented Oct 6, 2018

Original code output (StreamHTTPClient with the URL moved to the jigsaw.w3.org server)


SDK:2.2.1(cfd48f3)/Core:2.4.2-70-g9bc8ea1/lwIP:2.0.3(STABLE-2_0_3_RELEASE/glue:arduino-2.4.1-13-g163bb82)/BearSSL:f55a6ad



[SETUP] WAIT 4...
wifi evt: 2
scandone
state: 0 -> 2 (b0)
state: 2 -> 3 (0)
state: 3 -> 5 (10)
add 0
aid 3
cnt 

connected with NOBABIES, channel 10
dhcp client start...
wifi evt: 0
[SETUP] WAIT 3...
ip:192.168.1.138,mask:255.255.255.0,gw:192.168.1.1
wifi evt: 3
[SETUP] WAIT 2...
[SETUP] WAIT 1...
[WIFI][APlistAdd] add SSID: NOBABIES
[HTTP] begin...
[HTTP-Client][begin] url: http://jigsaw.w3.org/HTTP/connection.html
[HTTP-Client][begin] host: jigsaw.w3.org port: 80 url: /HTTP/connection.html
[HTTP] GET...
[hostByName] request IP for: jigsaw.w3.org
[hostByName] Host: jigsaw.w3.org IP: 128.30.52.21
:ref 1
[HTTP-Client] connected to jigsaw.w3.org:80
[HTTP-Client] sending request header
-----
GET /HTTP/connection.html HTTP/1.1
Host: jigsaw.w3.org
User-Agent: ESP8266HTTPClient
Connection: close
Accept-Encoding: identity;q=1,chunked;q=0.1,*;q=0

-----
:wr 160 0
:wrc 160 160 0
:ack 160
:rn 536
:rch 536, 284
:rcl
:abort
[HTTP-Client][handleHeaderResponse] RX: 'HTTP/1.1 200 OK'
[HTTP-Client][handleHeaderResponse] RX: 'Connection: extensionheader,close'
[HTTP-Client][handleHeaderResponse] RX: 'Date: Sat, 06 Oct 2018 00:06:57 GMT'
[HTTP-Client][handleHeaderResponse] RX: 'Content-Length: 550'
[HTTP-Client][handleHeaderResponse] RX: 'Content-Type: text/html'
[HTTP-Client][handleHeaderResponse] RX: 'Etag: "1giilod:q0efehi8"'
[HTTP-Client][handleHeaderResponse] RX: 'Last-Modified: Tue, 20 Jun 2000 13:33:22 GMT'
[HTTP-Client][handleHeaderResponse] RX: 'Server: Jigsaw/2.3.0-beta3'
[HTTP-Client][handleHeaderResponse] RX: 'ExtensionHeader: ExtensionValue'
[HTTP-Client][handleHeaderResponse] RX: ''
[HTTP-Client][handleHeaderResponse] code: 200
[HTTP-Client][handleHeaderResponse] size: 550
[HTTP] GET... code: 200
<HTML>
<HEAD>
  <!-- Created with AOLpress/2.0 -->
  <TITLE>Connection Header</TITLE>
</HEAD>
<BODY>
<P>
<IMG ALT="Jigsaw" BORDER="0" WIDTH="212" HEIGHT="49" SRC="/icons/jigsaw">
<H1>
  The <I>Connection</I> header
</H1>
<P>
This page will be served to you:c 1, 536, 820
 with the following headers:
<P>
<CODE>ExtensionHeader: ExtensionValue<BR>
Connection: ExtensionHeader</CODE>
<P>
If you're getting this page through a proxy, you should <I>not</I> see the
<I>ExtensionHeader</I> !
<P>
  <HR>
<BR>
<A HREF="mailto:jigsaw@w3.:c0 1, 284
org">jigsaw@w3.org</A>
</BODY></HTML>

[HTTP] connection closed or file end.
[HTTP-Client][end] tcp is closed
:ur 1
:del
pm open,type:2 0
Fix ::connect() to work for legacy API
The `_client==NULL` check was performed before any `_client` could
be set by the shim code.  Move the check past where the shim code
actually sets the variable.   Fixes any unmodified legacy code and the
old samples.
@earlephilhower

This comment has been minimized.

Copy link
Collaborator

commented Oct 6, 2018

@Jeroen88, hope you don't mind but the fix was simple so I just moved the problem NULL check to after the _client is actually set and committed. Now old samples seem to work fine with the new class.

@Jeroen88

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2018

@earlephilhower the check for _client should indeed be after the assignment, thnx for the fix!

@Jeroen88

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2018

@earlephilhower did you see my comment:

I still have a crash running the example streamHttpsClient.ino of this PR. I did not apply GIT head because I do not know how to do that (I am still a git newbie).
You are right that the problem is not the delete's. I am still not sure of the root cause, but: when the streamHttpsClient.ino-sketch calls delete client at the end of the loop ~WiFiClientSecure() is called before ~HTTPClient(). The ~HTTPClient() destructor calls _client->stop() with _client being a WiFiClientSecure and stop() is defined in WiFiClient.cpp and stop() calls flush() and _client->close(). However: the client context is already gone, because the ~WiFiClientSecure() destructor was already called. I think this is the problem. I do not know how it should be solved, in WiFiClientSecureBearSSL of in ESP8266HTTPClient. In case of the latter I also do not know how...

This is with streamHttpsClient.ino. Any suggestions?

@earlephilhower

This comment has been minimized.

Copy link
Collaborator

commented Oct 6, 2018

@Jeroen88, I'm going to commit this now, but if after this is committed you pull git head and still see the crash open an issue and we can look at it.

I'm running the StreamHTTPSClient (i.e. BSSL) example now and it's running just fine, so I have no idea what you're seeing, unfortunately.

@earlephilhower earlephilhower merged commit 13f3746 into esp8266:master Oct 6, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.