-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 gitlab.com ssh host keys as builtin #11564
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks!
Just some questions and ideas: Are there more vendors worth adding as built-in? I wonder How to draw a line between built-in and user opt-in. We've talked about introduce an interactive console but the time was limited as it was a CVE fix. I feel like we could explore more on this direction.
@weihanglo you bring up good points! I have only added gitlab in this PR because gitlab is probably the biggest git host after github, and I also have a gitlab account so I was able to test whether the keys work. If you want to join the rust-lang zulip, you are shown options to log in via four ways: a native zulip account, google, github, gitlab. If one wants to make a more extensive list than the two, one could probably do some googling to find lists on the internet of the biggest git hosters. That would still be quite arbitrary but it would give a starting point. E.g. this list or this list. I have checked the hosts from the union of the two lists for their host keys (plus source hut). Most of these only specify the fingerprints:
The host keys can still be obtained however, by e.g. running Google maintains a hsts preload list as well as a list of websites with pinned tls CAs. Both lists are quite long, so there is precedent in having non-trivially short hardcoded lists. Of course however, maintenance of those lists requires an overhead. Which brings us to the other point: probably such lists need to be kept up to date. SSH host keys don't change every day, but they probably still change every now and then. There should be some kind of automation to detect such changes. For two hosts github and gitlab we can probably get away with manual checking that these keys are up to date, but more niche hosts might not even have one user, so we might ship wrong keys for years until we notice problems. The most automatable way to obtain the keys would probably be to run |
IMO it should be quite uncontroversial to add gitlab, we can discuss these other questions (automation, which other hosts to add) orthogonally from having this PR merged. |
Thank you for your analysis and inputs! I am totally fine with adding GitLab. Let's wait for others opinions :) |
Nominating for further discussion. |
We discussed this in today's @rust-lang/cargo meeting. The only reason we include the Once we switch to sparse registries, we can consider dropping even the github SSH key. |
I don't really agree that this justification is enough. If someone edits their configuration and uses Personally the main impact I see in the hardcoding of those SSH keys is a symbolic one. It's documented in high level documentation that This worsens an already existing problem. Already before #11556 , cargo/crates.io has been considering github alternatives way too little. E.g. to publish on crates.io you must have a github account. #11556 has made the problem even worse. Having a close relationship to github for rust-the-oss-project (repo hosting, CI use, free index hosting, etc) is nice and totally fine, but it should not bleed into what Rust does facing towards its users as rust-the-technology. There should be a limit in the marketing value github.com gets from providing free cloud services to Rust. Having "Rust uses us" on their list should be enough, no need for having statements that amount to "Rust has made connecting to github.com especially easy" in the documentation. That's what rubs me wrong here. Gitlab would have helped at least to some degree. |
I think that's a fairly convincing argument for removing github hard coded keys. |
I would very much like to see those keys removed, rather than adding more, yes. Let's plan on doing that as soon as we switch to https indexes by default. |
PR #11556 has added github.com ssh host keys as builtin. This adds gitlab.com ssh host keys.
The "unknown SSH host key" error is quite annoying as, unlike ssh, it does not allow you to press
y
and continue. Instead you are asked to manually edit config files.I've tested it locally and I've confirmed both the error with latest master, as well as that the builtin key works with this PR applied.
The keys can be found in this URL.
r? @ehuss