cargo login encourages disclosure of secret token

[auspicacious]

Problem

I am new to Rust and working through the Rust Book.

cargo login's default syntax encourages users to pass their secret registry authentication token on the command line, e.g. cargo login [token]. The Rust Book also encourages this syntax.

The cargo login manpage makes it clear that this value requires protection:

Take care to keep the token secret, it should not be shared with anyone else.

However, in general, passing any secret as a command-line argument creates significant and unnecessary security risk. It is significant for at least these reasons:

• While the cargo login command is running, its arguments are visible to all users on the system. A malicious user monitoring processes could easily steal the token.
• The token is saved in the user's shell history file. Some shells may limit permissions on this file to only that user, but this is not guaranteed, and, regardless, exposes the token in a place the user may not anticipate.
• Users may be working on systems that monitor and log the commands they execute, such that the token could be persisted into a centralized logging system with even more relaxed access permissions and many potential vulnerabilities.

And it is unnecessary because there are other options available that do not create this risk:

• cargo login could be modified to prompt the user for input, using normal shell conventions for entering a password, and not echoing text back to the console. cargo login shows passwords in plain text #7813 touches on this.
  • This, of course, could lead to the token being persisted in a clipboard manager if it is copy/pasted, but mitigating that risk is out of scope for this issue and present regardless.
• The user could write the token into a file or a shell-oriented password manager like pass and pipe it into cargo login to avoid exposure.
• Even without modifications to cargo login, if using bash, and possibly some other shells, the user could use this combination of commands to avoid exposure:

IFS='' read -r -s token
cargo login <<< "$token"
unset token

I am honestly surprised that I could not find any duplicate issues for this, and I apologize if I have missed one that explains the rationale behind the current design. Likewise, I am not using the security vulnerability disclosure process for this bug report, as it is based entirely on public and obvious information.

However, I do believe that this is insecure by design, and encourages Rust developers to adopt patterns that are widely known to be insecure.

Steps

No response

Possible Solution(s)

No response

Notes

No response

Version

No response

[epage]

Note that the Cargo book shows the use of stdin, see https://doc.rust-lang.org/cargo/reference/publishing.html#before-your-first-publish

We can't remove the argument due to our compatibility guarantees.

Possible improvements from where we are at:

• Creating an issue against the book for this section
• Update cargo login -h with one of
  • Hide [token]
  • help message mentioning stdin as an alternative for more secure operation

[epage]

Also, in case you aren't aware, the token is stored in plain text. See https://doc.rust-lang.org/cargo/reference/registry-authentication.html for configuring an alternative storage. I'm working to deprecate us defaulting to plain text storage in #13343

[auspicacious]

Also, in case you aren't aware, the token is stored in plain text.

Yes, that is clear from the documentation, but there is still a significant difference between a single, well-known file with restricted permissions and the many unknowns that putting something on the command-line brings.

Thanks for linking to the registry -- I inferred while searching for other bugs that Cargo had support for other storage mechanisms but hadn't looked into it. (Which is good, and makes it even more compelling to deprecate this pattern!)

We can't remove the argument due to our compatibility guarantees.

• Is there a list somewhere to track deprecated things that should be removed in the next major version of Cargo?
• Compatibility guarantees are generally intended to enable automation, but automation that passes the token on the command-line could be leaving users open to significant vulnerabilities beyond what I described above, which assumed a user at a keyboard.
  • For example, a continuous integration pipeline that uses [token] could be logging that token to the CI system's build log. For many open-source projects, those logs may in fact be public. CircleCI operates this way. If I remember correctly, they make an effort to recognize and purge secrets from the log. However, this is a very fragile layer of defense in depth, indeed.
  • What would constitute a compelling enough security issue to drive a breaking change? Would this qualify?

Possible improvements from where we are at:

I think that both of these are reasonable if worded well. However, that is predicated on the decision that the compatibility guarantee trumps security issues and we have to work around it. I'm not entirely convinced of that.

Is it possible that the other, related security issue in #7813 will be fixed soon? It would be better to be able to update the Rust and Cargo books to show a reasonably secure pattern.

In other words, personally, if I were to update the documentation, the only way I would be comfortable writing it today would be the defensive Bash formula I mentioned above:

IFS='' read -r -s token
cargo login <<< "$token"
unset token

because that pattern will not echo to the console.

[weihanglo]

Is there a list somewhere to track deprecated things that should be removed in the next major version of Cargo?

The version of Cargo follows the release of Rust itself, hence unlikely to have major version bump unless Rust also did that. That being said, Cargo could and has been phasing out deprecated features slowly.

The other alternative is having asymmetric token being used by default, and phase out plain text token. That is tracked in #10519.

[auspicacious]

The other alternative is having asymmetric token being used by default, and phase out plain text token.

