Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hwio 2024 fixes #7

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Hwio 2024 fixes #7

wants to merge 6 commits into from

Conversation

pietrushnic
Copy link

@pietrushnic pietrushnic commented Oct 15, 2024

[15:38:11] pietrushnic:dasharo-sdk git:(hwio_2024_fixes*) $ ./build.sh
[+] Building 264.6s (12/12) FINISHED                                                                                                                                            docker:default
 => [internal] load build definition from Dockerfile                                                                                                                                      0.0s
 => => transferring dockerfile: 2.49kB                                                                                                                                                    0.0s
 => [internal] load metadata for docker.io/coreboot/coreboot-sdk:2024-02-18_732134932b                                                                                                    0.8s
 => [internal] load .dockerignore                                                                                                                                                         0.0s
 => => transferring context: 2B                                                                                                                                                           0.0s
 => CACHED [1/8] FROM docker.io/coreboot/coreboot-sdk:2024-02-18_732134932b@sha256:181452b3bc89ebc0eedcfc766d116224f0611ea4b5027857c6cfe530f5741a6c                                       0.0s
 => [2/8] RUN apt-get update &&     apt-get upgrade -y &&     apt-get install -y     gpg     imagemagick     uuid-runtime     unzip     libxcb-icccm4     libxcb-image0     libxcb-key  120.2s
 => [3/8] RUN wget https://github.com/LongSoft/UEFITool/releases/download/A68/UEFIExtract_NE_A68_x64_linux.zip &&     unzip UEFIExtract_NE_A68_x64_linux.zip &&     mv uefiextract /usr/  1.6s
 => [4/8] RUN git clone https://github.com/coreboot/coreboot.git  &&     cd coreboot &&     git checkout 05bb053e6356b30bfa2ae27d0b38e592e4c58111 &&     cd util/cbfstool &&     make &  47.1s
 => [5/8] RUN git clone https://github.com/wolfSSL/wolfssl.git -b v5.7.0-stable --depth=1 &&     cd wolfssl &&     ./autogen.sh &&     ./configure --libdir /lib/x86_64-linux-gnu/ &&    34.7s
 => [6/8] RUN cd coreboot &&     make -C util/ifdtool &&     make -C util/ifdtool install                                                                                                 1.2s
 => [7/8] RUN rm -rf coreboot &&     git clone https://review.coreboot.org/coreboot.git &&     cd coreboot &&     git fetch https://review.coreboot.org/coreboot refs/changes/29/67129/  33.7s
 => [8/8] RUN git clone https://github.com/Dasharo/vboot.git       -b master       --depth 1 &&     cd vboot &&     git checkout 22134690d7ced7b2ea824b71b597bb73586d99c6 &&     export  14.7s
 => exporting to image                                                                                                                                                                   10.4s
 => => exporting layers                                                                                                                                                                  10.4s
 => => writing image sha256:e35bbf4d6255b3b24008385cdb55ab4a67a023789285ea2317e423dd20f4f906                                                                                              0.0s
 => => naming to ghcr.io/dasharo/dasharo-sdk:latest                                                                                                                                       0.0s
[15:42:37] pietrushnic:dasharo-sdk git:(hwio_2024_fixes*) $ docker run -ti --rm -u root ghcr.io/dasharo/dasharo-sdk:latest /vboot/scripts/keygeneration/make_arv_root.sh
creating arv_root keypair (version = 1)...
vb2_read_packed_keyb() - wrong key size 3272 for algorithm 7

miczyg1 and others added 2 commits August 6, 2024 12:20
To set the GROUP_ID for the coreboot user one has to pass
two variables to docker environment: GROUP and GROUP_ID.
Most likely it was a copy-pasta error, which can be fixed
by using GROUP_ID in the IF condition.

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
Remove unneded entrypoint and switch back default user in container to
coreboot. Cleanup any source code directories after installing compiled
binaries.

Signed-off-by: Piotr Król <piotr.krol@3mdeb.com>
Signed-off-by: Piotr Król <piotr.krol@3mdeb.com>
@miczyg1
Copy link
Contributor

miczyg1 commented Oct 16, 2024

vb2_read_packed_keyb() - wrong key size 3272 for algorithm 7

This arv_root key still causing some problems? Unbelievable, even vboot itself cannot generate it properly?

@miczyg1
Copy link
Contributor

miczyg1 commented Oct 16, 2024

We also may change the coreboot version from which cbfstool and smmstoretool is built to 24.08. The latter tool is already there in tree. Keeping some random hash does not look good.

Or, keep this hash just for the smmstoretool. Then clone coreboot separately to 24.02 and build cbfstool and vboot from it.

@pietrushnic
Copy link
Author

vb2_read_packed_keyb() - wrong key size 3272 for algorithm 7

This arv_root key still causing some problems? Unbelievable, even vboot itself cannot generate it properly?

I ignored that; is this something to be concerned about? I never saw those scripts correctly running, so I can't say how bad it is. Also, I probably didn't see in the documentation what the correct output is. Generated keys worked for the process, but there were more warnings like that.

https://paste.dasharo.com/?eb1d2a8b2f6b9aed#4WPkBNCTFmZVUvvDShHKohdnK7Ri8JPwvrqYueFs54xd

We also may change the coreboot version from which cbfstool and smmstoretool is built to 24.08. The latter tool is already there in tree. Keeping some random hash does not look good.

