rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.84k stars 2.43k forks source link

Cargo built-in Git/SSH client doesn't support `@cert-authority` #11577

Open hds opened 1 year ago

hds commented 1 year ago

Edited: for remaining tasks, see https://github.com/rust-lang/cargo/issues/11577#issuecomment-1382706378.

Problem

Cargo parses SSH known hosts file. From the Cargo book (https://doc.rust-lang.org/cargo/appendix/git-authentication.html#ssh-known-hosts):

When connecting to an SSH host, Cargo must verify the identity of the host using "known hosts", which are a list of host keys. Cargo can look for these known hosts in OpenSSH-style known_hosts files located in their standard locations ...

However, there are some additional markers supported by at least some SSH clients (e.g. OpenSSH) to handle more complex cases than verifying a host via a single algorithm/key. The known ones are:

The Cargo SSH client doesn't support these directives. It is quite explicit about this in the code: https://github.com/rust-lang/cargo/blob/1cd6d3803dfb0b342272862a8590f5dfc9f72573/src/cargo/sources/git/known_hosts.rs#L490-L493

With the release of Rust 1.66.1 and the fix for CVE-2022-46176 (security advisory), Cargo is now performing host key checking, which will lead to more users needing this functionality because single host key verification may not be practical.

Proposed Solution

The solution to this issue would be to implement the missing support for the @cert-authority or @revoked markers.

There is useful documentation on these markers from the OpenSSH project:

This issue can be mitigated by telling cargo to use the command line Git client (net.git-fetch-with-cli = true) as mentioned by @weihanglo on this Zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Cargo.20SSH.20host.20key.20verification.20with.20.40cert-authority.20lines

As mentioned on that thread, a good mitigation step would be to add some text to the Cargo book section on SSH Known Hosts to suggest that users try net.git-fetch-with-cli = true if they find that Cargo's SSH behaviour is different to what they expect or different to how their command line Git client behaves.

Notes

Some further useful resources that I found related to creating an SSH Certificate Authority (CA) and then specifying it in the SSH Known Hosts file:

weihanglo commented 1 year ago

Thanks for the beautiful issue report! The manual of OpenSSH sshd is also considered as a reference.

As what aforementioned, there are at least a few independent tasks:

I would help mentor some of them, but for @cert-authority probably not.

est31 commented 1 year ago
  • Implementing @cert-authority marker

The certificate format is documented here.

There is an implementation of ssh certificates in Rust, from the rust-crypto team, via the ssh-key crate. The library contains also an entire authorized_keys parser. I guess it wasn't used for the CVE addressing because it was a third party component that wasn't audited. I think long term it might be best to move out much of the authorized_keys parsing into a different crate, maybe even a different repo.

hds commented 1 year ago

Thanks for the comments @weihanglo and @est31! I've added the links you each suggested into the main text of the Issue to make reading it less confusing. I've also switched from calling these lines directives (which seems to be wrong, I don't know where I got that from) to calling them markers.

There's a small docs PR for the first task on the list ready too.

hds commented 1 year ago

I don't think we want to close this issue, as the referenced PR was only for the first task. Or would you prefer to have separate issues for those?

weihanglo commented 1 year ago

Reopen as we can continue tracking sub-tasks here https://github.com/rust-lang/cargo/issues/11577#issuecomment-1382706378. Thanks for the suggestion :)