From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alex G. <mr.nuke.me@gmail.com>
Date: Thu, 28 Jan 2021 11:54:40 -0600
Subject: [PATCH v4 0/6] Add support for ECDSA image signing (with test)
In-Reply-To: <68d2a315-94a0-c73f-1c57-023d0ddacf3b@foss.st.com>
References: <20210108191737.615022-1-mr.nuke.me@gmail.com>
 <68d2a315-94a0-c73f-1c57-023d0ddacf3b@foss.st.com>
Message-ID: <2e3dd238-cd2b-6fb0-fd46-00a3f5640334@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 1/28/21 10:40 AM, Patrick DELAUNAY wrote:
> Hi Alexandru,
Hi Patrick

> 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).
[snip]

You are correct that the ECDSA path does not use the "key-name-hint". 
And this deviates from what RSA does. This is intentional.


[snip]> so today the RSA features seens more compete based on openssl (with
> ENGINE and pkcs11 support for exmaple)
> 
> and keydir / keyname seens clear enought...

The the common case with image signing is that one wants to sign an 
image with a key. keyname comes from the console, and keydir comes from 
the FIT image, which contradicts this simplicity.

Another issue is incorporating this into a bigger build system like 
yocto. Now mkimage would impose a specific directory structure for 
signing keys. This would not be ideal.

> PS: I think the engine part could be shared between RSA and EDCSA part.

I don't see the benefit of using the engine, and it seems highly 
libcrypto specific. It would be a lot more code, with no useful 
functionality -- we can ecdsa sign with the simpler code.

[snip]
> 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

Yes and no. It is temporary in that we'd like to update the RSA path to 
be consistent with the ECDSA path. It's permanent in that we want to 
have the -'k' option to specify the key filename instead of the key dir. 
At least that's my understanding given the previous discussion with Simon.


[snip]
> Sorry to open debate.
> I propose to change the test with previous proposal.
> 
[snip]
> with test/py/tests/vboot/sign-images-sha256.its
>  ?? ?fdt at 1 {
> ....
>  ?? ??? ?signature at 1 {
>  ??????????????????????????????? algo = "sha1,rsa2048";
> -??????????????????????????????? key-name-hint = "dev";
> +?????????????????????????????? key-name-hint = "ecdsa-test-key.pem";

This would go against us wanting to decouple the key filename from the 
key name. Consider haing several keys:

    dev-key-frobnozzle.pem
    prov-key-frobnozzle-r1.pem
    prov-key-frobnozzle-r2.pem
    prov-key-frobnozzle-r3-after-hack-mitigation.pem

One might not want to expose those key names in the .its. The issue is, 
when the fit-image is created, the key filename must be known. But when 
the signing happens on a separate machine, the filename really isn't known.

So we should really use the "key-name-hint" as a hint rather than a 
filename or part of a filename.

Alex