search

esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
16.03k stars 13.34k forks source link

OTA Unsigned binary after first one is signed, espota.py reports [INFO]: Result: OK but update fails on esp8266 #7162

Closed maribeiro closed 4 years ago

maribeiro commented 4 years ago

I have signed a binary and uploaded via OTA - so next OTA updates should be signed. Then tried to see if the esp8266 would accept an unsigned binary - it's expected that it should not accept. So I uploaded via OTA an unsigned binary, and espota.py reported [INFO]: Result: OK But it actually failed on the esp8266 side with Error[4]: End Failed First: why is espota reporting OK if it failed? And second, shouldn't the error on the esp side be UPDATE_ERROR_SIGN ?

I have used ArduinoOTA sketch for this tests.

devyte commented 4 years ago

@earlephilhower I seem to remember this was a choice to allow rollback. Is that correct?

earlephilhower commented 4 years ago

After digging through stuff, this is all in the ArduinoOTA, the BasicOTA sketch, and espota.py from what I can see and not a security risk as the Updater.cpp class is always rejecting invalid/unsigned bins.

In reverse order:

And second, shouldn't the error on the esp side be UPDATE_ERROR_SIGN ?

Updater (the guy who actually does the work) is returning UPDATE_ERROR_SIGN just fine. However, the ArduinoOTA class (wrapper which feeds Updater) doesn't define anything like SIGN_ERR and just returns ERROR_END when there's an error when calling Updater::end.

If we add a new error code from ArduinoOTA then existing apps may break/output incorrect error messages when a signing fails. Leaving that as-is, you get "Error End" which is correct, if not specific. @devyte , @d-a-v , thoughts?

First: why is espota reporting OK if it failed?

https://github.com/esp8266/Arduino/blob/41d271d9721f93ab8495b164188b556652d072d9/tools/espota.py#L190-L195

Guess what happens when an OTA error happens...it sends "ERROR xxxx". That's got an O, so as far as espota cares it's "OK". D'oh! edit: It's actually better than that. The received_ok var is set to True on every successful 1.4K chunk written to the ESP8266, but it is never reset to False. So the entire while loop is completely skipped and it jumps straight to the logging ok output!

I'll throw together a PR for the espota.py thing. The other bit, I think should leave as-is.

devyte commented 4 years ago

Agree with @earlephilhower. The behavior could be improved for the specific sign error, but if done it would have to be for next major.

maribeiro commented 4 years ago

Thank you very much for your time. It's not an urgent thing to do , so take your time :)