-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Commit 582246c
committed
Auto merge of #11635 - hds:ssh-known-hosts-markers, r=ehuss
Add partial support for SSH known hosts markers
### What does this PR try to resolve?
The SSH `known_hosts` file parsing in Cargo did not previously support
markers. Markers are modifiers on the lines (``@cert-authority`` and
``@revoked`)` which denote special behavior for the details on that line.
Lines were skipped entirely.
This silent skipping of marker lines can be confusing to a user, who
sees that their command line Git/SSH client works for some repository,
but Cargo reports that no host key is found.
This change adds support for the ``@revoked`` marker. This marker denotes
that a key should be rejected outright. It is of limited use without
``@cert-authority`` marker support. However, if it is present in a user's
`known_hosts` file, then Cargo definitely shouldn't accept that key and
probably shouldn't suggest that the user add it to their `known_hosts`
either.
The change also adds support for detecting ``@cert-authority`` markers in
`known_hosts` files. These lines cannot yet be used for host key
verification, but if one is found for a matching host, the user will be
informed that Cargo doesn't support ``@cert-authority`` markers in the
error message. Additionally, the user will be advised to use the
`net.git-fetch-with-cli` config option to use the command line git
client for fetching crates from Git.
Refs: #11577
### How should we test and review this PR?
The changes in this PR are covered by unit tests, all within
`src/cargo/sources/git/known_hosts.rs`.
Additionally, manual testing can be performed. For this you will need
an OpenSSH server (it doesn't need to be a Git server). I'll assume
that you have one running on your local machine at `127.0.0.1`.
#### Setup
1. Create a new Cargo project and add the following line to `[dependencies]`:
```toml
fake-crate = { git = "ssh://127.0.0.1/fake-crate.git" }
```
#### Test missing host key: `HostKeyNotFound` (existing functionality)
1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`.
2. Verify host key not present: `ssh 127.0.0.1`. SSH should tell you `The authenticity of host '127.0.0.1 (127.0.0.1)' can't be established.`
3. Run `cargo build`
4. Expect error from Cargo: `error: unknown SSH host key`
#### Test ``@revoked`` key: `HostKeyRevoked`
1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`.
2. Add host key: `ssh 127.0.0.1` answer `yes`
3. Find all lines in `known_hosts` beginning with `127.0.0.1` (there may be multiple).
4. Add ``@revoked` ` to the beginning of all lines in (3)
5. Run `cargo build`
6. Expect error from Cargo: error: Key has been revoked for `127.0.0.1`
#### Test `@cert-authority`` (not being supported): `HostHasOnlyCertAuthority`
1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`.
2. Run `cargo build`
3. Expect error from Cargo: `error: unknown SSH host key`
4. Check the line after ` The key to add is:` in the error message and copy the key type (e.g. `ecdsa-sha2-nistp256`)
5. Add a line to `known_hosts`: ``@cert-authority` 127.0.0.1 <key-type> AAAAB5Wm` (e.g. ``@cert-authority` 127.0.0.1 ecdsa-sha2-nistp256 AAAAB5Wm`)
7. Run `cargo build`
8. Expect error from Cargo: error: Found a ``@cert-authority`` marker for `127.0.0.1`
### Additional information
Cargo doesn't currently support a few things when checking host keys. This may affect the testing described above.
* Multiple host key types (OpenSSH negotiates the host key type and can support matching the one present in the `known_hosts` file even when it's not the preferred type of the server).
* Wildcard matching of host patterns (there's a FIXME for this)
More information about SSH known host markers can be found
on #11577.1 file changed
+312
-52
lines changed
0 commit comments