diff mbox series

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

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

Commit Message

Quirin Gylstorff Sept. 14, 2023, 6:51 p.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.

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 | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

Comments

Venkata Pyla Sept. 15, 2023, 3:33 a.m. UTC | #1 
>-----Original Message-----
>From: cip-dev@lists.cip-project.org <cip-dev@lists.cip-project.org> On Behalf Of
>Quirin Gylstorff via lists.cip-project.org
>Sent: Friday, September 15, 2023 12:21 AM
>To: cip-dev@lists.cip-project.org; felix.moessbauer@siemens.com;
>jan.kiszka@siemens.com; adriaan.schmidt@siemens.com
>Subject: [cip-dev][isar-cip-core][RFC 1/1] classes/verity: Set salt and uuid for
>reproducible builds
>
>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.
>
>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?

Yes, there are still open issues in reproducible builds, please check them in below link.

https://gitlab.com/cip-project/cip-core/isar-cip-core/-/issues/?label_name%5B%5D=Reproducible-Builds

>
>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 | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
>diff --git a/classes/verity.bbclass b/classes/verity.bbclass index 747a7ae..2cfeb28
>100644
>--- a/classes/verity.bbclass
>+++ b/classes/verity.bbclass
>@@ -18,6 +18,34 @@ VERITY_OUTPUT_IMAGE ?= "${IMAGE_FULLNAME}.verity"
> VERITY_IMAGE_METADATA = "${VERITY_OUTPUT_IMAGE}.metadata"
> VERITY_HASH_BLOCK_SIZE ?= "1024"
> VERITY_DATA_BLOCK_SIZE ?= "1024"
>+VERITY_IMAGE_SALT ?= ""
>+VERITY_IMAGE_UUID ?= ""
>+VERITY_IMAGE_SEED ?= ""
>+# TODO split if working
>+python derive_verity_salt_and_uuid() {
>+    import hashlib
>+    seed = d.getVar("VERITY_IMAGE_SEED")
>+    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 seed:
>+            verity_salt = hashlib.sha256(seed.encode()).hexdigest()
>+        elif target_uuid:
>+            verity_salt = hashlib.sha256(target_uuid.encode()).hexdigest()
>+        else:
>+            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_SEED are
>+ empty. Could not derive 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() {
>
>@@ -49,8 +77,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 +91,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 +101,4 @@ IMAGE_CMD:verity() {
>         >>"${WORKDIR}/${VERITY_IMAGE_METADATA}"
>     create_verity_env_file
> }
>+addtask do_image_verity after do_generate_image_uuid
>--
>2.40.1
Jan Kiszka Sept. 15, 2023, 6:10 a.m. UTC | #2 
On 15.09.23 00:21, 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.
> 
> 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 | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/classes/verity.bbclass b/classes/verity.bbclass
> index 747a7ae..2cfeb28 100644
> --- a/classes/verity.bbclass
> +++ b/classes/verity.bbclass
> @@ -18,6 +18,34 @@ VERITY_OUTPUT_IMAGE ?= "${IMAGE_FULLNAME}.verity"
>  VERITY_IMAGE_METADATA = "${VERITY_OUTPUT_IMAGE}.metadata"
>  VERITY_HASH_BLOCK_SIZE ?= "1024"
>  VERITY_DATA_BLOCK_SIZE ?= "1024"
> +VERITY_IMAGE_SALT ?= ""
> +VERITY_IMAGE_UUID ?= ""
> +VERITY_IMAGE_SEED ?= ""

So, how do I use these three variables now?

> +# TODO split if working

What does this mean?

> +python derive_verity_salt_and_uuid() {
> +    import hashlib
> +    seed = d.getVar("VERITY_IMAGE_SEED")
> +    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 seed:
> +            verity_salt = hashlib.sha256(seed.encode()).hexdigest()
> +        elif target_uuid:
> +            verity_salt = hashlib.sha256(target_uuid.encode()).hexdigest()
> +        else:
> +            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_SEED are empty. Could not derive 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() {
>  
> @@ -49,8 +77,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"

Why is VERITY_IMAGE_SEED not part of this list? Likely relates to my
question above.

>  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 +91,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 +101,4 @@ IMAGE_CMD:verity() {
>          >>"${WORKDIR}/${VERITY_IMAGE_METADATA}"
>      create_verity_env_file
>  }
> +addtask do_image_verity after do_generate_image_uuid

Jan
Quirin Gylstorff Sept. 15, 2023, 7:06 a.m. UTC | #3 
On 9/15/23 08:10, Jan Kiszka wrote:
> On 15.09.23 00:21, 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.
>>
>> 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 | 34 +++++++++++++++++++++++++++++++++-
>>   1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/classes/verity.bbclass b/classes/verity.bbclass
>> index 747a7ae..2cfeb28 100644
>> --- a/classes/verity.bbclass
>> +++ b/classes/verity.bbclass
>> @@ -18,6 +18,34 @@ VERITY_OUTPUT_IMAGE ?= "${IMAGE_FULLNAME}.verity"
>>   VERITY_IMAGE_METADATA = "${VERITY_OUTPUT_IMAGE}.metadata"
>>   VERITY_HASH_BLOCK_SIZE ?= "1024"
>>   VERITY_DATA_BLOCK_SIZE ?= "1024"
>> +VERITY_IMAGE_SALT ?= ""
>> +VERITY_IMAGE_UUID ?= ""
>> +VERITY_IMAGE_SEED ?= ""
> 
> So, how do I use these three variables now?
VERITY_IMAGE_SALT can be used to set the VERITY_IMAGE_SALT to fixed 
value, e.g. sha256sum of random string. If not set the 
IMAGE_UUID(TARGET_IMAGE_UUID from ${FULL_IMAGENAME}) will be decoded as 
sha256.

VERITY_IMAGE_UUID can set to a fixed UUID. If not set it will use 
IMAGE_UUID(TARGET_IMAGE_UUID from ${FULL_IMAGENAME})

VERITY_IMAGE_SEED will be removed - it was generating to apply sha256 on
a user defined string and set the verity setup salt to this checksum.


> 
>> +# TODO split if working
> 
> What does this mean?
Comment for me no longer applicable will be removed in a v2.

> 
>> +python derive_verity_salt_and_uuid() {
>> +    import hashlib
>> +    seed = d.getVar("VERITY_IMAGE_SEED")
>> +    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 seed:
>> +            verity_salt = hashlib.sha256(seed.encode()).hexdigest()
>> +        elif target_uuid:
>> +            verity_salt = hashlib.sha256(target_uuid.encode()).hexdigest()
>> +        else:
>> +            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_SEED are empty. Could not derive 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() {
>>   
>> @@ -49,8 +77,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"
> 
> Why is VERITY_IMAGE_SEED not part of this list? Likely relates to my
> question above.
>
I currently plan to remove VERITY_IMAGE_SEED as it has no benefit over 
setting the salt directly.


>>   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 +91,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 +101,4 @@ IMAGE_CMD:verity() {
>>           >>"${WORKDIR}/${VERITY_IMAGE_METADATA}"
>>       create_verity_env_file
>>   }
>> +addtask do_image_verity after do_generate_image_uuid
> 
> Jan
> 
Quirin
diff mbox series

Patch

diff --git a/classes/verity.bbclass b/classes/verity.bbclass
index 747a7ae..2cfeb28 100644
--- a/classes/verity.bbclass
+++ b/classes/verity.bbclass
@@ -18,6 +18,34 @@  VERITY_OUTPUT_IMAGE ?= "${IMAGE_FULLNAME}.verity"
 VERITY_IMAGE_METADATA = "${VERITY_OUTPUT_IMAGE}.metadata"
 VERITY_HASH_BLOCK_SIZE ?= "1024"
 VERITY_DATA_BLOCK_SIZE ?= "1024"
+VERITY_IMAGE_SALT ?= ""
+VERITY_IMAGE_UUID ?= ""
+VERITY_IMAGE_SEED ?= ""
+# TODO split if working
+python derive_verity_salt_and_uuid() {
+    import hashlib
+    seed = d.getVar("VERITY_IMAGE_SEED")
+    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 seed:
+            verity_salt = hashlib.sha256(seed.encode()).hexdigest()
+        elif target_uuid:
+            verity_salt = hashlib.sha256(target_uuid.encode()).hexdigest()
+        else:
+            bb.error("TARGET_IMAGE_UUID and VERITY_IMAGE_SEED are empty. Could not derive 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() {
 
@@ -49,8 +77,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 +91,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 +101,4 @@  IMAGE_CMD:verity() {
         >>"${WORKDIR}/${VERITY_IMAGE_METADATA}"
     create_verity_env_file
 }
+addtask do_image_verity after do_generate_image_uuid