Signed OTA updates not working at all

[maxgerhardt]

While integrating OTA updates (works) and signed OTA updates (doesn't work) into PlatformIO, I stumbled upon these problems:

First: Even though the platform.txt generates the Updater_Signing.h by using the command

arduino-pico/platform.txt

Line 107 in a2465f5

recipe.hooks.sketch.prebuild.pattern="{runtime.tools.pqt-python3.path}/python3" -I "{runtime.platform.path}/tools/signing.py" --mode header --publickey "{build.source.path}/public.key" --out "{build.path}/core/Updater_Signing.h"

in the path

{build.path}/core/Updater_Signing.h

This folder is not in the include path when the WiFi and Update etc. libraries are built. This is evident by looking at the verbose build command

"C:\Users\Max\AppData\Local\Arduino15\packages\rp2040\tools\pqt-gcc\1.4.0-c-0196c06/bin/arm-none-eabi-g++" -c -Werror=return-type -DCFG_TUSB_MCU=OPT_MCU_RP2040 -DUSB_VID=0x2e8a -DUSB_PID=0xf00a "-DUSB_MANUFACTURER="Raspberry Pi"" "-DUSB_PRODUCT="Pico W"" -DPICO_CYW43_ARCH_THREADSAFE_BACKGROUND=1 -DCYW43_LWIP=0 -DLWIP_IPV6=0 -DLWIP_IPV4=1 -DLWIP_IGMP=1 -DLWIP_CHECKSUM_CTRL_PER_NETIF=1 "-DARDUINO_VARIANT="rpipicow"" -march=armv6-m -mcpu=cortex-m0plus -mthumb -ffunction-sections -fdata-sections -fno-exceptions -DARM_MATH_CM0_FAMILY -DARM_MATH_CM0_PLUS -MMD "-iprefixC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0/" "@C:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0/lib/platform_inc.txt" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0/include" -fno-rtti -std=gnu++17 -g -DSERIALUSB_PID=0xf00a -DUSBD_MAX_POWER_MA=250 -DF_CPU=133000000L -DARDUINO=10819 -DARDUINO_RASPBERRY_PI_PICO_W "-DBOARD_NAME="RASPBERRY_PI_PICO_W"" -DARDUINO_ARCH_RP2040 -Os "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\cores\rp2040" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\variants\rpipicow" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\WiFi\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\Updater\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\MD5Builder\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\LEAmDNS\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\lwIP_Ethernet\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\ArduinoOTA\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\LittleFS\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\lwIP_CYW43\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\SPI\src" "-IC:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\PicoOTA\src" "C:\Users\Max\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\2.4.0\libraries\WiFi\src\BearSSLHelpers.cpp" -o "C:\Users\Max\AppData\Local\Temp\arduino_build_461914\libraries\WiFi\BearSSLHelpers.cpp.o"

There would have to be a reference to -I C:\Users\Max\AppData\Local\Temp\arduino_build_xxxx\core there since the file was generated there, which there isn't. This is using the latest Arduino IDE 1.8.19

So instead of the generated one they always get the default file in https://github.com/earlephilhower/arduino-pico/blob/master/libraries/Updater/src/Updater_Signing.h.

Second:

arduino-pico/libraries/WiFi/src/BearSSLHelpers.cpp

Lines 32 to 35 in a2465f5

	//#include <Updater_Signing.h>
	#ifndef ARDUINO_SIGNING
	#define ARDUINO_SIGNING 0
	#endif

being commented out, the BearSSLHelper.cpp does not recognize whether signing is on or off, and always assumes signing is off.

This leads (when issue 1 is fixed) to a later linking error when another library needs the objects that BearSSLHelper.cpp would have created and leads to a linking error.

Linking .pio\build\rpipicow_via_usb\firmware.elf
c:/users/max/.platformio/packages/toolchain-rp2040-earlephilhower/bin/../lib/gcc/arm-none-eabi/10.3.0/../../../../arm-none-eabi/bin/ld.exe: .pio\build\rpipicow_via_usb\lib771\Updater\Updater.cpp.o: in function `_ZN12UpdaterClassC1Ev':
Updater.cpp:(.text._ZN12UpdaterClassC2Ev+0x70): undefined reference to `_ZN7esp826622updaterSigningVerifierE'
c:/users/max/.platformio/packages/toolchain-rp2040-earlephilhower/bin/../lib/gcc/arm-none-eabi/10.3.0/../../../../arm-none-eabi/bin/ld.exe: Updater.cpp:(.text._ZN12UpdaterClassC2Ev+0x74): undefined reference to `_ZN7esp826618updaterSigningHashE'
collect2.exe: error: ld returned 1 exit status

And third: Even when both of these issues are fixed (added -I flag and commenting in the include), the update does accept the signed binary.

Progress: 5%
[...]
Error[4]: End Failed
ERROR[12]: Signature verification failed

So while in the Arduino IDE, the "Signed OTA" update example indeed "seems to work" because it generates the signed binary and uploading the binary via WiFi works, it actually does only work because it does not do any verification at all.

And fourth: The platform.txt only has the capability to upload the firmware.bin, never the firmware.bin.signed, so all signed OTA uploading must be done manually.

arduino-pico/platform.txt

Line 163 in a2465f5

tools.uf2conv.upload.network_pattern="{network_cmd}" -I "{runtime.platform.path}/tools/espota.py" -i "{serial.port}" -p "{network.port}" "--auth={network.password}" -f "{build.path}/{build.project_name}.bin"

[earlephilhower]

I could swear at one point I ran the signed OTA example and manually tried to upload the unsigned and signed BINs and only the signed one works, but maybe that's just the Mandela Effect.

While the code here may be borked, the workflow was taken from the ESP8266 where the same trickery with the auto-generated header was run by multiple people and worked. There may be some difference between PIO and IDE paths/etc. in this case.

In any case, let me get on the basic "actually check the damn cert" and worry about the auto-gen header next. Maybe it's always only enabled signing on the 2nd pass or something and nobody noticed.

[earlephilhower]

Crap, I see your point. The core build directory used to actually hold copies of the core files (4 years ago), but it looks like the latest arduino-builder learned how to reference the main installation directory. When it had all the C files, that generated H was definitely pulled in to the build. I need to now see how I can get back to that state. It's an infrastructure problem, now, not a crypto one.

Signing should now also be broken on the 8266, too. 😭

[maxgerhardt]

Oh well, that's quite the find.. But of course depends on how many users of signed OTA updates there actually are. I hope noone used it for something truely security-critical. Or maybe it wasn't broken for as long as we think?

But to get the auto-genned header in the builder system, can't we not add a -I{build.path}/core flag to the C/C++ recipes? Or better rework it all?

[earlephilhower]

Oh well, that's quite the find.. But of course depends on how many users of signed OTA updates there actually are. I hope noone used it for something truely security-critical. Or maybe it wasn't broken for as long as we think?

There were some 3rd party ESP8266 PRs a few months after it was added there (to make it conform to some standard signing format), so I am absolutely certain it was good.

In any case, I'll xfer whatever happens here to the 8266 once it's all shaken out

But to get the auto-genned header in the builder system, can't we not add a -I{build.path}/core flag to the C/C++ recipes? Or better rework it all?

That, or using -include signingfile (i.e. force it and remove any #includes and any dummy headers).

Sorry you put your work into getting this into PIO! I think it came to around 26,000 lines of PR for OTA, so I'm sure there are other things lurking...

[maxgerhardt]

The integration in PIO is really not hard, the current working version that generates the headers and signed OTA files etc and uploads them is really small. Half of that commit is improving UF2 file upload, and like 50 lines are generating the header, bin.signed and calling into espota.py, so nothing to worry about there. With the smaller build issues (header inclusion / path, commented-out #include) there only seems to be some logic bug in the signature check remaining really.

[earlephilhower]

I ended up including the build.path explicitly on the C++ command line as discussed.

Also found a bug introduced in signing verification a couple months back in the 8266 repo.

One more manual upload test...no false memories (error 12 == signing fail):

earle@amd:~/Arduino/hardware/pico/rp2040$ /home/earle/Arduino/hardware/pico/rp2040/system/python3/python3 -I /home/earle/Arduino/hardware/pico/rp2040/tools/espota.py -i 192.168.1.131 -p 2040 --auth= -f unsigned.bin
Uploading.......................................................................................................................................................................................................................................................................................................

14:23:47 [ERROR]: ERROR[12]: 
earle@amd:~/Arduino/hardware/pico/rp2040$ /home/earle/Arduino/hardware/pico/rp2040/system/python3/python3 -I /home/earle/Arduino/hardware/pico/rp2040/tools/espota.py -i 192.168.1.131 -p 2040 --auth= -f signed.bin
Uploading.......................................................................................................................................................................................................................................................................................................
Complete

[maxgerhardt]

Are you sure it's not needed in

arduino-pico/libraries/WiFi/src/BearSSLHelpers.cpp

Lines 32 to 35 in a2465f5

	//#include <Updater_Signing.h>
	#ifndef ARDUINO_SIGNING
	#define ARDUINO_SIGNING 0
	#endif

to activate the .h inclusion? Where is it getting the ARDUINO_SIGNING value from if not?

[earlephilhower]

Sorry, that change snuck in the PR just before this reporting NOFS properly. So if you check master or the current PR that file should be good.

[maxgerhardt]

Ah, indeed -- I'll quickly try this.

[maxgerhardt]

The PR is working for me.

Uploading .pio\build\rpipicow_via_ota\firmware.bin.signed
23:44:39 [DEBUG]: Options: {'esp_ip': '192.168.0.206', 'host_ip': '0.0.0.0', 'esp_port': 2040, 'host_port': 22601, 'auth': '', 'image': '.pio\\build\\rpipicow_via_ota\\firmware.bin.signed', 'spiffs': False, 'debug': True, 'progress': True}
[...]

Start updating sketch
Progress: 0%
[...]
Progress: 100%
Update Success

End
Rebooting...

I've also added print statements in BearSSLHelpers.cpp to make sure it's actually going into the expected RSA verification path, and it does.

Called SigningVerifier_verify()
Starting to verify against RSA key
Sig length: 256
Calculated hash should be: 39eadb78bc825993ce59aca015d022b857b1e5b0d82b99ddd68b22782e88ce
Got hash: 
39eadb78bc825993ce59aca015d022b857b1e5b0d82b99ddd68b22782e88ce
Update Success

End
Rebooting...

Still having some troubles on the PlatformIO side to generate the header with the right SCons constructs in all cases, but I'll figure it out.. (My previous method didn't work when using the "Upload" button instead of only thet build button first and then upload).