[Dnsmasq-discuss] [PATCH 2/2] dbus: add SetServersEx method

Dan Williams dcbw at redhat.com
Fri Aug 17 23:35:00 BST 2012


The SetServers method has a few deficiencies:

First, given its argument structure, you cannot actually
introspect it since it expects a variable number of arguments.
Typically these are encoded into arrays, but that's not the
case with SetServers.  This means it's hard to use the method
from dbus client libraries like dbus-glib, gdbus, or many bound
languages like python or javascript.

Second, given that the address arguments are either uint32
or a number of bytes, additional options like IPv6 scope or
DNS server port numbers are impossible.

To address these issues, create a new method SetServersEx that
performs the same function, but accepts arrays of strings as input.
This allows more complex parsing of addresses and easier groupings
of addresses and domains from clients.

For example, from Python:

import dbus

bus = dbus.SystemBus()
p = bus.get_object("uk.org.thekelleys.dnsmasq", "/uk/org/thekelleys/dnsmasq")
l = dbus.Interface(p, dbus_interface="uk.org.thekelleys.dnsmasq")

array = dbus.Array()
array.append(["1.2.3.5"])
array.append(["1.2.3.4#664", "foobar.com"])
array.append(["1003:1234:abcd::1%eth0", "eng.mycorp.com", "lab.mycorp.com"])
print l.SetServersEx(array)

Signed-off-by: Dan Williams <dcbw at redhat.com>
---
 dbus/DBus-interface |  42 ++++++
 src/dbus.c          | 419 +++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 371 insertions(+), 90 deletions(-)

diff --git a/dbus/DBus-interface b/dbus/DBus-interface
index 9fa08f2..58ae03c 100644
--- a/dbus/DBus-interface
+++ b/dbus/DBus-interface
@@ -95,6 +95,48 @@ Each call to SetServers completely replaces the set of servers
 specified by via the DBus, but it leaves any servers specified via the
 command line or /etc/dnsmasq.conf or /etc/resolv.conf alone.
 
+SetServersEx
+------------
+
+This function is more flexible and the SetServers function, in that it can
+handle address scoping, port numbers, and is easier for clients to use.
+
+Returns nothing. Takes a set of arguments representing the new
+upstream DNS servers to be used by dnsmasq. All addresses (both IPv4 and IPv6)
+are represented as STRINGS.  Each server address may be followed by one or more
+STRINGS, which are the domains for which the preceding server should be used.
+
+This function takes an array of STRING arrays, where each inner array represents
+a set of DNS servers and domains for which those servers may be used.  Each
+string represents a list of upstream DNS servers first, and domains second.
+Mixing of domains and servers within a the string array is not allowed.
+
+Examples.
+
+[
+  ["1.2.3.4", "foobar.com"],
+  ["1003:1234:abcd::1%eth0", "eng.mycorp.com", "lab.mycorp.com"]
+]
+
+is equivalent to
+
+--server=/foobar.com/1.2.3.4 \
+  --server=/eng.mycorp.com/lab.mycorp.com/1003:1234:abcd::1%eth0
+
+An IPv4 address of 0.0.0.0 is interpreted as "no address, local only",
+so
+
+[ ["0.0.0.0", "local.domain"] ]
+
+is equivalent to
+
+--local=/local.domain/
+
+
+Each call to SetServersEx completely replaces the set of servers
+specified by via the DBus, but it leaves any servers specified via the
+command line or /etc/dnsmasq.conf or /etc/resolv.conf alone.
+
 2. SIGNALS
 ----------
 
diff --git a/src/dbus.c b/src/dbus.c
index 16a05e8..9097c89 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -38,6 +38,9 @@ const char* introspection_xml_template =
 "    <method name=\"SetServers\">\n"
 "      <arg name=\"servers\" direction=\"in\" type=\"av\"/>\n"
 "    </method>\n"
+"    <method name=\"SetServersEx\">\n"
+"      <arg name=\"servers\" direction=\"in\" type=\"aas\"/>\n"
+"    </method>\n"
 "    <signal name=\"DhcpLeaseAdded\">\n"
 "      <arg name=\"ipaddr\" type=\"s\"/>\n"
 "      <arg name=\"hwaddr\" type=\"s\"/>\n"
