Tracking Issue for credential-process RFC 2730

[ehuss]

Summary

RFC: https://github.com/rust-lang/rfcs/pull/2730 Implementation: #8934 Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#credential-process Issues: https://github.com/rust-lang/cargo/labels/Z-credential-process https://github.com/rust-lang/cargo/labels/Command-logout

This feature provides a configuration option to specify a process to fetch a token to authenticate with a registry.

Unresolved issues

• [ ] Is this approach useful enough? Things like macos keychain don't protect against being executed to extract tokens (and I don't see a way to require a password, or force the process to be untrusted). The lack of signatures also cause issues (each update of the toolchain will cause it to become untrusted again).
• [ ] Does the {action} syntax need a way to pass the literal string "{action}" somehow to the command?
• [ ] Should {foo} generate an error to reserve the right for us to define {foo} in the future?
• Sample wrapper issues (https://github.com/rust-lang/cargo/tree/master/crates/credentials):
  • [ ] Should we commit to shipping and supporting these? Or instead of shipping them, maybe users should just use cargo install instead?
  • [ ] What to do about cargo-credential-gnome-secret? It cannot be built on the current rust-lang infrastructure, is it OK to just instruct users to build it manually? Does anyone actually use libsecret?
• [ ] Should the login API be changed? Currently it passes the token in via stdin, but this makes it awkward if the wrapper needs to request a password from the user. The 1password wrapper is an example, and it uses a hack of directly opening stdin (/dev/tty and CONIN$). It seems to work, but I am uneasy about it. Some alternatives:
  • Pass the token in through a separate descriptor/handle (see https://github.com/rust-lang/cargo/pull/8934#issuecomment-741986791).
  • Pass the token in an environment variable (which can be slightly insecure).
  • Force the user to set the appropriate OP_SESSION_ env var (to avoid requiring a password).
  • Don't support login if it needs console access, and to have the user manually enter the token into the vault themselves (which can be difficult to do correctly).
• [ ] Should the storage key off the registry name? RFC 3139 discusses problems with this.
• [ ] Should there be some support for multiple tokens for the same registry? For example, some people have different accounts on crates.io for different purposes.
• [ ] registry.credential-process, is used as both as the default credential process for alternative registries, and the credential process for crates.io. Should these two pieces of functionality be separate config's?

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

[jethrogb]

Also see the discussion in https://internals.rust-lang.org/t/securing-cargo-publishing-credentials/14050

[Eh2406]

There's still a lot of unanswered questions in this tracking issue. But to make it clear basic functionality from the RFC is implemented and available for testing. The questions are all about comparatively small details.

[ehuss]

I have posted a proposal to stabilize just the cargo logout portion at #11910.

[ehuss]

#12334 has been merged which is a redesign of this feature. It should land in nightly within the next week.

[BatmanAoD]

@ehuss should this issue be closed, then?

[ehuss]

@ehuss should this issue be closed, then?

I'm not sure I understand the question, why would this be closed? This is still tracking the credential-process feature. Its design has changed some, but it is still an unstable feature we are tracking.

[BatmanAoD]

@ehuss 🤦🏻 Sorry, I forgot that just because it's implemented doesn't mean it's stabilized.

[Fishrock123]

All the checkboxes in the original issue post here have been checked off - can an updated checklist be made if this is not ready to be stabilized?

[ehuss]

All the checkboxes in the original issue post here have been checked off - can an updated checklist be made if this is not ready to be stabilized?

I'm not sure which original issue you are referring to. Work is still actively being performed on this, mainly the amazing effort from @arlosi:

#12424 #12439 #12440 #12461 #12469 #12499 #12507 #12512 #12515 #12518 #12521 #12526 #12551 #12587 #12588 #12590 #12622 #12623 #12626 #12628

We are getting very close to being ready to stabilize. However, some of these changes haven't even made it to nightly, yet.

[BatmanAoD]

@Fishrock123 The checkbox list in the comment at the top of this thread are "unresolved issues", meaning design questions that needed to be answered. It is not a complete list of steps necessary to actually implement this feature.

[BatmanAoD]

...I agree this is a bit confusing, though. Several of those do look like implementation requirements, and are linked to merged PRs.

[Fishrock123]

I'm not sure which original issue you are referring to. Work is still actively being performed on this, mainly the amazing effort from @arlosi:

... pr links ...

We are getting very close to being ready to stabilize. However, some of these changes haven't even made it to nightly, yet.

All but one of those are merged. I am asking if it would be possible to update a high-level checklist like the one at the top of this issue with at least some kind of bullet point of what is left to be done.

[arlosi]

Checklist in this issue updated

[mojindri]

Hi, A question if "--token" for cargo build not included, is there any way to pass token to Private artifactory such as jfrog that requires token for crate download ?? Regards, Moji

[arlosi]

is there any way to pass token to Private artifactory such as jfrog that requires token for crate download

Yes, that's covered by the registry-auth unstable feature tracked as https://github.com/rust-lang/cargo/issues/10474

I'll be posting a stabilization PR for both registry-auth and credential-process soon. Hang in there.

[arlosi]

Stabilize registry-auth and credential-process

Users have been requesting a way to have authenticated private registries for some time, and the registry-auth feature has received significant testing, with multiple commercial and non-commercial offerings implementing the current unstable protocol.

The Cargo team has been reluctant to stabilize it as-is because it uses static plaintext tokens which are at greator risk of being leaked (accidentally or maliciously).

To improve the security situation, work continued on credential-process and asymmetric-token unstable features and the team agreed to only stabilize registry-auth if it was combined with one of these two approaches for improved security. credential-process allows the token to be stored securely by the operating system, or for people to build external credential providers that can generate short-lived tokens for registries. I believe the credential-process feature is ready to be stabilized, which also unblocks registry-auth.

Attempts to use an authenticated registry without a credential provider configured will encounter an error such as:

authenticated registries require a credential-provider to be configured
see https://doc.rust-lang.org/cargo/reference/registry-authentication.html for details

For contexts like CI where a more secure credential provider may not be available, the existing behavior of reading from config or the *_TOKEN environment variables can be enabled by selecting the cargo:token provider.

Stabilization summary (registry-auth) (#10474)

• The auth-required field in the registry index's config.json
  • Enables authenticated sparse index, crate downloads, and search API
• Automatically retrying requests to config.json in sparse registries with authorization included on HTTP 401 errors.
• Note: Attempting to use these features without a credential provider configured is an error.

Stabilization summary (credential-process):

• The built-in credential providers: cargo:token, cargo:wincred, cargo:macos-keychain, cargo:libsecret, cargo:token-from-stdout <path> <args>
• The credential provider v1 JSON protocol for external credential providers
• The registry.global-credential-providers config value
• The registries.<NAME>.credential-provider config value
• The [credential-alias] config table

To review these changes in more detail, I recommend looking at the documentation changes in the draft stabilization PR rust-lang/cargo#12649.

@rfcbot fcp merge

[rfcbot]

Team member @arlosi has proposed to merge this. The next step is review by the rest of the tagged team members:

• [x] @Eh2406
• [x] @Muscraft
• [x] @arlosi
• [x] @ehuss
• [x] @epage
• [ ] @joshtriplett
• [x] @weihanglo

Concerns:

• ~~future-compatibility~~ resolved by https://github.com/rust-lang/cargo/issues/8933#issuecomment-1722134167

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[epage]

Initially, I was concerned about allowing a plugin to be used multiple times with different configurations. This mostly came up as a theoretical because I was thinking or process plugins for per-user cache where that might be of more interest. Arlo pointed me to the fact that CLI flags can be used for this which I had forgotten about and overlooked it when I went back over the docs.

[epage]

@rfcbot concern future-compatibility

Built-in providers start with cargo: but on non-Windows systems, a binary could be created with a name like that or on all platforms a credential alias could be named that. Either approach means we have a potential compatibility hazard if we want to add more built-ins in the future.

EDIT: Do we care enough about this vs taking the risk of breaking people? Allowing this intentionally allows polyfills.

[arlosi]

We could reserve the cargo: namespace for credential providers, but I think we shouldn't.

• It's unlikely that a user will accidentally set up a credential-provider with a cargo: prefix, so the risk of unintentional shadowing is low (significantly less than the current issue of subcommand shadowing).
• If in the future Cargo adds a new built-in provider cargo:foo, it would be possible to make a polyfill provider available for older cargos that could be added via cargo install cargo-credential-foo and a credential-alias.

  [credential-alias]
  'cargo:foo' = 'cargo-credential-foo'
  
  

OTOH there may be a reason I'm not currently thinking of why we need to reserve something that we know can't be user-defined, and the current proposal does not allow for that.

[epage]

We also consider shadowing fine for built-in subcommands, aliases, and third-party subcommands though we provide warnings when it happens: https://github.com/rust-lang/cargo/blob/master/src/bin/cargo/cli.rs#L258

Maybe that is the route we should go with using that as our precedence.

[epage]

@rfcbot resolve future-compatibility

See #12671

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[BatmanAoD]

Will this be in the beta on October 5th and available in the stable channel on November 16th?

[Eh2406]

Yes, that is the plan and so far we are still on schedule for it.

[Fishrock123]

Stabilizing this without making cargo:token a default option (which would have respected the world as it functioned before 1.74) has broken all of our CI: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Cargo.201.2E74.20broke.20all.20our.20CI.20due.20to.20credentials-process