* [PATCH v2 0/3] binman: Add support for externally encrypted blobs
@ 2023-07-04 9:03 christian.taedcke-oss
2023-07-04 9:03 ` [PATCH v2 1/3] " christian.taedcke-oss
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: christian.taedcke-oss @ 2023-07-04 9:03 UTC (permalink / raw
To: u-boot
Cc: Christian Taedcke, Alper Nebi Yasak, Ivan Mikhaylov,
Jonas Karlman, Simon Glass, Stefan Herbrechtsmeier
From: Christian Taedcke <christian.taedcke@weidmueller.com>
This series adds the functionality to handle externally encrypted
blobs to binman. It includes the functionality itself and the
corresponding unit tests.
The following block shows an example on how to use this functionality.
In the device tree that is parsed by binman a new node encrypted is
used:
/ {
binman {
filename = "u-boot.itb";
fit {
...
images {
some-bitstream {
...
image_bitstream: blob-ext {
filename = "bitstream.bin";
};
encrypted {
content = <&image_bitstream>;
algo = "aes256-gcm";
iv-filename = "bitstream.bin.iv";
key-filename = "bitstream.bin.key";
};
...
This results in an generated fit image containing the following
information:
\ {
images {
...
some-bitstream {
...
data = [...]
cipher {
algo = "aes256-gcm";
key = <0x...>;
iv = <0x...>;
};
};
...
Changes in v2:
- remove global /cipher node
- replace key-name-hint with key-source property
- add entry documentation
- adapt tests for changed entry implementation
Christian Taedcke (3):
binman: Add support for externally encrypted blobs
binman: Allow cipher node as special section
binman: Add tests for etype encrypted
tools/binman/etype/encrypted.py | 149 ++++++++++++++++++
tools/binman/etype/section.py | 2 +-
tools/binman/ftest.py | 52 ++++++
.../binman/test/282_encrypted_no_content.dts | 15 ++
tools/binman/test/283_encrypted_no_algo.dts | 19 +++
.../test/284_encrypted_invalid_iv_file.dts | 23 +++
.../binman/test/285_encrypted_missing_key.dts | 28 ++++
.../binman/test/286_encrypted_key_source.dts | 29 ++++
tools/binman/test/287_encrypted_key_file.dts | 29 ++++
9 files changed, 345 insertions(+), 1 deletion(-)
create mode 100644 tools/binman/etype/encrypted.py
create mode 100644 tools/binman/test/282_encrypted_no_content.dts
create mode 100644 tools/binman/test/283_encrypted_no_algo.dts
create mode 100644 tools/binman/test/284_encrypted_invalid_iv_file.dts
create mode 100644 tools/binman/test/285_encrypted_missing_key.dts
create mode 100644 tools/binman/test/286_encrypted_key_source.dts
create mode 100644 tools/binman/test/287_encrypted_key_file.dts
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] binman: Add support for externally encrypted blobs
2023-07-04 9:03 [PATCH v2 0/3] binman: Add support for externally encrypted blobs christian.taedcke-oss
@ 2023-07-04 9:03 ` christian.taedcke-oss
2023-07-07 17:35 ` Simon Glass
2023-07-04 9:03 ` [PATCH v2 2/3] binman: Allow cipher node as special section christian.taedcke-oss
2023-07-04 9:03 ` [PATCH v2 3/3] binman: Add tests for etype encrypted christian.taedcke-oss
2 siblings, 1 reply; 7+ messages in thread
From: christian.taedcke-oss @ 2023-07-04 9:03 UTC (permalink / raw
To: u-boot; +Cc: Christian Taedcke, Alper Nebi Yasak, Simon Glass
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
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
+ 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] binman: Allow cipher node as special section
2023-07-04 9:03 [PATCH v2 0/3] binman: Add support for externally encrypted blobs christian.taedcke-oss
2023-07-04 9:03 ` [PATCH v2 1/3] " christian.taedcke-oss
@ 2023-07-04 9:03 ` christian.taedcke-oss
2023-07-07 17:35 ` Simon Glass
2023-07-04 9:03 ` [PATCH v2 3/3] binman: Add tests for etype encrypted christian.taedcke-oss
2 siblings, 1 reply; 7+ messages in thread
From: christian.taedcke-oss @ 2023-07-04 9:03 UTC (permalink / raw
To: u-boot
Cc: Christian Taedcke, Alper Nebi Yasak, Ivan Mikhaylov,
Jonas Karlman, Simon Glass, Stefan Herbrechtsmeier
From: Christian Taedcke <christian.taedcke@weidmueller.com>
The new encrypted etype generates a cipher node in the device tree
that should not be evaluated by binman, but still be kept in the
output device tree.
Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
---
(no changes since v1)
tools/binman/etype/section.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index c36edd1350..56abfd5129 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -178,7 +178,7 @@ class Entry_section(Entry):
Returns:
bool: True if the node is a special one, else False
"""
- return node.name.startswith('hash') or node.name.startswith('signature')
+ return node.name.startswith('hash') or node.name.startswith('signature') or node.name.startswith('cipher')
def ReadNode(self):
"""Read properties from the section node"""
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] binman: Add tests for etype encrypted
2023-07-04 9:03 [PATCH v2 0/3] binman: Add support for externally encrypted blobs christian.taedcke-oss
2023-07-04 9:03 ` [PATCH v2 1/3] " christian.taedcke-oss
2023-07-04 9:03 ` [PATCH v2 2/3] binman: Allow cipher node as special section christian.taedcke-oss
@ 2023-07-04 9:03 ` christian.taedcke-oss
2023-07-07 17:35 ` Simon Glass
2 siblings, 1 reply; 7+ messages in thread
From: christian.taedcke-oss @ 2023-07-04 9:03 UTC (permalink / raw
To: u-boot; +Cc: Christian Taedcke, Alper Nebi Yasak, Simon Glass
From: Christian Taedcke <christian.taedcke@weidmueller.com>
Add tests to reach 100% code coverage for the added etype encrypted.
Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
---
Changes in v2:
- adapt tests for changed entry implementation
tools/binman/ftest.py | 52 +++++++++++++++++++
.../binman/test/282_encrypted_no_content.dts | 15 ++++++
tools/binman/test/283_encrypted_no_algo.dts | 19 +++++++
.../test/284_encrypted_invalid_iv_file.dts | 23 ++++++++
.../binman/test/285_encrypted_missing_key.dts | 28 ++++++++++
.../binman/test/286_encrypted_key_source.dts | 29 +++++++++++
tools/binman/test/287_encrypted_key_file.dts | 29 +++++++++++
7 files changed, 195 insertions(+)
create mode 100644 tools/binman/test/282_encrypted_no_content.dts
create mode 100644 tools/binman/test/283_encrypted_no_algo.dts
create mode 100644 tools/binman/test/284_encrypted_invalid_iv_file.dts
create mode 100644 tools/binman/test/285_encrypted_missing_key.dts
create mode 100644 tools/binman/test/286_encrypted_key_source.dts
create mode 100644 tools/binman/test/287_encrypted_key_file.dts
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 43b4f850a6..d51139799f 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -94,6 +94,8 @@ ROCKCHIP_TPL_DATA = b'rockchip-tpl'
TEST_FDT1_DATA = b'fdt1'
TEST_FDT2_DATA = b'test-fdt2'
ENV_DATA = b'var1=1\nvar2="2"'
+ENCRYPTED_IV_DATA = b'123456'
+ENCRYPTED_KEY_DATA = b'1234567890123456'
PRE_LOAD_MAGIC = b'UBSH'
PRE_LOAD_VERSION = 0x11223344.to_bytes(4, 'big')
PRE_LOAD_HDR_SIZE = 0x00001000.to_bytes(4, 'big')
@@ -226,6 +228,10 @@ class TestFunctional(unittest.TestCase):
# Newer OP_TEE file in v1 binary format
cls.make_tee_bin('tee.bin')
+ # test files for encrypted tests
+ TestFunctional._MakeInputFile('encrypted-file.iv', ENCRYPTED_IV_DATA)
+ TestFunctional._MakeInputFile('encrypted-file.key', ENCRYPTED_KEY_DATA)
+
cls.comp_bintools = {}
for name in COMP_BINTOOLS:
cls.comp_bintools[name] = bintool.Bintool.create(name)
@@ -6676,6 +6682,52 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
['fit'])
self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
+ def testEncryptedNoContent(self):
+ with self.assertRaises(ValueError) as e:
+ self._DoReadFileDtb('282_encrypted_no_content.dts')
+ self.assertIn("Node '/binman/fit/images/u-boot/encrypted': Collection must have a 'content' property", str(e.exception))
+
+ def testEncryptedNoAlgo(self):
+ with self.assertRaises(ValueError) as e:
+ self._DoReadFileDtb('283_encrypted_no_algo.dts')
+ self.assertIn("Node '/binman/fit/images/u-boot/encrypted': 'encrypted' entry is missing properties: algo iv-filename", str(e.exception))
+
+ def testEncryptedInvalidIvfile(self):
+ with self.assertRaises(ValueError) as e:
+ self._DoReadFileDtb('284_encrypted_invalid_iv_file.dts')
+ self.assertIn("Filename 'invalid-iv-file' not found in input path",
+ str(e.exception))
+
+ def testEncryptedMissingKey(self):
+ with self.assertRaises(ValueError) as e:
+ self._DoReadFileDtb('285_encrypted_missing_key.dts')
+ self.assertIn("Node '/binman/fit/images/u-boot/encrypted': Provide either 'key-filename' or 'key-source'",
+ str(e.exception))
+
+ def testEncryptedKeySource(self):
+ data = self._DoReadFileDtb('286_encrypted_key_source.dts')[0]
+
+ dtb = fdt.Fdt.FromData(data)
+ dtb.Scan()
+
+ node = dtb.GetNode('/images/u-boot/cipher')
+ self.assertEqual('algo-name', node.props['algo'].value)
+ self.assertEqual('key-source-value', node.props['key-source'].value)
+ self.assertEqual(ENCRYPTED_IV_DATA, tools.to_bytes(''.join(node.props['iv'].value)))
+ self.assertNotIn('key', node.props)
+
+ def testEncryptedKeyFile(self):
+ data = self._DoReadFileDtb('287_encrypted_key_file.dts')[0]
+
+ dtb = fdt.Fdt.FromData(data)
+ dtb.Scan()
+
+ node = dtb.GetNode('/images/u-boot/cipher')
+ self.assertEqual('algo-name', node.props['algo'].value)
+ self.assertEqual(ENCRYPTED_IV_DATA, tools.to_bytes(''.join(node.props['iv'].value)))
+ self.assertEqual(ENCRYPTED_KEY_DATA, b''.join(node.props['key'].value))
+ self.assertNotIn('key-source', node.props)
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/282_encrypted_no_content.dts b/tools/binman/test/282_encrypted_no_content.dts
new file mode 100644
index 0000000000..03f7ffee90
--- /dev/null
+++ b/tools/binman/test/282_encrypted_no_content.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+ binman {
+ fit {
+ images {
+ u-boot {
+ encrypted {
+ };
+ };
+ };
+ };
+ };
+};
diff --git a/tools/binman/test/283_encrypted_no_algo.dts b/tools/binman/test/283_encrypted_no_algo.dts
new file mode 100644
index 0000000000..71975c0116
--- /dev/null
+++ b/tools/binman/test/283_encrypted_no_algo.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+ binman {
+ fit {
+ images {
+ u-boot {
+ encrypted {
+ content = <&data>;
+ };
+
+ data: data {
+ };
+ };
+ };
+ };
+ };
+};
diff --git a/tools/binman/test/284_encrypted_invalid_iv_file.dts b/tools/binman/test/284_encrypted_invalid_iv_file.dts
new file mode 100644
index 0000000000..1764d5e503
--- /dev/null
+++ b/tools/binman/test/284_encrypted_invalid_iv_file.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+ binman {
+ fit {
+ images {
+ u-boot {
+ blob: blob {
+ filename = "blobfile";
+ };
+
+ encrypted {
+ content = <&blob>;
+ algo = "some-algo";
+ key-source = "key";
+ iv-filename = "invalid-iv-file";
+ };
+ };
+ };
+ };
+ };
+};
diff --git a/tools/binman/test/285_encrypted_missing_key.dts b/tools/binman/test/285_encrypted_missing_key.dts
new file mode 100644
index 0000000000..9d342d6f45
--- /dev/null
+++ b/tools/binman/test/285_encrypted_missing_key.dts
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ fit {
+ description = "test desc";
+
+ images {
+ u-boot {
+ blob: blob {
+ filename = "blobfile";
+ };
+
+ encrypted {
+ content = <&blob>;
+ algo = "algo-name";
+ iv-filename = "encrypted-file.iv";
+ };
+ };
+ };
+ };
+ };
+};
diff --git a/tools/binman/test/286_encrypted_key_source.dts b/tools/binman/test/286_encrypted_key_source.dts
new file mode 100644
index 0000000000..d2529b9c3a
--- /dev/null
+++ b/tools/binman/test/286_encrypted_key_source.dts
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ fit {
+ description = "test desc";
+
+ images {
+ u-boot {
+ blob: blob {
+ filename = "blobfile";
+ };
+
+ encrypted {
+ content = <&blob>;
+ algo = "algo-name";
+ key-source = "key-source-value";
+ iv-filename = "encrypted-file.iv";
+ };
+ };
+ };
+ };
+ };
+};
diff --git a/tools/binman/test/287_encrypted_key_file.dts b/tools/binman/test/287_encrypted_key_file.dts
new file mode 100644
index 0000000000..71f1ab47b1
--- /dev/null
+++ b/tools/binman/test/287_encrypted_key_file.dts
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ fit {
+ description = "test desc";
+
+ images {
+ u-boot {
+ blob: blob {
+ filename = "blobfile";
+ };
+
+ encrypted {
+ content = <&blob>;
+ algo = "algo-name";
+ iv-filename = "encrypted-file.iv";
+ key-filename = "encrypted-file.key";
+ };
+ };
+ };
+ };
+ };
+};
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] binman: Add support for externally encrypted blobs
2023-07-04 9:03 ` [PATCH v2 1/3] " christian.taedcke-oss
@ 2023-07-07 17:35 ` Simon Glass
0 siblings, 0 replies; 7+ messages in thread
From: Simon Glass @ 2023-07-07 17:35 UTC (permalink / raw
To: christian.taedcke-oss; +Cc: u-boot, Christian Taedcke, Alper Nebi Yasak
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] binman: Allow cipher node as special section
2023-07-04 9:03 ` [PATCH v2 2/3] binman: Allow cipher node as special section christian.taedcke-oss
@ 2023-07-07 17:35 ` Simon Glass
0 siblings, 0 replies; 7+ messages in thread
From: Simon Glass @ 2023-07-07 17:35 UTC (permalink / raw
To: christian.taedcke-oss
Cc: u-boot, Christian Taedcke, Alper Nebi Yasak, Ivan Mikhaylov,
Jonas Karlman, Stefan Herbrechtsmeier
On Tue, 4 Jul 2023 at 10:03, <christian.taedcke-oss@weidmueller.com> wrote:
>
> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>
> The new encrypted etype generates a cipher node in the device tree
> that should not be evaluated by binman, but still be kept in the
> output device tree.
>
> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
> ---
>
> (no changes since v1)
>
> tools/binman/etype/section.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
Although this patch needs to change once rebased to u-boot-dm/mkim-working
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] binman: Add tests for etype encrypted
2023-07-04 9:03 ` [PATCH v2 3/3] binman: Add tests for etype encrypted christian.taedcke-oss
@ 2023-07-07 17:35 ` Simon Glass
0 siblings, 0 replies; 7+ messages in thread
From: Simon Glass @ 2023-07-07 17:35 UTC (permalink / raw
To: christian.taedcke-oss; +Cc: u-boot, Christian Taedcke, Alper Nebi Yasak
Hi Christian,
On Tue, 4 Jul 2023 at 10:03, <christian.taedcke-oss@weidmueller.com> wrote:
>
> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>
> Add tests to reach 100% code coverage for the added etype encrypted.
>
> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
> ---
>
> Changes in v2:
> - adapt tests for changed entry implementation
>
> tools/binman/ftest.py | 52 +++++++++++++++++++
> .../binman/test/282_encrypted_no_content.dts | 15 ++++++
> tools/binman/test/283_encrypted_no_algo.dts | 19 +++++++
> .../test/284_encrypted_invalid_iv_file.dts | 23 ++++++++
> .../binman/test/285_encrypted_missing_key.dts | 28 ++++++++++
> .../binman/test/286_encrypted_key_source.dts | 29 +++++++++++
> tools/binman/test/287_encrypted_key_file.dts | 29 +++++++++++
> 7 files changed, 195 insertions(+)
> create mode 100644 tools/binman/test/282_encrypted_no_content.dts
> create mode 100644 tools/binman/test/283_encrypted_no_algo.dts
> create mode 100644 tools/binman/test/284_encrypted_invalid_iv_file.dts
> create mode 100644 tools/binman/test/285_encrypted_missing_key.dts
> create mode 100644 tools/binman/test/286_encrypted_key_source.dts
> create mode 100644 tools/binman/test/287_encrypted_key_file.dts
>
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 43b4f850a6..d51139799f 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -94,6 +94,8 @@ ROCKCHIP_TPL_DATA = b'rockchip-tpl'
> TEST_FDT1_DATA = b'fdt1'
> TEST_FDT2_DATA = b'test-fdt2'
> ENV_DATA = b'var1=1\nvar2="2"'
> +ENCRYPTED_IV_DATA = b'123456'
> +ENCRYPTED_KEY_DATA = b'1234567890123456'
Can you make that different, e.g. abcd, since at present one is a
subset of the other. Does the length matter? If not, shorter is better
for the second one, so we can visually look at the output.
> PRE_LOAD_MAGIC = b'UBSH'
> PRE_LOAD_VERSION = 0x11223344.to_bytes(4, 'big')
> PRE_LOAD_HDR_SIZE = 0x00001000.to_bytes(4, 'big')
> @@ -226,6 +228,10 @@ class TestFunctional(unittest.TestCase):
> # Newer OP_TEE file in v1 binary format
> cls.make_tee_bin('tee.bin')
>
> + # test files for encrypted tests
> + TestFunctional._MakeInputFile('encrypted-file.iv', ENCRYPTED_IV_DATA)
> + TestFunctional._MakeInputFile('encrypted-file.key', ENCRYPTED_KEY_DATA)
> +
> cls.comp_bintools = {}
> for name in COMP_BINTOOLS:
> cls.comp_bintools[name] = bintool.Bintool.create(name)
> @@ -6676,6 +6682,52 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
> ['fit'])
> self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
>
> + def testEncryptedNoContent(self):
"""Test for missing content property"""
Please add a comment to following test functions
> + with self.assertRaises(ValueError) as e:
> + self._DoReadFileDtb('282_encrypted_no_content.dts')
> + self.assertIn("Node '/binman/fit/images/u-boot/encrypted': Collection must have a 'content' property", str(e.exception))
Please wrap to 80cols. It is OK not to split strings though, but
otherwise, please keep to 80.
self.assertIn(
"Node '/binman/fit/images/u-boot/encrypted': Collection must have
a 'content' property",
str(e.exception))
same below
> +
> + def testEncryptedNoAlgo(self):
> + with self.assertRaises(ValueError) as e:
> + self._DoReadFileDtb('283_encrypted_no_algo.dts')
> + self.assertIn("Node '/binman/fit/images/u-boot/encrypted': 'encrypted' entry is missing properties: algo iv-filename", str(e.exception))
> +
> + def testEncryptedInvalidIvfile(self):
> + with self.assertRaises(ValueError) as e:
> + self._DoReadFileDtb('284_encrypted_invalid_iv_file.dts')
> + self.assertIn("Filename 'invalid-iv-file' not found in input path",
> + str(e.exception))
> +
> + def testEncryptedMissingKey(self):
> + with self.assertRaises(ValueError) as e:
> + self._DoReadFileDtb('285_encrypted_missing_key.dts')
> + self.assertIn("Node '/binman/fit/images/u-boot/encrypted': Provide either 'key-filename' or 'key-source'",
> + str(e.exception))
> +
> + def testEncryptedKeySource(self):
> + data = self._DoReadFileDtb('286_encrypted_key_source.dts')[0]
> +
> + dtb = fdt.Fdt.FromData(data)
> + dtb.Scan()
> +
> + node = dtb.GetNode('/images/u-boot/cipher')
> + self.assertEqual('algo-name', node.props['algo'].value)
> + self.assertEqual('key-source-value', node.props['key-source'].value)
> + self.assertEqual(ENCRYPTED_IV_DATA, tools.to_bytes(''.join(node.props['iv'].value)))
> + self.assertNotIn('key', node.props)
> +
> + def testEncryptedKeyFile(self):
> + data = self._DoReadFileDtb('287_encrypted_key_file.dts')[0]
> +
> + dtb = fdt.Fdt.FromData(data)
> + dtb.Scan()
> +
> + node = dtb.GetNode('/images/u-boot/cipher')
> + self.assertEqual('algo-name', node.props['algo'].value)
> + self.assertEqual(ENCRYPTED_IV_DATA, tools.to_bytes(''.join(node.props['iv'].value)))
> + self.assertEqual(ENCRYPTED_KEY_DATA, b''.join(node.props['key'].value))
> + self.assertNotIn('key-source', node.props)
> +
>
> if __name__ == "__main__":
> unittest.main()
> diff --git a/tools/binman/test/282_encrypted_no_content.dts b/tools/binman/test/282_encrypted_no_content.dts
> new file mode 100644
> index 0000000000..03f7ffee90
> --- /dev/null
> +++ b/tools/binman/test/282_encrypted_no_content.dts
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> + binman {
> + fit {
> + images {
> + u-boot {
> + encrypted {
> + };
> + };
> + };
> + };
> + };
> +};
Do these need to be in a FIT? It is OK to do this, but I wonder if it
is necessary for the purposes of your tests?
Regards,
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-07 17:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-04 9:03 [PATCH v2 0/3] binman: Add support for externally encrypted blobs christian.taedcke-oss
2023-07-04 9:03 ` [PATCH v2 1/3] " christian.taedcke-oss
2023-07-07 17:35 ` Simon Glass
2023-07-04 9:03 ` [PATCH v2 2/3] binman: Allow cipher node as special section christian.taedcke-oss
2023-07-07 17:35 ` Simon Glass
2023-07-04 9:03 ` [PATCH v2 3/3] binman: Add tests for etype encrypted christian.taedcke-oss
2023-07-07 17:35 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.