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

Change the ssh key default scope to github.com #20512

Merged
merged 4 commits into from Oct 19, 2022
Merged

Conversation

alexmighty
Copy link
Contributor

@alexmighty alexmighty commented Sep 9, 2022

Why:

Closes #21333

With the current host wildcard suggested snippet configuration, the new key is scoped to all ssh-agent connection attempts regardless of host. This can become an issue when introducing this snippet to a (large) existing ssh-agent configuration in use by older systems which do not have knowledge of the recommended ed25519 algorithm. While the key created by following the instructions in this page could be reused for multiple purposes outside of github, I am proposing here to change the default scope of the snippet to the github.com host in order to avoid polluting the global namespace of the ssh-agent config.

What's being changed (if available, include any code snippets, screenshots, or gifs):

Change the recommended ssh-agent config snippet from:

Host *
  AddKeysToAgent yes
  IdentityFile ~/.ssh/id_ed25519
  UseKeychain yes

to

Host *.github.com
  AddKeysToAgent yes
  IdentityFile ~/.ssh/id_ed25519
  UseKeychain yes

Check off the following:

  • I have reviewed my changes in staging (look for the "Automatically generated comment" and click the links in the "Preview" column to view your latest changes).
  • For content changes, I have completed the self-review checklist.

Writer impact (This section is for GitHub staff members only):

  • This pull request impacts the contribution experience
    • I have added the 'writer impact' label
    • I have added a description and/or a video demo of the changes below (e.g. a "before and after video")

@welcome
Copy link

welcome bot commented Sep 9, 2022

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Sep 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

Automatically generated comment ℹ️

This comment is automatically generated and will be overwritten every time changes are committed to this branch.

The table contains an overview of files in the content directory that have been changed in this pull request. It's provided to make it easy to review your changes on the staging site. Please note that changes to the data directory will not show up in this table.


Content directory changes

You may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.

Source Preview Production What Changed
authentication/connecting-to-github-with-ssh/generating-a-new-ssh-key-and-adding-it-to-the-ssh-agent.md fpt
ghec
ghes@ 3.6 3.5 3.4 3.3 3.2
ghae
fpt
ghec
ghes@ 3.6 3.5 3.4 3.3 3.2
ghae

fpt: Free, Pro, Team
ghec: GitHub Enterprise Cloud
ghes: GitHub Enterprise Server
ghae: GitHub AE

@cmwilson21
Copy link
Contributor

@alexmighty Thanks for opening this PR! I noticed it's still a draft. Are you still working on it or is it ready for review? 👀

@alexmighty
Copy link
Contributor Author

Hey @cmwilson21 thanks for checking in! I've rebased the PR and transitioned it to the review state. I believe it is now ready for consideration, please let me know if there is anything missing like a ticket or anything else. cheers

@cmwilson21 cmwilson21 added content This issue or pull request belongs to the Docs Content team waiting for review Issue/PR is waiting for a writer's review ssh Content related to SSH authentication Content relating to authentication and removed triage Do not begin working on this issue until triaged by the team labels Sep 14, 2022
@cmwilson21
Copy link
Contributor

@alexmighty Thanks for letting me know! I'll get this triaged for review ⚡

@cmwilson21 cmwilson21 added the needs SME This proposal needs review from a subject matter expert label Oct 12, 2022
@github-actions
Copy link
Contributor

Thanks for opening a pull request! We've triaged this issue for technical review by a subject matter expert 👀

@vtbassmatt
Copy link
Contributor

👋 hi from the product manager of Git Systems at GitHub. Thanks for making this contribution! I've been asked to do a quick tech review.

I could go either way here. On one hand, using a wildcard "pushes you into the pit of success", so to speak, if you're a novice to the world of SSH config. It's not our intent to document all aspects of SSH configuration, and I think most individual users should be using a single key everywhere (for ease of management). On the other hand, fully specifying github.com shouldn't make this scenario any worse and would cover the more complex cases like yours.

My instinct is that this isn't common enough to warrant a change: you have to be both in a complex SSH environment and be unaware of how scopes in SSH config work. But I have no real data to back that up.

@ChrisKeefe
Copy link

ChrisKeefe commented Oct 14, 2022

+1 for narrowing the scope.

In my case, the recommended wildcard broke the ssh config on a managed work computer and locked me out of some important resources. I was previously unaware of how ssh config scopes work, as all prior config was provided by the company.

Incidentally, this PR probably closes #21333, which I opened after solving this problem for myself.

@vtbassmatt
Copy link
Contributor

Thanks for the additional perspective! I'm convinced, it's worth making a change.

We will need to fixup the wording for GHES customers, as *.github.com is not correct for them. I'm not sure how we've handled that elsewhere in the docs where we need to refer to the customer's domain name; hopefully one of my colleagues from the Content team can weigh in here.

With the current host wildcard default configuration, the new key applies to all ssh connection attempts regardless of host. This can become an issue on older systems that leverage existing keys in the ssh config and don't have knowledge of the "newer" recommended ed25519 algorithm. While this key can be reused for multiple purposes outside of github, I still propose to scope the default host to github.com to avoid a default that can pollute the global key namespace in the ssh config.
@alexmighty
Copy link
Contributor Author

Rebased. @vtbassmatt, I certainly appreciate how on the fence this one can be!

@vtbassmatt
Copy link
Contributor

@cmwilson21 are you the right person to advise on how we handle this for GHES?

@cmwilson21
Copy link
Contributor

@vtbassmatt, not me but I know some folks 😄

Thanks for the ping. I'll dig around and find out!

Copy link
Contributor

@vgrl vgrl left a comment

Choose a reason for hiding this comment

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

Thanks @alexmighty ! I am going to apply a few small changes, then I'll double check on preview and merge if all looks good! ⚡

Copy link
Contributor

@vgrl vgrl left a comment

Choose a reason for hiding this comment

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

Looks good on preview! 🎉

@vgrl vgrl removed the needs SME This proposal needs review from a subject matter expert label Oct 19, 2022
@vgrl vgrl added ready to merge This pull request is ready to merge and removed waiting for review Issue/PR is waiting for a writer's review labels Oct 19, 2022
@docubot docubot added this to Triage in Docs open source board Oct 19, 2022
@vgrl vgrl enabled auto-merge October 19, 2022 23:19
@vgrl vgrl merged commit 7783dc5 into github:main Oct 19, 2022
Docs open source board automation moved this from Triage to Done Oct 19, 2022
@github-actions
Copy link
Contributor

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues

@vtbassmatt
Copy link
Contributor

vtbassmatt commented Oct 20, 2022

@vgrl do we need a leading *. on the GHES/GHAE version? (Genuine question, I don't know if those products use multiple hostnames for various services like we do in dotcom.)

Edit: nevermind. Since this is for Git over SSH, it should only be one hostname anyhow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication Content relating to authentication content This issue or pull request belongs to the Docs Content team ready to merge This pull request is ready to merge ssh Content related to SSH
Development

Successfully merging this pull request may close these issues.

MacOS 12+ - Narrow Host definition in .ssh/config
5 participants