lib/repo-pull: Allow the keyring remote to be overridden #1810
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Don't merge this until the Flatpak side of it has been implemented |
|
☔ The latest upstream changes (presumably 6e935ee) made this pull request unmergeable. Please resolve the merge conflicts. |
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 9, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in flatpak considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "override-keyring-remotes" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In those latter cases the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We only use the "override-keyring-remotes" option in repo_pull() even though ostree_repo_pull_from_remotes_async() is also called in flatpak_dir_do_resolve_p2p_refs() because (a) in that case it's difficult to know if a result contains only refs that came from one configured remote and (b) it would be futile to prevent those commit metadata pulls because the later ostree_repo_find_remotes_async() call in repo_pull() would still pull the commit metadata from the malicious remote (but only the metadata). Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. Therefore it might make sense to have other mitigations for this issue beyond this commit. Partial fix for flatpak#1447 [1] ostreedev/ostree#1810
4f3e8e7 to
e823beb
Compare
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 14, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in flatpak considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. Fixes flatpak#1447 [1] ostreedev/ostree#1810
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 15, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in flatpak considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
Currently the P2P code requires you to trust every remote you have configured to the same extent, because a remote controlled by a malicious actor can serve updates to refs (such as Flatpak apps) installed from other remotes.[1] The way this attack would play out is that the malicious remote would deploy the same collection ID as the victim remote, and would then be able to serve updates for it. One possible remedy would be to make it an error to configure remotes such that two have the same collection ID but differing GPG keys. I attempted to do that in Flatpak[2] but it proved difficult because it is valid to configure two remotes with the same collection ID, and they may then each want to update their keyrings which wouldn't happen atomically. Another potential solution I've considered is to add a `trusted-remotes` option to ostree_repo_find_remotes_async() which would dictate which keyring to use when pulling each ref. However the ostree_repo_finder_resolve_async() API would still remain vulnerable, and changing that would require rewriting a large chunk of libostree's P2P support. So this commit represents a third attempt at mitigating this security hole, namely to have the client specify which remote to use for GPG verification at pull time. This way the pull will fail if the commits are signed with anything other than the keys we actually trust to serve updates. This is implemented as an option "ref-keyring-map" for ostree_repo_pull_from_remotes_async() and ostree_repo_pull_with_options() which dictates the remote to be used for GPG verification of each collection-ref. I think specifying a keyring remote for each ref is better than specifying a remote for each OstreeRepoFinderResult, because there are some edge cases where a result could serve updates to refs which were installed from more than one remote. The PR to make Flatpak use this new option is here[3]. [1] flatpak/flatpak#1447 [2] flatpak/flatpak#2601 [3] flatpak/flatpak#2705
c363334 to
eee6625
Compare
pwithnall
approved these changes
Feb 18, 2019
Is that this flatpak PR? It’s currently marked as WIP — so presumably this OSTree PR shouldn’t be merged yet? Maybe mark this one as WIP too if that’s the case. |
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 21, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in flatpak considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 21, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in flatpak considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 25, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in flatpak considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
Yes, that's the one, and it's now ready for review. |
cc @alexlarsson |
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 25, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in flatpak considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 27, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in OSTree considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Feb 28, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in OSTree considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
Right...I think I recall discussion of this in the original P2P code. I guess a lot of the complexity comes down to supporting key rotation. If the key can be rotated then it can't be used as a unique ID. (Perhaps what we should have done is defined "collection ID" = "original GPG key fingerprint" and required any later keys to be signed by the first one or something) I didn't look in detail at your flatpak PR but the idea is we're basically overriding with a "trusted remote"? And "trusted remotes" are ones defined statically and not discovered via P2P? |
Yes, the remotes passed to libostree by Flatpak in |
|
🙀 |
|
☀️ Test successful - status-atomicjenkins |
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Apr 2, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in OSTree considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
Apr 15, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in OSTree considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
mwleeds
added a commit
to mwleeds/flatpak
that referenced
this pull request
May 7, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in OSTree considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak#1447 [1] ostreedev/ostree#1810
rh-atomic-bot
pushed a commit
to flatpak/flatpak
that referenced
this pull request
May 9, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in OSTree considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes #1447 [1] ostreedev/ostree#1810 Closes: #2705 Approved by: alexlarsson
mwleeds
added a commit
to flatpak/flatpak
that referenced
this pull request
May 9, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in OSTree considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes #1447 [1] ostreedev/ostree#1810 Closes: #2705 Approved by: alexlarsson (cherry picked from commit caedda5)
mwleeds
added a commit
to endlessm/flatpak
that referenced
this pull request
May 9, 2019
With Flatpak you should only have to trust each remote to provide good updates for the apps provided by it. However the P2P support in OSTree considers each remote to be equally trustworthy, which opens a possible attack vector. For example if I have a flathub remote configured and apps installed from it and I also have a remote "sketchy-remote" configured which I have one app installed from, I expect the Flathub apps to update from Flathub (or to update from LAN/USB sources with Flathub GPG signatures) and not from the sketchy-remote. The way this attack would play out is that the sketchy-remote would deploy the same collection ID as the victim remote (in this case org.flathub.Stable) in order to serve updates for it. So this commit mitigates the issue by using the new "ref-keyring-map" option added to libostree[1], which means that pulls of updates to Flathub apps will always be verified using the Flathub GPG keyring, even if they're coming from another source like another configured remote or a LAN/USB source signed with the malicious remote's keyring. In the latter case the pull from the malicious source will fail, and flatpak should then do a successful pull from a legitimate source. We use the "ref-keyring-map" option in both flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use it in the latter place the ref could be resolved to the malicious commit (which would be checked with the malicious keyring), and then in repo_pull() we would try unsuccessfully to pull the malicious commit from the legitimate remote. Since pulls into the system installation already use the correct remote's keyring (see the use of ostree_repo_verify_commit_for_remote() in flatpak_dir_pull_untrusted_local()) this mitigation is only needed for per-user installations (or other scenarios that circumvent the system helper). It's also only needed since the commit "dir: Fix an edge case of resolving collection-refs" because before that commit this attack vector wasn't exploitable. Unfortunately this implementation is not perfect, because there's not always a one-to-one mapping between configured remotes and GPG keyrings. On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes added by Flatpak. However presumably you would only add a keyring to a global directory if you trust it to the same extent as the others. A subsequent commit will add a unit test for this. Fixes flatpak/flatpak#1447 [1] ostreedev/ostree#1810 Closes: #2705 Approved by: alexlarsson (cherry picked from commit caedda5)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the P2P code requires you to trust every remote you have
configured to the same extent, because a remote controlled by a
malicious actor can serve updates to refs (such as Flatpak apps)
installed from other remotes.[1] The way this attack would play out is
that the malicious remote would deploy the same collection ID as the
victim remote, and would then be able to serve updates for it.
One possible remedy would be to make it an error to configure remotes
such that two have the same collection ID but differing GPG keys. I
attempted to do that in Flatpak[2] but it proved difficult because it is
valid to configure two remotes with the same collection ID, and they may
then each want to update their keyrings which wouldn't happen
atomically.
Another potential solution I've considered is to add a
trusted-remotesoption to ostree_repo_find_remotes_async() which would dictate which
keyring to use when pulling each ref. However the
ostree_repo_finder_resolve_async() API would still remain vulnerable,
and changing that would require rewriting a large chunk of libostree's
P2P support.
So this commit represents a third attempt at mitigating this security
hole, namely to have the client specify which remote to use for GPG
verification at pull time. This way the pull will fail if the commits
are signed with anything other than the keys we actually trust to serve
updates.
This is implemented as an option "override-keyring-remotes" for
ostree_repo_pull_from_remotes_async() which dictates the remote to be
used for GPG verification of each OstreeRepoFinderResult. If set, this
value will become the value of the "override-remote-name" option of
ostree_repo_pull_with_options(). Currently "override-remote-name"
affects only local pulls and in that case affects both the name to be
used in the refspec and the remote used for GPG verification. This
commit changes the meaning so that it dictates what remote to use for
GPG verification regardless of whether the pull is local or not. This
seems cleaner than adding a new option and shouldn't be a backwards
compatibility concern because it only makes sense to use a remote in a
refspec name if it can also be used for GPG verification.
This solution is still not perfect because (a) commit metadata will have
already been pulled from the malicious remote during the execution of
ostree_repo_find_remotes_async() which would not be removed on pull
failure and (b) if a global keyring directory is being used rather than
a separate keyring for each remote the pull will still succeed even if
it's not from the intended remote.
[1] flatpak/flatpak#1447
[2] flatpak/flatpak#2601