[PATCH v2 1/3] aes: Allow to store randomly generated IV in the FIT
When the initialisation vector is randomly generated, its value shall be stored in the FIT together with the encrypted data. The changes allow to store the IV in the FIT also in the case where the key is not stored in the DTB but retrieved somewhere else at runtime.
Signed-off-by: Paul HENRYS paul.henrys_ext@softathome.com --- lib/aes/aes-encrypt.c | 7 +++++++ tools/image-host.c | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/aes/aes-encrypt.c b/lib/aes/aes-encrypt.c index e74e35eaa28..90e1407b4f0 100644 --- a/lib/aes/aes-encrypt.c +++ b/lib/aes/aes-encrypt.c @@ -84,6 +84,13 @@ int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest, char name[128]; int ret = 0;
+ if (!keydest && !info->ivname) { + /* At least, store the IV in the FIT image */ + ret = fdt_setprop(fit, node_noffset, "iv", + info->iv, info->cipher->iv_len); + goto done; + } + /* Either create or overwrite the named cipher node */ parent = fdt_subnode_offset(keydest, 0, FIT_CIPHER_NODENAME); if (parent == -FDT_ERR_NOTFOUND) { diff --git a/tools/image-host.c b/tools/image-host.c index 49ce7436bb9..3424b8d9a1d 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -535,7 +535,7 @@ fit_image_process_cipher(const char *keydir, void *keydest, void *fit, * size values * And, if needed, write the iv in the FIT file */ - if (keydest) { + if (keydest || (!keydest && !info.ivname)) { ret = info.cipher->add_cipher_data(&info, keydest, fit, node_noffset); if (ret) { fprintf(stderr,
The property 'fit,keys-directory' can be added to the configuration file passed to binman to specify a directory where keys are stored and can be used by mkimage to sign and cipher data.
Signed-off-by: Paul HENRYS paul.henrys_ext@softathome.com --- tools/binman/btool/mkimage.py | 5 ++++- tools/binman/entries.rst | 3 +++ tools/binman/etype/fit.py | 6 ++++++ 3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py index 39a4c8c1432..dbcf8daac30 100644 --- a/tools/binman/btool/mkimage.py +++ b/tools/binman/btool/mkimage.py @@ -22,7 +22,7 @@ class Bintoolmkimage(bintool.Bintool):
# pylint: disable=R0913 def run(self, reset_timestamp=False, output_fname=None, external=False, - pad=None, align=None): + pad=None, align=None, keys_dir=None): """Run mkimage
Args: @@ -34,6 +34,7 @@ class Bintoolmkimage(bintool.Bintool): other things to be easily added later, if required, such as signatures align: Bytes to use for alignment of the FIT and its external data + keys_dir: directory where keys are stored version: True to get the mkimage version """ args = [] @@ -45,6 +46,8 @@ class Bintoolmkimage(bintool.Bintool): args += ['-B', f'{align:x}'] if reset_timestamp: args.append('-t') + if keys_dir: + args += ['-k', keys_dir] if output_fname: args += ['-F', output_fname] return self.run_cmd(*args) diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 12482703782..eb33eb9eedf 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -864,6 +864,9 @@ The top-level 'fit' node supports the following special properties:
fit,fdt-list-dir = "arch/arm/dts
+ fit,keys-directory + Provides a directory where keys can be retrieved. + Substitutions ~~~~~~~~~~~~~
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index ee44e5a1cd6..d20906aab3b 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -96,6 +96,9 @@ class Entry_fit(Entry_section):
fit,fdt-list-dir = "arch/arm/dts
+ fit,keys-directory + Provides a directory where keys can be retrieved. + Substitutions ~~~~~~~~~~~~~
@@ -518,6 +521,9 @@ class Entry_fit(Entry_section): align = self._fit_props.get('fit,align') if align is not None: args.update({'align': fdt_util.fdt32_to_cpu(align.value)}) + keys_dir = self._fit_props.get('fit,keys-directory') + if keys_dir is not None: + args.update({'keys_dir': keys_dir.value}) if self.mkimage.run(reset_timestamp=True, output_fname=output_fname, **args) is None: if not self.GetAllowMissing():
Hi Paul,
On Mon, 5 Aug 2024 at 07:35, Paul HENRYS paul.henrys_ext@softathome.com wrote:
The property 'fit,keys-directory' can be added to the configuration file passed to binman to specify a directory where keys are stored and can be used by mkimage to sign and cipher data.
Environmental things like directories are best handled by binman itself, e.g. with the --indir argument.
In this case it seems that you want a specific directory, rather than finding the keys in one of many possible directories. So I suggest adding a new entryarg for the FIT which lets you specify this key dir.
Signed-off-by: Paul HENRYS paul.henrys_ext@softathome.com
tools/binman/btool/mkimage.py | 5 ++++- tools/binman/entries.rst | 3 +++ tools/binman/etype/fit.py | 6 ++++++ 3 files changed, 13 insertions(+), 1 deletion(-)
BTW when you change the fit docs you need to use 'binman entry-docs' to regenerate the entries.rts file, in the same patch.
diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py index 39a4c8c1432..dbcf8daac30 100644 --- a/tools/binman/btool/mkimage.py +++ b/tools/binman/btool/mkimage.py @@ -22,7 +22,7 @@ class Bintoolmkimage(bintool.Bintool):
# pylint: disable=R0913 def run(self, reset_timestamp=False, output_fname=None, external=False,
pad=None, align=None):
pad=None, align=None, keys_dir=None): """Run mkimage Args:@@ -34,6 +34,7 @@ class Bintoolmkimage(bintool.Bintool): other things to be easily added later, if required, such as signatures align: Bytes to use for alignment of the FIT and its external data
keys_dir: directory where keys are stored version: True to get the mkimage version """ args = []@@ -45,6 +46,8 @@ class Bintoolmkimage(bintool.Bintool): args += ['-B', f'{align:x}'] if reset_timestamp: args.append('-t')
if keys_dir:args += ['-k', keys_dir] if output_fname: args += ['-F', output_fname] return self.run_cmd(*args)diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 12482703782..eb33eb9eedf 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -864,6 +864,9 @@ The top-level 'fit' node supports the following special properties:
fit,fdt-list-dir = "arch/arm/dts
- fit,keys-directory
Provides a directory where keys can be retrieved.Substitutions
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index ee44e5a1cd6..d20906aab3b 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -96,6 +96,9 @@ class Entry_fit(Entry_section): fit,fdt-list-dir = "arch/arm/dts + fit,keys-directory + Provides a directory where keys can be retrieved. + Substitutions ~~~~~~~~~~~~~ @@ -518,6 +521,9 @@ class Entry_fit(Entry_section): align = self._fit_props.get('fit,align') if align is not None: args.update({'align': fdt_util.fdt32_to_cpu(align.value)}) + keys_dir = self._fit_props.get('fit,keys-directory') + if keys_dir is not None: + args.update({'keys_dir': keys_dir.value}) if self.mkimage.run(reset_timestamp=True, output_fname=output_fname, **args) is None: if not self.GetAllowMissing(): -- 2.25.1 -- This message and any attachments herein are confidential, intended solely for the addressees and are SoftAtHome’s ownership. Any unauthorized use or dissemination is prohibited. If you are not the intended addressee of this message, please cancel it immediately and inform the sender.
Regards, Simon
Test the property 'fit,keys-directory' which, when a cipher node is present, encrypts the data stored in the FIT.
Signed-off-by: Paul HENRYS paul.henrys_ext@softathome.com --- tools/binman/ftest.py | 39 +++++++++++++ tools/binman/test/326_fit_encrypt_data.dts | 53 ++++++++++++++++++ .../test/327_fit_encrypt_data_no_key.dts | 53 ++++++++++++++++++ tools/binman/test/aes256.bin | Bin 0 -> 32 bytes 4 files changed, 145 insertions(+) create mode 100644 tools/binman/test/326_fit_encrypt_data.dts create mode 100644 tools/binman/test/327_fit_encrypt_data_no_key.dts create mode 100644 tools/binman/test/aes256.bin
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 93f3d22cf57..64b7d0231de 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -7691,5 +7691,44 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertIsNone(dtb.GetNode('/node/other-node'))
+ def testSimpleFitEncryptedData(self): + """Test an image with a FIT containing data to be encrypted""" + data = self._DoReadFile('326_fit_encrypt_data.dts') + + fit = fdt.Fdt.FromData(data) + fit.Scan() + + # Extract the encrypted data and the IV from the FIT + node = fit.GetNode('/images/u-boot') + subnode = fit.GetNode('/images/u-boot/cipher') + data_size_unciphered = int.from_bytes(fit.GetProps(node)['data-size-unciphered'].bytes, + byteorder='big') + self.assertEqual(data_size_unciphered, len(U_BOOT_NODTB_DATA)) + + # Retrieve the key name from the FIT removing any null byte + key_name = fit.GetProps(subnode)['key-name-hint'].bytes.replace(b'\x00', b'') + with open(self.TestFile(key_name.decode('ascii') + '.bin'), 'rb') as file: + key = file.read() + iv = fit.GetProps(subnode)['iv'].bytes.hex() + enc_data = fit.GetProps(node)['data'].bytes + outdir = tools.get_output_dir() + enc_data_file = os.path.join(outdir, 'encrypted_data.bin') + tools.write_file(enc_data_file, enc_data) + data_file = os.path.join(outdir, 'data.bin') + + # Decrypt the encrypted data from the FIT and compare the data + tools.run('openssl', 'enc', '-aes-256-cbc', '-nosalt', '-d', '-in', + enc_data_file, '-out', data_file, '-K', key.hex(), '-iv', iv) + with open(data_file, 'r') as file: + dec_data = file.read() + self.assertEqual(U_BOOT_NODTB_DATA, dec_data.encode('ascii')) + + def testSimpleFitEncryptedDataMissingKey(self): + """Test an image with a FIT containing data to be encrypted but with a missing key""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('327_fit_encrypt_data_no_key.dts') + + self.assertIn("Can't open file ./aes256.bin (err=2 => No such file or directory)", str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/326_fit_encrypt_data.dts b/tools/binman/test/326_fit_encrypt_data.dts new file mode 100644 index 00000000000..3cd890063cd --- /dev/null +++ b/tools/binman/test/326_fit_encrypt_data.dts @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + fit,keys-directory = "tools/binman/test"; + description = "Test a FIT with encrypted data"; + #address-cells = <1>; + + images { + u-boot { + description = "U-Boot"; + type = "firmware"; + arch = "arm64"; + os = "U-Boot"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + cipher { + algo = "aes256"; + key-name-hint = "aes256"; + }; + u-boot-nodtb { + }; + }; + fdt-1 { + description = "Flattened Device Tree blob"; + type = "flat_dt"; + arch = "arm64"; + compression = "none"; + cipher { + algo = "aes256"; + key-name-hint = "aes256"; + }; + }; + }; + + configurations { + default = "conf-1"; + conf-1 { + description = "Boot U-Boot with FDT blob"; + firmware = "u-boot"; + fdt = "fdt-1"; + }; + }; + }; + }; +}; diff --git a/tools/binman/test/327_fit_encrypt_data_no_key.dts b/tools/binman/test/327_fit_encrypt_data_no_key.dts new file mode 100644 index 00000000000..b92cd2e4bd6 --- /dev/null +++ b/tools/binman/test/327_fit_encrypt_data_no_key.dts @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + fit,keys-directory = "."; + description = "Test a FIT with encrypted data"; + #address-cells = <1>; + + images { + u-boot { + description = "U-Boot"; + type = "firmware"; + arch = "arm64"; + os = "U-Boot"; + compression = "none"; + load = <00000000>; + entry = <00000000>; + cipher { + algo = "aes256"; + key-name-hint = "aes256"; + }; + u-boot-nodtb { + }; + }; + fdt-1 { + description = "Flattened Device Tree blob"; + type = "flat_dt"; + arch = "arm64"; + compression = "none"; + cipher { + algo = "aes256"; + key-name-hint = "aes256"; + }; + }; + }; + + configurations { + default = "conf-1"; + conf-1 { + description = "Boot U-Boot with FDT blob"; + firmware = "u-boot"; + fdt = "fdt-1"; + }; + }; + }; + }; +}; diff --git a/tools/binman/test/aes256.bin b/tools/binman/test/aes256.bin new file mode 100644 index 0000000000000000000000000000000000000000..09b8bf6254ada5c084039f32916bc7d30233bb2c GIT binary patch literal 32 ncmXpsGBz<aGq<obNK8sjNli=7$jr*l$<50zC@d;2DJ=s4pC}7U
literal 0 HcmV?d00001
Hi Paul,
On Mon, 5 Aug 2024 at 07:35, Paul HENRYS paul.henrys_ext@softathome.com wrote:
Test the property 'fit,keys-directory' which, when a cipher node is present, encrypts the data stored in the FIT.
Signed-off-by: Paul HENRYS paul.henrys_ext@softathome.com
tools/binman/ftest.py | 39 +++++++++++++ tools/binman/test/326_fit_encrypt_data.dts | 53 ++++++++++++++++++ .../test/327_fit_encrypt_data_no_key.dts | 53 ++++++++++++++++++ tools/binman/test/aes256.bin | Bin 0 -> 32 bytes 4 files changed, 145 insertions(+) create mode 100644 tools/binman/test/326_fit_encrypt_data.dts create mode 100644 tools/binman/test/327_fit_encrypt_data_no_key.dts create mode 100644 tools/binman/test/aes256.bin
Looks OK but for nits
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 93f3d22cf57..64b7d0231de 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -7691,5 +7691,44 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertIsNone(dtb.GetNode('/node/other-node'))
- def testSimpleFitEncryptedData(self):
"""Test an image with a FIT containing data to be encrypted"""data = self._DoReadFile('326_fit_encrypt_data.dts')fit = fdt.Fdt.FromData(data)fit.Scan()# Extract the encrypted data and the IV from the FIT
Please write out IV in full for clarity.
node = fit.GetNode('/images/u-boot')subnode = fit.GetNode('/images/u-boot/cipher')data_size_unciphered = int.from_bytes(fit.GetProps(node)['data-size-unciphered'].bytes,byteorder='big')self.assertEqual(data_size_unciphered, len(U_BOOT_NODTB_DATA))# Retrieve the key name from the FIT removing any null bytekey_name = fit.GetProps(subnode)['key-name-hint'].bytes.replace(b'\x00', b'')
Why do you need to replace the nul byte? Could you use this?
fdt_util.GetString(subnode, 'key-name-hint')
with open(self.TestFile(key_name.decode('ascii') + '.bin'), 'rb') as file:key = file.read()
tools.read_file(filename)
below also
iv = fit.GetProps(subnode)['iv'].bytes.hex()enc_data = fit.GetProps(node)['data'].bytesoutdir = tools.get_output_dir()enc_data_file = os.path.join(outdir, 'encrypted_data.bin')tools.write_file(enc_data_file, enc_data)data_file = os.path.join(outdir, 'data.bin')# Decrypt the encrypted data from the FIT and compare the datatools.run('openssl', 'enc', '-aes-256-cbc', '-nosalt', '-d', '-in',enc_data_file, '-out', data_file, '-K', key.hex(), '-iv', iv)with open(data_file, 'r') as file:dec_data = file.read()self.assertEqual(U_BOOT_NODTB_DATA, dec_data.encode('ascii'))- def testSimpleFitEncryptedDataMissingKey(self):
"""Test an image with a FIT containing data to be encrypted but with a missing key"""with self.assertRaises(ValueError) as e:self._DoReadFile('327_fit_encrypt_data_no_key.dts')self.assertIn("Can't open file ./aes256.bin (err=2 => No such file or directory)", str(e.exception))if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/326_fit_encrypt_data.dts b/tools/binman/test/326_fit_encrypt_data.dts new file mode 100644 index 00000000000..3cd890063cd --- /dev/null +++ b/tools/binman/test/326_fit_encrypt_data.dts @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;#size-cells = <1>;binman {fit {fit,keys-directory = "tools/binman/test";description = "Test a FIT with encrypted data";#address-cells = <1>;images {u-boot {description = "U-Boot";type = "firmware";arch = "arm64";os = "U-Boot";compression = "none";load = <00000000>;entry = <00000000>;cipher {algo = "aes256";key-name-hint = "aes256";};u-boot-nodtb {};};fdt-1 {description = "Flattened Device Tree blob";type = "flat_dt";arch = "arm64";compression = "none";cipher {algo = "aes256";key-name-hint = "aes256";};};};configurations {default = "conf-1";conf-1 {description = "Boot U-Boot with FDT blob";firmware = "u-boot";fdt = "fdt-1";};};};};+}; diff --git a/tools/binman/test/327_fit_encrypt_data_no_key.dts b/tools/binman/test/327_fit_encrypt_data_no_key.dts new file mode 100644 index 00000000000..b92cd2e4bd6 --- /dev/null +++ b/tools/binman/test/327_fit_encrypt_data_no_key.dts @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;#size-cells = <1>;binman {fit {fit,keys-directory = ".";description = "Test a FIT with encrypted data";#address-cells = <1>;images {u-boot {description = "U-Boot";type = "firmware";arch = "arm64";os = "U-Boot";compression = "none";load = <00000000>;entry = <00000000>;cipher {algo = "aes256";key-name-hint = "aes256";};u-boot-nodtb {};};fdt-1 {description = "Flattened Device Tree blob";type = "flat_dt";arch = "arm64";compression = "none";cipher {algo = "aes256";key-name-hint = "aes256";};};};configurations {default = "conf-1";conf-1 {description = "Boot U-Boot with FDT blob";firmware = "u-boot";fdt = "fdt-1";};};};};+}; diff --git a/tools/binman/test/aes256.bin b/tools/binman/test/aes256.bin new file mode 100644 index 0000000000000000000000000000000000000000..09b8bf6254ada5c084039f32916bc7d30233bb2c GIT binary patch literal 32 ncmXpsGBz<aGq<obNK8sjNli=7$jr*l$<50zC@d;2DJ=s4pC}7U
literal 0 HcmV?d00001
-- 2.25.1
-- This message and any attachments herein are confidential, intended solely for the addressees and are SoftAtHome’s ownership. Any unauthorized use or dissemination is prohibited. If you are not the intended addressee of this message, please cancel it immediately and inform the sender.
Regards, Simon
Hi Paul,
On Mon, 5 Aug 2024 at 07:35, Paul HENRYS paul.henrys_ext@softathome.com wrote:
When the initialisation vector is randomly generated, its value shall be stored in the FIT together with the encrypted data. The changes allow to store the IV in the FIT also in the case where the key is not stored in the DTB but retrieved somewhere else at runtime.
What is the IV? Can you please write it out in full?
Signed-off-by: Paul HENRYS paul.henrys_ext@softathome.com
lib/aes/aes-encrypt.c | 7 +++++++ tools/image-host.c | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/aes/aes-encrypt.c b/lib/aes/aes-encrypt.c index e74e35eaa28..90e1407b4f0 100644 --- a/lib/aes/aes-encrypt.c +++ b/lib/aes/aes-encrypt.c @@ -84,6 +84,13 @@ int image_aes_add_cipher_data(struct image_cipher_info *info, void *keydest, char name[128]; int ret = 0;
if (!keydest && !info->ivname) {/* At least, store the IV in the FIT image */ret = fdt_setprop(fit, node_noffset, "iv",info->iv, info->cipher->iv_len);goto done;}/* Either create or overwrite the named cipher node */ parent = fdt_subnode_offset(keydest, 0, FIT_CIPHER_NODENAME); if (parent == -FDT_ERR_NOTFOUND) {diff --git a/tools/image-host.c b/tools/image-host.c index 49ce7436bb9..3424b8d9a1d 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -535,7 +535,7 @@ fit_image_process_cipher(const char *keydir, void *keydest, void *fit, * size values * And, if needed, write the iv in the FIT file */
if (keydest) {
if (keydest || (!keydest && !info.ivname)) { ret = info.cipher->add_cipher_data(&info, keydest, fit, node_noffset); if (ret) { fprintf(stderr,-- 2.25.1
-- This message and any attachments herein are confidential, intended solely for the addressees and are SoftAtHome’s ownership. Any unauthorized use or dissemination is prohibited. If you are not the intended addressee of this message, please cancel it immediately and inform the sender.
Did you mean to include this?
Regards, Simon
participants (2)
-
Paul HENRYS -
Simon Glass