Skip to content

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 9, 2022
Merged

Consider additional default ssh keys for LibGit2. #44767

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions stdlib/LibGit2/src/callbacks.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,15 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Cvoid}}, p::CredentialPayload,
cred.user = unsafe_string(username_ptr)
end

cred.prvkey = Base.get(ENV, "SSH_KEY_PATH") do
default = joinpath(homedir(), ".ssh", "id_rsa")
if isempty(cred.prvkey) && isfile(default)
default
else
cred.prvkey
if haskey(ENV, "SSH_KEY_PATH")
cred.prvkey = ENV["SSH_KEY_PATH"]
elseif isempty(cred.prvkey)
for keytype in ("rsa", "ecdsa")
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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).

:)

private_key_file = joinpath(homedir(), ".ssh", "id_$keytype")
if isfile(private_key_file)
cred.prvkey = private_key_file
break
end
end
end

Expand Down