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

SSH host verification fails when IP changes (git succeeds) #2334

Closed
iamed2 opened this issue Jan 13, 2021 · 22 comments
Closed

SSH host verification fails when IP changes (git succeeds) #2334

iamed2 opened this issue Jan 13, 2021 · 22 comments

Comments

@iamed2
Copy link
Contributor

iamed2 commented Jan 13, 2021

Our git server is behind a load balancer, so the IP changes frequently. This is the git behaviour:

G1. On first clone, it says the authenticity of the host cannot be established, here's the fingerprint, do you want to trust it. If the user says yes, an entry is added to the ~/.ssh/known_hosts file in the form <hostname>,<ip> <key type> <fingerprint>.
G2. On the first clone after the IP changes, git sees an entry for the same host name with the same fingerprint and a different IP, adds an entry to the ~/.ssh/known_hosts file in the form <ip> <key type> <fingerprint>, and warns the user that this took place. This only happens when the host name and fingerprint are identical.

This is the Julia behaviour:

J1. On first clone, it shows "SSH host verification: the server <hostname> is not a known host. Please run ssh-keyscan <hostname> >> ~/.ssh/known_hosts in order to add the server to your known hosts file and then try again.".
J2. If the user does that, then entries in the form of <hostname> <key type> <fingerprint> are added to the ~/.ssh/known_hosts file are added.
J3. On the first clone after the IP changes, Julia succeeds

Mixing these can cause interesting behaviour.

J1, J2, G2 is fine, though notably git still adds those IP entries to the ~/.ssh/known_hosts file.

G1, J3 does not work. Instead Julia will fail and say "SSH host verification: the identity of the server <hostname> does not match its known hosts record.". This is the problem.

I say Julia should either a) allow the connection or b) give the prompt to run ssh-keyscan, which will cause J3 to succeed.

StefanKarpinski added a commit to JuliaLang/julia that referenced this issue Jan 19, 2021
Fixes #38777.
Might fix JuliaLang/Pkg.jl#2334.

It seems likely no one actually verifies SSH host identity with libgit2
because the callback doesn't give enough information do so correctly:

- It doesn't give the actual host key fingerprint, but rather three
  different hashes thereof; this means we cannot distinguish a known
  hosts entry that has a different type (`ssh-rsa`, `ssh-dsa`, etc.)
  versus an entry with a matching type and a fingerprint mismatch. The
  former should be treated as an unknown host whereas the latter is a
  host key mismatch; they cananot be distinguished with this patch.

- If the user connects on a non-default port (i.e. not 22), this is not
  passed to the callback in any way. Since there can be different known
  host entries for different ports and they should be treated as
  distinct, this also means the current API cannot be used to verify
  hosts serving SSH on non-standard ports. This patch passes the port.

I will try to upstream some version of this patch to libgit2. The same
patch has already been applied to the LibGit2 JLL.
StefanKarpinski added a commit to JuliaLang/julia that referenced this issue Jan 20, 2021
Fixes #38777.
Might fix JuliaLang/Pkg.jl#2334.

It seems likely no one actually verifies SSH host identity with libgit2
because the callback doesn't give enough information do so correctly:

- It doesn't give the actual host key fingerprint, but rather three
  different hashes thereof; this means we cannot distinguish a known
  hosts entry that has a different type (`ssh-rsa`, `ssh-dsa`, etc.)
  versus an entry with a matching type and a fingerprint mismatch. The
  former should be treated as an unknown host whereas the latter is a
  host key mismatch; they cananot be distinguished with this patch.

- If the user connects on a non-default port (i.e. not 22), this is not
  passed to the callback in any way. Since there can be different known
  host entries for different ports and they should be treated as
  distinct, this also means the current API cannot be used to verify
  hosts serving SSH on non-standard ports. This patch passes the port.

I will try to upstream some version of this patch to libgit2. The same
patch has already been applied to the LibGit2 JLL.
@StefanKarpinski
Copy link
Member

@iamed2: if you could please check if this fixes the issue with your server, that would be appreciated.

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 22, 2021

I'm getting a segfault now

               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.7.0-DEV.359 (2021-01-21)
 _/ |\__'_|_|_|\__'_|  |  Commit c242166ab0* (0 days old master)
|__/                   |

(@v1.7) pkg> registry up Invenia
    Updating registry at `~/.julia/registries/Invenia`
    Updating git-repo `git@gitlab.invenia.ca:invenia/PackageRegistry.git`

