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

Explain git commit signed commits and limitations (was Requesting Heads implement a signed release the way coreboot does) #1794

Closed
loftlifter31 opened this issue Sep 11, 2024 · 22 comments

Comments

@loftlifter31
Copy link

Is your feature request related to a problem? Please describe.
Github commits don't support user verifiable cryptographic signatures. A Heads user downloading the latest commit is trusting solely in TLS for download security. TLS as built on the current CA system is fatally flawed. CAs have been found to improperly issue certificates for domains they are not authorized for, issue certificates to untrustworthy servers, and to have lost control of their signing keys, as well as have multiple security failures. Certificate pinning can resolve some but not all of the issues underlying the CA dependent TLS security model.

Due to the broken nature of the TLS security model, a user downloading heads from a github commit is open to MITM attack where a malicious 3rd party can intercept the connection and impersonate github.com using a fraudulent certificate delivering a tampered version of the source code to introduce a backdoor or other vulnerability. The user has no way to verify the integrity of the download.

Describe the solution you'd like
Follow coreboot's example and issue releases with user verifiable pgp signature.

Describe alternatives you've considered
Could be as simple as once every couple of years upload the latest stable release of heads with a detached signature. Or as often as you like. Quarterly maybe. You could add a script to CircleCI to automatically sign builds using a private key local to that machine for nightly builds and for more major releases sign with the key of one of the principal developers. This adds an extra layer of security which would improve the overall security without much overhead or effort.

Additional context
Add any other context or screenshots about the feature request here.

@tlaurion
Copy link
Collaborator

tlaurion commented Sep 11, 2024

You could add a script to CircleCI to automatically sign builds using a private key local to that machine for nightly builds and for more major releases sign with the key of one of the principal developers.

Unfortunately not possible unless CircleCI has a private key to sign such trusted thing which wouldn't make it more trustable, key/secret leaks happen everyday and CI also made news leaking secrets.

Heads is rolling release based on reproducible build ideology. If one single line of code built was different, the rom produced would be different. What is missing today is the opposite of what you propose here @loftlifter31, based on the "distrusting infrastructures" principle: We miss a CircleCI orb that would ouput hashes of each produced ROM, just like embedded currently under zip files from CircleCI artififacts, that if unmatches would make the infrastructure distrusted (and what we currently wait for), where hashes.txt in build dir says where binary failed to be reproducible to help figure what went wrong.

Since #1661, every build for each PR/master made is reproducible, from local/CircleCI.

We could release code as tarballs as releases, but we couldn't release roms, since they contain blobs which we aren't permitted to redistribute. We can only redistribute blobs download scripts, which downloads and extract blobs and build reproducible roms. That was a 4 years ago and accomplished under #703. If we were to distribute roms as release, we might get the project shutdown without notice. This is legal grey area I won't play and worked really hard to get away of that redistribution issue doing exactly what is done today: a rolling release with CI producing ROMs, available for 30 days. And #1661 finally fixed the issue of being able to reproduce the same rom hashes for a commit forever, with the problem left of having mirrors that won't become inaccessible over time (seperate issue) to download tarballs which hashes are verified prior of decompression anyway, which satisfies also your requirements if I understand your concern well.