Or, keep this hash just for the smmstoretool. Then clone coreboot separately to 24.02 and build cbfstool and vboot from it.

We have a long-term plan for a fully boostable toolchain for coreboot, and things will move forward (posting stuff on the relevant channels on the matrix).

My main concern is whether I need more time to hack the solution if the issues are not critical. Switching the container to 24.08 for cbfstool and smmstoretool is easy. My primary inquiry with this PR is: why does our vboot behave differently (installing /vboot), and why can we not use one from upstream? We didn't care much about dropping support for our vboot in version 1.1.2+.

@miczyg1
Copy link
Contributor

miczyg1 commented Oct 16, 2024

I ignored that; is this something to be concerned about? I never saw those scripts correctly running, so I can't say how bad it is.

It must work to get the vboot signing or key generation script to work IIRC

@miczyg1
Copy link
Contributor

miczyg1 commented Oct 16, 2024

My primary inquiry with this PR is: why does our vboot behave differently (installing /vboot), and why can we not use one from upstream?

i already said that we should use upstream now. As to why there is no /vboot in the container now, I have no idea. ALso no idea how it got there.

Signed-off-by: Piotr Król <piotr.krol@3mdeb.com>
Signed-off-by: Piotr Król <piotr.krol@3mdeb.com>
Signed-off-by: Piotr Król <piotr.krol@3mdeb.com>
@pietrushnic
Copy link
Author

@miczyg1

One thing started to work:
https://paste.dasharo.com/?058296e447b4da49#3yZCArUS8KH8GppEtm7wfr6LBRy1tpgFDdqdY2aMCCTY

TL;DR

(...)
Firmware Preamble:
  Size:                  1688
  Header version:        2.1
  Firmware version:      1
  Kernel key algorithm:  7 RSA4096 SHA256
  Kernel key version:    1
  Kernel key sha1sum:    e41246691ba7980a1b004231d68da9dd91809b7a
  Firmware body size:    0
  Preamble flags:        0
  Body metadata hash:    SHA256 cf1128770ba7e9c2abba12abda4d368dbc4d2c54596ed9a868188635133d4879
  Body metadata hash valid!
Body verification succeeded.

Unfortunately, while generating keys vboot, I still see the same errors despite everything being built from the same code. Do you have any other ideas on how to make vboot happy?

user@OST2-VM:~/training_materials/src/dasharo-tools$ ./vboot/generate_keys new_keys
creating arv_root keypair (version = 1)...
vb2_read_packed_keyb() - wrong key size 3272 for algorithm 7
No version file found. Creating default key.versions.
creating root_key keypair (version = 1)...
vb2_read_packed_keyb() - wrong key size 6392 for algorithm 11
creating firmware_data_key keypair (version = 1)...
vb2_read_packed_keyb() - wrong key size 3272 for algorithm 7
creating kernel_subkey keypair (version = 1)...
vb2_read_packed_keyb() - wrong key size 3268 for algorithm 7
creating kernel_data_key keypair (version = 1)...
vb2_read_packed_keyb() - wrong key size 1704 for algorithm 4
creating recovery_key keypair (version = 1)...
(...)

@miczyg1
Copy link
Contributor

miczyg1 commented Oct 16, 2024

Main question is if the signing works and one can boot the platform in normal mode (no recovery). If yes, then those warning can be probably ignored. It may be some bug in vboot.

Also could you try make_arv_root.sh and create_new_keys.sh directly? I am not sure why do you use those wrappers fro mdasharo-tools when the yare not included in this PR nor Dockerfile

@pietrushnic
Copy link
Author

Main question is if the signing works and one can boot the platform in normal mode (no recovery). If yes, then those warning can be probably ignored. It may be some bug in vboot.

I will test that and get back with results.

Also could you try make_arv_root.sh and create_new_keys.sh directly? I am not sure why do you use those wrappers fro mdasharo-tools when the yare not included in this PR nor Dockerfile

dasharo-sdk is the base for dasharo-tools; I'm using those because those were instructions I got; the whole topic of fixing dasharo-sdk came from the issue that I already have coreboot-sdk and needed dasharo-sdk, which added 7GB to my hwio2024 VM image size.

From what I understand now, I need no dasharo-tools. I can use scripts directly in dasharo-sdk, but dasharo-tools OTOH are handy; there is just one command that signs everything. I already have quite a lot of copy and paste. Adding more will slow down training progress and potentially lead to more errors. Wrappers are good, especially if they are available publicly, like in the case of dasharo-tools. There is a need for balance between going too deep in the guts or wrapping everything.

I will try a direct call to see if this gives any better results.

@miczyg1
Copy link
Contributor

miczyg1 commented Oct 16, 2024

Wrappers are good, especially if they are available publicly, like in the case of dasharo-tools. There is a need for balance between going too deep in the guts or wrapping everything.

I agree. However, when debugging issues, wrappers may obfuscate some hidden problems, thus I asked to use the vboot script directly

@pietrushnic
Copy link
Author

@miczyg1 @macpijan do we see any value in those modifications?

@miczyg1
Copy link
Contributor

miczyg1 commented Oct 30, 2024

I think we should proceed with merging and tagging the container with a new version. There are improvements worth merging

@pietrushnic
Copy link
Author

@macpijan I would like to use this code for PC Engines build and get rid of pce-fw-builder. Do you see any issues with merging this branch?

@pietrushnic
Copy link
Author

@macpijan ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants