Fix failing virGetHostname.

We've been running into a lot of situations where
virGetHostname() is returning "localhost", where a plain
gethostname() would have returned the correct thing.  This
is because virGetHostname() is *always* trying to canonicalize
the name returned from gethostname(), even when it doesn't
have to.

This patch changes virGetHostname so that if the value returned
from gethostname() is already FQDN or localhost, it returns
that string directly.  If the value returned from gethostname()
is a shortened hostname, then we try to canonicalize it.  If
that succeeds, we returned the canonicalized hostname.  If
that fails, and/or returns "localhost", then we just return
the original string we got from gethostname() and hope for
the best.

Note that after this patch it is up to clients to check whether
"localhost" is an allowed return value.  The only place
where it's currently not is in qemu migration.

Signed-off-by: Chris Lalancette <clalance@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7aa7a0686d219ef7182c79ee40e2eafc2398fce2..6cb3d66f5cb5bd29ecc21c86d358d629e571080c 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -660,7 +660,6 @@ virExecDaemonize;
 virSetCloseExec;
 virSetNonBlock;
 virFormatMacAddr;
-virGetHostnameLocalhost;
 virGetHostname;
 virParseMacAddr;
 virFileDeletePid;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e6ce9c1f46b982612bfeb1be11829a9297e019d0..3b2b0d8dff5761c4e4fb0f5935bd1f06f6634108 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10171,7 +10171,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
     virDomainDefPtr def = NULL;
     virDomainObjPtr vm = NULL;
     int this_port;
-    char *hostname;
+    char *hostname = NULL;
     char migrateFrom [64];
     const char *p;
     virDomainEventPtr event = NULL;
@@ -10220,9 +10220,15 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
         if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0;
 
         /* Get hostname */
-        if ((hostname = virGetHostnameLocalhost(0)) == NULL)
+        if ((hostname = virGetHostname(NULL)) == NULL)
             goto cleanup;
 
+        if (STRPREFIX(hostname, "localhost")) {
+            qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                            _("hostname on destination resolved to localhost, but migration requires an FQDN"));
+            goto cleanup;
+        }
+
         /* XXX this really should have been a properly well-formed
          * URI, but we can't add in tcp:// now without breaking
          * compatability with old targets. We at least make the
@@ -10230,7 +10236,6 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
          */
         /* Caller frees */
         internalret = virAsprintf(uri_out, "tcp:%s:%d", hostname, this_port);
-        VIR_FREE(hostname);
         if (internalret < 0) {
             virReportOOMError();
             goto cleanup;
@@ -10334,10 +10339,10 @@ endjob:
         vm = NULL;
 
 cleanup:
+    VIR_FREE(hostname);
     virDomainDefFree(def);
-    if (ret != 0) {
+    if (ret != 0)
         VIR_FREE(*uri_out);
-    }
     if (vm)
         virDomainObjUnlock(vm);
     if (event)

diff --git a/src/util/util.c b/src/util/util.c
index 0642858ea6c78ceb08d954de2fe515dcaca134df..34cfc212e8c90affa40be4671a61255d4260795f 100644 (file)
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2416,11 +2416,31 @@ char *virIndexToDiskName(int idx, const char *prefix)
 # define AI_CANONIDN 0
 #endif
 
-char *virGetHostnameLocalhost(int allow_localhost)
+/* Who knew getting a hostname could be so delicate.  In Linux (and Unices
+ * in general), many things depend on "hostname" returning a value that will
+ * resolve one way or another.  In the modern world where networks frequently
+ * come and go this is often being hard-coded to resolve to "localhost".  If
+ * it *doesn't* resolve to localhost, then we would prefer to have the FQDN.
+ * That leads us to 3 possibilities:
+ *
+ * 1)  gethostname() returns an FQDN (not localhost) - we return the string
+ *     as-is, it's all of the information we want
+ * 2)  gethostname() returns "localhost" - we return localhost; doing further
+ *     work to try to resolve it is pointless
+ * 3)  gethostname() returns a shortened hostname - in this case, we want to
+ *     try to resolve this to a fully-qualified name.  Therefore we pass it
+ *     to getaddrinfo().  There are two possible responses:
+ *     a)  getaddrinfo() resolves to a FQDN - return the FQDN
+ *     b)  getaddrinfo() resolves to localhost - in this case, the data we got
+ *         from gethostname() is actually more useful than what we got from
+ *         getaddrinfo().  Return the value from gethostname() and hope for
+ *         the best.
+ */
+char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED)
 {
     int r;
     char hostname[HOST_NAME_MAX+1], *result;
-    struct addrinfo hints, *info, *res;
+    struct addrinfo hints, *info;
 
     r = gethostname (hostname, sizeof(hostname));
     if (r == -1) {
@@ -2430,6 +2450,21 @@ char *virGetHostnameLocalhost(int allow_localhost)
     }
     NUL_TERMINATE(hostname);
 
+    if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) {
+        /* in this case, gethostname returned localhost (meaning we can't
+         * do any further canonicalization), or it returned an FQDN (and
+         * we don't need to do any further canonicalization).  Return the
+         * string as-is; it's up to callers to check whether "localhost"
+         * is allowed.
+         */
+        result = strdup(hostname);
+        goto check_and_return;
+    }
+
+    /* otherwise, it's a shortened, non-localhost, hostname.  Attempt to
+     * canonicalize the hostname by running it through getaddrinfo
+     */
+
     memset(&hints, 0, sizeof(hints));
     hints.ai_flags = AI_CANONNAME|AI_CANONIDN;
     hints.ai_family = AF_UNSPEC;
