rust-lang / rustup

The Rust toolchain installer
https://rust-lang.github.io/rustup/
Apache License 2.0
6.19k stars 893 forks source link

Tracking: Simple PGP signature verification #2028

Open kinnison opened 5 years ago

kinnison commented 5 years ago

In order to implement simple signature verification for rustup to an extent that we're confident that it's good to proceed to thinking more about trust models, we need:


If anyone has ideas on what else needs doing, please comment below and I shall endeavour to keep this tracking issue up to date with the progress toward simple signature verification support.

dignifiedquire commented 5 years ago

I maintain https://github.com/rpgp/rpgp which should be easy to use for this as well and is pure rust, without c dependencies.

kinnison commented 5 years ago

Thanks @dignifiedquire I should indeed have a go with that in case I can get going more quickly than waiting for Sequoia to add Windows support which is currently blocking that prototype. When you say there's no C deps, is that definite?

dignifiedquire commented 5 years ago

When you say there's no C deps, is that definite?

Yes, the goal is a fully portable version (it is currently deployed through an app using it on ios,android, windows, macos and various unix distros), with all crypto being either pure rust, with possible optimizations in assembly.

dignifiedquire commented 5 years ago

Also probably of interest, the first security audit was just finished, with some minor things still on my list to fix, but no major issues found.

If you have a sketch of what you want this feature to exactly do, I'd be happy to help fill in the rpgp specific details, as the docs are not that amazing quite yet

kinnison commented 5 years ago

I threw together a very dodgy hack to try with Sequoia here: https://github.com/kinnison/rustup.rs/tree/signed-channels

If you wanted to have a go at an equivalent using rpgp, or at least point me at appropriate stuff that'd be awesome. Sequoia was easy for me to get going with because their sqv example did everything I needed, so I could crib from it.

dignifiedquire commented 5 years ago

I added some basics here: https://github.com/dignifiedquire/rustup.rs/tree/signed-channels-rpgp

Haven't had time to implement proper verification of the signature being on its own. Question, how are the signatures generated at the moment, just so I know what format this is expected to be in. (if you have an example one that would be great)

dignifiedquire commented 5 years ago

Update: the branch now includes a the code to validate signatures and a test validating some of the official signatures

kinnison commented 5 years ago

Just to ensure things are linked:

jiegec commented 4 years ago

Hi, will there be a switch(or further, make it the default behavior) to force signature validation?

pietroalbini commented 4 years ago

That's eventually the goal, but we want to have a mechanism to rotate the signing keys before we enable validation by default, otherwise older rustup clients might broke once we need to rotate keys.

Work on this started, but some of the people working on it got busy with other priorities.

therealbstern commented 3 years ago
  • [ ] Sign rustup releases

Isn't this step done too, if rustup is already validating the asc detached signatures? Or does this mean releases of rustup proper, not of the rust packages?

Anyway, this is awesome, I've been watching this for a while via #241, so I'm chuffed that this is working.

kinnison commented 3 years ago

@therealbstern No we don't sign rustup releases yet, only rust channels. We're working on some basic signature support by dint of unifying how rustup and rust releases are done to some extent, but that's going to be a long time in the making.

morriswinkler commented 2 years ago

@pietroalbini

What's the current status on that?

I am seeing download.rs#L231, so is this code still dormant.

That's eventually the goal, but we want to have a mechanism to rotate the signing keys before we enable validation by default, otherwise older rustup clients might broke once we need to rotate keys.

Why not just add a command line switch to load a new gpg key from file, I would expect that if the rust gpg key is replaced you probably only wan't to use it once to update rustup itself.

In any case initial setup with rustup-init needs to be verifiable with a separate sourced or cached gpg key.

est31 commented 2 years ago

Why not just add a command line switch to load a new gpg key from file

IMO that's the best solution to this. Key rotation can be done via adding support for multiple public keys to rustup clients, and then using the new key after a window period is over, say 4 years or something. For users on old keys, there could be a warning after 3 years that the key is going to be deprecated soon and that they should update their rustup.

For rustup users on too old versions, you can ask them to call rustup with a special env var, e.g. RUSTUP_ALLOW_PGP_FINGERPRINT=e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 rustup self update. The public key can be downloaded by rustup, all you need is the fingerprint.

morriswinkler commented 2 years ago

Ok I have been looking a bit further into the current implementation.

There is a code block in src/config.rs#L273-L297 that loads the embedded pgp key found in this repo under src/rust-key.pgp.ascii then checks for the environment variable RUSTUP_PGP_KEY and adds it as a second key and then checks the settings.toml for additional keys that are all added.

The verify step in src/dist/download.rs#L231 is currently dormant but if enabled would verify a downloaded tar.gz from https://static.rust-lang.org/... against all the keys and if one key checks out it passes.

I would rather like to load only one key from an external file, so I propose to add two more environment variables:

First environment variable RUSTUP_SINGLE_PGP_KEY holding a filepath to a pgp key and if set will load that key into the vector pgp_keys in src/config.rs and purge all other keys from it.

A second environment variable RUSTUP_PGP_VERIFY that enables the verification step in src/dist/download.rs#L231.

Here is an example of how that can be used in a Dockerfile then.


ENV RUST_VERSION 1.58.1
ENV RUSTUP_VERSION 1.24.3
ENV RUSTUP_INIT_SHA256 3dc5ef50861ee18657f9db2eeb7392f9c2a6c95c90ab41e45ab4ca71476b4338

ENV RUSTUP_SINGLE_PGP_KEY /my/cached/pgp-key.ascii
ENV RUSTUP_PGP_VERIFY

RUN TMPDIR=`mktemp -d` && cd ${TMPDIR} \
        && curl --proto '=https' --tlsv1.2 -OO -sSf https://static.rust-lang.org/rustup/archive/${RUSTUP_VERSION}/x86_64-unknown-linux-gnu/rustup-init \
        && echo "${RUSTUP_INIT_SHA256} *rustup-init" | sha256sum -c - \
        && chmod +x rustup-init \
        && ./rustup-init --default-toolchain=${RUST_VERSION} -y \
est31 commented 2 years ago

To support key rollovers in a smooth way, you either need support for multiple signatures of the payload, or support for multiple keys at the same time. According to @morriswinkler 's research (very thankful for pointing to the relevant location in the code!) the current implementation supports the latter.

If you allow multiple public keys, there should still be a way to turn off trust for builtin keys, which is useful e.g. when the key has been compromised (think of a hacker leaking the private key on the internet) or your migration has ended but you still can't use newer rustup versions for some reason.

I'd recommend adding an env var RUSTUP_NO_BUILTIN_PGP_KEYS that distrusts the builtin keys, but the approach that @morriswinkler suggests is also good to have an env var that distrusts and also populates the list of keys with the given key. It might in fact be better. It should be quite rare where you only want to disable builtin PGP keys so the ability to list them directly allows for shorter invocations.

Furthermore, it would be good to allow these env vars (RUSTUP_PGP_KEY) to take multiple pgp keys, e.g. as a : separated list like PATH. This might be useful in migration settings.

Also, I want to reiterate, a command that allows for direct copy-pasting is always better than having to first download and then verify some pgp key from the internet. Support for rustup to download a key from a hardcoded url and to then verify it matches the fingerprint given via an env var would be really convenient and more secure. Convenient as you wouldn't have to do the download, secure as you wouldn't be able to miss the verification step. Think of a CI script. With RUSTUP_PGP_KEY, you'd have to either include the key into the script via an echo command, or you'd have to manually verify the sha sum. The -c parameter of sha256sum only takes files and not sha hashes on the command line so you need to create that file as well, e.g. via an echo command. So you are looking at up to 3 additional commands here.

Kixunil commented 2 years ago

I'm happy seeing an interest in this, will try to find some time to help out!

I've thought about a bunch of things to consider. I'm not saying we should do them all at once but we should think about it in the design to leave the options open.

Key rolling

This is tricky topic, if we just blindly accept a new release with a new key then an attacker who compromised the key can just release a custom release with compromised key. Ideally, we would have automated revocation checking, key hierarchy, threshold signatures and possibly expiration.

Key hierarchy

Instead of having a single key have a master key that is super-securely stored ("offline") and signs sub-key that's actually used to sign binaries. Rustup checks that subkey is signed by master key and checks that the signature is valid. This minimizes opportunity for an attacker to compromise the master key and if a subkey is compromised it can be easily and safely replaced.

Revocation checking

Rustup should query a group of independent servers for a list of revoked keys on each update. If a key is compromised its fingerprint is signed by revocation key and uploaded to those servers. It should be hard for an attacker to compromise the release server, a release key and prevent the victim from accessing multiple independent servers. It's crucial that Rustup fails if it can't access a significant chunk of revocation servers to prevent sybil attacks. Revocation key can only be used for revoking a key, not anything else.

If the master key is revoked Rustup refuses to update anything until the user manually enters new key. This should be rare but provides "big red button; shit hit the fan; everything is broken" at the expense of possible DoS (revocation key must be properly secured)

Threshold signatures

Instead of having one key, have multiple keys held by independent Rust developers. Whenever a new release is made each of them deterministically builds the binaries, signs them and uploads the signatures. Rustup only accepts updates if majority of the signers signed the release. E.g. if there are 5 signers Rustup requires 3 signatures to accept an update.

This provides probably highest security, possibly making other approaches unneeded. It also protects against compromised builders and lost keys. The approach is already used by various security-critical projects.

tarcieri commented 2 years ago

@Kixunil as it were, you've just named a set of features provided by The Update Framework (TUF), which also specifies a detailed set of workflows to securely implement each of these features.

There are also (at least) two implementations of TUF in Rust: rust-tuf and tough.

If rustup could bootstrap a TUF-based root-of-trust, it could also be used for things like end-to-end security for crates.io.

Kixunil commented 2 years ago

@tarcieri oh, nice! I guess I previously misunderstood TUF, will have to look into it better. Are there any specific obstacles to using it here?

tarcieri commented 2 years ago

Thus far I think no one has done the work on such an integration, but also it would be a fairly major change which might be hard for the rustup team to find bandwidth to review.

I'd still personally encourage people to try, either as a patch to rustup or as a prototype out-of-tree tool. @erickt might be able to provide pointers if you're interested in using rust-tuf.

est31 commented 2 years ago

Personally I think revocation can be handled by rustup updates, at least for a first step. Just remove the offending key from the whitelist and push out an update to users. Note that signatures are only a second line of defense behind https, so even if a private key is published to the internet, we will "just" return to a situation that's secure as now. key hierarchies are not needed I think, for similar reasons. Don't perfect be the enemy of the good.

znewman01 commented 2 years ago

Thanks all for the interesting discussion and hard work so far. Warning: drive-by opinion from someone with no experience developing Rustup (though I'm relatively familiar with TUF).

+1 to the sentiment that there are easy wins to be had via incremental steps and that adding relatively simple signing is a good idea, even if it doesn't solve every problem.

But also +1 to the idea that as soon as you decide you want to do something fancy for revocation, rollover, delegation, etc. you should just do a full TUF integration. I bet that a prototype of integration would be a weekend project, but productionizing this would take a while as you need to update publication workflows etc.

In the long run, we probably do want all these features. However, we should build simple PGP verification first to understand all the places in Rustup (including both code and process) that need to be modified to do signing/verification (of any kind).

I unfortunately don't have bandwidth at the moment to do any implementation work, but I will try to note if any PRs flying by would cause difficulties for Rustup if you want to implement TUF later on.

jaisnan commented 9 months ago

Hello, are there any updates on this, or is this the latest update? Thanks for working on this!

djc commented 9 months ago

There is a proposed RFC here that's highly relevant: https://github.com/rust-lang/rfcs/pull/3579.

wiktor-k commented 1 month ago

I think this ticket can be closed. It was implemented in #2077 then changed in #2847 and then the entire feature was dropped in #3277.