Re: [libvirt] [PATCH v3 2/3] conf: add check if migration_host is a localhost address

8 Oct 2014

On 10/07/2014 06:07 AM, Chen Fan wrote:
...
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_conf.c     | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_conf.h     |  2 ++
 src/util/virsocketaddr.c | 24 +++++++++++++++++++++++
 src/util/virsocketaddr.h |  2 ++
 5 files changed, 79 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8ab1394..a104bc6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1911,6 +1911,7 @@ virSocketAddrGetIpPrefix;
 virSocketAddrGetPort;
 virSocketAddrGetRange;
 virSocketAddrIsNetmask;
+virSocketAddrIsNumericLocalhost;
 virSocketAddrIsPrivate;
 virSocketAddrIsWildcard;
 virSocketAddrMask;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index adc6caf..6b0ac5c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -707,6 +707,15 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
     GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox);
GET_VALUE_STR("migration_host", cfg->migrateHost);
+    if (cfg->migrateHost &&
+        qemuCheckLocalhost(cfg->migrateHost)) {
+        virReportError(VIR_ERR_CONF_SYNTAX,
+                       _("migration_host must not be the address of"
+                         " the local machine: %s"),
+                       cfg->migrateHost);
+        goto cleanup;
+    }
+
     GET_VALUE_STR("migration_address", cfg->migrationAddress);
GET_VALUE_BOOL("log_timestamp", cfg->logTimestamp);
@@ -1371,3 +1380,44 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
return qemuGetHugepagePath(&hugetlbfs[i]);
 }
+
+bool
+qemuCheckLocalhost(const char *addrStr)
+{
+    virSocketAddr addr;
+    char *hostname, *tmp;
+    bool encloseAddress = false;
+    int family;
+    bool ret = true;
+
+    if (VIR_STRDUP(hostname, addrStr) < 0)
+        return false;
+
+    tmp = hostname;
+
+    if (STRPREFIX(hostname, "[")) {
+        char *end = strchr(hostname, ']');
+        if (end) {
+            *end = '\0';
+            hostname++;
+            encloseAddress = true;
+        }
+    }
We don't format the qemu.conf back and we don't need the brackets for
anything. We can just store the migration host without them in cfg->migrationHost
...
+
+    if (STRPREFIX(hostname, "localhost"))
+        goto cleanup;
+
+    family = virSocketAddrNumericFamily(hostname);
+    if ((family == AF_INET && !encloseAddress) ||
+        family == AF_INET6) {
+        if (virSocketAddrParse(&addr, hostname, family) > 0 &&
+            virSocketAddrIsNumericLocalhost(&addr)) {
+            goto cleanup;
+        }
+    }
There's no need to check for family upfront.
...
+
+    ret = false;
+cleanup:
+    VIR_FREE(tmp);
+    return ret;
+}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index cb01fb6..c9ce53c 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -322,4 +322,6 @@ int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
 char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage);
 char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
                               size_t nhugetlbfs);
+
+bool qemuCheckLocalhost(const char *addrStr);
 #endif /* __QEMUD_CONF_H */
diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
index 7fe7a15..6d36689 100644
--- a/src/util/virsocketaddr.c
+++ b/src/util/virsocketaddr.c
@@ -878,3 +878,27 @@ virSocketAddrNumericFamily(const char *address)
     freeaddrinfo(res);
     return family;
 }
+
+/**
+ * virSocketAddrIsNumericLocalhost:
+ * @address: address to check
+ *
+ * Check if passed address is a numeric 'localhost' address.
+ *
+ * Returns: true if @address is a numeric 'localhost' address,
+ *          false otherwise
+ */
+bool
+virSocketAddrIsNumericLocalhost(const virSocketAddr *addr)
I've rewritten this function to take a 'const char *' argument.
Along with the virStringStripIPv6Brackets function I've sent for review
separately, this removes the need for a separate qemuCheckLocalhost function
and it can be inlined.
...
+{
+    struct in_addr tmp = { .s_addr = htonl(INADDR_LOOPBACK) };
+    switch (addr->data.stor.ss_family) {
+    case AF_INET:
+        return memcmp(&addr->data.inet4.sin_addr.s_addr, &tmp.s_addr,
+                      sizeof(addr->data.inet4.sin_addr.s_addr)) == 0;
+    case AF_INET6:
+        return IN6_IS_ADDR_LOOPBACK(&addr->data.inet6.sin6_addr);
+    }
+    return false;
+
+}
The diff I'll squash into this patch before pushing (after
virStringStripIPv6Brackets is sorted out):

