Ticket #4506 (closed defect: fixed)

Opened 14 months ago

Last modified 13 months ago

sftp: failure establishing SSH session (-5) due hashed host names in ~/.ssh/known_hosts

Reported by: wentasah Owned by: andrew_b
Priority: major Milestone: 4.8.31
Component: mc-vfs Version: master
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

MC fails with "sftp: failure establishing SSH session (-5)" even when connecting to the same host via the "ssh" command
works. I have analyzed the problem and found that whether the error appears or not depends on the content of ~/.ssh/known_hosts file. I have a patch with a fix.

Problem description:

Midnight Commander uses ~/.ssh/known_hosts for two reasons. Obviously,
one reason is checking for hostkey match after the SSH handshake. The
second reason is to set preferences which host key the remote side
should send us during the SSH handshake. And this is the problematic
place.

Entries in ~/.ssh/known_hosts store host names either in plain text or
in a hashed form. libssh2 does not export host name hashes, only plain
text host names. When mc tries to find a matching entry to set hostkey
preferences, it cannot cannot reliably match the hashed host names.
Before this change, mc assumed that any entry with hashed host name
matches the connecting host and set hostkey preference to the type of
that key. In many cases, this was incorrect. For example, when the
first hashed entry in ~/.ssh/known_hosts appeared before the matching
non-hashed one, and its key type was ssh-rsa, which is disabled by
default since OpenSSH 8.8 (released 2021-09-26), then mc requested
only the ssh-rsa host key from the remote host. Since this host key is
likely disabled these days, no key was sent by the remote host and mc
reported error -5 (LIBSSH2_ERROR_KEX_FAILURE).

Solution:

In this commit, we fix the problem as follows:

  1. When finding a matching known_hosts entry in order to set the preferred hostkey method, we ignore the entries with hashed host names. If we find no matching entry with the plain text host name, no preference is set, resulting in the server sending us whatever key it wants and our libssh2 supports it. Likely, that key will match an entry with hashed host name later during the host key check.
  1. If, on the other hand, a matching plain text entry is found, we use its type as a preference, but newly, we add other methods as a fallback. If the matched entry has a server-supported key type, it will be used. If it is not supported by the server (e.g. the old ssh-rsa type), the added fallback ensures that the server sends us some host key, which will likely match an entry with hashed host name later during the host key check.

This solution is not ideal, but I think it's good enough. For example,
the following situation is not solved ideally (I think): The
known_hosts file contains a single entry for some server. It has a
hashed host name and key of type B. Since we ignore hashed entries,
the server can send its host key as type A, which is higher on the
preference list. To the user, it will appear as that she has never
connected to that server before. After accepting the new key, it will
be added to known_hosts and the problem disappears.

Ideal solution would IMHO be to create libssh2_knownhost_find()
function in libssh2. It would allow finding all matching entries (even
with hashed host names) in known_hosts. Midnight commander would then
use all key types of found entries as its preference.

Note: Since the code modified by this commit was inspired by code from
curl, curl has the same problem. See
https://github.com/libssh2/libssh2/issues/676#issuecomment-1741877207.

Attachments

0001-sftpfs-Don-t-set-preferred-hostkey-methods-too-restr.patch (6.4 KB) - added by wentasah 14 months ago.
0001-sftpfs-Don-t-set-preferred-hostkey-methods-too-restr.2.patch (2.5 KB) - added by andrew_b 14 months ago.
Modified patch with fixed memory management

Change History

comment:1 Changed 14 months ago by andrew_b

Patch review:

  • patch requires libssh2 >= 1.11 released 2023-05-29, mc currently requires libssh2 >= 1.2.8. mc should work on LTS systems with libssh2 < 1.11
  • don't mix glib and glibc memory management. Modified patch attached.

Changed 14 months ago by andrew_b

Modified patch with fixed memory management

comment:2 Changed 14 months ago by wentasah

The patch does not require libssh2 >= 1.11. I've just verified that it builds and runs fine with libssh2 1.2.8:

$ ./src/mc --version
GNU Midnight Commander 4.8.30-15-g3d6a74afd
Built with GLib 2.76.4
Built with S-Lang 2.3.3 with terminfo database
Built with libssh2 1.2.8
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm
With internationalization support
With multiple codepages support
Virtual File Systems:
 cpiofs, tarfs, sfs, extfs, ftpfs, sftpfs, fish
Data types:
 char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

However, with this version, I cannot connect anywhere as all my remote hosts fail either with LIBSSH2_ERROR_KEX_FAILURE (-5) or LIBSSH2_ERROR_INVALID_MAC (-4). The following appears in server's log:

Unable to negotiate with 1.2.3.4 port 49948: no matching key exchange method found. Their offer: diffie-hellman-group14-sha1,diffie-hellman-group-exchange-sha1,diffie-hellman-group1-sha1 [preauth]

I guess, this is expected due to the use of nowadays insecure sha1.

comment:3 Changed 14 months ago by andrew_b

  • Status changed from new to accepted
  • Owner set to andrew_b
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.31

Thanks! Patch is applied with some minor modifications.

Branch: 4506_sftpfs_hashed_hostname
changeset:57738ef4cddb70b850375997299d9b196a35d984

comment:4 Changed 13 months ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:5 Changed 13 months ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

comment:6 Changed 13 months ago by andrew_b

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.