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 Issue for alternative registry authentication (RFC 3139) #10474

Closed
5 tasks done
ehuss opened this issue Mar 11, 2022 · 40 comments · Fixed by #12649
Closed
5 tasks done

Tracking Issue for alternative registry authentication (RFC 3139) #10474

ehuss opened this issue Mar 11, 2022 · 40 comments · Fixed by #12649
Labels
A-registries Area: registries A-registry-authentication Area: registry authentication and authorization (authn authz) C-tracking-issue Category: A tracking issue for something unstable. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns.

Comments

@ehuss
Copy link
Contributor

ehuss commented Mar 11, 2022

Summary

RFC: #3139
Implementation: #10592
Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#registry-auth
Issue: A-registry-authentication Area: registry authentication and authorization (authn authz)

This feature adds the ability to authenticate additional endpoints to a registry, including downloading crates.

Unresolved Issues

  • Do registries need a more fine-grained switch for which API commands require authentication?
  • The RFC mentions adding --token to additional commands like install and search, but we are leaning away from allowing tokens from being passed in on the command-line due to the ease of leaking. Should the --token flag be added or no? --token won't be added for now.
  • Consider changing the name and form of the X- header. See Cargo alternative registry auth rfcs#3139 (comment) and Cargo alternative registry auth rfcs#3139 (comment) Cargo now uses the www-authenticate header with the Cargo scheme and the login_url value, as in WWW-Authenticate: Cargo login_url="https://test-registry-login/me.
  • Will there be any concerns with the interaction with RFC 3231 (asymmetric tokens)?
  • Require a credential-provider to be defined in order to use authenticated registries

Stabilization tracked in #8933

Future Extensions

  • Support authentication with git indexes. Preferably, cargo will transition to HTTP indexes which will make this not necessary.

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.

@tarcieri
Copy link

The RFC mentions adding --token to additional commands like install and search, but we are leaning away from allowing tokens from being passed in on the command-line due to the ease of leaking. Should the --token flag be added or no?

I would recommend against providing any sort of secret as a command-line argument.

Command line arguments are exposed to the systemwide process environment, which is a risk to any machine running any sort of untrusted process.

Unless additional steps are taken like adding a leading space, it also exposes the secret to the shell's history, which is persisted to disk.

@Stargateur
Copy link

Stargateur commented Mar 31, 2022

Do registries need a more fine-grained switch for which API commands require authentication?

an app can just ignore the token if wanted

Consider changing the name and form of the X- header. See rust-lang/rfcs#3139 (comment) and rust-lang/rfcs#3139 (comment)

I think X- header are depreciated since RFC 6648 (nevermind second command already say it)

@vasanthkg
Copy link

Do we expect the solution for private registry authentication while dependencies download ?

@Eh2406
Copy link
Contributor

Eh2406 commented Apr 22, 2022

Yes, authentication will be sent for artifact download requests.

@vasanthkg

This comment was marked as off-topic.

@Eh2406

This comment was marked as off-topic.

@arlosi

This comment was marked as outdated.

bors added a commit that referenced this issue Nov 17, 2022
Implement RFC 3139: alternative registry authentication support

