[libvirt] [PATCH 0/2] Fix thrashing of user's known_hosts file.

Libvirt tried to use the user's default known_hosts file in .ssh/known_hosts for it's purposes, but libssh2 thrashes it if it contains unsupported keys. Whith this patchset, libvirt will maintain it's own known_hosts file in it's config directory. Peter Krempa (2): libssh2_session: Add support for creating known_hosts file client: Change default location of known_hosts file for libssh2 layer src/rpc/virnetclient.c | 17 ++++++++++------- src/rpc/virnetsocket.c | 4 ++-- src/rpc/virnetsshsession.c | 26 ++++++++++++++++---------- src/rpc/virnetsshsession.h | 9 +++++++-- 4 files changed, 35 insertions(+), 21 deletions(-) -- 1.7.8.6

The libssh2 code wasn't supposed to create the known_hosts file, but recent findings show, that we can't use the default created by OpenSSH as libssh2 might damage it. We need to create a private known_hosts file in the config path. This patch adds support for skipping error if the known_host file is not present and let libssh2 create a new one. --- src/rpc/virnetsocket.c | 4 ++-- src/rpc/virnetsshsession.c | 26 ++++++++++++++++---------- src/rpc/virnetsshsession.h | 9 +++++++-- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 530c081..5a48300 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -788,8 +788,8 @@ virNetSocketNewConnectLibSSH2(const char *host, host, portN, knownHosts, - false, - verify) != 0) + verify, + VIR_NET_SSH_HOSTKEY_FILE_CREATE) != 0) goto error; if (virNetSSHSessionSetChannelCommand(sess, command) != 0) diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index fe0197e..59013c7 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -1123,8 +1123,8 @@ virNetSSHSessionSetHostKeyVerification(virNetSSHSessionPtr sess, const char *hostname, int port, const char *hostsfile, - bool readonly, - virNetSSHHostkeyVerify opt) + virNetSSHHostkeyVerify opt, + unsigned int flags) { char *errmsg; @@ -1140,19 +1140,25 @@ virNetSSHSessionSetHostKeyVerification(virNetSSHSessionPtr sess, /* load the known hosts file */ if (hostsfile) { - if (libssh2_knownhost_readfile(sess->knownHosts, - hostsfile, - LIBSSH2_KNOWNHOST_FILE_OPENSSH) < 0) { - libssh2_session_last_error(sess->session, &errmsg, NULL, 0); + if (virFileExists(hostsfile)) { + if (libssh2_knownhost_readfile(sess->knownHosts, + hostsfile, + LIBSSH2_KNOWNHOST_FILE_OPENSSH) < 0) { + libssh2_session_last_error(sess->session, &errmsg, NULL, 0); + virReportError(VIR_ERR_SSH, + _("unable to load knownhosts file '%s': %s"), + hostsfile, errmsg); + goto error; + } + } else if (!(flags & VIR_NET_SSH_HOSTKEY_FILE_CREATE)) { virReportError(VIR_ERR_SSH, - _("unable to load knownhosts file '%s': %s"), - hostsfile, errmsg); + _("known hosts file '%s' does not exist"), + hostsfile); goto error; } /* set filename only if writing to the known hosts file is requested */ - - if (!readonly) { + if (!(flags & VIR_NET_SSH_HOSTKEY_FILE_READONLY)) { VIR_FREE(sess->knownHostsFile); if (!(sess->knownHostsFile = strdup(hostsfile))) goto no_memory; diff --git a/src/rpc/virnetsshsession.h b/src/rpc/virnetsshsession.h index eb92e43..1199eef 100644 --- a/src/rpc/virnetsshsession.h +++ b/src/rpc/virnetsshsession.h @@ -36,6 +36,11 @@ typedef enum { VIR_NET_SSH_HOSTKEY_VERIFY_IGNORE } virNetSSHHostkeyVerify; +typedef enum { + VIR_NET_SSH_HOSTKEY_FILE_READONLY = 1 << 0, + VIR_NET_SSH_HOSTKEY_FILE_CREATE = 1 << 1, +} virNetSSHHostKeyFileFlags; + int virNetSSHSessionSetChannelCommand(virNetSSHSessionPtr sess, const char *command); @@ -64,8 +69,8 @@ int virNetSSHSessionSetHostKeyVerification(virNetSSHSessionPtr sess, const char *hostname, int port, const char *hostsfile, - bool readonly, - virNetSSHHostkeyVerify opt); + virNetSSHHostkeyVerify opt, + unsigned int flags); int virNetSSHSessionConnect(virNetSSHSessionPtr sess, int sock); -- 1.7.8.6

On 08/21/2012 10:48 AM, Peter Krempa wrote:
The libssh2 code wasn't supposed to create the known_hosts file, but recent findings show, that we can't use the default created by OpenSSH as libssh2 might damage it. We need to create a private known_hosts file in the config path.
This patch adds support for skipping error if the known_host file is not
s/known_host/known_hosts/
present and let libssh2 create a new one. --- src/rpc/virnetsocket.c | 4 ++-- src/rpc/virnetsshsession.c | 26 ++++++++++++++++---------- src/rpc/virnetsshsession.h | 9 +++++++-- 3 files changed, 25 insertions(+), 14 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Unfortunately libssh2 doesn't support all types of host keys that can be saved in the known_hosts file. Also it does not report that parsing of the file failed. This results into truncated known_hosts files where the standard client stores keys also in other formats (eg. ecdsa-sha2-nistp256). This patch changes the default location of the known_hosts file into the libvirt private configuration directory, where it will be only written by the libssh2 layer itself. This prevents thrashing user's files. --- src/rpc/virnetclient.c | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 8ff5e09..4ecc703 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -417,23 +417,25 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, char *command = NULL; char *homedir = virGetUserDirectory(); + char *confdir = virGetUserConfigDirectory(); char *knownhosts = NULL; char *privkey = NULL; /* Use default paths for known hosts an public keys if not provided */ - if (homedir) { + if (confdir) { if (!knownHostsPath) { - virBufferAsprintf(&buf, "%s/.ssh/known_hosts", homedir); - if (!(knownhosts = virBufferContentAndReset(&buf))) - goto no_memory; - - if (!(virFileExists(knownhosts))) - VIR_FREE(knownhosts); + if (virFileExists(confdir)) { + virBufferAsprintf(&buf, "%s/known_hosts", confdir); + if (!(knownhosts = virBufferContentAndReset(&buf))) + goto no_memory; + } } else { if (!(knownhosts = strdup(knownHostsPath))) goto no_memory; } + } + if (homedir) { if (!privkeyPath) { /* RSA */ virBufferAsprintf(&buf, "%s/.ssh/id_rsa", homedir); @@ -501,6 +503,7 @@ cleanup: VIR_FREE(privkey); VIR_FREE(knownhosts); VIR_FREE(homedir); + VIR_FREE(confdir); VIR_FREE(nc); virObjectUnref(sock); return ret; -- 1.7.8.6

On 08/21/2012 10:48 AM, Peter Krempa wrote:
Unfortunately libssh2 doesn't support all types of host keys that can be saved in the known_hosts file. Also it does not report that parsing of the file failed. This results into truncated known_hosts files where the standard client stores keys also in other formats (eg. ecdsa-sha2-nistp256).
This patch changes the default location of the known_hosts file into the libvirt private configuration directory, where it will be only written by the libssh2 layer itself. This prevents thrashing user's files.
s/thrashing/trashing/
--- src/rpc/virnetclient.c | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/21/12 19:23, Eric Blake wrote:
On 08/21/2012 10:48 AM, Peter Krempa wrote:
Unfortunately libssh2 doesn't support all types of host keys that can be saved in the known_hosts file. Also it does not report that parsing of the file failed. This results into truncated known_hosts files where the standard client stores keys also in other formats (eg. ecdsa-sha2-nistp256).
This patch changes the default location of the known_hosts file into the libvirt private configuration directory, where it will be only written by the libssh2 layer itself. This prevents thrashing user's files.
s/thrashing/trashing/
--- src/rpc/virnetclient.c | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-)
ACK.
Series pushed. Thanks! Peter
participants (2)
-
Eric Blake
-
Peter Krempa