[libvirt] [PATCH] rpc: libssh: don't crash when known_hosts file path is not provided

Pino Toscano ptoscano at redhat.com
Tue Jan 10 15:08:03 UTC 2017


On Monday, 9 January 2017 15:16:46 CET Peter Krempa wrote:
> When connecting as root, the "hostsfile" variable would be NULL due to
> the code leading to this point. This would result into a crash when
> attempting to set the known hosts file path.

Almost: the issue happens generally when known_hosts does not exist.
Considering the following code in src/rpc/virnetclient.c (both
virNetClientNewLibSSH2 and virNetClientNewLibssh have it):

    if (confdir) {
        if (!knownHostsPath) {
            if (virFileExists(confdir)) {
                virBufferAsprintf(&buf, "%s/known_hosts", confdir);
                if (!(knownhosts = virBufferContentAndReset(&buf)))
                    goto no_memory;
            }
        } else {
            if (VIR_STRDUP(knownhosts, knownHostsPath) < 0)
                goto cleanup;
        }
    }

knownHostsPath is left NULL if known_hosts is not passed as parameter,
and either one of:
a) confdir is NULL: reading virGetUserConfigDirectory (actually,
   virGetXDGDirectory) this happens only when XDG_CONFIG_HOME is not
   set, and neither is HOME
b) confdir is not NULL, but it does not exist

Regarding (a), there is no other "fix" doable within virNetClient
itself, since all the ways to determine the user home failed (and thus
no path can be determined).

Regarding (b), IMHO virNetClient could drop the virFileExists check,
letting the libssh and libssh2 drivers to handle:
- libssh seems to handle a non-existing file fine, creating its
  directories and the file itself on its own when saving
- the libssh2 already checks whether the file exists, using it only in
  that case

> To avoid deviating from the approach taken in the libssh2 driver set the
> file to /dev/null so that all entries are discarded unless explicitly
> specified.

While this works (ssh_write_knownhost should handle /dev as existing
already), IMHO it's better to make the libssh driver behave more close
to the libssh2, i.e. setting knownHostsFile only when considered valid.
What about the attached patch instead?

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-rpc-libssh-allow-a-NULL-known_hosts-file.patch
Type: text/x-patch
Size: 3412 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170110/4c82fdb7/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170110/4c82fdb7/attachment-0001.sig>


More information about the libvir-list mailing list