signal (11): Segmentation fault: 11
in expression starting at none:0
_platform_strcmp at /usr/lib/system/libsystem_platform.dylib (unknown line)
Allocations: 3716815 (Pool: 3715382; Big: 1433); GC: 4
fish: './julia' terminated by signal SIGSEGV (Address boundary error)

Happens even in the scenario where the correct keys are present, or when no keys are present.

I ran make cleanall before building master but if there's something else I need to clean up I can do that too.

Julia Version 1.7.0-DEV.359
Commit c242166ab0* (2021-01-21 19:37 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin20.2.0)
  CPU: Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, icelake-client)

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 22, 2021

Ah yeah looks like I need to do something else as LibGit2.VERSION == v"1.1.0". Seems weird that this isn't covered by make cleanall...

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 22, 2021

Nope, it downloaded LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz but LibGit2.VERSION == v"1.1.0".

@omus
Copy link
Member

omus commented Jan 22, 2021

Maybe try make -C deps distclean-libgit2 && make -C deps install-libgit2 just to be sure. You can also verify the version number by looking at usr/lib

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 22, 2021

I did a full clean build with git clean -fxd. The build LibGit2-v1.2.1+0 just contains libgit2.1.1.0.dylib. Check out the release: https://github.com/JuliaBinaryWrappers/LibGit2_jll.jl/releases/tag/LibGit2-v1.2.1%2B0

@omus
Copy link
Member

omus commented Jan 22, 2021

The version used by build_tarballs.jl does point to libgit2 1.1.0 which is the latest version. I thought the JLL version was supposed to match the library version but I may be wrong about that.

KristofferC pushed a commit to JuliaLang/julia that referenced this issue Jan 22, 2021
It seems that no one actually verifies SSH host identity with libgit2
because the callback doesn't give enough information do so correctly:

- It doesn't give the actual host key fingerprint, but rather three
  different hashes thereof. This means we cannot distinguish a known
  hosts entry that has a different type (`ssh-rsa`, `ssh-dsa`, etc.)
  from an entry with a matching type and a fingerprint mismatch: the
  former should be treated as an unknown host whereas the latter is a
  host key mismatch; they cannot be distinguished without this patch.

- If the user connects on a non-default port (i.e. not 22), this is not
  passed to the callback in any way. Since there can be different known
  host entries for different ports and they should be treated as
  distinct, this also means the current API cannot be used to verify
  hosts serving SSH on non-standard ports. This patch passes the port.

I will try to upstream some version of this patch to libgit2. The same
patch has already been applied to the LibGit2 JLL.

Fixes #38777.
Might fix JuliaLang/Pkg.jl#2334.

(cherry picked from commit 2b13234)
@iamed2
Copy link
Contributor Author

iamed2 commented Jan 22, 2021

I found a slack message thanks to Mose that says this has changed and specifically LibGit2 was changed to have a greater version.

@StefanKarpinski
Copy link
Member

This was broken when the git URL used the user@host:path format. After JuliaLang/julia#39364 is merged, it should be fixed and you can hopefully try again (or try that branch).

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 25, 2021

G1 then updating the registry (no IP change) no longer works. In addition, it throws an UndefVarError.

(@v1.7) pkg> registry up Invenia
    Updating registry at `~/.julia/registries/Invenia`
    Updating git-repo `git@gitlab.invenia.ca:invenia/PackageRegistry.git`