Wow, yes, having asymmetric tokens will solve many of the problems of symmetric tokens. And I got to learn about PASETO.

That being said, Cargo could and has been phasing out deprecated features slowly.

Okay, so it's not exactly strict SemVer, but hopefully something a bit closer to Git's concept of "porcelain" and "plumbing" commands.

I'll summarize the work that's been discussed here. 1, 2, 3 probably will not affect Cargo's CLI in a backwards-incompatible way, but 4, 5, 6 probably will. This looks like rough chronological order to me.

1. Documentation updates to the Rust book, cargo login -h, and cargo help login to deprecate the passing of tokens on the command line. (I would personally prefer that these docs somehow reference cargo login shows passwords in plain text #7813 as an acknowledgement that this is still a work in progress)
2. Completion of cargo login shows passwords in plain text #7813 to stop showing typed/pasted passwords, including updates to the Rust and Cargo books to reflect this improvement
3. Better secret storage mechanisms automatically used in cargo:token not being used by default #13343
4. Asymmetric tokens used by default in Tracking Issue for RFC 3231: Asymmetric Tokens #10519
5. Removal of symmetric tokens from the command-line interface
6. Removal of symmetric tokens entirely

There's a lot of good news in that list!

The scope of this issue is, I think, limited to 1.

If it's OK with the others on this thread, I will try to take on the documentation changes myself. Given the evidence that much more significant improvements are on the way, I will try to keep the documentation focused on discouraging passing tokens on the command-line and avoid introducing the Bash formula that I mentioned above.

Does that sound reasonable, @epage @weihanglo ?

[weihanglo]

@auspicacious Nice write-up!

I'd like to correct my words. Cargo is guaranteed not to have breaking changes, pretty much SemVer (?). Usually only when them are identified as bugs or security vulnerabilities will we remove them, after a long deprecation period, after we scratched our head and couldn't come up with any better solution. Things in Cargo.toml might usually be fixable across edition boundaries. However, CLI breakage are generally not acceptable, as there is no way to check editions of a CLI flag now.

Back to the plan. Looks pretty great. However, as you said, 4, 5, and 6, especially 5 and 6, are not backward-compatible, so unlikely to execute them.

Sorry for giving you a wrong impression of how the project handle deprecations.

[epage]

I've created rust-lang/book#3866 to track the Book side of this.

At this point, it sounds like this issue is only about cargo login -h. Is that correct?

[auspicacious]

@epage Thanks, it looks like the Book authors will take up the edit.

At this point, it sounds like this issue is only about cargo login -h. Is that correct?

cargo login -h and cargo help login (the manpage) will both need to be updated, I think. The manpage uses neutral language right now, and should probably be updated to discourage use.

@weihanglo

Usually only when them are identified as bugs or security vulnerabilities will we remove them, after a long deprecation period
...
as you said, 4, 5, and 6, especially 5 and 6, are not backward-compatible, so unlikely to execute them.

I must continue to argue that this is a security vulnerability and that deprecation and removal is necessary. If you look at the most common and heavily scrutinized programs that deal with secret tokens, such as sudo, gpg, or cryptsetup, it would be unthinkable for them to support secret input on the command-line in the way that Cargo does. sudo and gpg, in fact, have elaborate subsystems dedicated to passphrase input.

Cargo is a critical part of supply-chain security and should be held to the same standard -- not to mention that it sets an example for other Rust programs and programmers.

To that end, it seems like there is a lot of progress happening. Asymmetric tokens will hopefully bring many benefits, but those benefits can only be realized by removing the old system, and I hope that the old system is removed someday.

I understand if the scope of this GitHub issue should be limited to changes in the documentation. However, this seems like a situation where breaking the CLI and removing this functionality would actually help users, by alerting them to and protecting them from, for instance, the potential CI system leaks that I mentioned above.

[epage]

I must continue to argue that this is a security vulnerability and that deprecation and removal is necessary

If this is a vulnerability, it should not be discussed here but reported through the standard reporting process. They can make an appropriate determination. See https://www.rust-lang.org/policies/security

[auspicacious]

If this is a vulnerability, it should not be discussed here but reported through the standard reporting process. They can make an appropriate determination. See https://www.rust-lang.org/policies/security

Fair enough. I sent a message after this comment. The team has been in contact, but I am waiting for a determination.

[auspicacious]

From the Rust Security Response WG. I've highlighted a suggestion they made that I don't think has been mentioned elsewhere:

While we agree that the current implementation and design of cargo login is sub-optimal, we don't see it as a security vulnerability requiring a coordinated disclosure and fix. The approach outlined in the issue to update the documentation and help messages, plus a deprecation warning shown when passing the token through the CLI discouraging its use, sounds good to us.

We leave the decision on whether to eventually remove support for passing the token to the CLI to the Cargo team.