diff mbox series

[isar-cip-core,RFC,v2] classes/verity: Set salt and uuid for reproducible builds

Message ID 20230915073426.1558460-1-Quirin.Gylstorff@siemens.com (mailing list archive)
State Accepted
Headers show
Series [isar-cip-core,RFC,v2] classes/verity: Set salt and uuid for reproducible builds | expand

Commit Message

Quirin Gylstorff Sept. 15, 2023, 7:34 a.m. UTC 
From: Quirin Gylstorff <quirin.gylstorff@siemens.com>

Currently veritysetup generates a random salt and uuid for the
verity file system. This leads to a changed root hash which makes
the verity image no longer reproducible and bootable.

This also fixes together with the option `kas/opt/reproducible.yml`
the issue that after a sstate build the image can no longer be
booted.

This introduces the following variables:
 - VERITY_IMAGE_SALT: set the verity salt the value of variable. If
   VERITY_IMAGE_SALT is not set the salt is derived from IMAGE_UUID
 - VERITY_IMAGE_UUID: set the verity UUID to the value of the variable.
   If VERITY_IMAGE_UUID is not set the UUID is set to the IMAGE_UUID

Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
---
Can we set the option in `kas/opt/reproducible.yml` as default or are
there still issues open?

This patch superseeds `[cip-dev][isar-cip-core][PATCH] initramfs-verity-hook:
Ensure sync on rebuild`[1].

[1]: https://lore.kernel.org/all/595d5791-a08d-f08f-5dee-6f9ed5d472e0@siemens.com/T/

 classes/verity.bbclass | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

Comments

Jan Kiszka Sept. 18, 2023, 7:41 a.m. UTC | #1 
On 15.09.23 09:34, Quirin Gylstorff wrote:
> From: Quirin Gylstorff <quirin.gylstorff@siemens.com>
> 
> Currently veritysetup generates a random salt and uuid for the
> verity file system. This leads to a changed root hash which makes
> the verity image no longer reproducible and bootable.
> 
> This also fixes together with the option `kas/opt/reproducible.yml`
> the issue that after a sstate build the image can no longer be
> booted.
> 
> This introduces the following variables:
>  - VERITY_IMAGE_SALT: set the verity salt the value of variable. If
>    VERITY_IMAGE_SALT is not set the salt is derived from IMAGE_UUID
>  - VERITY_IMAGE_UUID: set the verity UUID to the value of the variable.
>    If VERITY_IMAGE_UUID is not set the UUID is set to the IMAGE_UUID
> 
> Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
> ---
> Can we set the option in `kas/opt/reproducible.yml` as default or are
> there still issues open?

Venkata, even if there are other issues, I think the rootfs should be
fine already, thus also a dm-verity version of its squashfs, no? Then we
should set static salt and UUID, I would say.

