Hi Christian, On Tue, 27 Jun 2023 at 08:39, <christian.taedcke-...@weidmueller.com> wrote: > > From: Christian Taedcke <christian.taed...@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. > Optionally it creates a global /cipher node containing a key and iv > using the same nameing convention as used in boot/image-cipher.c. > > Signed-off-by: Christian Taedcke <christian.taed...@weidmueller.com> > --- > > tools/binman/etype/encrypted.py | 98 +++++++++++++++++++++++++++++++++ > 1 file changed, 98 insertions(+) > create mode 100644 tools/binman/etype/encrypted.py > > diff --git a/tools/binman/etype/encrypted.py b/tools/binman/etype/encrypted.py > new file mode 100644 > index 0000000000..005ae56acf > --- /dev/null > +++ b/tools/binman/etype/encrypted.py > @@ -0,0 +1,98 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +# Copyright 2023 Weidmüller Interface GmbH & Co. KG > +# Written by Christian Taedcke <christian.taed...@weidmueller.com> > +# > +# Entry-type module for cipher information of encrypted blobs/images > +# > + > +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 > + > + If the file providing this blob is missing, binman can optionally ignore > it > + and produce a broken image with a warning. > + > + In addition to the inherited 'collection' for Properties / Entry > arguments: > + - algo: The encryption algorithm
What possible values are available? Please list them > + - iv-name-hint: The name hint for the iv what is the iv? > + - key-name-hint: The name hint for the key > + - iv-filename: The name of the file containing the iv > + - key-filename: The name of the file containing the key > + > + This entry creates a cipher node in the entries' parent node (i.e. the entry's > + image). Optionally it also creates a cipher node in the root of the > device > + tree containg key and iv information. containing Overall I think this documentation needs to be expanded. I wonder why this needs to be an entry type? Could the node be added to the description by the user, instead of the entry adding it? It feels a little strange to me, but perhaps I just need more info. > + """ > + > + 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) > + # The property key-filename is not required, because some > implementations use keys that > + # are not embedded in the device tree, but e.g. in the device itself > + self.required_props = ['algo', 'key-name-hint', 'iv-filename'] > + self._algo = fdt_util.GetString(self._node, 'algo') > + self._iv_name_hint = fdt_util.GetString(self._node, 'iv-name-hint') > + self._key_name_hint = fdt_util.GetString(self._node, 'key-name-hint') > + self._filename_iv = fdt_util.GetString(self._node, 'iv-filename') > + self._filename_key = fdt_util.GetString(self._node, 'key-filename') Here you should set these variables to None. Move the reading to ReadNode() Sorry if there are counter-examples in the source code. But this is the correct way. > + > + def ReadNode(self): > + super().ReadNode() > + > + iv_filename = tools.get_input_filename(self._filename_iv) > + self._iv = tools.read_file(iv_filename, binary=True) Please only read the node in this method. Move file reading until where it is needed. > + > + self._key = None > + if self._filename_key: > + key_filename = tools.get_input_filename(self._filename_key) > + self._key = tools.read_file(key_filename, binary=True) > + > + def gen_entries(self): > + super().gen_entries() > + > + cipher_node = state.AddSubnode(self._node.parent, "cipher") > + cipher_node.AddString("algo", self._algo) > + cipher_node.AddString("key-name-hint", self._key_name_hint) > + if self._iv_name_hint: > + cipher_node.AddString("iv-name-hint", self._iv_name_hint) > + else: > + cipher_node.AddData("iv", self._iv) > + > + if self._key or self._iv_name_hint: > + # add cipher node in root > + root_node = self._node.parent.parent.parent The root node is self.GetImage()._node But why are you adding something to the root node? This seems quite strange. > + name = "cipher" > + cipher_node = root_node.FindNode(name) > + if not cipher_node: > + cipher_node = state.AddSubnode(root_node, name) > + key_node_name = ( > + > f"key-{self._algo}-{self._key_name_hint}-{self._iv_name_hint}" > + if self._iv_name_hint > + else f"key-{self._algo}-{self._key_name_hint}" > + ) > + key_node = cipher_node.FindNode(key_node_name) > + if not key_node: This behaviour is not clearly documented above. > + key_node = state.AddSubnode(cipher_node, key_node_name) > + if self._key: > + key_node.AddData("key", self._key) > + if self._iv_name_hint: > + key_node.AddData("iv", self._iv) > + > + def ObtainContents(self): > + # ensure that linked content is not added to the device tree again > from this entry > + self.SetContents(b'') > + return True > + > + def ProcessContents(self): > + # ensure that linked content is not added to the device tree again > from this entry > + return self.ProcessContentsUpdate(b'') > -- > 2.34.1 > Regards, Simon