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

Add missing maintainer keys to qubes-secpack #9199

Open
quantumpacket opened this issue May 8, 2024 · 7 comments
Open

Add missing maintainer keys to qubes-secpack #9199

quantumpacket opened this issue May 8, 2024 · 7 comments
Labels
C: other P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.

Comments

@quantumpacket
Copy link

quantumpacket commented May 8, 2024

The problem you're addressing (if any)

This was suggested in the Matrix room by @marmarek


@drivingreturns and I are putting together the Code Audit Program as discussed on the forum. As a first step, we want to be able to fully verify the authenticity of the source code.

According to the docs, we should be able to verify any signed tag. So we are retroactively verifying every tag to best ensure the integrity of the repositories. However, there are many public keys missing from the Qubes Security Pack, which hinders us from doing a full verification.

All these missing keys have been used to sign tags:

Abel Luck <abel/at/outcomedubious.im> | 8DA68FC57E6D2A54
Agnieszka Kostrzewa <a.kostrzewa/at/alx.pl> | 8828AF3D12F27588
Alex Tereshkin <alex.tereshkin/at/gmail.com> | 9C7ED5A5BCEB51E1
Alexander Tereshkin <alex.tereshkin/at/gmail.com> | 9C7ED5A5BCEB51E1
Alexandre Bezroutchko <abb/at/gremwell.com> | DB04F0670980ECF0
Andrew David Wong <adw/at/qubes-os.org> | 650EEB0985F48F785E9C61F5DB4DD3BC39503030
Andrew David Wong <adw/at/qubes-os.org> | 987806DC7544F57C
Andrew David Wong <adw/at/qubes-os.org> | DB4DD3BC39503030
Axon <axon/at/openmailbox.org> | 987806DC7544F57C
Bahtiar `kalkin-` Gadimov <bahtiar/at/gadimov.de> | 07799AE179ED4FD4
Demi Marie Obenour <demi/at/invisiblethingslab.com> | 8F3BD112BD78566DEABA584FCFA776934CEF6E3F
Elliot Killick <elliotkillick/at/zohomail.eu> | 018FB9DE6DFA13FB18FB5552F9B90D44F83DD5F2
Frederic Pierret (Epitre) <frederic.epitre/at/orange.fr> | 42FCB65E84F3267C
Frédéric Pierret (fepitre) <frederic.epitre/at/orange.fr> | C7A3BA7D2DDAF042C77CA4D21DABC232BE02201E
Frédéric Pierret (fepitre) <frederic.pierret/at/qubes-os.org> | 9FA64B92F95E706BF28E2CA6484010B5CDC576E2
Frédéric Pierret <frederic.epitre/at/orange.fr> | 1DABC232BE02201E
Jason Mehring <nrgaway/at/gmail.com> | 1BB9B1FB5A4C6DAD
Jean-Philippe Ouellet <jpo/at/vt.edu> | 4747332C27533622
Jean-Philippe Ouellet <jpo/at/vt.edu> | CF12F22491766DD94ED901834747332C27533622
Joanna Rutkowska <joanna/at/invisiblethingslab.com> | A1C341B8FF112935
Olivier MEDOC <o_medoc/at/yahoo.fr> | 2043E7ACC1833B9C
Patrick Schleizer <adrelanos/at/riseup.net> | CB8D50BB77BB3C48
Patrick Schleizer <adrelanos/at/whonix.org> | 6E979B28A6F37C43BE30AFA1CB8D50BB77BB3C48
Rafal Wojtczuk <rafal/at/invisiblethingslab.com> | 46D37762727FEAEC
Rafal Wojtczuk <rafal/at/invisiblethingslab.com> | D46BAD0DF9EE5F59
Reg Tiangha <reg/at/reginaldtiangha.com> | B56BF4C0A24ED097
Tomasz Sterna <tomek/at/xiaoka.com> | 258651A998DF8E52
Tomasz Sterna <tomek/at/xiaoka.com> | 3E9213B0E15BDCC4
Wojciech Zygmunt Porczyk <wojciech/at/porczyk.eu> | 0E0BCB3897BA8CC9
Wojciech Zygmunt Porczyk <wojciech/at/porczyk.eu> | 5BA92D6DAF975A7D
Wojciech Zygmunt Porczyk <woju/at/invisiblethingslab.com> | 70483D0A15CE40BF
Wojtek Porczyk <woju/at/invisiblethingslab.com> | 500BF9EC3C677AEC
Wojtek Porczyk <woju/at/invisiblethingslab.com> | 70483D0A15CE40BF
marmot1791 <marmot1791/at/protonmail.com> | E6D399BF7AD6027DC7E030893E88197B2E2EAD6C

The solution you'd like

All keys that have been used to sign a tag in any of the QubesOS repositories should have their public key added to the Qubes Security Pack. Not all of them are/were core developers as far as I am aware, so those keys should probably go in a separate directory. Some keys are still active, others are expired/retired.

