Hi Alexandru,
On 1/8/21 8:17 PM, Alexandru Gagniuc wrote:
## Purpose and intent > > The purpose of this series is to enable ECDSA as an alternative to RSA for FIT signing. As new chips have built-in support for ECDSA >
verified boot, it makes sense to stick to one signing algorithm, > instead of resorting to RSA for u-boot images. > > The focus of this series is signing an existing FIT image: > > mkimage -F some-existing.fit --signing-key some/key.pem > > Signing while assembling a FIT is not a tested use case. # > Implementation > > ## Code organization > > Unlike the RSA path, which mixes host and firmware code in the same, > source files, this series keeps a very clear distinction. > ecdsa-libcrypto.c is intended to be used for host code and only for > host code. There is more opportunity for code reuse this way. > > ## Signing > > There is one major difference from the RSA path. The 'key-name-hint' > property is ignored in the ECDSA path. There are two reasons: (1) The > intent of 'key-name-hint' is not clear (2) Initial implementation is > much easier to review > I found in doc/uImage.FIT/signature.txt the description
- key-name-hint: Name of key to use for signing. The keys will normally be in a single directory (parameter -k to mkimage). For a given key <name>, its private key is stored in <name>.key and the certificate is stored in <name>.crt.
And i check the code and the result is a little different:
include/image.h:1000:#define FIT_KEY_HINT "key-name-hint"
common/image-fit-sig.c:89: info->keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL);
lib/rsa/rsa-sign.c:742: ret = fdt_setprop_string(keydest, node, FIT_KEY_HINT, info->keyname);
tools/image-host.c: info->keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL);
=> "keyname" provided the name oft he node in device tree for verify
lib/rsa/rsa-verify.c:518:
snprintf(name, sizeof(name), "key-%s", info->keyname); node = fdt_subnode_offset(blob, sig_node, name); ret = rsa_verify_with_keynode(info, hash, sig, sig_len, node);
=> "keyname" provide the name of the key provider (pem file or key prefix for engine ) in keydir directory
lib/rsa/rsa-sign.c:537: ret = rsa_get_priv_key(info->keydir, info->keyname, e, &rsa);
lib/rsa/rsa-sign.c:702: ret = rsa_get_pub_key(info->keydir, info->keyname, e, &rsa);
dir and namle are used to get crt or key file.
=> rsa_pem_get_priv_key = "%s/%s.key", keydir, name
=> rsa_pem_get_pub_key = "%s/%s.crt", keydir, name
so today the RSA features seens more compete based on openssl (with ENGINE and pkcs11 support for exmaple)
and keydir / keyname seens clear enought...
PS: I think the engine part could be shared between RSA and EDCSA part.
There is an intentional side-effect. The RSA path takes > 'key-name-hint' to decide which key file to read from disk. In the >
context of "which fdt node describes my signing key", this makes > sense. On the other hand, 'key-name-hint' is also used as the > basename of where the key is on the filesystem. This leads to some > funny search paths, such as > > "some/dir/(null).key" So I am using the -K option to mkimage as the > _full_ path to the key file. It doesn't have to be named .key, it > doesn't have to be named .crt, and it doesn't have to exist in a > particular directory (as is the case for the RSA path). I understand > and recognize that this discrepancy must be resolved, but resolving > it right now would make the initial implementation much harder and > longer. marex@denx.de
This new option -K with full path will be permanent added to mkimage
or it is a temporarily solution (for initial ECDSA implementation).
If it is permanent it should be also supported in RSA mode ?
=> for example: -K allow to override the "key-name-hint" value
Why you can't easily could use the existing -k option and "key-name-hint" to keep the current RSA behavior for EDCSA (even without ENGINE support)
For me, EDCSA part can rely on "key-name-hint" to chose the expected key file to read the pem from disk
info->keydir / params->keydir (from option -k)
info->keyname
perhaps : edcsa_get_priv_key() => "%s/%s.pem", keydir, keyname
in prepare_ctx() function
and in the test:
keyname = "ecdsa-test-key"
keydir = "${tempdir}"
or I miss something.....
key_file > > # Testing > > test/py/tests/test_fit_ecdsa.py is implemented withe
the goal to > check that the signing is done correctly, and that the signature is > encoded in the proper raw format. Verification is done with > pyCryptodomex, so this test will catch both coding errors and openssl > bugs. This is the only scope of testing proposed here. > > > # Things not yet resolved: - is mkimage '-k' supposed to be a > directory or file path I'm hoping I can postpone answering this > question pending further discussion.
Sorry to open debate.
I propose to change the test with previous proposal.
mkimage -F /local/home/frq07632/views/u-boot/build-sandbox/test.fit -k /local/home/frq07632/views/u-boot/build-sandbox/ecdsa-test-key.pem
=> -k use directory as RSA
mkimage -F /local/home/frq07632/views/u-boot/build-sandbox/test.fit -k /local/home/frq07632/views/u-boot/build-sandbox/
with test/py/tests/vboot/sign-images-sha256.its fdt@1 { .... signature@1 { algo = "sha1,rsa2048"; - key-name-hint = "dev"; + key-name-hint = "ecdsa-test-key.pem";
}; };
Or change key_file = f'{tempdir}/ecdsa-test-key.pem'
to '{tempdir}/dev.pem' file (as: key-name-hint = "dev";)
Regards.
Patrick