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

Tracking: Simple PGP signature verification #2028

Open
5 of 13 tasks
kinnison opened this issue Sep 28, 2019 · 25 comments
Open
5 of 13 tasks

Tracking: Simple PGP signature verification #2028

kinnison opened this issue Sep 28, 2019 · 25 comments
Assignees
Labels
security tracking This is a tracking issue
Milestone

Comments

@kinnison
Copy link
Contributor

kinnison commented Sep 28, 2019

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:

  • Select an OpenPGP implementation to use (rpgp)
    • Develop a prototype using that implementation (done on Validate pgp signatures #2077)
    • Ensure the OpenPGP implementation supports all platforms we need it to
  • Basic verification of signatures over channel manifests (in the .asc files)
    • Ensure that the signature-verification-failed message is good enough that users won't just re-run and/or file bugs on rustup which aren't actual bugs for us.
    • Switch warnings for errors, ensuring that GPG signatures are always checked and valid.
  • Sign rustup releases
    • Ensure we have at least release signed before we deploy signature verification in a release
  • Verify the signature on rustup releases in a basic way during self-update
  • Verify the signature on the rustup-init downloaded by rustup-init.sh
    • Determine which tools we can use and how to detect them
    • Decide how we distribute the key to verify against
    • Implement the verification check

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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

@kinnison
Copy link
Contributor Author

Just to ensure things are linked:

dignifiedquire added a commit to dignifiedquire/rustup.rs that referenced this issue Oct 26, 2019
Uses the pgp crate to validate signatures on downloaded artifacts when they are available and warns if those are not valid.

Ref rust-lang#2028
@jiegec
Copy link

jiegec commented Jun 8, 2020

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

@pietroalbini
Copy link
Member

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
Copy link

  • 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
Copy link
Contributor Author

kinnison commented Dec 8, 2020

@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.

legoktm added a commit to freedomofpress/securedrop that referenced this issue Feb 11, 2022
As an extra defense, pin the rustup version used and verify the
hash of the downloaded rustup-init binary. Previously we were downloading
the hash from the same place we were downloading the binary, so it didn't
really offer any extra protection besides making sure the download wasn't
corrupted (which HTTPS does for us).

This does not completely protect us, as rustup-init downloads rustup
without verifying signatures, but that will hopefully be fixed
soon: <rust-lang/rustup#2028>.

This shouldn't add a significant amount of maintenance overhead, as old
rustup versions can still be used to download newer Rust versions.

Fixes freedomofpress/securedrop-security#70.
@morriswinkler
Copy link

@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
Copy link
Member

est31 commented Jun 27, 2022

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
Copy link

morriswinkler commented Jul 4, 2022

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
Copy link
Member

est31 commented Jul 4, 2022

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
Copy link

Kixunil commented Jul 22, 2022

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
Copy link

@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
Copy link

Kixunil commented Jul 24, 2022

@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
Copy link

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
Copy link
Member

est31 commented Jul 24, 2022

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
Copy link

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
Copy link

jaisnan commented Mar 1, 2024

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

@djc
Copy link
Contributor

djc commented Mar 1, 2024

There is a proposed RFC here that's highly relevant: rust-lang/rfcs#3579.

@rami3l rami3l added this to the On Deck milestone Mar 13, 2024
@wiktor-k
Copy link

wiktor-k commented Oct 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security tracking This is a tracking issue
Projects
None yet
Development

No branches or pull requests