Quaterly signed releases: Who would sign them? coreboot releases are TBH, kinda random in terms of stability historically and picked/post-poned with now releases not being versioned but date based. I would sign code tarballs? Above my pay grade as well unfortunately. :(

Current model is downstream doing releases (Nitrokey/novacustom/Purism/others) choosing an upstream commit they tested well enough to decide that release meets their requirements in term of support, and challenges upstream in PR prior of those PR being merged, and test the CircleCI/local builds themselves and have their own release cycles.

That will be subject of my talk https://cfp.3mdeb.com/qubes-os-summit-2024/talk/HL9MSV/ sunday Sept 22 2024, and I will take this into account into the discussions. That will be filmed and will be available live and after the fact, if you want to be there and be part of the discussions as well.

Also see #1793 OP, which will also be covered in that talk/discussion to find the best way forward.


The alternative desired solution: If someone has experience and time to implement CircleCI orb to comment on PR upon successful builds and output direct links to zip and external roms URLS and final hashes of roms: that would be awesome.

I will tag this with Help wanted @loftlifter31 without renaming the issue until I feel you come to a common understanding here.

Side note: Libreboot borrowed Heads logic of blobs download script+extraction and changed their policies to minimal blobs policy; but they do not provide full roms, leaving it as an exercice to do by the end user; producing half-backed roms, left to be inected with blobs by the end user; and they do not try to fill the reproducible builds requirements AFAIK. Same for Skulls, same for every FOSS firmware projects I know, and same for coreboot not providing roms either. Signed tarballs? Why if we have reproducible builds and users can raise issues if there is a reproducible build issue? Not reproducible? You are targeted/Ci was targeted.


If what you want, still, is signed releases of tarballs, then that would "release of sourcecode could be done under a orb too, but I would have to download them all, and detach signed them myself and then add detached signatures myself: after the fact. There is no way I would have a CI have a private key of some sort, and CI history shows secrets having leaked in the past. This is not a question of if but when, again, and I would not go in that direction with public infrastructures we use (and kind of abuse their free tier with Heads).

And no, I checked: there is no way of interrupting a CircleCI build and login through SSH unless the build fails to pass scdaemon + socket back using local GPG keyring+ smartcard to safely do this. I can only log in through ssh into a build when builds fail, and those builds will never be marked as successful. If CircleCI was permitting otherwise, that would be a security risk for build tampering as well.


TLDR: Heads is rolling release backed with reproducible builds principles. If a user arrive to a different hash then what was built by CircleCI for same commit: that is a good reason to open an issue, today and since #1661.

If retrying to build a old commit, the end user is left to check which reproducible docker image was used under CircleCI config file for that build. If the user wants to recreate the docker image reproducibly, he has to go back in time and check from which commit that docker image was build, since there is still uninvestigated strings embedded in the docker image that are related to the commit that was used to build that docker image reproducibly for the commit id.

Otherwise, latest docker image will always permit to build master and PRs reproducibly, where CircleCI PINs to versioned docker images, not latest, for reproducible builds reasons.


@loftlifter31 you can close this issue and open another one if that satisfies your curiosity on the reasons why linuxboot/heads won't provide releases, even less sign them, anytime soon.

Meanwhile: Thanks for challengin ideas/code and your contributions, well appreciated.

@loftlifter31
Copy link
Author

@tlaurion Let's leave aside producing full roms with proprietary code blobs as that was not my request. I'm not sure if I'm understanding you correctly, but it sounds like you have 2 reasons: 1) You have a reproducible build system which you think is superior to signed releases. 2) Signing releases is "above your pay grade".

Responding to point 1, let me see if I understand this correctly. The idea is that a user will build locally using master and compare hashes for the build artifacts to hashes for the latest CircleCI build. If both hashes match, there was no tampering with the source code download. Is that right?

But if we assume a MITM attacker, why wouldn't the attacker just modify the CircleCI hashes in transit to match whatever the tainted code they injected will build as? Signed releases avoid this issue because the key used to sign the realease can be signed by other trusted parties in the web of trust. Also the signing key would only be downloaded once, possibly months or years in advance of the download of the signed code. An MITM attacker is highly unlikely to maintain persistent MITM access for months or years.

Regarding point 2, I'm afraid I don't understand this. "Above my pay grade" is military slang for division of responsibilities. An O-3 can sign off on things an O-2 can't. I'm not clear on how this relates to our current discussion. Is there someone "above" you in a heirarchy who would be permitted to sign code where you can't?

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 2, 2024

@tlaurion Let's leave aside producing full roms with proprietary code blobs as that was not my request. I'm not sure if I'm understanding you correctly, but it sounds like you have 2 reasons: 1) You have a reproducible build system which you think is superior to signed releases. 2) Signing releases is "above your pay grade".

Responding to point 1, let me see if I understand this correctly. The idea is that a user will build locally using master and compare hashes for the build artifacts to hashes for the latest CircleCI build. If both hashes match, there was no tampering with the source code download. Is that right?

Yes.

But if we assume a MITM attacker, why wouldn't the attacker just modify the CircleCI hashes in transit to match whatever the tainted code they injected will build as? Signed releases avoid this issue because the key used to sign the realease can be signed by other trusted parties in the web of trust. Also the signing key would only be downloaded once, possibly months or years in advance of the download of the signed code. An MITM attacker is highly unlikely to maintain persistent MITM access for months or years.

I don't understand your concern with git. Signed commits + TLS is enough.
Please do git log --show-signature from command line after having cloned repo.

Signed release is a way of distribution, and coreboot takes point in time and create a tarball, yes. But git is far better at giving origin of changes and better integrity+authenticity of changes through git that is, not tarballs downloads. Signed tarballs permits to make sure that the release was signed, yes, but nothing more.

Regarding point 2, I'm afraid I don't understand this. "Above my pay grade" is military slang for division of responsibilities. An O-3 can sign off on things an O-2 can't. I'm not clear on how this relates to our current discussion. Is there someone "above" you in a heirarchy who would be permitted to sign code where you can't?

No. That simply means that I invested a lot of time and resource into making builds reproducibles and following best practices for code signing and PR reviews, where tarballs would encourage users to use that instead of git. I want users to use git, use that reproducible docker image that CircleCI uses (see circleci config to see versioned image) and verify that buikds are reproducibles.

Signed tarballs would be inferior, yes. I do not plan to do this. If there is anything still unclear on the whys, please quote and reply. When I say above my pay grade, its because we don't want CI to have private keys (that could leak). I do not intend to provision private keys, and doing releases would just be point in time. I plan to do branches, though, and feature freezes.

More details in QubesOS mini-summit discussion at https://youtu.be/mAb_kHrF6SQ?list=PLuISieMwVBpL5S7kPUHKenoFj_YJ8Y0_d

@loftlifter31
Copy link
Author

Are you serious? The git log shows me there are more than 24 code signers on this project and not all of them even use the same keyserver to host thier keys. I spent a couple hours today hunting down and downloading as many keys as I could find and still couldn't get them all. And you expect users to review the entire git log for every commit to make sure the signature is valid? That is a huge investment in time. Users might as well audit every line of code personally while they are at it. And then we find little gems like this:

commit d6ef65c
gpg: Signature made Thu 26 Sep 2024 07:59:42 AM CDT
gpg: using RSA key 8735540225E98BDBC82491B41E9C3CA91AE25114
gpg: issuer "[email protected]"
gpg: Good signature from "Jonathon Hall [email protected]" [expired]
gpg: Note: This key has expired!
Primary key fingerprint: 8735 5402 25E9 8BDB C824 91B4 1E9C 3CA9 1AE2 5114
Author: Jonathon Hall [email protected]
Date: Thu Sep 26 08:59:01 2024 -0400

bin/fetch_source_archive.sh: Add storage.puri.st mirror

storage.puri.st is an alternate host name for storage.puri.sm, in case
there is another issue with the .sm name registration.

Signed-off-by: Jonathon Hall <[email protected]>

Seems several commits have been signed with an expired key. Should the code be trusted? A coreboot user can download the latest tarball and verify 1 signature for code integrity up to present date. A heads user has do hours of research and downloading to accomplish the same task. If you are so elitist you don't care about making code verification easy for users, that is your choice, but you are also creating an attack vector for users who can't be bothered to jump through all of these hoops required to verify code and simply trust what they are presented.

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 2, 2024

Are you serious? The git log shows me there are more than 24 code signers on this project and not all of them even use the same keyserver to host thier keys. I spent a couple hours today hunting down and downloading as many keys as I could find and still couldn't get them all. And you expect users to review the entire git log for every commit to make sure the signature is valid? That is a huge investment in time. Users might as well audit every line of code personally while they are at it. And then we find little gems like this:

commit d6ef65c
gpg: Signature made Thu 26 Sep 2024 07:59:42 AM CDT
gpg: using RSA key 8735540225E98BDBC82491B41E9C3CA91AE25114
gpg: issuer "[email protected]"
gpg: Good signature from "Jonathon Hall [email protected]" [expired]
gpg: Note: This key has expired!
Primary key fingerprint: 8735 5402 25E9 8BDB C824 91B4 1E9C 3CA9 1AE2 5114
Author: Jonathon Hall [email protected]
Date: Thu Sep 26 08:59:01 2024 -0400

bin/fetch_source_archive.sh: Add storage.puri.st mirror

storage.puri.st is an alternate host name for storage.puri.sm, in case
there is another issue with the .sm name registration.

Signed-off-by: Jonathon Hall <[email protected]>

Seems several commits have been signed with an expired key. Should the code be trusted? A coreboot user can download the latest tarball and verify 1 signature for code integrity up to present date. A heads user has do hours of research and downloading to accomplish the same task. If you are so elitist you don't care about making code verification easy for users, that is your choice, but you are also creating an attack vector for users who can't be bothered to jump through all of these hoops required to verify code and simply trust what they are presented.

@JonathonHall-Purism seems like you have to extend your public key expiration date and upload it back to github.

@loftlifter31 I don't know how to handle the rudeness in your tone.

Heads depends on modules/* which enforces tarballs checksum validation with pinned versions and known good hashes.

Otherwise, scripts in codebase under initrd/* all were part of github pull requests, merged by 3 maintainers in the past. @osresearch @tlaurion and @JonathonHall-Purism.

If I was to detach sign a branch, turned into a version (data tag Ala coreboot), and that being signed with my key, would that make you more confortable? Why? You say circle of trust, but would you sign it? After which verification? Why would you trust it more then git today where all PRs were scrutinized prior of merging? I'm not sure how to handle this issue or explain better the reasoning here.

I merge pull request after verification. @JonathonHall-Purism approves my PRs prior of merging/let me merge it. There were moments where Bugfixes needed to be merged fast because regressions were introduced (see bug tag) which still goes through PR, but as maintainer, I chose to merge prior of review because bug was intruduced prior of weekend and bug could result in a brick.

I understand your worries, but I'm not sure I understand why what you propose would change anything about them.

See hashes.txt of Circleci artifacts. Follow reproductivity notes. It's actually the consensual best approach to address your concern: you are sure that all the build process produced the same final hash of rom, where anything changing one byte of either cpio included in initrd (heads.cpio: scripts, modules.cpio: on demand loaded kernel modules, tools.cpio: modules binaries/libraries compiled -> stitches in initrd +kernel stitched in coreboot payload +coreboot) is reproducible. To my opinion, it cannot get better than that because of reproducible builds guarantees.

@JonathonHall-Purism maybe my wording sounds elitist but that is not my intention. If you have more reassuring words or agree that versioned code tarballs + detached signatures would be better (how) then please chime in. This is not dictatorship, just my opinion. As always, I welcome other opinions but my time is limited explaining past choices.

Yes there is collaborators to this project, we are lucky. Their code was proposed through PR, those were signed commits otherwise I created superseeding PRs co-authoring their commits with my gpg key after reviewing the code. I'm not sure I understand your criticism. Looking at coreboot codebase will show some commits that aren't signed either, but that doesn't matter because code review is what matters. This is also git, but you seem to trust this. You would be correct based on "more eyes" premises and you would be right. Same applies to linux kernel. This is a supply chain problem. Only thing that we can do here is what is done under modules/*, and sign commits for authenticity /integrity validation. Yes keys expires. That a reality of gpg when public keys have expiration dates. That doesn't mean the key wasn't expired when it was signed though: just that the key expired since signed.

@JonathonHall-Purism
Copy link
Collaborator

commit d6ef65c
gpg: Signature made Thu 26 Sep 2024 07:59:42 AM CDT
gpg: using RSA key 8735540225E98BDBC82491B41E9C3CA91AE25114
gpg: issuer "[email protected]"
gpg: Good signature from "Jonathon Hall [email protected]" [expired]
gpg: Note: This key has expired!
Primary key fingerprint: 8735 5402 25E9 8BDB C824 91B4 1E9C 3CA9 1AE2 5114
Author: Jonathon Hall [email protected]
Date: Thu Sep 26 08:59:01 2024 -0400

I extended this key back in June and uploaded it both to GitHub and keys.puri.sm. Are you still getting the expired key somewhere?

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 2, 2024

commit d6ef65c
gpg: Signature made Thu 26 Sep 2024 07:59:42 AM CDT
gpg: using RSA key 8735540225E98BDBC82491B41E9C3CA91AE25114
gpg: issuer "[email protected]"
gpg: Good signature from "Jonathon Hall [email protected]" [expired]
gpg: Note: This key has expired!
Primary key fingerprint: 8735 5402 25E9 8BDB C824 91B4 1E9C 3CA9 1AE2 5114
Author: Jonathon Hall [email protected]
Date: Thu Sep 26 08:59:01 2024 -0400

I extended this key back in June and uploaded it both to GitHub and keys.puri.sm. Are you still getting the expired key somewhere?

2024-10-02-151646

I cannot replicate ffrom GUI (expected). This means @loftlifter31 got it from somewhere where it was not updated redoing

I don't understand your concern with git. Signed commits + TLS is enough.
Please do git log --show-signature from command line after having cloned repo.

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 2, 2024

@JonathonHall-Purism Maybe we could public key in repo for others to use git log --show-signature without friction? And some gpg instructions in a README.md under that directory?

@loftlifter31 would that satisfy your requirement?

@JonathonHall-Purism
Copy link
Collaborator

@JonathonHall-Purism seems like you have to extend your public key expiration date and upload it back to github.

I did this back in June. If the old key is still up somewhere, I'd be happy to re-upload (asked above).

@JonathonHall-Purism maybe my wording sounds elitist but that is not my intention. If you have more reassuring words or agree that versioned code tarballs + detached signatures would be better (how) then please chime in. This is not dictatorship, just my opinion. As always, I welcome other opinions but my time is limited explaining past choices.

Signing a commit signs the entire content of the repo at that point in time, not just that change (that's how Git works). If you trust the signer of the last commit (which you'd have to do to trust a signed tarball), you don't have to check all the signatures.

For signing a tarball to have more value, the key(s) used would have to be handled in some way that makes it more trustworthy than either @tlaurion's or my own individual keys. E.g. in some way that requires both us to take action to sign a release, so one of us cannot create a signature on our own. (I'm not going to go down the rabbit hole of how this can be done, there are ways.)

Heads also does not currently have releases. Downstream distributions do (e.g. PureBoot - I run thorough tests for each release to ensure it is functional on all Purism devices, but Heads master will always be ahead as a result.)

While it is certainly possible to address each of those issues, I don't feel that either @tlaurion or I have spare bandwidth available to spend on this implementation at this time for free. If anyone is offering to construct this solution, such that the two of us would approve, generate keys, create releases following some as-yet-undefined criteria, and start signing, I'd be supportive. Or if someone is offering the funding for one or both of our organizations to execute this, I'd be happy to discuss a funded effort.

@JonathonHall-Purism
Copy link
Collaborator

@JonathonHall-Purism Maybe we could public key in repo for others to use git log --show-signature without friction? And some gpg instructions in a README.md under that directory?

You'd have a chicken-egg problem. How do you know the key in the repository is my real key? I guess because it's part of a signed commit or tarball. How do you check the commit/tarball signature? I guess with the key in the repository. How do you know that's my real key? [....etc., until heat death of the universe]

Obtaining the key from another source (GitHub, keys.puri.sm, or Purism's GitLab) builds confidence that it is my real key because an imposter would have to control one of those accounts to change it (or all of them).

Of course, those three links I just posted are all still anchored in TLS. But you can distribute that trust by checking all three of them, comparing the certs used, etc. If anybody would like to invite me to a key-signing party, let me know what kind of pizza you will have 😉

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 2, 2024

@JonathonHall-Purism : Well, this led to a small practical experiment, which shows some keys not being easily found publicly. Opened a draft PR for others to see results, showing keys that should be on gpg public servers but aren't and some that were that now aren't.

See #1804


@loftlifter31 :

ROMs contain blobs because those are needed to be flashed internally/externally AND tested by the community owning the boards, code reviewed in PR that produces those roms.

There is no release because Heads is the upstream Open Source project, and hosting those blobs (in releases) could have this project shutdown.

Downstream forks do what they feel best to do for their businesses and customers to thrive, and coreboot/forks are the ones dealing with blobs redistribution issues/whatnot, not Heads, which is user of those coreboot forks providing the blobs.

Heads is upstream and tries to stay neutral/dodge legal grey areas as much as possible providing code that will download/extract/generate blobs where needed, and tries to fix bugs as fast as possible with resources available so downstream can deploy in their systems following their release cycles, where downstream also reviews changes between releases and extensively test their releases, based on Heads upstream commit + their rebranding and their own coreboot fork changes.

If you can propose something better, feel free to open a PR proving your point. We will happily review, comment and discuss there. This is why I pointed you to the conferences I just did. You are not the first one asking this. But this is the same answer. This won't happen for before mentioned reasons.


@loftlifter31 feel free to close this issue/ comment/tag me wherever needed.

Once again, I didn't intend to shock before.

I'm here to collaborate for the the whole ecosystem benefit, following Pareto principle as much as I can, even more today since downstram releases are multiplying.

What you proposed is high effort for low gains.

I didn't want to sound rude. I restate that what we have today under Heads is way better compared to so many other open source firmware projects. It can be built locally, local scripts changes tested in qemu prior of doing a PR, built from your own Circleci instance if you follow your fork from there, where Ci uses reproducible tagged version docker images and produce the same rom image everywhere if you do so. Anywhere, as long as instructions to do so are read, followed and reproduced. As far as I know, no other project does that nor produce fully working rom images. Brick? Flash last used version externally. It cannot get simpler in my opinion.

Feel free to contribute any improvement you see beneficial to this project and the whole ecosystem.

I'm extensively replying here to you because of your interest for your galp5 system76 platform and the benefit that this could have if we collaborated better. As said in that draft PR, where I tried to show you in code where things should be addressed, the more contributions upstream the better for the whole ecosystem.

A lot of people ask for X or Y. But when asked back to do Z for X or Y to happen, people disappear and work in their own silo. This is not how open source project thrives.

Maintainers then feel they lost their precious time, even if they showed white paws and fixed things along the way, even if X or Y is not delivered yet... because Z never happened.

@loftlifter31
Copy link
Author

@JonathonHall-Purism I got your expired key from keys.openpgp.org. I didn't know any authoritative source and was searching blindly different keyservers.

@tlaurion Sorry. I misunderstood what you were asking me to do. I didn't realize I only needed to verify the latest commit signature to verify the whole codebase. I thought I would need to do each individually. Since heads uses a different system than other software projects people may be more familiar with, it may be helpful to other users who want to verify code to have some documentation on how to do that.

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 4, 2024

@tlaurion Sorry. I misunderstood what you were asking me to do. I didn't realize I only needed to verify the latest commit signature to verify the whole codebase. I thought I would need to do each individually.

Well. If you wanted to inspect the origin of each commit, you would have somehow to trace it back to the origin commiter and his signature at that time. But then again, Heads didn't forced signed commits until the repo moved under linuxboot org. For integrity validation of the codebase, this is different story with git which is superior for aforementioned reasons. If you aimed to build code yourself as opposed to download pre-built roms, and felt the need to verify commiter origin and signatures, this is the path you would need to follow, but your thought expirent led me to do PoC showing that some keys are not easily available, and I haven't digged deeper but I'm pretty sure some commits weren't signed in tree. But they were reviewed, and then you kinda have to trust the reviewers there. But you could trace those in git, which obvipily a tarball wouldn't permit.

Since heads uses a different system than other software projects people may be more familiar with, it may be helpful to other users who want to verify code to have some documentation on how to do that.

@loftlifter31 git is pretty generally used nowadays. If you mean something else, feel free to detail which parts you feel should be documented or better, submit a PR/seperate issue against heads-wiki.

There, commits don't need to be signed as opposed to code here. And issues there are linked to documentation, unless you think this should be part of the global README.md of this repo and not there.

Feel free to close this issue and open another one. We prefer one subject per issue. Others will find this one closed, if they follow documentation and contribution guidelines.

@JonathonHall-Purism
Copy link
Collaborator

@JonathonHall-Purism I got your expired key from keys.openpgp.org. I didn't know any authoritative source and was searching blindly different keyservers.

Thanks. I uploaded my current PGP public key there (same key, extended validity).

@loftlifter31
Copy link
Author

@tlaurion Perhaps I'm still not understanding this, but git doesn't seem to be verifying the downloaded code. git log --show-signature seems to be verifying the commit was signed when it was uploaded to github. It says nothing about the integrity of the codebase downloaded/cloned from github, so it doesn't seem to protect against a MITM attacker inserting malicious modifications into the code as it is dowloaded from github. Additionally some newer commits don't appear to be signed at all.
For example:

root@debian:/home/user/heads# git log --show-signature
commit 1683309f9c4c68c0ea0a96bc8cdaed6f36dbd967 (HEAD -> master, origin/master, 
origin/HEAD)
Author: Thierry Laurion <[email protected]>
Date:   Sat Sep 7 16:45:24 2024 -0400

    kexec-iso-init: Always show kernel arguments suppressions/additions override
s
    
    Signed-off-by: Thierry Laurion <[email protected]>

root@debian:/home/user/heads# git verify-commit 1683309f9c4c68c0ea0a96bc8cdaed6f36dbd967
root@debian:/home/user/heads# 

git verify-commit produces null output even though I have the signing key imported into my keystore.

@loftlifter31
Copy link
Author

I'll close the issue, but for the sake of posterity here is a list of signing keys in case anyone wants to follow up on this at some point. Also including a tarball of the keys I was able to download from public keyservers and a list of those not found, probbaly because the code signer never uploaded the public key to any keyserver. Presumably github would have a copy somewhere. Also I was surprised to discover github itself signs many of the commits, which seems to contradict your policy of don't trust the infrastructure.
keystoget.txt
keys.txt
keys.tar.gz
gpg.txt

@tlaurion
Copy link
Collaborator

tlaurion commented Dec 5, 2024

I'll close the issue, but for the sake of posterity here is a list of signing keys in case anyone wants to follow up on this at some point. Also including a tarball of the keys I was able to download from public keyservers and a list of those not found, probbaly because the code signer never uploaded the public key to any keyserver. Presumably github would have a copy somewhere. Also I was surprised to discover github itself signs many of the commits, which seems to contradict your policy of don't trust the infrastructure. keystoget.txt keys.txt keys.tar.gz gpg.txt

@loftlifter31 I did a similar effort under #1804 if you want to collaborate there into something that should exist in tree. A comment in a closed issue won't resolve the issue you opened at the first place and other people will ask the same question in the future.

Note:

Also I was surprised to discover github itself signs many of the commits, which seems to contradict your policy of don't trust the infrastructure.

The merge commits are signed by Github, while individual commiter's work is signed by collaborators. I or @JonathonHall-Purism or @daringer can merge PR (hit the merge button), which Github then adds its own commit as a merge commit, referring to the PR that was merged. Does that answer your concern/misunderstanding?

I didn't reply nor close this issue because I wasn't unsure what was left to answer here to adress possibile misunderstandings of how things work. As detailed as extensively as I could under #1804 there were commits that weren't signed prior of DCO enforced by linuxboot org (when heads was under osresearch), and old public key that cannot be fetched anymore and/or expired public keys that were used to validly sign commits in the past but which keys cannot be fetched anymore.
TLDR:

Also poked Dasahro team for them to publish their keys under 3mdeb/3mdeb-secpack#75
Nothing more I could do to try to explain, show things as they are without spreading FUD on my side.

@tlaurion tlaurion changed the title Requesting Heads implement a signed release the way coreboot does Explain git commit signed commits and limitations (was Requesting Heads implement a signed release the way coreboot does) Dec 5, 2024
@loftlifter31
Copy link
Author

The merge commits are signed by Github, while individual commiter's work is signed by collaborators. I or @JonathonHall-Purism or @daringer can merge PR (hit the merge button), which Github then adds its own commit as a merge commit, referring to the PR that was merged. Does that answer your concern/misunderstanding?

So, if I understand correctly, then code which is part of a PR that gets merged is signed by github's key. The PR should hopefully have been signed by the author prior to the approval. However from the end user point of view, how are they to verify the code delivered by github is trustworthy? What is to stop a bad actor who has control of the github signing key from inserting some backdoor into the code and signing it with the github key? I thought that is what don't trust the infrastructure means. Don't trust that github is not acting maliciously, rather verify code to the author.

Also there remains the issue that there are recent commits which remain unsigned. For example, commit 1683309

I did a similar effort under #1804 if you want to collaborate there into something that should exist in tree. A comment in a closed issue won't resolve the issue you opened at the first place and other people will ask the same question in the future.

You were able to locate more signing keys than I did, so I have nothing to add really.

@tlaurion
Copy link
Collaborator

tlaurion commented Dec 17, 2024

@loftlifter31 should we reopen/document it elsewhere then in a closed issue?

Also there remains the issue that there are recent commits which remain unsigned. For example, commit 1683309

True and I see what you mean now. Some commits are DCO validated as being "Singed-off-by" without being gpg signed.

Point from @JonathonHall-Purism is still valid though, but I guess we could force better checks by github-actions to prvent unsighed GPG commits:

Signing a commit signs the entire content of the repo at that point in time, not just that change (that's how Git works). If you trust the signer of the last commit (which you'd have to do to trust a signed tarball), you don't have to check all the signatures.


I guess the point of the discussion here is, when checked with git log --show-signature:

  • Some commits are signed-off-by and gpg signed with publicly available public keys
commit f4db4b791c7abfd049835cf9487c56152e825976
gpg: Signature made Mon 06 May 2024 03:22:12 PM EDT
gpg:                using RSA key 575F80D1599EA6D2C70AA9A19A53E1BB3FF00461
gpg: Good signature from "Insurgo Open Technologies/Technologies Libres (With key material backup) <[email protected]>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 0ACC B2B6 64EE 17E0 54B0  5E0B 4A38 DA8B EB9C 8396
     Subkey fingerprint: 575F 80D1 599E A6D2 C70A  A9A1 9A53 E1BB 3FF0 0461
Author: Thierry Laurion <[email protected]>
Date:   Mon May 6 14:12:05 2024 -0400

    README.md qemu.md + CircleCI: point to images for building and using nix developed created docker image
    
    - push v0.1.3 and have latest point to the same image, add repro notes inside of README.md
    - modify qemu.md to also refer to using docker images
    
    TODO: remove NIX_REPRO_NOTES prior of merging
    
    Signed-off-by: Thierry Laurion <[email protected]>
  • Some are signed-off-by and gpg signed but with expired keys
commit 65ca94b1843d370f5f4a1008f2e001815b05236d
gpg: Signature made Tue 11 Jun 2024 01:01:57 PM EDT
gpg:                using RSA key 8735540225E98BDBC82491B41E9C3CA91AE25114
gpg:                issuer "[email protected]"
gpg: Good signature from "Jonathon Hall <[email protected]>" [expired]
gpg: Note: This key has expired!
Primary key fingerprint: 8735 5402 25E9 8BDB C824  91B4 1E9C 3CA9 1AE2 5114
Author: Jonathon Hall <[email protected]>
Date:   Wed Oct 11 09:06:06 2023 -0400

    modules/coreboot: Update Purism coreboot to 4.22.01-Purism-1
    
    Update Purism coreboot to 4.22.01-Purism-1.
    
    Signed-off-by: Jonathon Hall <[email protected]>
  • Some commits are "signed-off-by" text, but not gpg signed
Author: Thierry Laurion <[email protected]>
Date:   Mon May 6 14:12:05 2024 -0400

    README.md qemu.md + CircleCI: point to images for building and using nix developed created docker image
    
    - push v0.1.3 and have latest point to the same image, add repro notes inside of README.md
    - modify qemu.md to also refer to using docker images
    
    TODO: remove NIX_REPRO_NOTES prior of merging
    
    Signed-off-by: Thierry Laurion <[email protected]>

  • some commits are "signed-off-by" but public keys are not published on public servers
commit 7323fef60432ddc2e41132b9905ffa0bce297265
gpg: Signature made Tue 10 Dec 2024 12:24:47 PM EST
gpg:                using RSA key 3A07364F010D7C71552FAFA687F342A528DFD8E5
gpg:                issuer "[email protected]"
gpg: Can't check signature: No public key
Author: Michał Kopeć <[email protected]>
Date:   Tue Dec 10 18:24:47 2024 +0100

    modules/coreboot: bump for MTL S3
    
    Signed-off-by: Michał Kopeć <[email protected]>


@loftlifter31 correct? But to be honest, I haven't thoroughly checked this i other repos up to now. Maybe I should but again, we rely on trusting the maintainer of the projects here and code review prior of merging, there is no escape on this. Signing is an attestation of integrity+authenticity which doesn't replace code editing: its a trust anchor (trust but still audit code) here, not a replacement of code inspection/review prior of merge. Not sure how to turn this issue into a solution, to be honest. And enforcing better checks/stop gaps would prevent collaboration further more. I welcome any solution here, reminding that a signed commit again, signs the whole code base on each commit because that's how git works.

@JonathonHall-Purism ideas on how to enforce better github actions checks on this? But then again, how to enfocr better checks without preventing collaboration or refusing commits that are gpg signed but where public keys are not part of public key servers as it is the case now?

@tlaurion tlaurion reopened this Dec 17, 2024
@tlaurion
Copy link
Collaborator

tlaurion commented Dec 17, 2024

I did a little experiement here, and pushed a change directly under heads repo without signing and forcing merge without review on README.md (doc related, but under a code repo: I exploited a law in my own contribution guidelines here to prove a point) on master a couple of minutes ago:

commit fa0f90cbecfd4b8e545c2a77bc5338e033652f2c (osresearch/master)
gpg: Signature made Tue 17 Dec 2024 11:23:30 AM EST
gpg:                using RSA key B5690EEEBB952194
gpg: Good signature from "GitHub <[email protected]>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 9684 79A1 AFF9 27E3 7D1A  566B B569 0EEE BB95 2194
Author: Thierry Laurion <[email protected]>
Date:   Tue Dec 17 11:23:30 2024 -0500

    Put usage of ./docker_repro.sh (docker images with docker-ce) first

As can be seen, signed by github, not "signed-off-by" me as part of git commit log, and therefore not signed with my dongle but by github, proving somehow that the changes were authenticated by login instead and made through GUI, not console based.

Authenticity+integrity enforced (accountability) is on github, not commiter+github when "signed-off-by" + dev public key fingerprint. If anything github would be accountable if pushed code for commit would be not what I have modified under this commit, but code reviewers inspecting git with git log --show-signature will see that its not with my security dongle nor public key fingerprint they are used to see. Point is: nobody would see this unless they deep dive into accountability. Not trusting the infra, but auditing still required on code to prove something was added by a commit or merge commit, where merge commit is just a commit log, accepting empty changes.

I guess the question is what do we want here, considering that signed tarball doesn't bring anything more to what git provides, but where signature forced by git itself seems to have failed me before, and where public key expiration happens by design if signers rotate their keys and leave the old one expiring: but all of this doesn't void the guarantee provided by any of those signatures here.

I would need to understand once more the left over concerns here to try to think of a solution (PR). Someone?

@loftlifter31
Copy link
Author

There are 2 issues to come my mind. 1) Jonathan states the most recent commit signs the entire tree. We are assuming the whole codebase up to that point has been checked out by the signer and they take responsibility for it when signing. But what about the case where the most recent commit is unsigned or is signed by github? How can the user be sure of the codebase integrity without having to trust infrastructure (ie github)?

As can be seen, signed by github, not "signed-off-by" me as part of git commit log, and therefore not signed with my dongle but by github, proving somehow that the changes were authenticated by login instead and made through GUI, not console based.

This seems like a problem to me. If changes can be authenticated by login rather than pgp key what is to stop an evil github admin who can impersonate any user on github from pushing bad code into repo as the repo owner? If they make a change to the public repo then maybe a legitimate code author sees the bad code and removes it. But what if they only make the change to the code as it is being delivered to the user? The code in transit rather than the code at rest.

If I as a user clone a repo there are a number of steps. 1. Make TLS connection to github. 2. Transfer some compressed byte stream to local storage. 3. unpack the compressed data into files and folders 4. verify integrity Yes?

Now we have a list of objects and commits and the actual files and folders those objects represent. So my question is what is to stop either github or a MITM attacker who can compromise the github TLS session with a fake cert from inserting a change into the codebase if they forge an unsigned commit? The attacker injects a fake commit into the object store and then links it to some backdoor code they have created. Is this change to code in transit detectable by the user if there is a forged unsigned commit added to the tree? Is there a way to verify the code which was downloaded is identical to the published code in the repo?

@tlaurion
Copy link
Collaborator

There are 2 issues to come my mind. 1) Jonathan states the most recent commit signs the entire tree. We are assuming the whole codebase up to that point has been checked out by the signer and they take responsibility for it when signing. But what about the case where the most recent commit is unsigned or is signed by github? How can the user be sure of the codebase integrity without having to trust infrastructure (ie github)?

As can be seen, signed by github, not "signed-off-by" me as part of git commit log, and therefore not signed with my dongle but by github, proving somehow that the changes were authenticated by login instead and made through GUI, not console based.

This seems like a problem to me. If changes can be authenticated by login rather than pgp key what is to stop an evil github admin who can impersonate any user on github from pushing bad code into repo as the repo owner? If they make a change to the public repo then maybe a legitimate code author sees the bad code and removes it. But what if they only make the change to the code as it is being delivered to the user? The code in transit rather than the code at rest.

If I as a user clone a repo there are a number of steps. 1. Make TLS connection to github. 2. Transfer some compressed byte stream to local storage. 3. unpack the compressed data into files and folders 4. verify integrity Yes?

Now we have a list of objects and commits and the actual files and folders those objects represent. So my question is what is to stop either github or a MITM attacker who can compromise the github TLS session with a fake cert from inserting a change into the codebase if they forge an unsigned commit? The attacker injects a fake commit into the object store and then links it to some backdoor code they have created. Is this change to code in transit detectable by the user if there is a forged unsigned commit added to the tree? Is there a way to verify the code which was downloaded is identical to the published code in the repo?

This is not how git works. Git was created to provide patches on top of a codebase. Each commit are patches. When tu download code from git, it download the whole codebase of all the changes that were ever done in that tree. Through tls or ssh which are just transport mechanisms, and then git validates the whole tree by pointing to its last commit.

I feel you ask me to explain to you both tls and internals of github and also pgp here?

Lets say you have git repo local. If you do git diff HEAD^ you compare tip to prior commit. If I work on the tree on its master state, I work commits, so patches, on top of tip, and a PR is set of those patches to be applied to tip of that branch. If there is conflict, git will report that the patches, or the commits, can't apply. That has nothing to do with tls, which makes sure that nobody can tamper between the two hosts that communicate, and injecting objects in that stream won't pass neither git delta alrorithms to see if the tree is valid when applying the patches on top of each other to arrive to the tree tip. Pgp signatures are there to attest of the origin of a change, and requiring signed commits is to be able to have accountability, and the person doing those patches to be applied to the tip downloads the same tree to work on it, and creates a branch where he pushes it's changes to be reviewed by others, which will download that branch, which is a série of patches applied on top of each other.

I am not really interested into debunking all the things arriving randomly in this repo for me to answer to any unrelated to Heads specifics.

The point is, if you build Heads with whatever commit you attempt to build, you should arrive to the same final image hashes on any computer. Yes, we love accountability offered by pgp, tls augmenting trust we have in the transport layer. But at the end of the day, github here, as any other git host, is hosting git and a GUI that permits us to have this discussion. And each pull request here is supposed to be reviewed prior of merge, both functionally and by code review, where the dev creating a PR does best effort to comply with repo requirements codestyle etc and the responsibility is left to the reviewers (this is where the most eyes the better) to fix possible errors prior of things getting merged. And sometimes, bugs are introduced and they are fixed later.

The simpler the changes, the better, in a PR. Each commit has a log that ideally explain what code changes this change does, assembled in a pull request which is logically created to target a new functionality. Unfortunately, sometimes, to arrive to the final goal, a lot of things needs to be changed to arrive to à final goal, and the biggest the pull request, the longer to review, and the longer to review, the most bitrot because tip of the repository will change, and patches cannot be applied cleanly anymore. And this is why pull requests get abandoned in the open source world, and why forks of repositories happen, too.

We mix a bit too much here.

My interest in this issue is what can be done better to enforce or bring light to commits that aren't signed, or to deal in a better way with public key that aren't publicly available from pgp key servers, in the scope of heads.

What you are bringing here are concepts of stream protection being possibly weakened, keys leakage by github and pull requests being possibly done by bad actors.

I do not remeber having merged once code that looked fishy, by whoever proposed it. But I also merged my own Bugfixes because things broke for everybody unless I did so.

Yes, someone could steal my credentials on GitHub somehow. Somehow create a PR in my name. Your theoretical attack would mean that code review from second person would blindfully trust that whatever I propose as patches in a PR is not reviewed by its value proposition nor functionality changes, but simply because it visibly came from me? Do I get this right? This could happen, but I highly doubt so. It coukd happen on GitHub, of course.

I don't have a clear answer to all your doubts. The answer to your concerns is trust but verify. But you, downloading code that was reviewed by others, I'm sorry but there is not much you can do but trust that there's more technical actually reviewed the code before doing their PR approval. The only thing you can do is get curious, subscribe to this repo, and read the discussions that happen here. Or subscribe to the channel heads-changes, and see each pull request, issue created, and have a way to engage with those developers if you have to.

But you, User of heads, can download code from github that was reviewed by others, and build a rom for a platform supported for a commit id identified by a hash, see that commit id in the rom name, and go to CirecleCI and see that the same filename is built, see the hashes.txt file alongside it, and see that the hashes matches for all components built up to the hash of the final rom. As a user, that's the contract that you can see respected, and open an issue in github if anything weird is seen there, if the final hash for the rom differs.

Considering all can do the same. Considering that in the past, some things were discovered non-reprducible but that this problem should be resolved forever now, reproducible builds was always the goal, was attain at numerous times in the past but switching to Nix as a build system to create reproducible docker images to build Heads should resolve this problem once and for all. You can rebuild an older commit and arrive to the same hashes that are available under Circleci.

I would suggest checking this avenue which should resolve most of your doubts. And roms under both local builds and circleci are packed under a zip file, which final sha256sum is part of the zip file as well, which should match on both Circleci and local builds for all commits of Heads.

Tldr

  • tls is transport protection against eavesdropping, key coukd leak from Microsoft but won't easily corrupt github guarantees
  • pgp helps to attest of tree integrity/authenticity with renewed validation in time. Goal would be that all commits related to development are signed.
  • git offers it's own guarantees for repo integrity, github is just where Heads is hosted, where it is agreed that github signing commits is suboptimal, where I still don't get, really, what it changes considering previous git point, and the fact that patchset are reviewed prior of being merged.

I woukd suggest creating smaller issues. This one is getting too big to be actionable anymore @loftlifter31 and I cannot answer all your questions.

We addressed that signed tar.gz archives wouldn't address your concerns in any way better then git. You distrust tls, I understand, but in the context of git, I disagree with your concerns. Signed commits are a plus, added to how git works. Woukd be ideal, but is not catastrophic in this context, if and only code review is actually a practice. Code signing is nowhere to be confounded with code review.

I will close this issue now. Smaller issues in the future. I won't be able to deal with big issues like this. Issues need to be actionable. The bigger they are, the lower the possibility to be transformed in pull request which should be the desired outcome of any created issue. Otherwise, please open discussions.

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

No branches or pull requests

3 participants