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

Validate gpg signatures of downloaded nightlies #34598

Closed
alexcrichton opened this issue Jul 1, 2016 · 13 comments
Closed

Validate gpg signatures of downloaded nightlies #34598

alexcrichton opened this issue Jul 1, 2016 · 13 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@alexcrichton
Copy link
Member

Right now in rustbuild we download nightlies and verify the sha256, but we should also validate the gpg signature at the very least if a gpg tool is available. This probably also wants to coordinate with rustup which I believe also wants this behavior as well.

@alexcrichton alexcrichton added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 1, 2016
@nagisa
Copy link
Member

nagisa commented Jul 1, 2016

You should also check that the gpg tool in question has the key imported before doing the check and by no means attempt to import one automatically, without prompting the user.

@sorear
Copy link
Contributor

sorear commented Jul 3, 2016

Isn't the signature check redundant? Signature checks rely on hash checks internally, so the signature adds nothing over the hash check, no?

@nagisa
Copy link
Member

nagisa commented Jul 3, 2016

@sorear all hash check can do is checking whether data has changed since hash has been computed, however what it doesn’t and cannot do is verify that data (and signature itself) comes from the party who claims to be “The Rust Developers”. The evil 3rd party can easily tweak both data and hash making you think you have unchanged data, whereas they cannot tweak signature in the same manner (today, at least).

Hash check is redundant in case signature is checked, but not the other way around.

@sorear
Copy link
Contributor

sorear commented Jul 3, 2016

@nagisa I had assumed that the hash was hard-coded in the build script (and if you can change the hash, you can just as well remove the check entirely), but having checked the code I see that's wrong. Carry on then.

@ollie27
Copy link
Member

ollie27 commented Jul 5, 2016

Can we just add the checksums to the repo? That would give authenticity and allow for reproducible builds.

Isn't the current hash check completely pointless? The file is downloaded over HTTPS which provides integrity. EDIT: nevermind, it probably helps if the server sends the wrong file but the right sha256 file.

@alexcrichton
Copy link
Member Author

Discussed a bit on #32926, but it was intentionally avoided adding checksums to the repo primarily because:

  • They're cumbersome to maintain
  • It doesn't match what rustup is doing today
  • New platforms spring up and it's cumbersome to keep adding checksums for each new platform

Ideally both rustup and this download script would mirror strategies in how they download and verify toolchains. And yeah it's not entirely pointless as it fixes issues like various cache invalidation bugs, some proxy in the middle swapping something out accidentally, etc.

Also to be clear we'd probably commit into the repository the signing key and use that verbatim to verify downloads at least at the beginning.

@ollie27
Copy link
Member

ollie27 commented Jul 5, 2016

It will be cumbersome but I'm sure a script or two will help.

This is different to rustup because we know beforehand exactly which files we need so we may as well take advantage of that.

Keeping the checksums in the repo means we know we're getting the correct file, not just one signed with the correct key.

As an aside we also need to keep checksums of everything cargo downloads in the repo.

@est31
Copy link
Member

est31 commented Nov 25, 2016

GPG signatures are inferior to hashes security wise as GPG keys can be compromised and sign an "evil" second version, while the hashes verify that the version is the same for all people who have the same source code.

Plus, GPG signatures are not future proof, see quantum computers.

If maintaining is cumbersome, automation might be the solution. Maybe integrate this into cargo vendor somehow?

@nagisa
Copy link
Member

nagisa commented Nov 25, 2016

while the hashes verify that the version is the same for all people who have the same source code.

Provided you have a method to securely transfer the hash in question, which is has all the problems GPG has and probably some more.

@est31
Copy link
Member

est31 commented Nov 25, 2016

I was assuming that the hash was made part of the source code, and updated each time the version updates. Then they will have the same hash as they "have the same source code". Obviously, if the hash is downloaded together with the binary, there is no real security benefit in having it at all, as if the download channel is compromised, the hash can be swapped just as easily.

If you don't want to maintain a hash for each single platform in the source code, you can host a file containing all the hashes in the appropriate dir in static.rlo and then store a hash of that in the source. It will only have to update when the stage0 changes, which is fairly seldom.

@est31
Copy link
Member

est31 commented Nov 25, 2016

Apparently there are already channel.toml files on static.rlo, which contain hashes of builds for each platform...

@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 25, 2017
@Mark-Simulacrum Mark-Simulacrum added this to the impl period milestone Sep 15, 2017
@aturon aturon removed this from the impl period milestone Sep 15, 2017
@jyn514
Copy link
Member

jyn514 commented Jun 27, 2022

We currently store a hash of the signatures for the stage0 toolchain in src/stage0.json. I think that's sufficient verification; I'm not sure what the GPG key would add (and like @est31 mentioned, hashes give us the advantage that there's only one canonical hash, rather than the GPG key making multiple signatures).

@jyn514 jyn514 closed this as completed Jun 27, 2022
@est31
Copy link
Member

est31 commented Jun 27, 2022

Yeah this issue should probably have been closed when #88362 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

8 participants