[PATCH v4 2/3] tools: binman: add 'fit, encrypt' property to pass keys directory to mkimage

[Paul HENRYS]

mkimage can be used for both signing the FIT or encrypt its content and the
option '-k' can be used to pass a directory where both signing and encryption
keys can be retrieved. Adding 'fit,encrypt' property to the 'fit' node, leads to
try to find keys directory among binman include directories.
_get_priv_keys_dir() is renamed as _get_keys_dir() and adapted to support both
signing and encryption nodes in the FIT.

Signed-off-by: Paul HENRYS <paul.henrys_ext at softathome.com>
---
Changes for v4:
- Add 'fit,encrypt' property to enable encryption in FIT
- Remove the previous implementation to have a common property to pass the keys
  directory to mkimage for both signing and encrypting the FIT
- Update the entries.rst file

Reacting to Alexander's message pointing at Simon's previous message in August, speaking about entryargs. At my level, it doesn't really matter whether a specific directory is specified or if it comes from one of the binman input directories and Alexander's implementation, that retrieves the signing keys from one of the binman input directory is handy for that.
On my side, it would sufficient to retrieve the ciphering keys from one of binman input directories, like for the signing keys but with the current implementation, I would need to add the property 'fit,sign' which is a bit counterintuitive. 'fit-keys-directory' might also not be right, it could be something like 'fit,keys'. What I meant is that afaiu, 'fit,sign' is not a property that will enable the signature but provides the keys directories to mkimage. Signing the FIT data is based on the presence of 'signature' nodes in the ITS. The same is true for the encryption with mkimage but with 'cipher' nodes.

Regarding the other entryargs for signing with binman, this is another way of signing any piece of data (file, binary blob...) which globally signs the data and prepends a header. When using the FIT embedded signature, that does not sign all FIT metadata, I guess it's better to keep it in mkimage, which is the tool generating the FIT.

I am pushing here a v4, where I add a property 'fit,encrypt' that does the same than 'fit,sign', passing the option '-k' to mkimage. That might help our discussions because I am a bit confuse at the moment on the approach to be taken. Thx for your help.


 tools/binman/btool/mkimage.py |  8 ++++----
 tools/binman/entries.rst      |  7 +++++++
 tools/binman/etype/fit.py     | 25 ++++++++++++++++++-------
 3 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py
index 78d3301bc1..3f84220fb1 100644
--- a/tools/binman/btool/mkimage.py
+++ b/tools/binman/btool/mkimage.py
@@ -22,7 +22,7 @@ class Bintoolmkimage(bintool.Bintool):
 
     # pylint: disable=R0913
     def run(self, reset_timestamp=False, output_fname=None, external=False,
-            pad=None, align=None, priv_keys_dir=None):
+            pad=None, align=None, keys_dir=None):
         """Run mkimage
 
         Args:
@@ -34,7 +34,7 @@ class Bintoolmkimage(bintool.Bintool):
                 other things to be easily added later, if required, such as
                 signatures
             align: Bytes to use for alignment of the FIT and its external data
-            priv_keys_dir: Path to directory containing private keys
+            keys_dir: Path to directory containing private and encryption keys
             version: True to get the mkimage version
         """
         args = []
@@ -46,8 +46,8 @@ class Bintoolmkimage(bintool.Bintool):
             args += ['-B', f'{align:x}']
         if reset_timestamp:
             args.append('-t')
-        if priv_keys_dir:
-            args += ['-k', f'{priv_keys_dir}']
+        if keys_dir:
+            args += ['-k', f'{keys_dir}']
         if output_fname:
             args += ['-F', output_fname]
         return self.run_cmd(*args)
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index e918162fb4..53024acad4 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -871,6 +871,13 @@ The top-level 'fit' node supports the following special properties:
         -k flag. All the keys required for signing FIT must be available at
         time of signing and must be located in single include directory.
 
+    fit,encrypt
+        Enable data encryption in FIT images via mkimage. If the property
+        is found, the keys path is detected among binman include
+        directories and passed to mkimage via  -k flag. All the keys
+        required for encrypting the FIT must be available at the time of
+        encrypting and must be located in a single include directory.
+
 Substitutions
 ~~~~~~~~~~~~~
 
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index b5afbda41b..70be9bea47 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -110,6 +110,13 @@ class Entry_fit(Entry_section):
             available at time of signing and must be located in single include
             directory.
 
+        fit,encrypt
+            Enable data encryption in FIT images via mkimage. If the property
+            is found, the keys path is detected among binman include
+            directories and passed to mkimage via  -k flag. All the keys
+            required for encrypting the FIT must be available at the time of
+            encrypting and must be located in a single include directory.
+
     Substitutions
     ~~~~~~~~~~~~~
 
@@ -518,14 +525,14 @@ class Entry_fit(Entry_section):
         # are removed from self._entries later.
         self._priv_entries = dict(self._entries)
 
-    def _get_priv_keys_dir(self, data):
-        """Detect private keys path among binman include directories
+    def _get_keys_dir(self, data):
+        """Detect private and encryption keys path among binman include directories
 
         Args:
             data: FIT image in binary format
 
         Returns:
-            str: Single path containing all private keys found or None
+            str: Single path containing all keys found or None
 
         Raises:
             ValueError: Filename 'rsa2048.key' not found in input path
@@ -533,11 +540,14 @@ class Entry_fit(Entry_section):
         """
         def _find_keys_dir(node):
             for subnode in node.subnodes:
-                if subnode.name.startswith('signature'):
+                if (subnode.name.startswith('signature') or
+                    subnode.name.startswith('cipher')):
                     if subnode.props.get('key-name-hint') is None:
                         continue
                     hint = subnode.props['key-name-hint'].value
-                    name = tools.get_input_filename(f"{hint}.key")
+                    name = tools.get_input_filename(
+                        f"{hint}.key" if subnode.name.startswith('signature')
+                        else f"{hint}.bin")
                     path = os.path.dirname(name)
                     if path not in paths:
                         paths.append(path)
@@ -587,8 +597,9 @@ class Entry_fit(Entry_section):
         align = self._fit_props.get('fit,align')
         if align is not None:
             args.update({'align': fdt_util.fdt32_to_cpu(align.value)})
-        if self._fit_props.get('fit,sign') is not None:
-            args.update({'priv_keys_dir': self._get_priv_keys_dir(data)})
+        if (self._fit_props.get('fit,sign') is not None or
+            self._fit_props.get('fit,encrypt') is not None):
+            args.update({'keys_dir': self._get_keys_dir(data)})
         if self.mkimage.run(reset_timestamp=True, output_fname=output_fname,
                             **args) is None:
             if not self.GetAllowMissing():
-- 
2.43.0