Allows registries to request Cargo to send the authentication token for all requests, rather than just publish/yank, implementing [RFC 3139](#10474).

### Items from the [tracking issue](#10474)

> Do registries need a more fine-grained switch for which API commands require authentication?

This PR uses the `auth_required` boolean as described in the RFC.

> The RFC mentions adding --token to additional commands like install and search

These flags are not added by this PR.

> Consider changing the name and form of the X- header

Changed to the `www-authenticate` header as suggested by the comments.

> Will there be any concerns with the interaction with rust-lang/rfcs#3231

Not that I know of.

-------------

Adds a new field `"auth-required": true` to `config.json` that indicates Cargo should include the token in all requests to a registry.

For HTTP registries, Cargo first attempts an un-authenticated request, then if that fails with HTTP 401, an authenticated request is attempted. The registry server may include a `www-authenticate` header with the HTTP 401 to instruct Cargo with URL the user can visit to acquire a token (crates.io/me).

Since the API URL is not known (because it's stored in the index), the unstable credential provider feature is modified to key off the index url, and the registry name is no longer provided.

To handle the case where an alternative registry's name is not known (such as coming from a lock file, or via `--index`), Cargo can now look up the token in the configuration by matching on the index URL. This introduces a new error if two alternative registries are configured with the same index URL.

Several operations, such as `cargo install` could have had a `--token` argument added, however it appears that Cargo would like to move away from passing the token on the command line for security reasons. In this case, users would need to configure the registry via the config file (or environment variables) when using `cargo install --index ...` or similar.
@ehuss ehuss added the A-registry-authentication Area: registry authentication and authorization (authn authz) label Dec 11, 2022
@sassman

This comment was marked as off-topic.

@Eh2406

This comment was marked as off-topic.

@sassman
Copy link

sassman commented Dec 17, 2022

@Eh2406 thanks for pointing that out. I was also uncertain if here would be the right place. I will move the suggestion over to #11460

@ehuss ehuss added the Z-asymmetric-token Nightly: asymmetric-token authentication label Jan 3, 2023
@dtretyakov
Copy link

Since sparse registry protocol is stable now, this issue becomes stopper in it's adoption in private registries because not everyone is ready to use nightly toolchain only to provide HTTP authentication.

Do we have roughly estimate which issues are preventing to allow HTTP auth in stable toolchain?

It looks like currently registry-auth feature flag covers also Assymetric tokens, but maybe it could be hidden by dedicated feature flag to not postpone HTTP auth stabilization?

@ehuss
Copy link
Contributor Author

ehuss commented Jun 2, 2023

@dtretyakov There's still ongoing discussions about how this will move forward. The initial plan was to gate on #3231, but there are discussions about separating them. See https://hackmd.io/I-HV7Sh0RMmNo6EwcPqomA and https://internals.rust-lang.org/t/pre-rfc-jwts-for-private-cargo-registry-authentication/18701. I believe @arlosi is still working on a new proposal. Discussing this is next on the team's agenda when we have everyone together. If you have further questions or want to engage in the discussion, I recommend following up on #t-cargo on Zulip (there are some other discussions there).

@fasterthanlime
Copy link

fasterthanlime commented Jun 19, 2023

I just want there to be on the public record that "we can't stabilize symmetric tokens because then small registries will do the insecure thing (which is symmetric tokens)" is laughable because what small registries are doing right now are:

  • passing symmetric tokens through the user-agent header (you know, the one everyone harvests for analytics)
  • generating dummy symmetric tokens (like 0000) and then never checking them
  • adding a reverse proxy in front of their small registry software that unconditionally adds the correct symmetric token (as an Authorization header)

Seeing as this is something people have been asking for since 2019, this feels like a "perfect is the enemy of not as bad as whatever people's workarounds are right now" situation.

I'm sympathetic to the general lack of bandwidth on the cargo team but trust me, asymmetric tokens or whatever "the right thing" ends up being, can wait a little more.

Stalling symmetric tokens is making everyone's security posture worse for the time being.

@ThatGeoGuy
Copy link

To add onto what @fasterthanlime has said - current best practices aren't great and it is very frustrating that this isn't something that has been stabilized yet.

For reference, using something like cloudsmith it is most often the case that one has to use:

[registries.OWNER-REPOSITORY]
index = "https://dl.cloudsmith.io/TOKEN/OWNER/REPOSITORY/cargo/index.git"

with the TOKEN in the URL. This is problematic especially if you have binaries where you're saving the Cargo.lock — you often end up saving the whole URL and your token in the lockfile.

It's just a plain hassle to manage this, so you end up with people making basic mistakes like putting tokens in lockfiles very easily, and they're actively being recommended to do this by providers today because the other mechanisms aren't always as easy to set up and provision across multiple machines.

@secana
Copy link

secana commented Jul 9, 2023

I implemented a working authentication in cargo two years ago, based on the http-registry work by @jonhoo, but it never got merged (https://github.com/secana/cargo/tree/http-registry), as the http-registry wasn't done yet.

If there is a concrete concept everyone agrees on, I can implement it in cargo, again.

It's the most requested feature by users of kellnr (the free private registry). The not-so-nice alternative would be a fork of cargo that implements auth. and can only be used with kellnr. But I would appreciate a common solution that works with all private registries.

@weihanglo
Copy link
Member

Some relevant updates:

@arlosi
Copy link
Contributor

arlosi commented Jul 10, 2023

If there is a concrete concept everyone agrees on, I can implement it in cargo, again.

@secana does the current unstable implementation available on nightly under -Zregistry-auth work for kellnr? That would be a good data point to encourage stabilization of what we have.

@secana
Copy link

secana commented Jul 12, 2023

@arlosi I'll have a look as soon as I've got time.

@secana
Copy link

secana commented Jul 15, 2023

@arlosi I implemented the current RFC (sync. tokens) in kellnr and it works fine for the git index and sparse registry. Easy to implement, easy to use. I would give it a go :)

@Noah-Kennedy
Copy link

I've written a registry implementation as well, and I'd like to add a few thoughts to the conversation.

First, echoing @fasterthanlime, I believe that this is absolutely critical to adoption of third-party registries. The overwhelming majority of users of third party registries need them because they are working with private source code which cannot be published to crates.io for the obvious reason that the code is proprietary. Nearly everyone using a private registry right now is thus needing to do one of the things he mentioned.

Second, many organizations need the ability to supply temporary tokens generated via security software on the machine for source code pulls, a use case which I believe the credential-process RFC to be potentially perfect for. As a result, I believe that the credential-process RFC can solve many of the potential issues with this system better than other RFCs can. One alternative could be the usage of OAuth, however OAuth is quite complex and many things would need to be worked out, and I believe that it would be best to add support for that later via supplemental RFCs, rather than block this one on a lengthy RFC, especially when credential-process solves many of the same issues, and is likely far easier to stabilize.

Finally, I would be willing to contribute to Cargo and provide other assistance if it would help get this and the credential-process RFCs stabilized and implemented. I would also be willing to be a guinea pig for both RFCs.

@Noah-Kennedy
Copy link

What needs done right now to drive this forward?

@arlosi
Copy link
Contributor

arlosi commented Aug 2, 2023

The current plan for stabilizing registry-auth is to first stabilize credential-process (#8933), then require a credential-provider or global-credential-provider to be defined to use an authenticated registry. Cargo will include built-in providers such as cargo:macos-keychain or cargo:wincred that securely store the token.

The Cargo team would like to move away from storing plaintext tokens in credentials.toml, as they can easily be leaked. We want to take this opportunity to require configuring a credential-provider in order to use an authenticated registry.

So registry-auth itself is considered feature complete, and its stabilization is essentially blocked on credential-process now.

many organizations need the ability to supply temporary tokens generated via security software on the machine for source code pulls

I recently made changes to credential providers in #12334, switching it to use a JSON based protocol. Implementing a credential provider and/or giving feedback about the design would be the best way to get it towards stabilization.

Work is still ongoing for asymmetric token support (#10519), which is now implemented by a built-in credential-provider. This is not a blocker for stabilizing registry-auth, however.

@Noah-Kennedy
Copy link

The current plan for stabilizing registry-auth is to first stabilize credential-process (#8933), then require a credential-provider or global-credential-provider to be defined to use an authenticated registry. Cargo will include built-in providers such as cargo:macos-keychain or cargo:wincred that securely store the token.

The Cargo team would like to move away from storing plaintext tokens in credentials.toml, as they can easily be leaked. We want to take this opportunity to require configuring a credential-provider in order to use an authenticated registry.

So registry-auth itself is considered feature complete, and its stabilization is essentially blocked on credential-process now.

many organizations need the ability to supply temporary tokens generated via security software on the machine for source code pulls

I recently made changes to credential providers in #12334, switching it to use a JSON based protocol. Implementing a credential provider and/or giving feedback about the design would be the best way to get it towards stabilization.

Work is still ongoing for asymmetric token support (#10519), which is now implemented by a built-in credential-provider. This is not a blocker for stabilizing registry-auth, however.

@arlosi I'll implement a credential provider over the next week or two and start testing it internally with others. Would you like feedback to be provided in the thread for #12334, or elsewhere?

@tarcieri
Copy link

tarcieri commented Aug 2, 2023

Cargo will include built-in providers such as cargo:macos-keychain

@arlosi very curious about macOS Keychain support for Cargo. Is that tracked in #8933?

@Noah-Kennedy
Copy link

The current plan for stabilizing registry-auth is to first stabilize credential-process (#8933), then require a credential-provider or global-credential-provider to be defined to use an authenticated registry. Cargo will include built-in providers such as cargo:macos-keychain or cargo:wincred that securely store the token.
The Cargo team would like to move away from storing plaintext tokens in credentials.toml, as they can easily be leaked. We want to take this opportunity to require configuring a credential-provider in order to use an authenticated registry.
So registry-auth itself is considered feature complete, and its stabilization is essentially blocked on credential-process now.

many organizations need the ability to supply temporary tokens generated via security software on the machine for source code pulls

I recently made changes to credential providers in #12334, switching it to use a JSON based protocol. Implementing a credential provider and/or giving feedback about the design would be the best way to get it towards stabilization.
Work is still ongoing for asymmetric token support (#10519), which is now implemented by a built-in credential-provider. This is not a blocker for stabilizing registry-auth, however.

@arlosi I'll implement a credential provider over the next week or two and start testing it internally with others. Would you like feedback to be provided in the thread for #12334, or elsewhere?

Ah, tracking issue is #8933.

@arlosi
Copy link
Contributor

arlosi commented Aug 2, 2023

I'll implement a credential provider over the next week or two and start testing it internally with others. Would you like feedback to be provided in the thread for #12334, or elsewhere?

@Noah-Kennedy thanks, that sounds great. Currently you have to reference the cargo-credential crate from the Cargo repo, as the copy on crates.io hasn't been updated yet. I'll look in to getting it published.

General feedback can go on the tracking issue. If you have a specific problem, or change you'd like, please create a new issue. You can also create a thread on Zulip in t-cargo, and I'd be happy to have a discussion there too.

@arlosi
Copy link
Contributor

arlosi commented Aug 2, 2023

very curious about macOS Keychain support for Cargo. Is that tracked in #8933?

@tarcieri yes. You can try it out on nightly by adding the following to ~/.cargo/config.toml

[unstable]
credential-process = true

[registry]
# Set up macOS keychain as the preferred credential provider with a fallback 
# to `cargo:token` (`credentials.toml`) if a token can't be found.
global-credential-providers = ["cargo:token", "cargo:macos-keychain"]

If you then do a cargo login, your token should be stored in the macOS keychain. If you hit any problems, please file a new issue.

@Fishrock123
Copy link

Fishrock123 commented Aug 3, 2023

Why is this blocked on credential-process?

Surely just sending tokens to the registry for every request is not worse than private packages needing to have publicly accessible download links?

In my case I'm much prefer to just always send an auth token to artifactory than have the download links be public. Some auth would be better than none at all.

Edit: it's not so clear what this "feature" all covers but I am specifically interested in the "auth-required": true part and the behavior that surrounds that.

@BatmanAoD
Copy link
Member

Does #12334 unblock this?

@arlosi arlosi removed the S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. label Aug 29, 2023
@Fishrock123
Copy link

Looks like the tracking issue for credentials-process stability is #8933

@arlosi
Copy link
Contributor

arlosi commented Sep 8, 2023

This feature is being now proposed for stabilization in the FCP in #8933.

@bors bors closed this as completed in 7149418 Sep 18, 2023
@weihanglo weihanglo removed the Z-asymmetric-token Nightly: asymmetric-token authentication label Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-registries Area: registries A-registry-authentication Area: registry authentication and authorization (authn authz) C-tracking-issue Category: A tracking issue for something unstable. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.