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

[Simon Glass]

Hi Christian,

On Tue, 4 Jul 2023 at 10:03, <christian.taedcke-oss at weidmueller.com> wrote:
>
> From: Christian Taedcke <christian.taedcke at 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 at 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 at 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
>