SSH host verification: the identity of the server `gitlab.invenia.ca:22` does not match its known hosts record. Someone could be trying to man-in-the-middle your connection. It is also possible that the server has changed its key, in which case you should check with the server administrator and if they confirm that the key has been changed, update your known hosts file.
ERROR: UndefVarError: PkgError not defined
Stacktrace:
  [1] (::Pkg.Registry.var"#46#49"{Base.TTY, Vector{Tuple{String, String}}, String, Pkg.Registry.RegistryInstance})(repo::LibGit2.GitRepo)
    @ Pkg.Registry ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/Pkg/src/Registry/Registry.jl:328
  [2] with(f::Pkg.Registry.var"#46#49"{Base.TTY, Vector{Tuple{String, String}}, String, Pkg.Registry.RegistryInstance}, obj::LibGit2.GitRepo)
    @ LibGit2 ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/LibGit2/src/types.jl:1150
  [3] update(regs::Vector{Pkg.Registry.RegistrySpec}; io::Base.TTY, force::Bool)
    @ Pkg.Registry ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/Pkg/src/Registry/Registry.jl:311
  [4] update(regs::Vector{Pkg.Registry.RegistrySpec})
    @ Pkg.Registry ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/Pkg/src/Registry/Registry.jl:280
  [5] do_cmd!(command::Pkg.REPLMode.Command, repl::REPL.LineEditREPL)
    @ Pkg.REPLMode ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/Pkg/src/REPLMode/REPLMode.jl:407
  [6] do_cmd(repl::REPL.LineEditREPL, input::String; do_rethrow::Bool)
    @ Pkg.REPLMode ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/Pkg/src/REPLMode/REPLMode.jl:385
  [7] do_cmd
    @ ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/Pkg/src/REPLMode/REPLMode.jl:376 [inlined]
  [8] (::Pkg.REPLMode.var"#24#27"{REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::IOBuffer, ok::Bool)
    @ Pkg.REPLMode ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/Pkg/src/REPLMode/REPLMode.jl:549
  [9] #invokelatest#2
    @ ./essentials.jl:710 [inlined]
 [10] invokelatest
    @ ./essentials.jl:708 [inlined]
 [11] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/REPL/src/LineEdit.jl:2441
 [12] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL ~/repos/juliamaster/usr/share/julia/stdlib/v1.7/REPL/src/REPL.jl:1126
 [13] (::REPL.var"#44#49"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL ./task.jl:406

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 25, 2021

Hmm, this one is because because git only records the ecdsa-sha2-nistp256 host key, but libssh2 does not support that (https://www.libssh2.org/libssh2-vs-libssh.html)

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 25, 2021

I think in this case it's wrong to say SSH host verification: the identity of the server `gitlab.invenia.ca:22` does not match its known hosts record. and there should be a better, more specific message.

@fredrikekre fredrikekre reopened this Jan 25, 2021
@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jan 25, 2021

If you use ssh-keyscan it should record all of the of keys that the server has. We're already patching libgit2 because its SSH host verification support is so broken; I really don't know that it's reasonable to also try to fix libssh2 to support this or even to try to distinguish reasons why libssh2 can't verify a host. The workaround here may be to export JULIA_SSH_NO_VERIFY_HOSTS=gitlab.invenia.ca and go back to the old behavior of simply not verifying hosts. The solution may be to stop using libgit2 at all.

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 25, 2021

What is libssh2 telling you in this case? Are you getting LIBSSH2_KNOWNHOST_CHECK_MISMATCH or a different error value?

I'd really like to see the error message in this case suggest running ssh-keyscan, if possible.

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 25, 2021

Hmmm, libssh2 should support the modern keys as of version 1.9, but only with OpenSSL: https://www.libssh2.org/changes.html

I guess that won't work since we use MbedTLS?

The MbedTLS version supports ecdsa as of libssh2/libssh2#385 but that hasn't been released yet AFAICT.

@StefanKarpinski
Copy link
Member

Are you getting LIBSSH2_KNOWNHOST_CHECK_MISMATCH or a different error value?

Yes. You can see the logic here:

https://github.com/JuliaLang/julia/blob/f31ef767ef9cb0eb1de0/stdlib/LibGit2/src/callbacks.jl#L412-L413

If that error message gets printed then libssh2_knownhost_checkp returned LIBSSH2_KNOWNHOST_CHECK_MISMATCH. There's no other information that the call provides to indicate any difference between a real fingerprint mismatch and this situation where it just doesn't know about the fingerprint type.

Hmmm, libssh2 should support the modern keys as of version 1.9, but only with OpenSSL [...]
I guess that won't work since we use MbedTLS?

Yes, we use MbedTLS. We could probably switch to OpenSSL, although I'm not sure about the license compatibility. Personally ain't doing all that and it's way too late to be switching TLS engines for the 1.6 release.

The MbedTLS version supports ecdsa as of libssh2/libssh2#385 but that hasn't been released yet AFAICT.

Great, I guess this problem will fix itself when the make a new release.

Can you confirm if using ssh-keyscan and/or setting JULIA_SSH_NO_VERIFY_HOSTS works?

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 25, 2021

Can you confirm if using ssh-keyscan and/or setting JULIA_SSH_NO_VERIFY_HOSTS works?

Yes, both of those work.

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 25, 2021

It also successfully verifies when the IP changes (if I manually test with the ssh-rsa key and a changing IP), so this issue is technically resolved.

@iamed2 iamed2 closed this as completed Jan 25, 2021
@StefanKarpinski
Copy link
Member

If you want to, you can open an issue on https://github.com/JuliaLang/julia about libgit2 not supporting ecdsa-sha2-nistp256, which we can close when we upgrade to a version that does support it. Another possible course of action would be to file an issue with libssh2 requesting that they return a different code when a check fails because the fingerprint type isn't supported.

@iamed2
Copy link
Contributor Author

iamed2 commented Jan 25, 2021

I'll do both!

KristofferC pushed a commit to JuliaLang/julia that referenced this issue Feb 1, 2021
It seems that no one actually verifies SSH host identity with libgit2
because the callback doesn't give enough information do so correctly:

- It doesn't give the actual host key fingerprint, but rather three
  different hashes thereof. This means we cannot distinguish a known
  hosts entry that has a different type (`ssh-rsa`, `ssh-dsa`, etc.)
  from an entry with a matching type and a fingerprint mismatch: the
  former should be treated as an unknown host whereas the latter is a
  host key mismatch; they cannot be distinguished without this patch.

- If the user connects on a non-default port (i.e. not 22), this is not
  passed to the callback in any way. Since there can be different known
  host entries for different ports and they should be treated as
  distinct, this also means the current API cannot be used to verify
  hosts serving SSH on non-standard ports. This patch passes the port.

I will try to upstream some version of this patch to libgit2. The same
patch has already been applied to the LibGit2 JLL.

Fixes #38777.
Might fix JuliaLang/Pkg.jl#2334.

(cherry picked from commit 2b13234)
@racinmat

This comment has been minimized.

@goretkin
Copy link
Contributor

I might be experiencing the issue here. I had opened an issue at: #2428

git remote -v of the .julia/registries/MyRegistry shows URL git@gitlab.com:myregistry/localjuliaregistry.git so it is supposed to use ssh. How can I provide more information or figure out if it is indeed the same issue?

ElOceanografo pushed a commit to ElOceanografo/julia that referenced this issue May 4, 2021
…ang#39324)

It seems that no one actually verifies SSH host identity with libgit2
because the callback doesn't give enough information do so correctly:

- It doesn't give the actual host key fingerprint, but rather three
  different hashes thereof. This means we cannot distinguish a known
  hosts entry that has a different type (`ssh-rsa`, `ssh-dsa`, etc.)
  from an entry with a matching type and a fingerprint mismatch: the
  former should be treated as an unknown host whereas the latter is a
  host key mismatch; they cannot be distinguished without this patch.

- If the user connects on a non-default port (i.e. not 22), this is not
  passed to the callback in any way. Since there can be different known
  host entries for different ports and they should be treated as
  distinct, this also means the current API cannot be used to verify
  hosts serving SSH on non-standard ports. This patch passes the port.

I will try to upstream some version of this patch to libgit2. The same
patch has already been applied to the LibGit2 JLL.

Fixes JuliaLang#38777.
Might fix JuliaLang/Pkg.jl#2334.
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this issue May 9, 2021
…ang#39324)

It seems that no one actually verifies SSH host identity with libgit2
because the callback doesn't give enough information do so correctly:

- It doesn't give the actual host key fingerprint, but rather three
  different hashes thereof. This means we cannot distinguish a known
  hosts entry that has a different type (`ssh-rsa`, `ssh-dsa`, etc.)
  from an entry with a matching type and a fingerprint mismatch: the
  former should be treated as an unknown host whereas the latter is a
  host key mismatch; they cannot be distinguished without this patch.

- If the user connects on a non-default port (i.e. not 22), this is not
  passed to the callback in any way. Since there can be different known
  host entries for different ports and they should be treated as
  distinct, this also means the current API cannot be used to verify
  hosts serving SSH on non-standard ports. This patch passes the port.

I will try to upstream some version of this patch to libgit2. The same
patch has already been applied to the LibGit2 JLL.

Fixes JuliaLang#38777.
Might fix JuliaLang/Pkg.jl#2334.
staticfloat pushed a commit to JuliaLang/julia that referenced this issue Dec 23, 2022
It seems that no one actually verifies SSH host identity with libgit2
because the callback doesn't give enough information do so correctly:

- It doesn't give the actual host key fingerprint, but rather three
  different hashes thereof. This means we cannot distinguish a known
  hosts entry that has a different type (`ssh-rsa`, `ssh-dsa`, etc.)
  from an entry with a matching type and a fingerprint mismatch: the
  former should be treated as an unknown host whereas the latter is a
  host key mismatch; they cannot be distinguished without this patch.

- If the user connects on a non-default port (i.e. not 22), this is not
  passed to the callback in any way. Since there can be different known
  host entries for different ports and they should be treated as
  distinct, this also means the current API cannot be used to verify
  hosts serving SSH on non-standard ports. This patch passes the port.

I will try to upstream some version of this patch to libgit2. The same
patch has already been applied to the LibGit2 JLL.

Fixes #38777.
Might fix JuliaLang/Pkg.jl#2334.

(cherry picked from commit 2b13234)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants