espressif / esptool

Espressif SoC serial bootloader utility
https://docs.espressif.com/projects/esptool
GNU General Public License v2.0
5.6k stars 1.39k forks source link

espsecure.py sign_data with hsm result in Payload Signing Failed (ESPTOOL-697) #889

Closed rretanubun closed 1 year ago

rretanubun commented 1 year ago

Operating System

ubuntu 22.04

Esptool Version

4.6.1

Python Version

Python 3.10.6

Full Esptool Command Line that Was Run

python3 bin/espsecure.py sign_data --version 2 --hsm --hsm-config hsm-config.cfg --output huhu-signed.bin huhu.bin espsecure.py v4.6.1

Esptool Output

espsecure.py v4.6.1
Padding data contents by 4091 bytes so signature sector aligns at sector boundary
Trying to establish a session with the HSM.
Session creation successful with HSM slot 0.
Trying to extract public key from the HSM.
Got public key with label esp32-secure-boot.
Connection closed successfully
Trying to establish a session with the HSM.
Session creation successful with HSM slot 0.
Got private key metadata with label esp32-secure-boot.
Signing payload using the HSM.
<class 'pkcs11.exceptions.DataLenRange'> Mechanism.SHA256_RSA_PKCS_PSS
Payload Signing Failed

What is the Expected Behaviour?

sign_data and verify_image works with hsm

More Information

I see the error on opensc version 0.22.0-1ubuntu2 -- I updated to version 0.23.0-0.1ubuntu1 same result on both.

the HSM is nitrokey HSM2 and it passes pkcs11-tool --login --test and pkcs15-tool -D outputs this for the key:

Private RSA Key [esp32-secure-boot]
    Object Flags   : [0x03], private, modifiable
    Usage          : [0x0E], decrypt, sign, signRecover
    Access Flags   : [0x1D], sensitive, alwaysSensitive, neverExtract, local
    Algo_refs      : 0
    ModLength      : 3072
    Key ref        : 2 (0x02)
    Native         : yes
    Auth ID        : 01
    ID             : <snip>
    MD:guid        : <snip>

Other Steps to Reproduce

No response

Harshal5 commented 1 year ago

Hello @rretanubun,

DataLenRange exception occurs when the input length is "bad" (too long or too short) ref. But I am not sure if input length would be an issue while generating a signature using the SHA256-RSA-PKCS-PSS mechanism and I am also not able to reproduce the issue using softhsm2. Still, could you please let me know what is the size of the input binary image (huhu.bin)?

Probably we could also start off debugging by checking if we are able to generate a signature externally using the pkcs11-tool command,

pkcs11-tool --module=<path_to_pkcs11_lib> --slot <slot_number> --login --pin <credentials> -s -m SHA256-RSA-PKCS-PSS -i huhu.bin -o signature.bin

Let me know if the above command works for you.

rretanubun commented 1 year ago

Hello @Harshal5

huhu.bin is generated using echo "huhu" > huhu.bin, its contents is like this:

xxd huhu.bin 
00000000: 6875 6875 0a                             huhu.

huhu.bin is just a simple test input that is easy to replicate. The real artifacts we want to sign are the application.bin (1835008 bytes) and bootloader.bin (26256 bytes)

the pkcs11-tool command you shared seems to have error in the command line args? I was not able to execute it. I manage to sign via openSSL like this

openssl dgst -sha256 -engine pkcs11 -keyform engine -sign "pkcs11:model=<pkcs11uri>" -out huhu-openssl-dgst-sign.bin huhu.bin

replacing the pkcs11-uri with one supplied from p11tool --list-token-urls

out of curiosity, what is the output of this command in your setup? pkcs11-tool -M | grep SHA256-RSA-PKCS-PSS

for my NitroKey-HSM2 - FW-version-3.5 it looks like this: SHA256-RSA-PKCS-PSS, keySize={1024,4096}, sign, verify

Harshal5 commented 1 year ago

@rretanubun I have updated the command, could you check running it once?

out of curiosity, what is the output of this command in your setup?

SHA256-RSA-PKCS-PSS, keySize={512,16384}, sign, verify

rretanubun commented 1 year ago

@Harshal5 : Thanks for updating the command hints, I am able to run the command and this is the output for me:

