Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
rretanubun opened this issue Jun 13, 2023 · 21 comments

Comments

@rretanubun
Copy link
Contributor

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

@github-actions github-actions bot changed the title espsecure.py sign_data with hsm result in Payload Signing Failed espsecure.py sign_data with hsm result in Payload Signing Failed (ESPTOOL-697) Jun 13, 2023
@Harshal5
Copy link
Contributor

Harshal5 commented Jun 14, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor

@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
Copy link
Contributor Author

@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
Copy link

@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
Copy link
Contributor Author

@Harshal5 : FYI - I also activated debug messages from OpenSC and checking in on the project on this post OpenSC/OpenSC#2802

@Harshal5
Copy link
Contributor

Harshal5 commented Jun 15, 2023

@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

  • softhsm2-util --init-token --slot 0 --label softhsm-test-token --pin xxxx --so-pin xxxxxx

Generate the key pair

  • pkcs11-tool --module /usr/local/lib/softhsm/libsofthsm2.so -l --keypairgen --key-type RSA:3072 --label esp32-secure-boot

Create the hsm config file

  • [hsm_config]
    pkcs11_lib = /usr/local/lib/softhsm/libsofthsm2.so
    credentials = xxxx
    slot = 1655111737
    label = esp32-secure-boot
    label_pubkey = esp32-secure-boot

Generate the signed image using espsecure.py

  • espsecure.py sign_data --version 2 --hsm --hsm-config softhsm_v2.ini --output huhu_signed.bin huhu.bin
    (huhu.bin has been generated similarly as mentioned by @rretanubun)

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
Copy link

@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 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
Copy link
Contributor

@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
Copy link

@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:

digest = digest = hashlib.sha256()
digest.update(image_content[:-SECTOR_SIZE])
digest = digest.digest()
valid = False
for sig_blk_num in range(SIG_BLOCK_MAX_COUNT):
sig_blk = validate_signature_block(image_content, sig_blk_num)
if sig_blk is None:
print(f"Signature block {sig_blk_num} invalid. Skipping.")
continue
_, version, blk_digest = struct.unpack("<BBxx32s", sig_blk[:36])
if blk_digest != digest:
raise esptool.FatalError(
"Signature block image digest does not match "
f"the actual image digest {digest}. Expected {blk_digest}."
)
try:
if isinstance(vk, rsa.RSAPublicKey):
_, _, _, _, signature, _ = struct.unpack(
"<384sI384sI384sI16x", sig_blk[36:]
)
vk.verify(
signature[::-1],
digest,
padding.PSS(mgf=padding.MGF1(hashes.SHA256()), salt_length=32),
utils.Prehashed(hashes.SHA256()),
)

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
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor

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
Copy link
Contributor

Harshal5 commented Jun 21, 2023

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
Copy link

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
Copy link
Contributor

@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
Copy link

@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
Copy link
Member

@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
Copy link
Contributor

@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
Copy link
Contributor

Harshal5 commented Jul 26, 2023

There seems to be no update on OpenSC/OpenSC#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
Copy link
Collaborator

Closing this ticket, as it has been confirmed to be an upstream issue in this tracker: OpenSC/OpenSC#2802

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants