httpUpdateSigned crashes on verification when the digital signature does not match #7145
Comments
|
There is also a memory leak in Arduino/cores/esp8266/Updater.cpp Line 264 in 845241b The sig variable and the buffer needs to be freed. But it doesn't fix the problem.. |
|
I made a simple sketch to verify the signature on the running sketch (no uploading from the internet etc). The code is taken from Arduino/cores/esp8266/Updater.cpp Line 233 in 845241b And it is running fine. So the problem won't be simply in the RSA signature, unfortunately. If you want to play with the code, note that Arduino IDE uploads unsigned sketch. You need to upload the signed one using python script manually. Edit: |
|
Good catch on the memory leak. Because the PC has gone bad it feels like a stack corruption issue (as-in stack overflow). We're now using the standard format signature and not just the SHA256 result, so a change of 1 byte in the signature may end up causing the packed format to go wacky. BearSSL should still not crash things, in that case, though. Can you give the exact pub/priv key and the exact offset from the end of the .bin.signed you changed, so I can rerun the same changes on my own builds to debug? |
|
The keys are in verifySig.zip in the post above. Edit: I edited the bin file so that I left the signature intact and changed another byte. Crashing as well. Edit2: You asked about my changes: I changed offset 0xf0 to value 0 or 0xff to value 0, both crashed. |
|
@earlephilhower: Doesn't it mean stack overflow? Edit: a similar behaviour I see in my application: |
|
That looks like a smoking gun right there, @JiriBilek. I may need to allocate/use the 2nd stack for the verification, or check that the Updater stack usage is minimized. |
Fixes esp8266#7145 When doing a signed update, the signature calculation can use a lot of stack, so move it silently to the BearSSL second stack. Also fix a memory leak of signature-bytes found by @JiriBilek
* Use 2nd stack for update signature verification Fixes #7145 When doing a signed update, the signature calculation can use a lot of stack, so move it silently to the BearSSL second stack. Also fix a memory leak of signature-bytes found by @JiriBilek * Reset state on any error condition in Updater::end
Basic Infos
Platform
Settings in IDE
Problem Description
The OTA update with default signature verification (MANUAL_SIGNING = 0) work fine up to the moment when I tamper the signature in the bin file.
Any byte changed in the 256 byte signature will cause wdt or various crashes. Changing the last 4 bytes (the signature length) does not crash the CPU, it correctly throws an error and resumes the sketch.
I traced the problem up to the file Updater.cpp. Changing the line
Arduino/cores/esp8266/Updater.cpp
Line 264 in 845241b
if (sig[0xf0] == 0) {will allow playing with the signature and not crashing the CPU. Of course, it removes the digital signature verification :), it is only for tracing the bug.I am asking for help as I don't understand the RSA stuff in
Arduino/libraries/ESP8266WiFi/src/BearSSLHelpers.cpp
Line 873 in 845241b
Thanks for your time.
MCVE Sketch
Standard example Arduino/libraries/ESP8266httpUpdate/examples/httpUpdateSecure/httpUpdateSecure.ino, no code changes, only RSA keys changed, generated by OpenSSL. I can upload them if it's worth doing it.
Debug Messages
wdt or Fatal exception 2(InstructionRetchErrorCause) at various addresses (epc1)
I will decode the stack and update the post shortly.
Edit: the dump
The text was updated successfully, but these errors were encountered: