NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

bin/43900: ypbind(8) fails to handle multiple domains correcly



>Number:         43900
>Category:       bin
>Synopsis:       ypbind(8) fails to handle multiple domains correcly
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Sep 23 13:45:00 +0000 2010
>Originator:     Wolfgang Stukenbrock
>Release:        NetBSD 5.0.2
>Organization:
Dr. Nagler & Company GmbH
>Environment:
        
        
System: NetBSD s051 5.0.2 NetBSD 5.0.2 (NSW-S051) #2: Thu Aug 12 18:30:48 CEST 
2010 wgstuken@s051:/usr/src/sys/arch/amd64/compile/NSW-S051 amd64
Architecture: x86_64
Machine: amd64
>Description:
        The current implementation of ypbind will only handle multiple domains 
correctly
        if it runs in broadcast mode. Direct binding and ypset-mode may not 
handle
        different sets of ypservers for different domains correcly.
        The cause for the problem is that in ypbind.c some state information is 
stored
        from global variables and not in domain specific data.
        These global variables are correct for the default domain, but not for 
any
        additional domain.

        The current implementation will use the 
/var/yp/bind/<defaultdomain>.ypservers for
        any domain ypbind is ask for.
        And in the current implementation ypset will set the server for the 
specified domain
        but switches to "ypset-mode" for all domains. So all other domains not 
explitly bound by
        a separate ypset call will fail.
>How-To-Repeat:
        Setup multiple ypserver for different domains on different hosts.
        Setup a client /var/yp/binding/... for the domains to the correct hosts.
        The client will fail to contact the domain, that is not the default 
domain.
        Try to use "ypbind -ypset" and "ypset -d <domain> <host>". It will set 
the domain
        to that host, but all other domains may not be resolved anymore.
>Fix:
        As a workaround ypbind(8) can be forced to broadcast mode (for all 
domains) so the
        additionals domains can be used. But useing broadcast can be a security 
problem.

        The following patch eliminates the problem.
        It will move some previously global varaibles into the per domain 
structure.
        It will correct a minor bug in yp_log() by adding the missing '\n' if 
debug != 0.

        This patch will not search for any existing 
/var/yp/binding/xxxxx.ypservers file and
        prepare an internal structure with this information. This additional 
change may be usefull
        to avoid the (rare) situation, that a DOS attack is run against the 
system and all (100)
        allowed domain-names are setup before a configured one is loaded by a 
request from a valid
        client.

--- ypbind.c    2010/09/23 12:21:31     1.1
+++ ypbind.c    2010/09/23 13:19:03
@@ -76,6 +76,10 @@
 #define YPSERVERSSUFF  ".ypservers"
 #define BINDINGDIR     (_PATH_VAR_YP "binding")
 
+typedef enum {
+       YPBIND_DIRECT, YPBIND_BROADCAST, YPBIND_SETLOCAL, YPBIND_SETALL
+} ypbind_mode_t;
+
 struct _dom_binding {
        struct _dom_binding *dom_pnext;
        char dom_domain[YPMAXDOMAIN + 1];
@@ -88,6 +92,14 @@
        int dom_lockfd;
        int dom_alive;
        u_int32_t dom_xid;
+       FILE *direct_fp;
+       ypbind_mode_t bind_mode;
+/*
+ * If ypbindmode is YPBIND_SETLOCAL or YPBIND_SETALL, this indicates
+ * whether or not we've been "ypset".  If we haven't, we behave like
+ * YPBIND_BROADCAST.  If we have, we behave like YPBIND_DIRECT.
+ */
+       int been_ypset;
 };
 
 static char *domainname;
@@ -95,18 +107,7 @@
 static struct _dom_binding *ypbindlist;
 static int check;
 
-typedef enum {
-       YPBIND_DIRECT, YPBIND_BROADCAST, YPBIND_SETLOCAL, YPBIND_SETALL
-} ypbind_mode_t;
-
-ypbind_mode_t ypbindmode;
-
-/*
- * If ypbindmode is YPBIND_SETLOCAL or YPBIND_SETALL, this indicates
- * whether or not we've been "ypset".  If we haven't, we behave like
- * YPBIND_BROADCAST.  If we have, we behave like YPBIND_DIRECT.
- */
-int been_ypset;
+ypbind_mode_t default_ypbindmode;
 
 #ifdef DEBUG
 static int debug;
@@ -142,7 +143,7 @@
 static struct _dom_binding *xid2ypdb(u_int32_t);
 static u_int32_t unique_xid(struct _dom_binding *);
 static int broadcast(char *, int);
-static int direct(char *, int);
+static int direct(struct _dom_binding *, char *, int);
 static int direct_set(char *, int, struct _dom_binding *);
 
 static void
@@ -168,7 +169,7 @@
 
 #if defined(DEBUG)
        if (debug)
-               (void)vprintf(fmt, ap);
+               { (void)vprintf(fmt, ap); (void)printf("\n"); }
        else
 #endif
                vsyslog(pri, fmt, ap);
@@ -179,14 +180,43 @@
 makebinding(const char *dm)
 {
        struct _dom_binding *ypdb;
+       struct stat st;
+       char pathname[MAXPATHLEN];
 
        if ((ypdb = (struct _dom_binding *)malloc(sizeof *ypdb)) == NULL) {
-               yp_log(LOG_ERR, "makebinding");
+               yp_log(LOG_ERR, "makebinding out of memory");
                exit(1);
        }
 
        (void)memset(ypdb, 0, sizeof *ypdb);
        (void)strlcpy(ypdb->dom_domain, dm, sizeof ypdb->dom_domain);
+
+/*
+ * do some common initialisation for a new domain entry ...
+ */
+
+       if ((ypdb->bind_mode = default_ypbindmode) == YPBIND_DIRECT) {
+               (void)snprintf(pathname, sizeof(pathname), "%s/%s%s", 
BINDINGDIR,
+                   dm, YPSERVERSSUFF);
+
+               if (stat(pathname, &st) < 0) {
+#ifdef DEBUG
+                       if (debug)
+                               (void)printf("%s does not exist, defaulting to "
+                                   "broadcast\n", pathname);
+#endif
+                       ypdb->bind_mode = YPBIND_BROADCAST;
+               } else if ((ypdb->direct_fp = fopen(pathname, "r")) == NULL) {
+                       yp_log(LOG_WARNING, "%s exists, but is not readable - 
failing to serve domain %s",
+                           pathname, dm);
+               }
+       }
+       ypdb->dom_xid = unique_xid(ypdb);
+       ypdb->dom_vers = YPVERS;
+       ypdb->dom_alive = 0;
+       ypdb->dom_lockfd = -1;
+       removelock(ypdb);
+
        return ypdb;
 }
 
@@ -266,11 +296,6 @@
 
        if (ypdb == NULL) {
                ypdb = makebinding(arg);
-               ypdb->dom_vers = YPVERS;
-               ypdb->dom_alive = 0;
-               ypdb->dom_lockfd = -1;
-               removelock(ypdb);
-               ypdb->dom_xid = unique_xid(ypdb);
                ypdb->dom_pnext = ypbindlist;
                ypbindlist = ypdb;
                check++;
@@ -334,7 +359,8 @@
        (void)memset(&res, 0, sizeof(res));
        fromsin = svc_getcaller(transp);
 
-       switch (ypbindmode) {
+/* only need to check default bindmode here ... */
+       switch (default_ypbindmode) {
        case YPBIND_SETLOCAL:
                if (fromsin->sin_addr.s_addr != htonl(INADDR_LOOPBACK)) {
 #ifdef DEBUG
@@ -347,7 +373,6 @@
                /* FALLTHROUGH */
 
        case YPBIND_SETALL:
-               been_ypset = 1;
                break;
 
        case YPBIND_DIRECT:
@@ -385,7 +410,8 @@
 
 #ifdef DEBUG
        if (debug)
-               (void)printf("ypset to %s succeeded\n", 
inet_ntoa(bindsin.sin_addr));
+               (void)printf("ypset for %s to %s succeeded\n", 
sd->ypsetdom_domain,
+                   inet_ntoa(bindsin.sin_addr));
 #endif
        res = 1;
        return &res;
@@ -459,8 +485,6 @@
        fd_set fdsr;
        int width, lockfd;
        int evil = 0, one;
-       char pathname[MAXPATHLEN];
-       struct stat st;
 
        setprogname(argv[0]);
        (void)yp_get_default_domain(&domainname);
@@ -474,28 +498,18 @@
         * Note that we can still override direct mode by passing
         * the -broadcast flag.
         */
-       (void)snprintf(pathname, sizeof(pathname), "%s/%s%s", BINDINGDIR,
-           domainname, YPSERVERSSUFF);
-       if (stat(pathname, &st) < 0) {
-#ifdef DEBUG
-               if (debug)
-                       (void)printf("%s does not exist, defaulting to "
-                           "broadcast\n", pathname);
-#endif
-               ypbindmode = YPBIND_BROADCAST;
-       } else
-               ypbindmode = YPBIND_DIRECT;
+       default_ypbindmode = YPBIND_DIRECT;
 
        while (--argc) {
                ++argv;
                if (!strcmp("-insecure", *argv))
                        insecure = 1;
                else if (!strcmp("-ypset", *argv))
-                       ypbindmode = YPBIND_SETALL;
+                       default_ypbindmode = YPBIND_SETALL;
                else if (!strcmp("-ypsetme", *argv))
-                       ypbindmode = YPBIND_SETLOCAL;
+                       default_ypbindmode = YPBIND_SETLOCAL;
                else if (!strcmp("-broadcast", *argv))
-                       ypbindmode = YPBIND_BROADCAST;
+                       default_ypbindmode = YPBIND_BROADCAST;
 #ifdef DEBUG
                else if (!strcmp("-d", *argv))
                        debug++;
@@ -561,10 +575,6 @@
 
        /* build initial domain binding, make it "unsuccessful" */
        ypbindlist = makebinding(domainname);
-       ypbindlist->dom_vers = YPVERS;
-       ypbindlist->dom_alive = 0;
-       ypbindlist->dom_lockfd = -1;
-       removelock(ypbindlist);
 
        checkwork();
 
@@ -791,10 +801,10 @@
                        yp_log(LOG_WARNING, "nag_servers: sendto: %m");
        }
 
-       switch (ypbindmode) {
+       switch (ypdb->bind_mode) {
        case YPBIND_SETALL:
        case YPBIND_SETLOCAL:
-               if (been_ypset)
+               if (ypdb->been_ypset)
                        return direct_set(buf, outlen, ypdb);
                /* FALLTHROUGH */
 
@@ -802,7 +812,7 @@
                return broadcast(buf, outlen);
 
        case YPBIND_DIRECT:
-               return direct(buf, outlen);
+               return direct(ypdb, buf, outlen);
        }
        /*NOTREACHED*/
        return -1;
@@ -861,70 +871,60 @@
 }
 
 static int
-direct(char *buf, int outlen)
+direct(struct _dom_binding *ypdb, char *buf, int outlen)
 {
-       static FILE *df;
-       static char ypservers_path[MAXPATHLEN];
        char line[_POSIX2_LINE_MAX];
        char *p;
        struct hostent *hp;
        struct sockaddr_in bindsin;
        int i, count = 0;
 
-       if (df)
-               rewind(df);
-       else {
-               (void)snprintf(ypservers_path, sizeof(ypservers_path),
-                   "%s/%s%s", BINDINGDIR, domainname, YPSERVERSSUFF);
-               df = fopen(ypservers_path, "r");
-               if (df == NULL) {
-                       yp_log(LOG_ERR, "%s: ", ypservers_path);
-                       exit(1);
-               }
-       }
+       if (ypdb->direct_fp) {
+               rewind(ypdb->direct_fp);
 
-       (void)memset(&bindsin, 0, sizeof bindsin);
-       bindsin.sin_family = AF_INET;
-       bindsin.sin_len = sizeof(bindsin);
-       bindsin.sin_port = htons(PMAPPORT);
+               (void)memset(&bindsin, 0, sizeof bindsin);
+               bindsin.sin_family = AF_INET;
+               bindsin.sin_len = sizeof(bindsin);
+               bindsin.sin_port = htons(PMAPPORT);
 
-       while(fgets(line, (int)sizeof(line), df) != NULL) {
-               /* skip lines that are too big */
-               p = strchr(line, '\n');
-               if (p == NULL) {
-                       int c;
+               while(fgets(line, (int)sizeof(line), ypdb->direct_fp) != NULL) {
+                       /* skip lines that are too big */
+                       p = strchr(line, '\n');
+                       if (p == NULL) {
+                               int c;
 
-                       while ((c = getc(df)) != '\n' && c != EOF)
-                               ;
-                       continue;
-               }
-               *p = '\0';
-               p = line;
-               while (isspace((unsigned char)*p))
-                       p++;
-               if (*p == '#')
-                       continue;
-               hp = gethostbyname(p);
-               if (!hp) {
-                       yp_log(LOG_WARNING, "%s: %s", p, hstrerror(h_errno));
-                       continue;
-               }
-               /* step through all addresses in case first is unavailable */
-               for (i = 0; hp->h_addr_list[i]; i++) {
-                       (void)memcpy(&bindsin.sin_addr, hp->h_addr_list[0],
-                           hp->h_length);
-                       if (sendto(rpcsock, buf, outlen, 0,
-                           (struct sockaddr *)(void *)&bindsin,
-                           (socklen_t)sizeof(bindsin)) < 0) {
-                               yp_log(LOG_WARNING, "direct: sendto: %m");
+                               while ((c = getc(ypdb->direct_fp)) != '\n' && c 
!= EOF)
+                                       ;
                                continue;
-                       } else
-                               count++;
+                       }
+                       *p = '\0';
+                       p = line;
+                       while (isspace((unsigned char)*p))
+                               p++;
+                       if (*p == '#')
+                               continue;
+                       hp = gethostbyname(p);
+                       if (!hp) {
+                               yp_log(LOG_WARNING, "%s: %s", p, 
hstrerror(h_errno));
+                               continue;
+                       }
+                       /* step through all addresses in case first is 
unavailable */
+                       for (i = 0; hp->h_addr_list[i]; i++) {
+                               (void)memcpy(&bindsin.sin_addr, 
hp->h_addr_list[0],
+                                   hp->h_length);
+                               if (sendto(rpcsock, buf, outlen, 0,
+                                   (struct sockaddr *)(void *)&bindsin,
+                                   (socklen_t)sizeof(bindsin)) < 0) {
+                                       yp_log(LOG_WARNING, "direct: sendto: 
%m");
+                                       continue;
+                               } else
+                                       count++;
+                       }
                }
        }
        if (!count) {
-               yp_log(LOG_WARNING, "no contactable servers found in %s",
-                   ypservers_path);
+               yp_log(LOG_WARNING, "no contactable servers found for domain 
%s",
+                   ypdb->dom_domain);
                return -1;
        }
        return 0;
@@ -951,7 +951,7 @@
 
        if ((fd = open(path, O_SHLOCK|O_RDONLY, 0644)) == -1) {
                yp_log(LOG_WARNING, "%s: %m", path);
-               been_ypset = 0;
+               ypdb->been_ypset = 0;
                return -1;
        }
 
@@ -969,7 +969,7 @@
        if (bytes != (iov[0].iov_len + iov[1].iov_len)) {
                /* Binding file corrupt? */
                yp_log(LOG_WARNING, "%s: %m", path);
-               been_ypset = 0;
+               ypdb->been_ypset = 0;
                return -1;
        }
 
@@ -1141,7 +1141,6 @@
                if (force == 0)
                        return;
                ypdb = makebinding(dom);
-               ypdb->dom_lockfd = -1;
                ypdb->dom_pnext = ypbindlist;
                ypbindlist = ypdb;
        }
@@ -1161,7 +1160,6 @@
            sizeof ypdb->dom_server_addr);
        /* recheck binding in 60 seconds */
        ypdb->dom_check_t = time(NULL) + 60;
-       ypdb->dom_vers = YPVERS;
        ypdb->dom_alive = 1;
 
        if (ypdb->dom_lockfd != -1)
@@ -1195,6 +1193,11 @@
                removelock(ypdb);
                ypdb->dom_lockfd = -1;
        }
+/* we know that force != 0 only for the ypset-call - avoid search for the 
pointer
+ * again by the caller - set been_ypset here
+ */
+       if (force != 0)
+               ypdb->been_ypset = 1;
 }
 
 static struct _dom_binding *

>Unformatted:
        
        


Home | Main Index | Thread Index | Old Index