Skip to content

Conversation

@JonathonHall-Purism
Copy link
Collaborator

@JonathonHall-Purism JonathonHall-Purism commented Oct 28, 2023

Proof of concept for built-in ROM digest verification. This method is backward compatible with older Heads and also supports external/manual flashing with the same artifact (since the artifact is still just the ROM).

@tlaurion @daringer Following up on discussion in #1514, I propose this method instead of the NPF format. (Please check out my last few comments there for some findings on the NPF support.) Please let me know what you think 😁

I have not yet integrated this into the existing flashing scripts, it's just standalone scripts to calculate the digest or check it. Check out the usage notes in those scripts to try them out.

I added a "--replace" operation to cbfs to have a clear guarantee that it would replace the digest data in-place without altering any other part of the ROM, which is necessary for digest verification. A delete+add generally does work at this point since the file placement/header are exactly the same, but adding "--replace" is pretty easy and much more robust as tooling might change in the future.

We should be able to pretty easily calculate the digest during the build so no further manual steps are needed too.

edit: Whoops tagged the wrong person, had intended to tag @daringer re: #1485 which introduced NPF, my mistake @krystian-hebel

Add --replace command to cbfs in flashtools, ensures data is replaced
in-place without altering any other part of ROM.

Add calc_digest.sh to calculate or zero digest in ROM file.

Add check_digest.sh to verify integrity of ROM using built-in digest.

Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
@tlaurion
Copy link
Collaborator

tlaurion commented Oct 28, 2023

Ok so if I get this right, the idea would be to inject the calculated rom hash at build time, invalidating builds hashes.txt produced at build time?

@JonathonHall-Purism
Copy link
Collaborator Author

Ok so if I get this right, the idea would be to inject the calculated rom hash at build time

Yes, the idea is to insert this at build time.

invalidating builds hashes.txt produced at build time?

No, if the integrity digest is inserted before hashes.txt is calculated, hashes.txt will be fine.

@daringer
Copy link
Collaborator

Generally we have no strict preference for one or another approach. We chose the .npf idea because this delivers two features:

  • it is a plain zip so we can tell customers to unzip it in order to get the original .rom
  • one can verify the rom by using default tools (sha256sum + unzip)

Nevertheless, including the checksum info the cbfs is clearly the more backwards compatible way. Please correct me, but the cbfs-based approach does something like this to verify the image:

  • extract the checksum
  • copy the rom and nullify the checksum
  • calculate checksum for nullified-checksum-rom
  • compare with extracted checksum

This is quite some added complexity for the sake of backwards compatibility, which is not necessarily a bad thing. Without heads tooling it is not really possible to verify the integrity of the .rom - this feels like a drawback.

On the other side we could also improve the npf approach.

Under the line I personally cannot see that one approach is clearly "better" - for me this looks like a design-decision.

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 30, 2023

No, if the integrity digest is inserted before hashes.txt is calculated, hashes.txt will be fine.

Yes, but the rom won't match that verification checksum by itself and will require additional tooling/scripts to validate. Not against, but will require transaparency and Makefile modifications at least stating on CI and on builder's machines that the rom is modified with added cbfs file and that that cbfs file needs to be removed prior of checking rom against to get positive hash match. Just writing this convinces me this might be overcomplication, while hiding implementation details under Heads might be enticing, but a bit against Heads total transparency goal.

Will continue to think on this, but I would prefer a simpler approach

  • distribute zip files on CI with hash files in, modify npf-builder to generate zip files (npf are just zip files renamed with npf).
  • generalize npf codepath and erywhere npf is searched for/prompted for/messaging user for: just .npf/.zip those.
  • progress with this POC: Add tooling for built-in ROM digest verification #1518 concept in parallel.

But as stated under #1514, master state with this npf promoting without Heads providing such npf files is making me unconfortable.

I would simply continue to have CI provide rom+zip files and modify that PR so that npf/zip files are promoted as of that PR being merged, correcting master's state.

And I think continuing promoting rom files as of today with manual user verification works. People flashing #1514 merged result, when we agree, would permit people to flash zip/npf later on, where we should adapt our things to provide such files downloadable/buildable.

@JonathonHall-Purism
Copy link
Collaborator Author

Closing this in favor of #1526 - our support team agrees with plans to slowly migrate to the ZIP format, so the new PR improves that instead.

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.

3 participants