--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -707,8 +707,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
     GET_VALUE_LONG("seccomp_sandbox", cfg->seccompSandbox);

     GET_VALUE_STR("migration_host", cfg->migrateHost);
+    virStringStripIPv6Brackets(cfg->migrateHost);
     if (cfg->migrateHost &&
-        qemuCheckLocalhost(cfg->migrateHost)) {
+        (STRPREFIX(cfg->migrateHost, "localhost") ||
+         virSocketAddrIsNumericLocalhost(cfg->migrateHost))) {
         virReportError(VIR_ERR_CONF_SYNTAX,
                        _("migration_host must not be the address of"
                          " the local machine: %s"),
@@ -1380,44 +1382,3 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,

     return qemuGetHugepagePath(&hugetlbfs[i]);
 }
-
-bool
-qemuCheckLocalhost(const char *addrStr)
-{
-    virSocketAddr addr;
-    char *hostname, *tmp;
-    bool encloseAddress = false;
-    int family;
-    bool ret = true;
-
-    if (VIR_STRDUP(hostname, addrStr) < 0)
-        return false;
-
-    tmp = hostname;
-
-    if (STRPREFIX(hostname, "[")) {
-        char *end = strchr(hostname, ']');
-        if (end) {
-            *end = '\0';
-            hostname++;
-            encloseAddress = true;
-        }
-    }
-
-    if (STRPREFIX(hostname, "localhost"))
-        goto cleanup;
-
-    family = virSocketAddrNumericFamily(hostname);
-    if ((family == AF_INET && !encloseAddress) ||
-        family == AF_INET6) {
-        if (virSocketAddrParse(&addr, hostname, family) > 0 &&
-            virSocketAddrIsNumericLocalhost(&addr)) {
-            goto cleanup;
-        }
-    }
-
-    ret = false;
-cleanup:
-    VIR_FREE(tmp);
-    return ret;
-}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index c9ce53c..cb01fb6 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -322,6 +322,4 @@ int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
 char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage);
 char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
                               size_t nhugetlbfs);
-
-bool qemuCheckLocalhost(const char *addrStr);
 #endif /* __QEMUD_CONF_H */

diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
index 6d36689..a19e3af 100644
--- a/src/util/virsocketaddr.c
+++ b/src/util/virsocketaddr.c
@@ -889,15 +889,24 @@ virSocketAddrNumericFamily(const char *address)
  *          false otherwise
  */
 bool
-virSocketAddrIsNumericLocalhost(const virSocketAddr *addr)
+virSocketAddrIsNumericLocalhost(const char *addr)
 {
+    struct addrinfo *res;
     struct in_addr tmp = { .s_addr = htonl(INADDR_LOOPBACK) };
-    switch (addr->data.stor.ss_family) {
+    struct sockaddr_in *inet4;
+    struct sockaddr_in6 *inet6;
+
+    if (virSocketAddrParseInternal(&res, addr, AF_UNSPEC, false) < 0)
+        return false;
+
+    switch (res->ai_addr->sa_family) {
     case AF_INET:
-        return memcmp(&addr->data.inet4.sin_addr.s_addr, &tmp.s_addr,
-                      sizeof(addr->data.inet4.sin_addr.s_addr)) == 0;
+        inet4 = (struct sockaddr_in*) res->ai_addr;
+        return memcmp(&inet4->sin_addr.s_addr, &tmp.s_addr,
+                      sizeof(inet4->sin_addr.s_addr)) == 0;
     case AF_INET6:
-        return IN6_IS_ADDR_LOOPBACK(&addr->data.inet6.sin6_addr);
+        inet6 = (struct sockaddr_in6*) res->ai_addr;
+        return IN6_IS_ADDR_LOOPBACK(&(inet6->sin6_addr));
     }
     return false;

diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
index fa9e98b..053855b 100644
--- a/src/util/virsocketaddr.h
+++ b/src/util/virsocketaddr.h
@@ -127,5 +127,5 @@ bool virSocketAddrIsWildcard(const virSocketAddr *addr);

 int virSocketAddrNumericFamily(const char *address);

-bool virSocketAddrIsNumericLocalhost(const virSocketAddr *addr);
+bool virSocketAddrIsNumericLocalhost(const char *addr);
 #endif /* __VIR_SOCKETADDR_H__ */

Jan

    

Re: [libvirt] [PATCH v3 2/3] conf: add check if migration_host is a localhost address

Ján Tomko