Re: [PATCH v2 1/3] binman: Add support for externally encrypted blobs

Hi Christian,

On Tue, 4 Jul 2023 at 10:03, christian.taedcke-oss@weidmueller.com wrote:

From: Christian Taedcke christian.taedcke@weidmueller.com

This adds a new etype encrypted that is derived from collection.

It creates a new cipher node in the related image similar to the cipher node used by u-boot, see boot/image-cipher.c.

Signed-off-by: Christian Taedcke christian.taedcke@weidmueller.com

Changes in v2:

• remove global /cipher node
• replace key-name-hint with key-source property
• add entry documentation

tools/binman/etype/encrypted.py | 149 ++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 tools/binman/etype/encrypted.py

Please can you regenerate entries.rst in the same patch?

Also please rebase your series on u-boot-dm/mkim-working as it has a fix to section.py (if that is a pain for you to do, I can do it).

diff --git a/tools/binman/etype/encrypted.py b/tools/binman/etype/encrypted.py new file mode 100644 index 0000000000..feb2b4f1de --- /dev/null +++ b/tools/binman/etype/encrypted.py @@ -0,0 +1,149 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2023 Weidmüller Interface GmbH & Co. KG +# Written by Christian Taedcke christian.taedcke@weidmueller.com +# +# Entry-type module for cipher information of encrypted blobs/binaries +#

+from binman.etype.collection import Entry_collection +from dtoc import fdt_util +from u_boot_pylib import tools

+# This is imported if needed +state = None

+class Entry_encrypted(Entry_collection):

• """Externally built encrypted binary blob
• This entry provides the functionality to include information about how to
• decrypt an encrypted binary. This information is added to the
• resulting device tree by adding a new cipher node in the entry's parent
• node (i.e. the binary).
• The key that must be used to decrypt the binary is either directly embedded
• in the device tree or indirectly by specifying a key source. The key source
• can be used as an id of a key that is stored in an external device.
• Using an embedded key
• This is an example using an embedded key::

•    encrypted_blob: blob-ext {
  

•        filename = "encrypted-blob.bin";
  

•    };
  

•    encrypted {
  

•        content = <&encrypted_blob>;
  

•        algo = "aes256-gcm";
  

•        iv-filename = "encrypted-blob.bin.iv";
  

•        key-filename = "encrypted-blob.bin.key";
  

•    };
  

• This entry generates the following device tree structure form the example
• above::

•    data = [...]
  

•    cipher {
  

•        algo = "aes256-gcm";
  

•        key = <0x...>;
  

•        iv = <0x...>;
  

•    };
  

• The data property is generated by the blob-ext etype, the cipher node and
• its content is generated by this etype.
• Using an external key
• Instead of embedding the key itself into the device tree, it is also
• possible to address an externally stored key by specifying a 'key-source'
• instead of the 'key'::

•    encrypted_blob: blob-ext {
  

•        filename = "encrypted-blob.bin";
  

•    };
  

•    encrypted {
  

•        content = <&encrypted_blob>;
  

•        algo = "aes256-gcm";
  

•        iv-filename = "encrypted-blob.bin.iv";
  

•        key-source = "external-key-id";
  

•    };
  

• This entry generates the following device tree structure form the example
• above::

•    data = [...]
  

•    cipher {
  

•        algo = "aes256-gcm";
  

•        key-source = "external-key-id";
  

•        iv = <0x...>;
  

•    };
  

• Properties
• In addition to the inherited 'collection' for Properties / Entry arguments:

•    - algo: The encryption algorithm. Currently no algorithm is supported
  

•            out-of-the-box. Certain algorithms will be added in future
  

•            patches.
  

•    - iv-filename: The name of the file containing the initialization
  

•                   vector (in short iv). See
  

•                   https://en.wikipedia.org/wiki/Initialization_vector
  

•    - key-filename: The name of the file containing the key. Either
  

•                    key-filename or key-source must be provided.
  

•    - key-source: The key that should be used. Either key-filename or
  

•                  key-source must be provided.
  

• """
• def __init__(self, section, etype, node):

•    # Put this here to allow entry-docs and help to work without libfdt
  

•    global state
  

•    from binman import state
  

•    super().__init__(section, etype, node)
  

•    self.required_props = ['algo', 'iv-filename']
  

•    self._algo = None
  

•    self._iv_filename = None
  

•    self._key_name_hint = None
  

•    self._key_filename = None
  

• def ReadNode(self):

•    super().ReadNode()
  

•    self._algo = fdt_util.GetString(self._node, 'algo')
  

•    self._iv_filename = fdt_util.GetString(self._node, 'iv-filename')
  

•    self._key_filename = fdt_util.GetString(self._node, 'key-filename')
  

•    self._key_source = fdt_util.GetString(self._node, 'key-source')
  

•    if self._key_filename is None and self._key_source is None:
  

•        self.Raise("Provide either 'key-filename' or 'key-source'")
  

• def gen_entries(self):

•    super().gen_entries()
  

•    iv_filename = tools.get_input_filename(self._iv_filename)
  

•    iv = tools.read_file(iv_filename, binary=True)
  

•    cipher_node = state.AddSubnode(self._node.parent, "cipher")
  

•    cipher_node.AddString("algo", self._algo)
  

•    cipher_node.AddData("iv", iv)
  

•    if self._key_filename:
  

•        key_filename = tools.get_input_filename(self._key_filename)
  

•        key = tools.read_file(key_filename, binary=True)
  

•        cipher_node.AddData("key", key)
  

•    if self._key_source:
  

•        cipher_node.AddString("key-source", self._key_source)
  

• def ObtainContents(self):

•    # ensure that linked content is not added to the device tree again from this entry
  

Function comments should have a single-line header """Here is my comment""", then a blank line and any more details, with 80cols being the max.

So can you please either reword this to shorten it, or add a summary on the first line, with the other details below.

•    self.SetContents(b'')
  

•    return True
  

• def ProcessContents(self):

•    # ensure that linked content is not added to the device tree again from this entry
  

Same here

•    return self.ProcessContentsUpdate(b'')
  

-- 2.34.1