[v3,10/15] tools: binman: pre-load: add support of ecdsa

[Simon Glass]

Hi Philippe,

On 2026-03-31T10:00:34, Philippe Reynes <philippe.reynes at softathome.com> wrote:
> diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py
> @@ -152,6 +152,67 @@ class Entry_pre_load(Entry_collection):
> +    def _CreateHeaderEcdsa(self, hash_name, sign_name, padding_name, key_name):
> +        # Check hash and signature name/type
> +        if hash_name not in SHAS:
> +            self.Raise(hash_name + " is not supported")
> +
> +        # Read the key
> +        key = ECC.import_key(tools.read_file(key_name))
> +
> +        # Check if the key has the expected size
> +        # if key.size_in_bytes() != ECDSAS[sign_name]:
> +        #    self.Raise("The key " + self.key_name + " don't have the expected size")

Please can you either enable this key size check or remove the
commented-out code. Leaving dead code in isn't ideal and suggests the
feature is incomplete. If there's a reason the check cannot work, a
brief comment explaining why would be helpful.

> diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py
> @@ -152,6 +152,67 @@ class Entry_pre_load(Entry_collection):
> +        # Compute the signature
> +        signer = DSS.new(key, 'fips-186-3')
> +        sig = signer.sign(hash_image)
> +
> +        # print("len = %d\n", len(sig))

Please remove this debug line.

> diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py
> @@ -152,6 +152,67 @@ class Entry_pre_load(Entry_collection):
> +    def _CreateHeaderEcdsa(self, hash_name, sign_name, padding_name, key_name):

The padding_name argument is passed here but never used in the
function body. ECDSA does not use padding like RSA, so this parameter
serves no purpose. Please can you either document that it is
intentionally ignored (for API consistency) or remove it from the
signature and update the caller.

> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> @@ -5894,6 +5894,37 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
> +    def testPreLoadEcdsaInvalidSha(self):
> +        """Test an image with a pre-load ecdsa header with an invalid hash"""
> +        entry_args = {
> +            'pre-load-key-path': os.path.join(self._binman_dir, 'test'),
> +        }
> +        with self.assertRaises(ValueError) as e:
> +            self._DoReadFileDtb('352_pre_load_ecdsa_invalid_sha.dts',
> +                                entry_args=entry_args)
> +
> +    def testPreLoadEcdsaInvalidAlgo(self):
> +        """Test an image with a pre-load header with an invalid algo"""
> +        with self.assertRaises(ValueError) as e:
> +            data = self._DoReadFile('353_pre_load_ecdsa_invalid_algo.dts')

The variable 'e' from assertRaises is captured but not used in either
test. It would be good to verify the exception message contains the
expected error text, similar to how testPreLoadNoKey() does with
assertIn().

Also exc is better as we try to avoid single-char var names.

Regards,
Simon