Updater signature validation - format incompatible w/RFC8017

[qistoph]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: ESP8285 device
• Core Version: 2.5.2
• Development Env: Arduino IDE
• Operating System: Ubuntu

Settings in IDE (although not relevant imho)

• Module: LOLIN(WEMOS) D1 D2 & mini
• Flash Mode: unknown
• Flash Size: 4MB
• lwip Variant: v2 Lower Memory
• Reset Method: unknown
• Flash Frequency: 40Mhz
• CPU Frequency: 80Mhz
• Upload Using: SERIAL
• Upload Speed: 921600

Problem Description

The signed updates code (#5213) does not correctly implement PKCS#1. This makes it harder to verify the updates in other applications, like a python script.

According to the RFC 8017 section 9.2 step 2:

Encode the algorithm ID for the hash function and the hash value into an ASN.1 value of type DigestInfo with the DER.

Currently the signed data looks like this:

00000000  00 01 ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000010  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000020  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000030  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000040  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000050  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000060  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000070  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000080  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000090  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
000000a0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
000000b0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
000000c0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
000000d0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff 00  |................|
000000e0  3c 10 ae 9a e8 d3 e6 42  bf c2 a9 c5 71 3f d3 2d  |<......B....q?.-|
000000f0  a5 d3 1d 12 b5 bb dd 86  1b 0a 30 60 4a 2e a2 bd  |..........0`J...|

RFC 8017 step 5 says the encoded message is a concatenation of:
EM = 0x00 || 0x01 || PS || 0x00 || T

T should be the DER of DigestInfo. In this case the DigestInfo should look like:

    0:d=0  hl=2 l=  49 cons: SEQUENCE          
    2:d=1  hl=2 l=  13 cons:  SEQUENCE          
    4:d=2  hl=2 l=   9 prim:   OBJECT            :sha256
   15:d=2  hl=2 l=   0 prim:   NULL              
   17:d=1  hl=2 l=  32 prim:  OCTET STRING      [HEX DUMP]:3C10AE9AE8D3E642BFC2A9C5713FD32DA5D31D12B5BBDD861B0A30604A2EA2BD

That would encode to EM:

00000000  00 01 ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000010  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000020  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000030  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000040  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000050  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000060  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000070  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000080  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000090  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
000000a0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
000000b0  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
000000c0  ff ff ff ff ff ff ff ff  ff ff ff ff 00 30 31 30  |.............010|
000000d0  0d 06 09 60 86 48 01 65  03 04 02 01 05 00 04 20  |...`.H.e....... |
000000e0  3c 10 ae 9a e8 d3 e6 42  bf c2 a9 c5 71 3f d3 2d  |<......B....q?.-|
000000f0  a5 d3 1d 12 b5 bb dd 86  1b 0a 30 60 4a 2e a2 bd  |..........0`J...|

The currently used version of BearSSL seems to support proper PKCS#1 signatures, with an OID.

The signing.py is executing:
openssl rsautl -sign -inkey <privatekey> on a pre-computed hash.
To create a signature with a valid PKCS#1 padded signature it should use openssl dgst -sha256 -sign <privatekey> on the raw binary.

Changing this might require additional effort to make it backwards compatible. Old firmware will most likely not accept these new signatures. Intermediate firmware might be required that checks for the new signature format, but is signed with the old method. This might be challenging for users, though since signing updates is relatively new (Oct 2018) it is probably better to do as soon as possible, before even more people are affected.

I'm willing to look into writing a PR for this, but would first like to check if that's appreciated and what the requirements would be.

[earlephilhower]

Just updated your title to be a little clearer what the issue is when we look at these things.

While I'm not quite sure of the use case for this, it's probably better to handle things in a standardized way. A PR would be great, if you can supply one, otherwise it will get onto the backlog here and we'll track it.

A transition signing menu option (ugh, another one), or a config file/#define in the code might make the 1-time transition less of a chore to get right.

[qistoph]

So, if I'm correct there are currently 4 methods to update the ESP. Below are these four methods and my suggestions on guidance to update to the new signature format.

We'll have to provide two files to allow people to upgrade and use signed firmware:

• new firmware with old signature (verifiable by old firmware instances)
• new firmware with new signature (verifiable by new firmware instances)

Suggestions for clear names/extensions are most welcome. For now I'm using:

• Sketch.ino.bin - unsigned binary
• Sketch.ino.bin.signed - signed with new signature
• Sketch.ino.bin.legacy_sig - signed with old signature

Serial update
Serial updates are not affected by the signature format change, because the signature is not checked at all. Just building and uploading the code will send over the new firmware with the new signature.

Web Browser OTA and HTTP Server OTA
For the Web Browser and HTTP Server OTA a user is manually selecting the file to use. This option is probably used by users with a little more experience, though might know nothing about signatures or crypto at all. In these cases I think it will suffice if we: (and)

• provide firmware with old and new signatures,
• make names as straight forward as possible, to have people the right file,
• update documentation to include instructions to upgrade using new firmware with old signature.

Arduino IDE OTA
This one will be the most challenging one. Though, it appears that this option currently isn't even supporting signed updates. The espota.py script is always called with the unsigned .bin as argument:

Arduino/platform.txt

Line 144 in 653f58e

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

Users that are calling the espota.py script manually are already selecting the signed file to upload, so these will fall into the same category as the two above.

My work so far is in my branch pkcs1_fix. I'm really looking forward to your ideas and suggestions for further improvement. We could use this issue or I could create a PR, whatever suits your needs.

Terms used:

• old signature - a signature like on v2.5.2, without DigestInfo
• new signature - a signature as implemented in the branch pkcs1_fix, with DigestInfo
• old firmware - firmware build with code expecting an old signature
• new firmware - firmware build with code expecting a new signature

[earlephilhower]

Howdy, @qistoph . I looked quickly at your branch and it looks good so far. I didn't see the ASN.1 yet, but I imagine that's en-route. BearSSL has a reader for ASN.1, but not a writer, so we may want to consider a simple hardcoded ASN.1 wrapper with signature replacement to match the RFC requirements.

For ArduinoOTA, I would not worry too much. ArduinoOTA is not a routable protocol AFAIK (or at least shouldn't be used over the internet), and since signing was only added in 2.5.2 I doubt there are any users concerned with it and signing now.

Extensions seem fine, and are a minor detail.

I've been working on other bits of the core recently, but look forward to seeing more updates on this. Thanks!

[qistoph]

Thanks for your feedback @earlephilhower!

The ASN.1 verification is actually already in BearSSL. The changes in my first commit are actually really enough to make the signature check use it.

Instead of NULL, now an OID is passed to vrfy which is passed through some helper functions and eventually to

br_rsa_pkcs1_sig_unpad(const unsigned char *sig, size_t sig_len,
	const unsigned char *hash_oid, size_t hash_len,
	unsigned char *hash_out)

This part of BearSSL creates the ASN.1 part to verify against the signed data at line 89.

[earlephilhower]

Ah, got it. That's very elegant!

[earlephilhower]

Closing, your PR is now merged.