I would also like to suggest this be policy going forward. Such that a tag should not be signed with a key unless that key is in the Qubes Security Pack. Otherwise, it puts the signed tag in a less than ideal state for verification. One must now find the correct key, verify it, etc. How do we know the QubesOS team trusts this key for tagging?

The value to a user, and who that user might be

The user has the convenience to trust all the verified keys in the Qubes Security Pack, opposed to trying to painstakingly locate them one by one, verifying the fingerprints, etc. This allows the user the ability to verify any tag in any of the repositories. This is vital for a source code audit as it will prevent auditors from potentially being served an alternate version of the code signed by an unknown key, as a common threat model among QubesOS users includes Microsoft and state actors.

@quantumpacket quantumpacket added P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. labels May 8, 2024
@marmarek
Copy link
Member

marmarek commented May 8, 2024

qubes-secpack is not a replacement for keyservers (however broken they are). It's a place for keys that should be trusted to sign various official releases. Various repositories do contain commits (or tags) signed by other people (because we do accept contributions!), but that doesn't mean you should trust any of them. qubes-builder takes care of verifying if there is a tag signed with a trusted key. Especially, it is not the goal to have key for every contributor in the qubes-secpack repo. To put it differently, qubes-secpack should include keys of all maintainers, not necessarily all developers.
In practice, I think the only missing keys are of @fepitre (maintainer of contrib-related repositories), @adrelanos (maintainer of template-whonix repository)

@quantumpacket
Copy link
Author

quantumpacket commented May 8, 2024

Sorry if I'm misunderstanding the process here...

qubes-builder takes care of verifying if there is a tag signed with a trusted key

I see here that it is checking the maintainer's keys in the committed keyring.

However, for tags signed by developers, where in the verification chain is QubeOS vouching that these signed tags and the code being committed has been reviewed by QubesOS? Shouldn't the tag be signed by a trusted source (QubesOS) as final approval that the code is legit?

Especially, it is not the goal to have key for every contributor in the qubes-secpack repo.

Why are contributors signing tags, opposed to just signing commits? I would expect the tag signing to be done by one of the trusted keys.

In your Matrix response you said:

commits are signed by their authors, which may or may not be part of the core team; what you should use for sources verification is signed tag, which should be signed by one of the core team member, with a key included in the secpack

So only trust tags signed by a core team member? Any tags after or prior of such a trusted tag should be considered untrusted? If so, we will advise the auditors to only get the latest trusted tag to audit.

@drivingreturns
Copy link

Have I understood correctly that qubes-builder does check for trusted keys, but that the secpack is also missing a couple of active maintainer keys? Does this block them from releasing in practice? If the process as documented isn't what is actually in use, that would already be a useful audit finding.

I understand from this that there's no concern over the existence of unverifiable signed tags, just that they shouldn't be trusted.

@andrewdavidwong andrewdavidwong changed the title Add Missing Public Keys To Qubes SecPack Add missing maintainer keys to qubes-secpack May 8, 2024
@marmarek
Copy link
Member

marmarek commented May 8, 2024

Why are contributors signing tags, opposed to just signing commits? I would expect the tag signing to be done by one of the trusted keys.

See https://www.qubes-os.org/doc/code-signing/

However, for tags signed by developers, where in the verification chain is QubeOS vouching that these signed tags and the code being committed has been reviewed by QubesOS? Shouldn't the tag be signed by a trusted source (QubesOS) as final approval that the code is legit?

The benefit of tags is that a single commit can have multiple of them. This allows multi-stage process - for example one developer writes something and signs it, another reviews it and adds its own tag and finally maintainer reviews too (or trust the review of the other developer and just verifies its tag) and adds its own tag. All tags are left in place to keep evidence, but for the actual release only the maintainer tag counts.

Have I understood correctly that qubes-builder does check for trusted keys, but that the secpack is also missing a couple of active maintainer keys? Does this block them from releasing in practice? If the process as documented isn't what is actually in use, that would already be a useful audit finding.

You can see which key is used where here (for R4.2). qubes-builder repo has its own copy of the keys for easier bootstrapping

@quantumpacket
Copy link
Author

but for the actual release only the maintainer tag counts

That makes a lot more sense. Thanks for clearing that up.

@drivingreturns
Copy link

Query on If you come across a repo with any unsigned commits, you should not add any of your own signed tags or commits on top of them unless you personally vouch for the trustworthiness of the unsigned commits. (source): do maintainers ever trust that a developer has done this? If not, is this guidance needed? If so, should it also be added to https://www.qubes-os.org/doc/code-signing/ ? Without knowing how maintainers treat this (is this also documented somewhere?), I see potential for commits to go unreviewed here.

@marmarek
Copy link
Member

marmarek commented May 8, 2024

Any pull request is reviewed based on previous trusted version (regardless of what github ui shows), so if somebody include some extra commits found somewhere, they will get reviewed in the process too (and if they weren't included intentionally, that will get noticed there).
But also, the convenient way of getting sources is to use qubes-builder, that does verify relevant tags

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: other P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.
Projects
None yet
Development

No branches or pull requests

4 participants