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

Rejecting part of HURD PAM patch--help appreciated



[Please CC me; I'm not on the list]

As part of  upgrading our PAM to the latest release, I'm evaluating
all the Debian local patches.  I've chosen to reject part of a patch
labeled 020_hurd from the PAM currently in sid; I suspect this means
that PAM 0.75 will not compile on hurd once uploaded to sid and I'd
appreciate your help fixing this situation.

The part of the patch I'm rejecting has to do with
Linux-PAM/modules/pam_rhosts/pam_rhosts_auth.c
.

I'm rejecting several hunks related to maxhostnamelen and maxpathlen.
  The patch radically changes the behavior of the code if these two
  symbols are not defined using dynamic allocation rather than
  asserting a reasonable maximum buffer length.  I think I would be
  happy saying you should always do dynamic allocation and never use
  the preprocessor symobl, but I'm not happy introducing dynamic
  allocation sometimes.

I think the correct solution is in fact to always do the dynamic
allocation as I'm not at all convinced maxhostnamelen is being used
correctly; I suspect it is being used for domains read over the
network rather than for the return value of gethostname()
.

I'm not prepared to assert that behavior is correct for all
architectures during this patch review pass.  Eventually I'll get
around to looking at it again, but if someone else looks at the code
first and confirms that the hurd behavior would be an improvement
everywhere it would be appreciated.

As a side note, the patch was well written; it removed some rather
unfortunate bugs in tty handling, string handling, etc.  Thanks a lot,
Igor Khavkine !

Here are the rejected hunks:

@@ -359,11 +359,27 @@
     register const char *user;
     register char *p;
     int hcheck, ucheck;
+#ifdef MAXHOSTNAMELEN
     char buf[MAXHOSTNAMELEN + 128];                       /* host + login */
+#else
+		char *buf = NULL, *old_buf = 0;
+		size_t buf_len = 0;
+#endif
 
+#ifdef MAXHOSTNAMELEN
     buf[sizeof (buf)-1] = '\0';                 	/* terminate line */
 
-    while (fgets(buf, sizeof(buf), hostf) != NULL) {   /* hostf file line */
+    while (fgets(buf, sizeof(buf), hostf) != NULL)   /* hostf file line */
+#else
+    while (getline(&buf, &buf_len, hostf) > 0)
+#endif
+    {
+#ifndef MAXHOSTNAMELEN
+        if (old_buf)
+          free(old_buf);
+        old_buf = buf;
+        buf_len = 0;
+#endif
         p = buf;                              /* from beginning of file.. */
 
 	/* Skip empty or comment lines */
@@ -371,6 +386,7 @@
 	    continue;
 	}
 
+#ifdef MAXHOSTNAMELEN
 	/* Skip lines that are too long. */
 	if (strchr(p, '\n') == NULL) {
 	    int ch = getc(hostf);
@@ -379,6 +395,7 @@
 		ch = getc(hostf);
 	    continue;
 	}
+#endif
 
 	/*
 	 * If there is a hostname at the start of the line.  Set it to
@@ -432,6 +449,10 @@
 	    /* Neither, go on looking for match */
 	}
     }
+#ifndef MAXHOSTNAMELEN
+    free(buf);
+    buf = NULL;
+#endif
 
     return (1);
 }
@@ -457,7 +477,11 @@
     FILE *hostf;
     uid_t uid;
     int answer;
+#ifdef MAXPATHLEN
     char pbuf[MAXPATHLEN];               /* potential buffer overrun */
+#else
+    char *pbuf = 0;
+#endif
 
     if ((!superuser||opts->opt_hosts_equiv_rootok) && !opts->opt_no_hosts_equiv ) {
 
@@ -491,6 +515,7 @@
 	return(1);
     }
 
+#ifdef MAXPATHLEN
     /* check for buffer overrun */
     if (strlen(pwd->pw_dir) + sizeof(USER_RHOSTS_FILE) + 2 >= MAXPATHLEN) {
 	if (opts->opt_debug)
@@ -500,6 +525,26 @@
 
     (void) strcpy(pbuf, pwd->pw_dir);
     (void) strcat(pbuf, USER_RHOSTS_FILE);
+#else
+    {
+        long int pbuf_len = strlen(pwd->pw_dir) + sizeof(USER_RHOSTS_FILE);
+        long int path_max;
+        pbuf = (char *)malloc(pbuf_len + 1);
+        if (!pbuf) {
+           if (opts->opt_debug)
+               _pam_log(LOG_DEBUG,
+                   "not enough memory to store home directory for `%s'", luser);
+            return 1;
+        }
+        snprintf(pbuf, pbuf_len, "%s%s", pwd->pw_dir, USER_RHOSTS_FILE);
+        path_max = pathconf(pbuf, _PC_PATH_MAX);
+        if(!(path_max == -1 && !errno) && pbuf_len >= path_max)  {
+	    if (opts->opt_debug)
+		_pam_log(LOG_DEBUG,"home directory for `%s' is too long",luser);
+	    return 1;                               /* to dangerous to try */
+	}
+    }
+#endif
 
     /*
      * Change effective uid while _reading_ .rhosts. (not just
@@ -593,6 +638,9 @@
 
     if (hostf != NULL)
         (void) fclose(hostf);
+#ifndef MAXPATHLEN
+    free(pbuf);
+#endif
 
     return answer;
 }


-- 
To UNSUBSCRIBE, email to debian-hurd-request@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org



Reply to: