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

feature request: flatpak info should show full project revision (against collision attacks) #4634

Open
dc2xl opened this issue Oct 24, 2023 · 6 comments

Comments

@dc2xl
Copy link

dc2xl commented Oct 24, 2023

For security reasons I suggest that flatpak info shows the complete commit ID of the application (e.g. the revision in https://github.com/flathub/org.signal.Signal/) as a fast and convenient way to check that a certain build matches a certain revision.

Currently only a very short abbreviation (probably generated by the buildbot) is shown.
e.g. flatpak info org.signal.Signal shows
Subject: Update signal-desktop.deb to 6.34.0 (2ba12be1)

While collisions for full SHA-1 hashes are possible to generate, git makes the hashes unique by not accepting any commit for an already used hash, also if that commit gets deleted afterwards (if not getting garbage collected). To the best of my knowledge, on github that means one would have to delete and recreate the whole repository in order to get a different content for a certain revision in a certain repository. And in the flathub case that is under control of the flathub project.

AFAIU: the short hash does not have any security guarantees. Collisions for that short prefix should be trivial to generate (rather seconds on consumer graphic cards). If one had both revisions (the valid and the maliciuous one) in the workspace, one would get the 'ambiguous argument' warning. But if somebody does not have the complete repository history (e.g. on a shallow checkout) one won't have both revisions; I am also pretty sure deleted revisions do not get synced to a local workspace.

My current workaround:

N.B: i failed also with forum help to find an easier way to get the project revision from the flatpak revision.

Another possibility would be able to query the flatpak to show the manifest used by the build bot. I quickly googled but did not find a flatpak feature for adding more complex metadata/ to be shown in the info.

So I guess I suggest to change the subject generated to include the full hash.

@hfiguiere
Copy link
Contributor

The flathub project doesn't maintain the flatpak project.

You are barking at the wrong tree.

@dc2xl
Copy link
Author

dc2xl commented Oct 24, 2023

The flathub project doesn't maintain the flatpak project.

I am aware of that flathub just provides flatpaks/build infrastructure for flatpaks.

So you are saying that flatpak(-builder (?)) is actually setting the subject and flathub is not able to change it?

@razzeee
Copy link
Member

razzeee commented Oct 24, 2023

No, you should report at https://github.com/flatpak/flatpak

@bbhtt
Copy link
Contributor

bbhtt commented Oct 24, 2023

Builder doesn't pass a default subject, it's coming from buildbot https://github.com/flathub/buildbot/blob/df5a5dd91cc0a75e39915760a9a7aefca3dc74b6/master/buildbot/flathub_master.py#L1394, it's stripping got_revesion to 8 characters

@dc2xl
Copy link
Author

dc2xl commented Nov 23, 2023

@razzeee as the info/source found by bbhtt, the flathub/buildhub is truncating the revision before setting the subject. I can not find a way to open an issue against flathub/buildhub, so flathub/flathub is the right place? Should I open a PR without the truncation?

Builder doesn't pass a default subject, it's coming from buildbot https://github.com/flathub/buildbot/blob/df5a5dd91cc0a75e39915760a9a7aefca3dc74b6/master/buildbot/flathub_master.py#L1394, it's stripping got_revesion to 8 characters

@razzeee
Copy link
Member

razzeee commented Nov 23, 2023

so flathub/flathub is the right place?

yes, it's fine here then

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

4 participants