Skip to content

Commit

Permalink
sftpfs: don't set preferred hostkey methods too restrictively.
Browse files Browse the repository at this point in the history
This fixes "sftp: failure establishing SSH session (-5)" error that
may appear on some systems when using SFTP link feature. The error
appears even when connecting to the same host via the "ssh" command
works. Whether the error appears or not depends on the content of
~/.ssh/known_hosts file.

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.

2. 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
libssh2/libssh2#676 (comment).

Signed-off-by: Andrew Borodin <[email protected]>
  • Loading branch information
wentasah authored and aborodin committed Oct 22, 2023
1 parent 789f951 commit 8303cca
Showing 1 changed file with 44 additions and 3 deletions.
47 changes: 44 additions & 3 deletions src/vfs/sftpfs/connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,37 @@ static const char *const hostkey_method_ssh_ecdsa_256 = "ecdsa-sha2-nistp256";
static const char *const hostkey_method_ssh_rsa = "ssh-rsa";
static const char *const hostkey_method_ssh_dss = "ssh-dss";

/* *INDENT-OFF* */
static const char *default_hostkey_methods =
#ifdef LIBSSH2_KNOWNHOST_KEY_ECDSA_256
"ecdsa-sha2-nistp256,"
#endif
#ifdef LIBSSH2_KNOWNHOST_KEY_ECDSA_384
"ecdsa-sha2-nistp384,"
#endif
#ifdef LIBSSH2_KNOWNHOST_KEY_ECDSA_521
"ecdsa-sha2-nistp521,"
#endif
#ifdef LIBSSH2_KNOWNHOST_KEY_ECDSA_256
"[email protected],"
#endif
#ifdef LIBSSH2_KNOWNHOST_KEY_ECDSA_384
"[email protected],"
#endif
#ifdef LIBSSH2_KNOWNHOST_KEY_ECDSA_521
"[email protected],"
#endif
#ifdef LIBSSH2_KNOWNHOST_KEY_ED25519
"ssh-ed25519,"
"[email protected],"
#endif
"rsa-sha2-256,"
"rsa-sha2-512,"
"ssh-rsa,"
"[email protected],"
"ssh-dss";
/* *INDENT-ON* */

/**
*
* The current implementation of know host key checking has following limitations:
Expand Down Expand Up @@ -257,8 +288,10 @@ sftpfs_read_known_hosts (struct vfs_s_super *super, GError ** mcerror)
continue;

if (store->name == NULL)
found = TRUE;
else if (store->name[0] != '[')
/* Ignore hashed hostnames. Currently, libssh2 offers no way for us to match it */
continue;

if (store->name[0] != '[')
found = strcmp (store->name, super->path_element->host) == 0;
else
{
Expand All @@ -285,6 +318,7 @@ sftpfs_read_known_hosts (struct vfs_s_super *super, GError ** mcerror)
{
int mask;
const char *hostkey_method = NULL;
char *hostkey_methods;

mask = store->typemask & LIBSSH2_KNOWNHOST_KEY_MASK;

Expand Down Expand Up @@ -326,8 +360,15 @@ sftpfs_read_known_hosts (struct vfs_s_super *super, GError ** mcerror)
return FALSE;
}

/* Append the default hostkey methods (with lower priority).
* Since we ignored hashed hostnames, the actual matching host
* key might have different type than the one found in
* known_hosts for non-hashed hostname. Methods not supported
* by libssh2 it are ignored. */
hostkey_methods = g_strdup_printf ("%s,%s", hostkey_method, default_hostkey_methods);
rc = libssh2_session_method_pref (sftpfs_super->session, LIBSSH2_METHOD_HOSTKEY,
hostkey_method);
hostkey_methods);
g_free (hostkey_methods);
if (rc < 0)
goto err;
}
Expand Down

0 comments on commit 8303cca

Please sign in to comment.