-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Consider additional default ssh keys for LibGit2. #44767
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
Consider additional default ssh keys for LibGit2. #44767
Conversation
if haskey(ENV, "SSH_KEY_PATH") | ||
cred.prvkey = ENV["SSH_KEY_PATH"] | ||
elseif isempty(cred.prvkey) | ||
for keytype in ("rsa", "ecdsa") |
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.
Would it make sense to prefer ecdsa
if it exists?
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.
I thought about it but it may be considered breaking for people who are using RSA keys with Julia (sufficiently old ones still work with GitHub and other services may accept them) and also have an ECDSA key for other purposes but not registered where Julia is used (or just have one but doesn't use it).
I have no strong opinion whether that outweighs the advantage for many people of having an ECDSA key chosen over an RSA key. Either way I guess it needs to be communicated.
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.
FWIW ssh will try pretty much anything. You can confirm this with ssh -vvvv
. may be good to also have ed25519
, id_ecdsa_sk
, and id_ed25519_sk
on here as well.
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.
In fact, the GitHub docs instruct you to generate ed25519
keys now anyway, so that should at least be here as well.
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.
Command line ssh is quite capable, yes, but this code is for LibGit2/LibSSH2. Do you have any indications that those ciphers will help here?
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.
No, I misread:
Arguably the list of keys should be longer but at this point RSA and ECDSA keys are the only ones Julia's LibSSH2 understands (up to 1.7, RSA only).
:)
(cherry picked from commit b6b0874)
When LibGit2 needs an ssh key, it will by default only look for
~/.ssh/id_rsa
. Since GitHub dropped support for certain keys, new RSA keys will not work with Julia's default build of LibGit2/LibSSH2 to connect to GitHub (sufficiently old RSA keys should still be ok and other git services than GitHub may still work fine).For Julia 1.8 and later, ECDSA keys work with LibGit2/LibSSH2 and are accepted by GitHub, but Julia will not find that key without prompting unless you specify it with
SSH_KEY_PATH
.With this PR Julia will default to
~/.ssh/id_ecdsa
if no~/.ssh/id_rsa
is found. Arguably the list of keys should be longer but at this point RSA and ECDSA keys are the only ones Julia's LibSSH2 understands (up to 1.7, RSA only).Further discussion in JuliaLang/Pkg.jl#3030.