> 
> This patch superseeds `[cip-dev][isar-cip-core][PATCH] initramfs-verity-hook:
> Ensure sync on rebuild`[1].
> 
> [1]: https://lore.kernel.org/all/595d5791-a08d-f08f-5dee-6f9ed5d472e0@siemens.com/T/
> 
>  classes/verity.bbclass | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/classes/verity.bbclass b/classes/verity.bbclass
> index 747a7ae..bacf592 100644
> --- a/classes/verity.bbclass
> +++ b/classes/verity.bbclass
> @@ -19,6 +19,37 @@ VERITY_IMAGE_METADATA = "${VERITY_OUTPUT_IMAGE}.metadata"
>  VERITY_HASH_BLOCK_SIZE ?= "1024"
>  VERITY_DATA_BLOCK_SIZE ?= "1024"
>  
> +# Set the salt used to generate a verity image to a fixed value
> +# if not set it is derived from TARGET_IMAGE_UUID
> +VERITY_IMAGE_SALT ?= ""
> +
> +# Set the UUID used to generate a verity image to a fixed value
> +# if not set it is set to TARGET_IMAGE_UUID
> +VERITY_IMAGE_UUID ?= ""
> +
> +python derive_verity_salt_and_uuid() {
> +    import hashlib
> +
> +    verity_salt = d.getVar("VERITY_IMAGE_SALT")
> +    verity_uuid = d.getVar("VERITY_IMAGE_UUID")
> +    target_uuid = d.getVar("TARGET_IMAGE_UUID")
> +
> +    if not verity_salt:
> +        if target_uuid:
> +            verity_salt = hashlib.sha256(target_uuid.encode()).hexdigest()
> +        else:
> +            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_SALT are empty. Could not set VERITY_SALT.")
> +
> +    if not verity_uuid:
> +        if target_uuid:
> +            verity_uuid = target_uuid
> +        else:
> +            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_UUID are empty. Could not set VERITY_UUID.")
> +
> +    d.setVar("VERITY_IMAGE_SALT_OPTION", "--salt=" + str(verity_salt))
> +    d.setVar("VERITY_IMAGE_UUID_OPTION", "--uuid=" + str(verity_uuid))
> +}
> +
>  create_verity_env_file() {
>  
>      local ENV="${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}.verity.env"
> @@ -49,8 +80,9 @@ python calculate_verity_data_blocks() {
>      d.setVar("VERITY_DATA_BLOCKS", str(size // data_block_size))
>  }
>  
> +do_image_verity[vardeps] += "VERITY_IMAGE_UUID VERITY_IMAGE_SALT"
>  do_image_verity[cleandirs] = "${WORKDIR}/verity"
> -do_image_verity[prefuncs] = "calculate_verity_data_blocks"
> +do_image_verity[prefuncs] = "calculate_verity_data_blocks derive_verity_salt_and_uuid"
>  IMAGE_CMD:verity() {
>      rm -f ${DEPLOY_DIR_IMAGE}/${VERITY_OUTPUT_IMAGE}
>      rm -f ${WORKDIR}/${VERITY_IMAGE_METADATA}
> @@ -62,6 +94,8 @@ IMAGE_CMD:verity() {
>          --data-block-size "${VERITY_DATA_BLOCK_SIZE}"  \
>          --data-blocks "${VERITY_DATA_BLOCKS}" \
>          --hash-offset "${VERITY_INPUT_IMAGE_SIZE}" \
> +        "${VERITY_IMAGE_SALT_OPTION}" \
> +        "${VERITY_IMAGE_UUID_OPTION}" \
>          "${PP_DEPLOY}/${VERITY_OUTPUT_IMAGE}" \
>          "${PP_DEPLOY}/${VERITY_OUTPUT_IMAGE}" \
>          >"${WORKDIR}/${VERITY_IMAGE_METADATA}"
> @@ -70,3 +104,4 @@ IMAGE_CMD:verity() {
>          >>"${WORKDIR}/${VERITY_IMAGE_METADATA}"
>      create_verity_env_file
>  }
> +addtask do_image_verity after do_generate_image_uuid

Looks good to me. Anyone any other comments on this?

Jan
Venkata Pyla Sept. 19, 2023, 6:54 a.m. UTC | #2 
> -----Original Message-----
> From: Jan Kiszka <jan.kiszka@siemens.com>
> Sent: Monday, September 18, 2023 1:12 PM
> To: Quirin Gylstorff <Quirin.Gylstorff@siemens.com>; cip-dev@lists.cip-
> project.org; felix.moessbauer@siemens.com;
> adriaan.schmidt@siemens.com; pyla venkata(ＴＳＩＰ TMIEC ODG Porting)
> <Venkata.Pyla@toshiba-tsip.com>
> Subject: Re: [cip-dev][isar-cip-core][RFC v2] classes/verity: Set salt and uuid
> for reproducible builds
> 
> On 15.09.23 09:34, Quirin Gylstorff wrote:
> > From: Quirin Gylstorff <quirin.gylstorff@siemens.com>
> >
> > Currently veritysetup generates a random salt and uuid for the verity
> > file system. This leads to a changed root hash which makes the verity
> > image no longer reproducible and bootable.
> >
> > This also fixes together with the option `kas/opt/reproducible.yml`
> > the issue that after a sstate build the image can no longer be booted.
> >
> > This introduces the following variables:
> >  - VERITY_IMAGE_SALT: set the verity salt the value of variable. If
> >    VERITY_IMAGE_SALT is not set the salt is derived from IMAGE_UUID
> >  - VERITY_IMAGE_UUID: set the verity UUID to the value of the variable.
> >    If VERITY_IMAGE_UUID is not set the UUID is set to the IMAGE_UUID
> >
> > Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
> > ---
> > Can we set the option in `kas/opt/reproducible.yml` as default or are
> > there still issues open?
> 
> Venkata, even if there are other issues, I think the rootfs should be fine
> already, thus also a dm-verity version of its squashfs, no? Then we should set
> static salt and UUID, I would say.
Yes the rootfs is reproducible already (including squashfs), the current reproducible build issues are in file system headers.

> 
> >
> > This patch superseeds `[cip-dev][isar-cip-core][PATCH] initramfs-verity-hook:
> > Ensure sync on rebuild`[1].
> >
> > [1]:
> > https://lore.kernel.org/all/595d5791-a08d-f08f-5dee-6f9ed5d472e0@sieme
> > ns.com/T/
> >
> >  classes/verity.bbclass | 37 ++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/classes/verity.bbclass b/classes/verity.bbclass index
> > 747a7ae..bacf592 100644
> > --- a/classes/verity.bbclass
> > +++ b/classes/verity.bbclass
> > @@ -19,6 +19,37 @@ VERITY_IMAGE_METADATA =
> "${VERITY_OUTPUT_IMAGE}.metadata"
> >  VERITY_HASH_BLOCK_SIZE ?= "1024"
> >  VERITY_DATA_BLOCK_SIZE ?= "1024"
> >
> > +# Set the salt used to generate a verity image to a fixed value # if
> > +not set it is derived from TARGET_IMAGE_UUID VERITY_IMAGE_SALT ?= ""
> > +
> > +# Set the UUID used to generate a verity image to a fixed value # if
> > +not set it is set to TARGET_IMAGE_UUID VERITY_IMAGE_UUID ?= ""
> > +
> > +python derive_verity_salt_and_uuid() {
> > +    import hashlib
> > +
> > +    verity_salt = d.getVar("VERITY_IMAGE_SALT")
> > +    verity_uuid = d.getVar("VERITY_IMAGE_UUID")
> > +    target_uuid = d.getVar("TARGET_IMAGE_UUID")
> > +
> > +    if not verity_salt:
> > +        if target_uuid:
> > +            verity_salt = hashlib.sha256(target_uuid.encode()).hexdigest()
> > +        else:
> > +            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_SALT are
> > + empty. Could not set VERITY_SALT.")
> > +
> > +    if not verity_uuid:
> > +        if target_uuid:
> > +            verity_uuid = target_uuid
> > +        else:
> > +            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_UUID are
> > + empty. Could not set VERITY_UUID.")
> > +
> > +    d.setVar("VERITY_IMAGE_SALT_OPTION", "--salt=" + str(verity_salt))
> > +    d.setVar("VERITY_IMAGE_UUID_OPTION", "--uuid=" +
> > +str(verity_uuid)) }
> > +
> >  create_verity_env_file() {
> >
> >      local ENV="${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}.verity.env"
> > @@ -49,8 +80,9 @@ python calculate_verity_data_blocks() {
> >      d.setVar("VERITY_DATA_BLOCKS", str(size // data_block_size))  }
> >
> > +do_image_verity[vardeps] += "VERITY_IMAGE_UUID VERITY_IMAGE_SALT"
> >  do_image_verity[cleandirs] = "${WORKDIR}/verity"
> > -do_image_verity[prefuncs] = "calculate_verity_data_blocks"
> > +do_image_verity[prefuncs] = "calculate_verity_data_blocks
> derive_verity_salt_and_uuid"
> >  IMAGE_CMD:verity() {
> >      rm -f ${DEPLOY_DIR_IMAGE}/${VERITY_OUTPUT_IMAGE}
> >      rm -f ${WORKDIR}/${VERITY_IMAGE_METADATA}
> > @@ -62,6 +94,8 @@ IMAGE_CMD:verity() {
> >          --data-block-size "${VERITY_DATA_BLOCK_SIZE}"  \
> >          --data-blocks "${VERITY_DATA_BLOCKS}" \
> >          --hash-offset "${VERITY_INPUT_IMAGE_SIZE}" \
> > +        "${VERITY_IMAGE_SALT_OPTION}" \
> > +        "${VERITY_IMAGE_UUID_OPTION}" \
> >          "${PP_DEPLOY}/${VERITY_OUTPUT_IMAGE}" \
> >          "${PP_DEPLOY}/${VERITY_OUTPUT_IMAGE}" \
> >          >"${WORKDIR}/${VERITY_IMAGE_METADATA}"
> > @@ -70,3 +104,4 @@ IMAGE_CMD:verity() {
> >          >>"${WORKDIR}/${VERITY_IMAGE_METADATA}"
> >      create_verity_env_file
> >  }
> > +addtask do_image_verity after do_generate_image_uuid
> 
> Looks good to me. Anyone any other comments on this?
> 
> Jan
> 
> --
> Siemens AG, Technology
> Linux Expert Center
Jan Kiszka Sept. 19, 2023, 10:13 a.m. UTC | #3 
On 15.09.23 09:34, Quirin Gylstorff wrote:
> From: Quirin Gylstorff <quirin.gylstorff@siemens.com>
> 
> Currently veritysetup generates a random salt and uuid for the
> verity file system. This leads to a changed root hash which makes
> the verity image no longer reproducible and bootable.
> 
> This also fixes together with the option `kas/opt/reproducible.yml`
> the issue that after a sstate build the image can no longer be
> booted.
> 
> This introduces the following variables:
>  - VERITY_IMAGE_SALT: set the verity salt the value of variable. If
>    VERITY_IMAGE_SALT is not set the salt is derived from IMAGE_UUID
>  - VERITY_IMAGE_UUID: set the verity UUID to the value of the variable.
>    If VERITY_IMAGE_UUID is not set the UUID is set to the IMAGE_UUID
> 
> Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
> ---
> Can we set the option in `kas/opt/reproducible.yml` as default or are
> there still issues open?
> 
> This patch superseeds `[cip-dev][isar-cip-core][PATCH] initramfs-verity-hook:
> Ensure sync on rebuild`[1].
> 
> [1]: https://lore.kernel.org/all/595d5791-a08d-f08f-5dee-6f9ed5d472e0@siemens.com/T/
> 
>  classes/verity.bbclass | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/classes/verity.bbclass b/classes/verity.bbclass
> index 747a7ae..bacf592 100644
> --- a/classes/verity.bbclass
> +++ b/classes/verity.bbclass
> @@ -19,6 +19,37 @@ VERITY_IMAGE_METADATA = "${VERITY_OUTPUT_IMAGE}.metadata"
>  VERITY_HASH_BLOCK_SIZE ?= "1024"
>  VERITY_DATA_BLOCK_SIZE ?= "1024"
>  
> +# Set the salt used to generate a verity image to a fixed value
> +# if not set it is derived from TARGET_IMAGE_UUID
> +VERITY_IMAGE_SALT ?= ""
> +
> +# Set the UUID used to generate a verity image to a fixed value
> +# if not set it is set to TARGET_IMAGE_UUID
> +VERITY_IMAGE_UUID ?= ""
> +
> +python derive_verity_salt_and_uuid() {
> +    import hashlib
> +
> +    verity_salt = d.getVar("VERITY_IMAGE_SALT")
> +    verity_uuid = d.getVar("VERITY_IMAGE_UUID")
> +    target_uuid = d.getVar("TARGET_IMAGE_UUID")
> +
> +    if not verity_salt:
> +        if target_uuid:
> +            verity_salt = hashlib.sha256(target_uuid.encode()).hexdigest()
> +        else:
> +            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_SALT are empty. Could not set VERITY_SALT.")
> +
> +    if not verity_uuid:
> +        if target_uuid:
> +            verity_uuid = target_uuid
> +        else:
> +            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_UUID are empty. Could not set VERITY_UUID.")
> +
> +    d.setVar("VERITY_IMAGE_SALT_OPTION", "--salt=" + str(verity_salt))
> +    d.setVar("VERITY_IMAGE_UUID_OPTION", "--uuid=" + str(verity_uuid))
> +}
> +
>  create_verity_env_file() {
>  
>      local ENV="${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}.verity.env"
> @@ -49,8 +80,9 @@ python calculate_verity_data_blocks() {
>      d.setVar("VERITY_DATA_BLOCKS", str(size // data_block_size))
>  }
>  
> +do_image_verity[vardeps] += "VERITY_IMAGE_UUID VERITY_IMAGE_SALT"
>  do_image_verity[cleandirs] = "${WORKDIR}/verity"
> -do_image_verity[prefuncs] = "calculate_verity_data_blocks"
> +do_image_verity[prefuncs] = "calculate_verity_data_blocks derive_verity_salt_and_uuid"
>  IMAGE_CMD:verity() {
>      rm -f ${DEPLOY_DIR_IMAGE}/${VERITY_OUTPUT_IMAGE}
>      rm -f ${WORKDIR}/${VERITY_IMAGE_METADATA}
> @@ -62,6 +94,8 @@ IMAGE_CMD:verity() {
>          --data-block-size "${VERITY_DATA_BLOCK_SIZE}"  \
>          --data-blocks "${VERITY_DATA_BLOCKS}" \
>          --hash-offset "${VERITY_INPUT_IMAGE_SIZE}" \
> +        "${VERITY_IMAGE_SALT_OPTION}" \
> +        "${VERITY_IMAGE_UUID_OPTION}" \
>          "${PP_DEPLOY}/${VERITY_OUTPUT_IMAGE}" \
>          "${PP_DEPLOY}/${VERITY_OUTPUT_IMAGE}" \
>          >"${WORKDIR}/${VERITY_IMAGE_METADATA}"
> @@ -70,3 +104,4 @@ IMAGE_CMD:verity() {
>          >>"${WORKDIR}/${VERITY_IMAGE_METADATA}"
>      create_verity_env_file
>  }
> +addtask do_image_verity after do_generate_image_uuid

Thanks, applied.

Jan
diff mbox series

Patch

diff --git a/classes/verity.bbclass b/classes/verity.bbclass
index 747a7ae..bacf592 100644
--- a/classes/verity.bbclass
+++ b/classes/verity.bbclass
@@ -19,6 +19,37 @@  VERITY_IMAGE_METADATA = "${VERITY_OUTPUT_IMAGE}.metadata"
 VERITY_HASH_BLOCK_SIZE ?= "1024"
 VERITY_DATA_BLOCK_SIZE ?= "1024"
 
+# Set the salt used to generate a verity image to a fixed value
+# if not set it is derived from TARGET_IMAGE_UUID
+VERITY_IMAGE_SALT ?= ""
+
+# Set the UUID used to generate a verity image to a fixed value
+# if not set it is set to TARGET_IMAGE_UUID
+VERITY_IMAGE_UUID ?= ""
+
+python derive_verity_salt_and_uuid() {
+    import hashlib
+
+    verity_salt = d.getVar("VERITY_IMAGE_SALT")
+    verity_uuid = d.getVar("VERITY_IMAGE_UUID")
+    target_uuid = d.getVar("TARGET_IMAGE_UUID")
+
+    if not verity_salt:
+        if target_uuid:
+            verity_salt = hashlib.sha256(target_uuid.encode()).hexdigest()
+        else:
+            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_SALT are empty. Could not set VERITY_SALT.")
+
+    if not verity_uuid:
+        if target_uuid:
+            verity_uuid = target_uuid
+        else:
+            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_UUID are empty. Could not set VERITY_UUID.")
+
+    d.setVar("VERITY_IMAGE_SALT_OPTION", "--salt=" + str(verity_salt))
+    d.setVar("VERITY_IMAGE_UUID_OPTION", "--uuid=" + str(verity_uuid))
+}
+
 create_verity_env_file() {
 
     local ENV="${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}.verity.env"
@@ -49,8 +80,9 @@  python calculate_verity_data_blocks() {
     d.setVar("VERITY_DATA_BLOCKS", str(size // data_block_size))
 }
 
+do_image_verity[vardeps] += "VERITY_IMAGE_UUID VERITY_IMAGE_SALT"
 do_image_verity[cleandirs] = "${WORKDIR}/verity"
-do_image_verity[prefuncs] = "calculate_verity_data_blocks"
+do_image_verity[prefuncs] = "calculate_verity_data_blocks derive_verity_salt_and_uuid"
 IMAGE_CMD:verity() {
     rm -f ${DEPLOY_DIR_IMAGE}/${VERITY_OUTPUT_IMAGE}
     rm -f ${WORKDIR}/${VERITY_IMAGE_METADATA}
@@ -62,6 +94,8 @@  IMAGE_CMD:verity() {
         --data-block-size "${VERITY_DATA_BLOCK_SIZE}"  \
         --data-blocks "${VERITY_DATA_BLOCKS}" \
         --hash-offset "${VERITY_INPUT_IMAGE_SIZE}" \
+        "${VERITY_IMAGE_SALT_OPTION}" \
+        "${VERITY_IMAGE_UUID_OPTION}" \
         "${PP_DEPLOY}/${VERITY_OUTPUT_IMAGE}" \
         "${PP_DEPLOY}/${VERITY_OUTPUT_IMAGE}" \
         >"${WORKDIR}/${VERITY_IMAGE_METADATA}"
@@ -70,3 +104,4 @@  IMAGE_CMD:verity() {
         >>"${WORKDIR}/${VERITY_IMAGE_METADATA}"
     create_verity_env_file
 }
+addtask do_image_verity after do_generate_image_uuid