@@ -2444,54 +2479,25 @@ char *virGetHostnameLocalhost(int allow_localhost)
     /* Tell static analyzers about getaddrinfo semantics.  */
     sa_assert (info);
 
-    /* if we aren't allowing localhost, then we iterate through the
-     * list and make sure none of the IPv4 addresses are 127.0.0.1 and
-     * that none of the IPv6 addresses are ::1
-     */
-    if (!allow_localhost) {
-        res = info;
-        while (res) {
-            if (res->ai_family == AF_INET) {
-                if (htonl(((struct sockaddr_in *)res->ai_addr)->sin_addr.s_addr) == INADDR_LOOPBACK) {
-                    virUtilError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                 _("canonical hostname pointed to localhost, but this is not allowed"));
-                    freeaddrinfo(info);
-                    return NULL;
-                }
-            }
-            else if (res->ai_family == AF_INET6) {
-                if (IN6_IS_ADDR_LOOPBACK(&((struct sockaddr_in6 *)res->ai_addr)->sin6_addr)) {
-                    virUtilError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                 _("canonical hostname pointed to localhost, but this is not allowed"));
-                    freeaddrinfo(info);
-                    return NULL;
-                }
-            }
-            res = res->ai_next;
-        }
-    }
+    if (info->ai_canonname == NULL ||
+        STRPREFIX(info->ai_canonname, "localhost"))
+        /* in this case, we tried to canonicalize and we ended up back with
+         * localhost.  Ignore the canonicalized name and just return the
+         * original hostname
+         */
+        result = strdup(hostname);
+    else
+        /* Caller frees this string. */
+        result = strdup (info->ai_canonname);
 
-    if (info->ai_canonname == NULL) {
-        virUtilError(VIR_ERR_INTERNAL_ERROR,
-                     "%s", _("could not determine canonical host name"));
-        freeaddrinfo(info);
-        return NULL;
-    }
+    freeaddrinfo(info);
 
-    /* Caller frees this string. */
-    result = strdup (info->ai_canonname);
-    if (!result)
+check_and_return:
+    if (result == NULL)
         virReportOOMError();
-
-    freeaddrinfo(info);
     return result;
 }
 
-char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED)
-{
-    return virGetHostnameLocalhost(1);
-}
-
 /* send signal to a single process */
 int virKillProcess(pid_t pid, int sig)
 {

diff --git a/src/util/util.h b/src/util/util.h
index abc268822f5da5f06b6a3fb0908176c2a9a62531..476eac43b793cdd93362f83101c5ea6741e87184 100644 (file)
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -254,7 +254,6 @@ static inline int getuid (void) { return 0; }
 static inline int getgid (void) { return 0; }
 # endif
 
-char *virGetHostnameLocalhost(int allow_localhost);
 char *virGetHostname(virConnectPtr conn);
 
 int virKillProcess(pid_t pid, int sig);