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 documentation that SSH markers aren't supported #11586

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

hds
Copy link
Contributor

@hds hds commented Jan 16, 2023

What does this PR try to resolve?

Cargo doesn't support the @cert-authority or @revoked markers in SSH
Known Hosts files. The lines are silently ignored.

If a user is depending on these lines to connect to a Git server via
SSH, then their command line Git client will work, but Cargo will fail
with an error that the host key doesn't match.

This change adds a note explaining that Cargo doesn't support these
markers and suggests that the user change their cargo configuration to
fetch with the CLI client instead.

This PR fixes the first part (of 4) of the suggested tasks to fix #11577.

How should we test and review this PR?

This change only modifies the Cargo book source. Running mdbook build
and checking the SSH Known Hosts section of the appendix on Git
authentication should be sufficient to test the PR.

Additional information

The note in this section repeats what is said in the higher section on
SSH authentication, however given the recently implemented host key
checking, it seems worth calling out the limitation that Cargo doesn't
support these markers in the SSH known hosts file explicitly. Hopefully,
it reduces support requests and questions as well.

Cargo doesn't support the `@cert-authority` or `@revoked` markers in SSH
Known Hosts files. The lines are silently ignored.

If a user is depending on these lines to connect to a Git server via
SSH, then their command line Git client will work, but Cargo will fail
with an error that the host key doesn't match.

This change adds a note explaining that Cargo doesn't support these
markers and suggests that the user change their cargo configuration to
fetch with the CLI client instead.

Refs: rust-lang#11577
@rustbot
Copy link
Collaborator

rustbot commented Jan 16, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 16, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice. Thank you for taking care of this!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 16, 2023

📌 Commit f460ac2 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2023
@bors
Copy link
Collaborator

bors commented Jan 16, 2023

⌛ Testing commit f460ac2 with merge 8681a5a...

@bors
Copy link
Collaborator

bors commented Jan 16, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 8681a5a to master...

@bors bors merged commit 8681a5a into rust-lang:master Jan 16, 2023
@hds hds deleted the ssh-known-hosts-doc-update branch January 16, 2023 14:45
weihanglo added a commit to weihanglo/rust that referenced this pull request Jan 17, 2023
9 commits in 1cd6d3803dfb0b342272862a8590f5dfc9f72573..a5d47a72595dd6fbe7d4e4f6ec20dc5fe724edd1
2023-01-12 18:40:36 +0000 to 2023-01-16 18:51:50 +0000

- Add network container tests (rust-lang/cargo#11583)
- Show progress of crates.io index update even `net.git-fetch-with-cli` option enabled (rust-lang/cargo#11579)
- `cargo metadata` supports artifact dependencies (rust-lang/cargo#11550)
- fix(docs): add required "inherits" option to example profile (rust-lang/cargo#11504)
- add documentation that SSH markers aren't supported (rust-lang/cargo#11586)
- Fix typo (rust-lang/cargo#11585)
- Enable source_config_env test on Windows (rust-lang/cargo#11582)
- Support `codegen-backend` and `rustflags` in profiles in config file (rust-lang/cargo#11562)
- ci: reflect to clap updates (rust-lang/cargo#11578)
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 18, 2023
Update cargo

9 commits in 1cd6d3803dfb0b342272862a8590f5dfc9f72573..a5d47a72595dd6fbe7d4e4f6ec20dc5fe724edd1 2023-01-12 18:40:36 +0000 to 2023-01-16 18:51:50 +0000

- Add network container tests (rust-lang/cargo#11583)
- Show progress of crates.io index update even `net.git-fetch-with-cli` option enabled (rust-lang/cargo#11579)
- `cargo metadata` supports artifact dependencies (rust-lang/cargo#11550)
- fix(docs): add required "inherits" option to example profile (rust-lang/cargo#11504)
- add documentation that SSH markers aren't supported (rust-lang/cargo#11586)
- Fix typo (rust-lang/cargo#11585)
- Enable source_config_env test on Windows (rust-lang/cargo#11582)
- Support `codegen-backend` and `rustflags` in profiles in config file (rust-lang/cargo#11562)
- ci: reflect to clap updates (rust-lang/cargo#11578)

r? `@ghost`
@weihanglo
Copy link
Member

Congrats @hds! Your doc enhancement is live on https://doc.rust-lang.org/nightly/cargo/appendix/git-authentication.html#ssh-known-hosts. Check it out. Also thank you again!

@hds
Copy link
Contributor Author

hds commented Jan 20, 2023

Fantastic! Thanks for the review and for letting me know!

@ehuss ehuss added this to the 1.68.0 milestone Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cargo built-in Git/SSH client doesn't support @cert-authority
5 participants