Re: [PATCH v2 3/3] binman: Add tests for etype encrypted

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