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

Add client certificates option to cargo registries #10641

Open
ETKNeil opened this issue May 9, 2022 · 2 comments
Open

Add client certificates option to cargo registries #10641

ETKNeil opened this issue May 9, 2022 · 2 comments
Labels
A-networking Area: networking issues, curl, etc. A-registries Area: registries A-registry-authentication Area: registry authentication and authorization (authn authz) C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.

Comments

@ETKNeil
Copy link

ETKNeil commented May 9, 2022

Problem

Currently when using a private registry, it will pull down the assets via http(s) from a public facing registry, however in the context of a company we don't want such public facing registry to be left unauthentificated.

One way to mitigate the problem is to implement what is call mutual TLS or client certificate. The client come with a certificate which is derived from a CA known to the server and the proxy such as NGINX, Apache, Haproxy or squid will serve either a 403 or the registry.

Proposed Solution

Fortunately for us curl (the backend used by cargo to make its request) already support client ssl certificates. Such certificates are composed of 2(3) parts, a certificate (public key), a private key and optionally a password protecting the private key.

I propose to use the curl::easy::Easy::ssl_cert curl::easy::Easy::ssl_key and curl::easy::Easy::key_password which are well tested methods in curl source code :

I would then expose those options inside the [http] section of the $HOME/.cargo/config.toml :

[http]
client-ssl-cert = "/etc/ssl/client.pem"
client-ssl-key = "/etc/ssl/key.pem"
client-ssl-key-password = "<password>"

Here is the PR : #10630

Notes

The certificate public and private key must be accessible by the user, so anyone accessing this user account has access to those information, we can optionally encrypt the private key with a password, this mitigate such broad access however here we would store it as plain text.

The password will be stored as plain text, the issue could be that it can then be read by anyone, but let me remind you that the registry token is also stored as plain text inside credentials.toml in the .cargo home directory.
We can not securely store it as asking it each time that you make a request to the registry would be counter productive for the user. In my threat model I assumed that the user would have the specific config with the plain password only in its home directory and that the home security would be assumed by the user.

Testing such real world scenario would need a mocking framework for a server with a client certificate authentication, however as these options are well tested in libcurl, I don't see the point of testing such feature. A regression in such area would likely mean also that all TLS exchange are broken as well.

In the PR, I set the key type to PEM, however it's already the default : https://github.com/curl/curl/blob/master/docs/libcurl/opts/CURLOPT_SSLKEYTYPE.3#L44 , I made this change so its explicit and fixed from an usage perspective. We could also provide the option to support DER format but since changing from DER to PEM is a one liner, having it as a constant would help to fix it once and for all.

@ETKNeil ETKNeil added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label May 9, 2022
@Eh2406
Copy link
Contributor

Eh2406 commented May 9, 2022

How does this proposal interact with the two excepted RFCs (RFC 3139 and RFC 3231) related to securing registries?

Why is secret material stored in config.toml and not in credentials.toml?

Similarly, how will this integrate with credential-process RFC 2730?

I made an issue explaining the design decisions, but to be honest the password one is impossible to solve without building a proper password manager in cargo.

You are not the first to use this language, in what way is credential-process not an interface for a "proper password manager"? What is improper about that design? It is unstable because we are not yet happy with the design. Genuinely, how do we make it a better design?

@ehuss ehuss added A-registries Area: registries A-networking Area: networking issues, curl, etc. labels May 24, 2022
@ehuss ehuss added the A-registry-authentication Area: registry authentication and authorization (authn authz) label Dec 11, 2022
@bbucko
Copy link

bbucko commented Apr 17, 2023

You are not the first to use this language, in what way is credential-process not an interface for a "proper password manager"? What is improper about that design? It is unstable because we are not yet happy with the design. Genuinely, how do we make it a better design?

Some organizations are not using passwords and rely mainly on client certificates for authentication. Because mTLS is not supported in Cargo we are unable to deploy our internal crate registry (or rather reuse the existing artifact repository).

@epage epage added the S-triage Status: This issue is waiting on initial triage. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Area: networking issues, curl, etc. A-registries Area: registries A-registry-authentication Area: registry authentication and authorization (authn authz) C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

5 participants