dasharo-issues icon indicating copy to clipboard operation
dasharo-issues copied to clipboard

Protectli Vault VP2420 Dasharo Release v1.1.0 - review

Open pietrushnic opened this issue 1 year ago • 17 comments

Below is a similar review to #408 - I tried to avoid duplication, but duplicated problems were extracted to have a public record of issues we continuously repeat.

  • [x] The new release introduces new features such as ... - low quality, better would be The new release introduces features like USB stack and SMM BIOS write protection enable/disable options.
  • [ ] SMM BIOS write protection enable/disable option - link leads to Dasharo Security Options, this feature is used every time we do update and improve the security posture of the device in a significant way, it deserves better write up and clean assignment of Dasharo PID including validations procedures.
  • [x] asciinema was posted from a private account instead of using 3mdeb one - we should get rid of asciinema and replace it with a generic signature validation procedure (manual) or DTS (automatic)
    • [x] For me procedure described in asciinema generate different results in a clean VM (I can't see 3mdeb Dasharo Master Key nor Protectli signing key signatures), --fetch-keys has other behavior than --import. Please check the gpg command output difference. We should use import instead of fetch-keys. It took me quite a lot of time to figure out why my output is different - let's save users time.
  • [x] link for downloads leads to 3mdeb.com instead of dl.3mdeb.com - maybe we didn't entirely switch yet, but changing will invalidate this newsletter unless we set redirection.
  • [x] release keys are generated as SCAE; there should be only a signing key here; others are not needed (https://github.com/3mdeb/3mdeb-secpack/pull/124/files)
  • [ ] no social media announcement scheduled
  • [ ] This submenu allows configuring UEFI Secure Boot functionality. By default, Dasharo firmware released after October 2022 has Secure Boot disabled default with no keys and certificates provisioned. link - as a user, I would like to know why.
  • [x] https://github.com/Dasharo/dasharo-issues/issues/420
  • [ ] Initial deployment requires flashrom compilation. The whole procedure could be more precise. First, it says Ubuntu 22.04 would be used, then it points to DTS, then asks to compile flashrom. We should support only DTS-based deployment.
  • [ ] https://github.com/Dasharo/dasharo-issues/issues/421
  • [ ] We have two issues with pfSense booting, but pfSense tests are PASS, and there is no indication of any problem associated with the given test. How is it possible? Either we indicated difficulties as part of the correct test scope issue or we extended the test scope and linked to the bug. I can't see how Dasharo Transparent Validation can grow without that strategy.
  • [x] Overview page could have more information e.g., referral link to shop, picture of hardware, where to report issues.
  • [x] Recovery page is under construction. Can't we reuse the recovery procedure from another platform?
  • [ ] Building manual procedure is misleading
    • [x] it doesn't tell how to obtain protectli_blobs.zip - from my understanding, it comes from some private repository; some readers may have access but not all of the, which means we should inform them at the beginning, not at the end of the manual
    • [x] If I'm a user who has access to that private repository, which version of blobs should I download? For tags downloading file name pattern is protectli-blobs-x.y.z.{zip,tar.gz}
    • [x] Obtain the Protectli blobs package and extract it to 3rdparty/blobs/mainboard directory (or keep it as protectli_blobs.zip file in the coreboot directory? The build script will remove it if needed in step 5). - why not provide command which will do that? It would avoid the problem of misinterpreting this description. For example, when I downloaded a zip file from the private repo and used a command from the script build.sh unzip protectli_blobs.zip -d 3rdparty/blobs/mainboard s/protectli_blobs.zip/protectli-blobs-1.0.18.zip/ this of course doesn't work because 3rdparty/blobs/mainboard will contain protectli-blobs-1.0.18 but it should contain files from inside. So there is one step more to accomplish the correct build tree mv 3rdparty/blobs/mainboard/protectli-blobs-1.0.18 3rdparty/blobs/mainboard/protectli
    • [ ] Build script throws following errors:
    (venv) [22:59:10] pietrushnic:coreboot git:(e36a117dd7cd*) $ ./build.sh vp2420
--2023-04-23 22:59:21--  https://cloud.3mdeb.com/index.php/s/FzF5fjqieEyQX4e/download
Resolving cloud.3mdeb.com (cloud.3mdeb.com)... 84.10.27.202
Connecting to cloud.3mdeb.com (cloud.3mdeb.com)|84.10.27.202|:443... connected.
HTTP request sent, awaiting response... 404 Not Found
2023-04-23 22:59:21 ERROR 404: Not Found.                                                                
                                                    
Archive:  protectli_blobs.zip            
  End-of-central-directory signature not found.  Either this file is not
  a zipfile, or it constitutes one disk of a multi-part archive.  In the
  latter case the central directory and zipfile comment will be found on
  the last disk(s) of this archive.
unzip:  cannot find zipfile directory in one of protectli_blobs.zip or
        protectli_blobs.zip.zip, and cannot find protectli_blobs.zip.ZIP, period.
  • :white_check_mark: build reproducibility confirmed with protectli-blobs-1.0.18.zip

pietrushnic avatar Apr 23 '23 19:04 pietrushnic

link for downloads leads to 3mdeb.com instead of dl.3mdeb.com - maybe we didn't entirely switch yet, but changing will invalidate this newsletter unless we set redirection.

Redirection is a part of the migration plan, so this point will be covered

artur-rs avatar Apr 23 '23 22:04 artur-rs

Recovery page is under construction. Can't we reuse the recovery procedure from another platform?

This is already scheduled after initial review

artur-rs avatar Apr 23 '23 22:04 artur-rs

Redirection is a part of the migration plan, so this point will be covered

How and where we can track it?

This is already scheduled after initial review

Can we link to PR?

pietrushnic avatar Apr 24 '23 10:04 pietrushnic

@pietrushnic I pushed some improvements regarding the blobs in building manual: https://github.com/Dasharo/docs/pull/570

macpijan avatar May 23 '23 13:05 macpijan

I expanded the recovery page: https://github.com/Dasharo/docs/pull/559

Pokisiekk avatar May 25 '23 05:05 Pokisiekk

link for downloads leads to 3mdeb.com instead of dl.3mdeb.com - maybe we didn't entirely switch yet, but changing will invalidate this newsletter unless we set redirection.

We do have redirection in place. That was the prerequisite of moving the files domain in the first place.

macpijan avatar May 31 '23 08:05 macpijan

The new release introduces new features such as ... - low quality, better would be The new release introduces features like USB stack and SMM BIOS write protection enable/disable options.

fix in review

Psotas avatar Jun 27 '23 12:06 Psotas

updated checkboxes (subissues) status in the first comment

artur-rs avatar Jun 28 '23 12:06 artur-rs

Overview page could have more information e.g., referral link to shop, picture of hardware, where to report issues.

Done: https://github.com/Dasharo/docs/pull/615

Mixss avatar Jul 21 '23 09:07 Mixss

asciinema was posted from a private account instead of using 3mdeb one - we should get rid of asciinema and replace it with a generic signature validation procedure (manual) or DTS (automatic) For me procedure described in asciinema generate different results in a clean VM (I can't see 3mdeb Dasharo Master Key nor Protectli signing key signatures), --fetch-keys has other behavior than --import. Please check the gpg command output difference. We should use import instead of fetch-keys. It took me quite a lot of time to figure out why my output is different - let's save users time.

@pietrushnic changed by this PR, new procedure is published here: Dasharo release signature verification.

BeataZdunczyk avatar Sep 07 '23 10:09 BeataZdunczyk

@BeataZdunczyk --import was used so it resolve the sub-issue mentioned above.

pietrushnic avatar Sep 07 '23 11:09 pietrushnic

The whole procedure could be more precise. First, it says Ubuntu 22.04 would be used, then it points to DTS, then asks to compile flashrom. We should support only DTS-based deployment.

Protectli uses their own flashli for deployments. We have not supported any Protectli platform in DTS. EIther we implement support for it, or simply add instructions how to flash the ROM from DTS shell...

miczyg1 avatar Apr 03 '24 12:04 miczyg1

This submenu allows configuring UEFI Secure Boot functionality. By default, Dasharo firmware released after October 2022 has Secure Boot disabled default with no keys and certificates provisioned

There is no such text in SB menu documentation anymore. Should we consider it resolved are UX improvements from Qubes OS Summit 2023?

miczyg1 avatar Apr 03 '24 12:04 miczyg1

Regarding the Proteclti blobs ZIP, now we have the following description in building manual:

Obtain the Protectli blobs package (only v1.1.0 or older):

Replace <PROTECTLI_BLOBS_REPO> with a a proper path to the repository in a form of: [email protected]. You should checkout to the same tag as in case aof the coreboot repository.


cd 3rdparty/blobs/mainboard/
git init
git remote add origin <PROTECTLI_BLOBS_REPO>
git fetch origin && git checkout protectli_vault_ehl_v1.1.0
cd -

miczyg1 avatar Apr 03 '24 12:04 miczyg1

@pietrushnic please take a look at above 2 comments and let me know which of them we can consider resolved.

Regarding the flashing, we need some improvements overall for Protectli, as we don't support DTS. Suggestions welcome.

miczyg1 avatar Apr 03 '24 12:04 miczyg1

low quality, better would be The new release introduces features like USB stack and SMM BIOS write protection enable/disable options.

I guess reviewers need to do a better job and overall do much more...

no social media announcement scheduled

Release process says:

[EX035] Social Media
Founder is responsible for this part of a process.   <-------------------------

Clients, community and end-users deserve high-quality information about what the new release is bringing and how it will improve their experience.

This process is performed according to Social Media Campaign Tracker [documentation](URL obfuscated)

@pietrushnic you may want to change this...

miczyg1 avatar Apr 03 '24 12:04 miczyg1

I guess reviewers need to do a better job and overall do much more...

Probably. Either reviewers would improve, or the initial writer would improve. Descriptive PRs and commits are the essence of good release. I guess if we have descriptive PR or commits, then ChatGPT should do relatively well in wrapping up.

Founder is responsible for this part of a process.

This means I will publish, but adding to the schedule is PM work @BeataZdunczyk cc

pietrushnic avatar Apr 10 '24 02:04 pietrushnic