pkcs11-tool --module=/usr/lib/x86_64-linux-gnu/opensc-pkcs11.so --slot 0 --login --pin <credentials> -s -m SHA256-RSA-PKCS-PSS -i huhu.bin -o signature.bin

Using signature algorithm SHA256-RSA-PKCS-PSS
PSS parameters: hashAlg=SHA256, mgf=MGF1-SHA256, salt_len=32 B
error: PKCS11 function C_SignFinal failed: rv = CKR_DATA_LEN_RANGE (0x21)
Aborting.
mmb-davidsmith commented 1 year ago

@Harshal5: I'm a colleague of Richard's and wanted to ensure we're generating the key as you would expect. The command we used to generate was:

pkcs11-tool --module /Library/OpenSC/lib/opensc-pkcs11.so -l --keypairgen --key-type RSA:3072 --label esp32-secure-boot
rretanubun commented 1 year ago

@Harshal5 : FYI - I also activated debug messages from OpenSC and checking in on the project on this post https://github.com/OpenSC/OpenSC/discussions/2802

Harshal5 commented 1 year ago

@Harshal5: I'm a colleague of Richard's and wanted to ensure we're generating the key as you would expect. The command we used to generate was:

pkcs11-tool --module /Library/OpenSC/lib/opensc-pkcs11.so -l --keypairgen --key-type RSA:3072 --label esp32-secure-boot

Yes, I tried generating the key pair using the above command and I was successfully able to generate a signed binary image.

I used SoftHSMv2 for verifying the workflow using the following steps:

Initialize a new token

Generate the key pair

Create the hsm config file

Generate the signed image using espsecure.py

Following is my output for the above command:

espsecure.py v4.6.2
Padding data contents by 4091 bytes so signature sector aligns at sector boundary
Trying to establish a session with the HSM.
Session creation successful with HSM slot 1655111737.
Trying to extract public key from the HSM.
Got public key with label esp32-secure-boot.
Connection closed successfully
Trying to establish a session with the HSM.
Session creation successful with HSM slot 1655111737.
Got private key metadata with label esp32-secure-boot.
Signing payload using the HSM.
Signature generation successful.
Connection closed successfully
Pre-calculated signatures found
1 signing key(s) found.
Signed 4096 bytes of data from huhu.bin. Signature sector now has 1 signature blocks.
mmb-davidsmith commented 1 year ago

@Harshal5: Looking at the error, it seems like it has to do with hashing. I looked into the script and adjusted it so that the hashing occurs on the machine and then only the signature portion occurs in the script. Please take a look at the commit https://github.com/espressif/esptool/compare/master...mmbnetworks:esptool:nitrokey-hsm2-signing to see what I've changed.

Does that approach make sense for now to deploy signed images in the short term using the HSM until we can determine where the underlying issue is around having the HSM perform the HASH + Signature.

Harshal5 commented 1 year ago

@mmb-davidsmith I am not sure if this would work as I fear due to this parameter it would re-hash the payload. Also if we do not pass that parameter, SHA1 will be chosen by default.

mmb-davidsmith commented 1 year ago

@Harshal5 - @rretanubun is in the process of testing on actual ESP32 hardware, but running an untouched version of the verification tool we're getting

espsecure.py v4.6.2
Signature block 0 is valid (RSA).
Signature block 0 verification successful using the supplied key (RSA).
Signature block 1 invalid. Skipping.
Signature block 2 invalid. Skipping.

an older version which doesn't even support the HSM gives:

espsecure.py verify_signature --version 2 --keyfile ~/work/mmb/esptool-v461/esp32-secure-boot.pubkey.pem ~/work/mmb/esptool-v461/fromDavid/bayview.signed.bin 
espsecure.py v3.2-dev
Signature block 0 is valid. 
Signature block 0 verification successful with /home/rretanubun/work/mmb/esptool-v461/esp32-secure-boot.pubkey.pem.

My understanding from the pkcs11 library documentation is that by specifying RSA_PKCS_PSS it doesn't do the hash internally. This still provides parameters about the hash with the mechanism_params tuple which indicate that the hash should be a SHA256 instead of SHA1.

Looking at the verify path: https://github.com/espressif/esptool/blob/f542148d6942b3541cd2abee0503318006e9c1d7/espsecure/__init__.py#L820-L849

The hash is being computed locally, and it's explicitly specifying the values for PSS padding which need to be used. Would that not indicate that if it validates, it's using the right parameters on the signing side given the parameters are explicitly specified in the validation function?

Harshal5 commented 1 year ago

Right, if the verification is successful then it validates the signing process.

Also, before trying on the ESP32 hardware, you could also verify by emulating it using qemu for esp32. Here are some sets of instructions to use qemu: https://github.com/espressif/qemu/wiki.

rretanubun commented 1 year ago

@Harshal5 : I am able to run @mmb-davidsmith modified version on actual ESP32 HW. I confirmed that both securre_boot_v2 and encrypted_ota image signing check is working. To cross-check I used espsecure sign_data from his branch and ran espsecure verify_signature from the v4.6.1 release (and even an older v3.2) both are able to verify the binary signed by the modified espsecure.

Can you see if it works in your use case as well?

Harshal5 commented 1 year ago

To cross-check I used espsecure sign_data from his branch and ran espsecure verify_signature from the v4.6.1 release (and even an older v3.2) both are able to verify the binary signed by the modified espsecure.

If esptool verification works then I think that would confirm the image being correctly signed.

Harshal5 commented 1 year ago

I think we could close this issue as the issue seems more specific to NitroKey HSM's OpenSC. Also including the above patch would introduce a breaking change as initially we used to expect the image as payload but after the patch we would expect its hash as the payload. cc: @radimkarnis

mmb-davidsmith commented 1 year ago

Closing the issue is ok. I don't agree with your assertion about this being a breaking change. The outcome from the call to the PKCS#11 driver is the same as the original code (in fact I didn't even change the call to private_key.sign). All it's doing is offloading the SHA256 calculation from the HSM, but the signature that function returns is the same (and it doesn't modify payload outside the function)

Harshal5 commented 1 year ago

@mmb-davidsmith There might be use cases where a user directly uses the sign_payload() method or any other method of the esp_hsm_sign module. For such cases, the input param changing from the image binary to its hash could be a breaking change.

mmb-davidsmith commented 1 year ago

@Harshal5 - I'm confused at the assertion that you're making. Can you walk me through the code path that would fail.

As I understand (and use) my change, there is no change to the contract of sign_payload. You still provide the same binary, and it internally performs the hash.

The change to the get_mechanism could be an issue, but the only place that get_mechanism is called is within the repository is from within sign_payload. If someone was calling get_mechanism and then doing something with that, it could maybe cause issues.

Overall, I would think the desire of the tool would be to support signing with a variety of available HSMs. I'm not sure if SoftHSM would be acceptable to production customers as on the face it appears to bypasses the whole point of an HSM (ie, key is stored on physically secured hardware which makes private keys inaccessible). It seems like a good option to support for dev testing, but for production I would expect users to make use of hardware & cloud keys such as options from nitrokey, yubikey, thales, amazon, etc.

As such, when issues with such hardware devices are required, they should be considered for inclusion.

mahavirj commented 1 year ago

@mmb-davidsmith

Overall, I would think the desire of the tool would be to support signing with a variety of available HSMs. I'm not sure if SoftHSM would be acceptable to production customers as on the face it appears to bypasses the whole point of an HSM (ie, key is stored on physically secured hardware which makes private keys inaccessible). It seems like a good option to support for dev testing, but for production I would expect users to make use of hardware & cloud keys such as options from nitrokey, yubikey, thales, amazon, etc.

You are absolutely right and I completely agree with you. Our HSM signing feature is tested with following:

As such, when issues with such hardware devices are required, they should be considered for inclusion.

Of-course adding NitroKey to the this list is more than desired but I am still failing to understand the hash issue here. I would request that we evaluate this further before coming to any conclusion (especially the upstream issue you filed on OpenSC repo).

Harshal5 commented 1 year ago

@mmb-davidsmith Oh my bad, the above patch would not be a breaking change. Actually, I had tried such a local implementation for the same and it causes a breaking change. I somehow got confused between both implementations and thus I mentioned the breaking change here.

Harshal5 commented 1 year ago

There seems to be no update on https://github.com/OpenSC/OpenSC/discussions/2802. @mmb-davidsmith Regarding the fix, our take is that it would be a sort of workaround that ideally should not be required. What do you think about it?

radimkarnis commented 1 year ago

Closing this ticket, as it has been confirmed to be an upstream issue in this tracker: https://github.com/OpenSC/OpenSC/discussions/2802

It should get fixed in the next OpenSC release, please follow the related discussion there.