@@ -99,20 +102,125 @@ static void remove_watch(DBusWatch *watch, void *data)
   w = data; /* no warning */
 }
 
-static void dbus_read_servers(DBusMessage *message)
+static void add_update_server(union mysockaddr *addr,
+                              union mysockaddr *source_addr,
+                              int flags,
+                              const char *interface,
+                              const char *domain)
+{
+  struct server *serv;
+
+  /* See if this is already there, and unmark */
+  for (serv = daemon->servers; serv; serv = serv->next)
+    if ((serv->flags & SERV_FROM_DBUS) &&
+       (serv->flags & SERV_MARK))
+      {
+       if ((serv->flags & SERV_HAS_DOMAIN) &&
+           (!domain ||
+            !hostname_isequal(domain, serv->domain)))
+          continue;
+
+        if (serv->flags & SERV_HAS_SOURCE)
+          {
+            /* Not all SERV_HAS_SOURCE servers have an interface */
+            if (interface &&
+                serv->interface &&
+                (strcmp(interface, serv->interface) != 0))
+             continue;
+           else if (interface != serv->interface)
+             continue;
+          }
+
+        serv->flags &= ~SERV_MARK;
+        break;
+      }
+
+  if (!serv && (serv = whine_malloc(sizeof (struct server))))
+    {
+      /* Not found, create a new one. */
+      memset(serv, 0, sizeof(struct server));
+
+      if (domain)
+        serv->domain = whine_malloc(strlen(domain)+1);
+
+      if (domain && !serv->domain)
+        {
+          free(serv);
+          serv = NULL;
+        }
+      else
+        {
+         serv->next = daemon->servers;
+         daemon->servers = serv;
+         serv->flags = SERV_FROM_DBUS;
+         if (domain)
+           {
+             strcpy(serv->domain, domain);
+             serv->flags |= SERV_HAS_DOMAIN;
+           }
+          if (interface)
+            {
+             strcpy(serv->interface, interface);
+             serv->flags |= SERV_HAS_SOURCE;
+            }
+       }
+    }
+
+  if (serv)
+    {
+      serv->flags |= flags;
+      if (source_addr->in.sin_family == AF_INET &&
+          addr->in.sin_addr.s_addr == 0 &&
+          serv->domain)
+        serv->flags |= SERV_NO_ADDR;
+      else
+        {
+          serv->flags &= ~SERV_NO_ADDR;
+          serv->addr = *addr;
+          serv->source_addr = *source_addr;
+        }
+    }
+}
+
+static void mark_dbus(void)
+{
+  struct server *serv;
+
+  /* mark everything from DBUS */
+  for (serv = daemon->servers; serv; serv = serv->next)
+    if (serv->flags & SERV_FROM_DBUS)
+      serv->flags |= SERV_MARK;
+}
+
+static void cleanup_dbus()
 {
   struct server *serv, *tmp, **up;
+
+  /* unlink and free anything still marked. */
+  for (serv = daemon->servers, up = &daemon->servers; serv; serv = tmp) 
+    {
+      tmp = serv->next;
+      if (serv->flags & SERV_MARK)
+       {
+         server_gone(serv);
+         *up = serv->next;
+         free(serv);
+       }
+      else 
+       up = &serv->next;
+    }
+}
+
+static void dbus_read_servers(DBusMessage *message)
+{
   DBusMessageIter iter;
   union  mysockaddr addr, source_addr;
   char *domain;
   
   dbus_message_iter_init(message, &iter);
-  
-  /* mark everything from DBUS */
-  for (serv = daemon->servers; serv; serv = serv->next)
-    if (serv->flags & SERV_FROM_DBUS)
-      serv->flags |= SERV_MARK;
-  
+
+  mark_dbus();
+
   while (1)
     {
       int skip = 0;
@@ -170,7 +278,8 @@ static void dbus_read_servers(DBusMessage *message)
       else
        /* At the end */
        break;
-      
+
+      /* process each domain */
       do {
        if (dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_STRING)
          {
@@ -181,83 +290,204 @@ static void dbus_read_servers(DBusMessage *message)
          domain = NULL;
        
        if (!skip)
-         {
-           /* See if this is already there, and unmark */
-           for (serv = daemon->servers; serv; serv = serv->next)
-             if ((serv->flags & SERV_FROM_DBUS) &&
-                 (serv->flags & SERV_MARK))
-               {
-                 if (!(serv->flags & SERV_HAS_DOMAIN) && !domain)
-                   {
-                     serv->flags &= ~SERV_MARK;
-                     break;
-                   }
-                 if ((serv->flags & SERV_HAS_DOMAIN) && 
-                     domain &&
-                     hostname_isequal(domain, serv->domain))
-                   {
-                     serv->flags &= ~SERV_MARK;
-                     break;
-                   }
-               }
-           
-           if (!serv && (serv = whine_malloc(sizeof (struct server))))
-             {
-               /* Not found, create a new one. */
-               memset(serv, 0, sizeof(struct server));
-               
-               if (domain)
-                 serv->domain = whine_malloc(strlen(domain)+1);
-               
-               if (domain && !serv->domain)
-                 {
-                   free(serv);
-                   serv = NULL;
-                 }
-               else
-                 {
-                   serv->next = daemon->servers;
-                   daemon->servers = serv;
-                   serv->flags = SERV_FROM_DBUS;
-                   if (domain)
-                     {
-                       strcpy(serv->domain, domain);
-                       serv->flags |= SERV_HAS_DOMAIN;
-                     }
-                 }
-             }
-
-           if (serv)
-             {
-               if (source_addr.in.sin_family == AF_INET &&
-                   addr.in.sin_addr.s_addr == 0 &&
-                   serv->domain)
-                 serv->flags |= SERV_NO_ADDR;
-               else
-                 {
-                   serv->flags &= ~SERV_NO_ADDR;
-                   serv->addr = addr;
-                   serv->source_addr = source_addr;
-                 }
-             }
-         }
-       } while (dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_STRING);
+           add_update_server(&addr, &source_addr, 0, NULL, domain);
+      } while (dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_STRING);
     }
-  
-  /* unlink and free anything still marked. */
-  for (serv = daemon->servers, up = &daemon->servers; serv; serv = tmp) 
+
+  cleanup_dbus();
+}
+
+static const char *parse_address(char *str, union mysockaddr *addr,
+                                 union mysockaddr *source_addr,
+                                 int *flags, char *interface)
+{
+  int source_port = 0, serv_port = NAMESERVER_PORT;
+  char *portno, *source;
+#ifdef HAVE_IPV6
+  int scope_index = 0;
+  char *scope_id;
+#endif
+
+  if ((source = split_chr(str, '@')) && /* is there a source. */
+      (portno = split_chr(source, '#')) &&
+      !atoi_check16(portno, &source_port))
+    return _("bad port");
+
+  if ((portno = split_chr(str, '#')) && /* is there a port no. */
+      !atoi_check16(portno, &serv_port))
+    return _("bad port");
+
+#ifdef HAVE_IPV6
+  scope_id = split_chr(str, '%');
+#endif
+
+  if ((addr->in.sin_addr.s_addr = inet_addr(str)) != (in_addr_t) -1)
     {
-      tmp = serv->next;
-      if (serv->flags & SERV_MARK)
-       {
-         server_gone(serv);
-         *up = serv->next;
-         free(serv);
+      addr->in.sin_port = htons(serv_port);
+      source_addr->in.sin_port = htons(source_port);
+      addr->sa.sa_family = source_addr->sa.sa_family = AF_INET;
+#ifdef HAVE_SOCKADDR_SA_LEN
+      source_addr->in.sin_len = addr->in.sin_len = sizeof(struct sockaddr_in);
+#endif
+      if (source)
+        {
+          *flags |= SERV_HAS_SOURCE;
+          if ((source_addr->in.sin_addr.s_addr = inet_addr(source)) == (in_addr_t) -1)
+            {
+#if defined(SO_BINDTODEVICE)
+              source_addr->in.sin_addr.s_addr = INADDR_ANY;
+              strncpy(interface, source, IF_NAMESIZE - 1);
+#else
+              return _("interface binding not supported");
+#endif
+           }
        }
-      else 
-       up = &serv->next;
+      else
+       source_addr->in.sin_addr.s_addr = INADDR_ANY;
+    }
+#ifdef HAVE_IPV6
+  else if (inet_pton(AF_INET6, str, &addr->in6.sin6_addr) > 0)
+    {
+      if (scope_id && (scope_index = if_nametoindex(scope_id)) == 0)
+       return _("bad interface name");
+
+      addr->in6.sin6_port = htons(serv_port);
+      addr->in6.sin6_scope_id = scope_index;
+      source_addr->in6.sin6_port = htons(source_port);
+      source_addr->in6.sin6_scope_id = 0;
+      addr->sa.sa_family = source_addr->sa.sa_family = AF_INET6;
+      addr->in6.sin6_flowinfo = source_addr->in6.sin6_flowinfo = 0;
+#ifdef HAVE_SOCKADDR_SA_LEN
+      addr->in6.sin6_len = source_addr->in6.sin6_len = sizeof(addr->in6);
+#endif
+      if (source)
+        {
+         *flags |= SERV_HAS_SOURCE;
+         if (inet_pton(AF_INET6, source, &source_addr->in6.sin6_addr) == 0)
+           {
+#if defined(SO_BINDTODEVICE)
+             source_addr->in6.sin6_addr = in6addr_any;
+             strncpy(interface, source, IF_NAMESIZE - 1);
+#else
+             return _("interface binding not supported");
+#endif
+            }
+        }
+      else
+       source_addr->in6.sin6_addr = in6addr_any;
+    }
+  else
+    {
+      return _("invalid IP address");
+    }
+#endif
+  return NULL;
+}
+
+static DBusMessage* dbus_read_servers_ex(DBusMessage *message)
+{
+  DBusMessageIter iter, array_iter, string_iter;
+  DBusMessage *error = NULL;
+  const char *addr_err;
+
+  if (!dbus_message_iter_init(message, &iter))
+    {
+      return dbus_message_new_error(message, DBUS_ERROR_INVALID_ARGS,
+                                    "Failed to initialize dbus message iter");
+    }
+
+  /* check that the message contains an array of arrays */
+  if ((dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY) ||
+      (dbus_message_iter_get_element_type(&iter) != DBUS_TYPE_ARRAY))
+    {
+      return dbus_message_new_error(message, DBUS_ERROR_INVALID_ARGS,
+                                    "Expected array of string arrays");
     }
 
+  mark_dbus();
+
+  /* array_iter points to each "as" element in the outer array */
+  dbus_message_iter_recurse(&iter, &array_iter);
+  while (dbus_message_iter_get_arg_type(&array_iter) != DBUS_TYPE_INVALID)
+    {
+      const char *str = NULL;
+      union  mysockaddr addr, source_addr;
+      int flags = 0;
+      char interface[IF_NAMESIZE];
+      char *str_addr;
+
+      /* check the types of the struct and its elements */
+      if ((dbus_message_iter_get_arg_type(&array_iter) != DBUS_TYPE_ARRAY) ||
+          (dbus_message_iter_get_element_type(&array_iter) != DBUS_TYPE_STRING))
+        {
+          error = dbus_message_new_error(message, DBUS_ERROR_INVALID_ARGS,
+                                         "Expected inner array of strings");
+          break;
+        }
+
+      /* string_iter points to each "s" element in the inner array */
+      dbus_message_iter_recurse(&array_iter, &string_iter);
+      if (dbus_message_iter_get_arg_type(&string_iter) != DBUS_TYPE_STRING)
+        {
+          /* no IP address given */
+          error = dbus_message_new_error(message, DBUS_ERROR_INVALID_ARGS,
+                                         "Expected IP address");
+          break;
+        }
+
+      dbus_message_iter_get_basic(&string_iter, &str);
+      if (!str || !strlen (str))
+        {
+          error = dbus_message_new_error(message, DBUS_ERROR_INVALID_ARGS,
+                                         "Empty IP address");
+          break;
+        }
+
+      memset(&addr, 0, sizeof(addr));
+      memset(&source_addr, 0, sizeof(source_addr));
+      memset(&interface, 0, sizeof(interface));
+
+      /* dup the string because it gets modified during parsing */
+      str_addr = strdup(str);
+      if (!str_addr)
+        {
+          error = dbus_message_new_error(message, DBUS_ERROR_INVALID_ARGS,
+                                         "Out of memory parsing IP address");
+          break;
+        }
+
+      /* parse the IP address */
+      addr_err = parse_address(str_addr, &addr, &source_addr, &flags, &interface[0]);
+      free(str_addr);
+
+      if (addr_err)
+        {
+          error = dbus_message_new_error_printf(message, DBUS_ERROR_INVALID_ARGS,
+                                                "Invalid IP address '%s': %s",
+                                                str, addr_err);
+          break;
+        }
+
+      /* jump past the address to the domain list (if any) */
+      dbus_message_iter_next (&string_iter);
+
+      /* parse domains and add each server/domain pair to the list */
+      do {
+        str = NULL;
+       if (dbus_message_iter_get_arg_type(&string_iter) == DBUS_TYPE_STRING)
+         dbus_message_iter_get_basic(&string_iter, &str);
+       dbus_message_iter_next (&string_iter);
+
+       add_update_server(&addr, &source_addr, flags, interface, str);
+      } while (dbus_message_iter_get_arg_type(&string_iter) == DBUS_TYPE_STRING);
+
+      /* jump to next element in outer array */
+      dbus_message_iter_next(&array_iter);
+    }
+
+  cleanup_dbus();
+
+  return error;
 }
 
 DBusHandlerResult message_handler(DBusConnection *connection, 
@@ -265,11 +495,10 @@ DBusHandlerResult message_handler(DBusConnection *connection,
                                  void *user_data)
 {
   char *method = (char *)dbus_message_get_member(message);
+  DBusMessage *reply = NULL;
    
   if (dbus_message_is_method_call(message, DBUS_INTERFACE_INTROSPECTABLE, "Introspect"))
     {
-      DBusMessage *reply;
-
       /* string length: "%s" provides space for termination zero */
       if (!introspection_xml && 
          (introspection_xml = whine_malloc(strlen(introspection_xml_template) + strlen(daemon->dbus_name))))
@@ -278,20 +507,15 @@ DBusHandlerResult message_handler(DBusConnection *connection,
       if (introspection_xml)
        {
          reply = dbus_message_new_method_return(message);
-         
          dbus_message_append_args(reply, DBUS_TYPE_STRING, &introspection_xml, DBUS_TYPE_INVALID);
-         dbus_connection_send (connection, reply, NULL);
-         dbus_message_unref (reply);
        }
     }
   else if (strcmp(method, "GetVersion") == 0)
     {
       char *v = VERSION;
-      DBusMessage *reply = dbus_message_new_method_return(message);
-      
+
+      reply = dbus_message_new_method_return(message);
       dbus_message_append_args(reply, DBUS_TYPE_STRING, &v, DBUS_TYPE_INVALID);
-      dbus_connection_send (connection, reply, NULL);
-      dbus_message_unref (reply);
     }
   else if (strcmp(method, "SetServers") == 0)
     {
@@ -299,6 +523,12 @@ DBusHandlerResult message_handler(DBusConnection *connection,
       dbus_read_servers(message);
       check_servers();
     }
+  else if (strcmp(method, "SetServersEx") == 0)
+    {
+      my_syslog(LOG_INFO, _("setting upstream servers from DBus"));
+      reply = dbus_read_servers_ex(message);
+      check_servers();
+    }
   else if (strcmp(method, "ClearCache") == 0)
     clear_cache_and_reload(dnsmasq_time());
   else
@@ -306,8 +536,17 @@ DBusHandlerResult message_handler(DBusConnection *connection,
   
   method = user_data; /* no warning */
 
+  /* If no reply or no error, return nothing */
+  if (!reply)
+    reply = dbus_message_new_method_return(message);
+
+  if (reply)
+    {
+      dbus_connection_send (connection, reply, NULL);
+      dbus_message_unref (reply);
+    }
+
   return (DBUS_HANDLER_RESULT_HANDLED);
- 
 }
  
 
-- 
1.7.11.2






More information about the Dnsmasq-discuss mailing list