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

Re: proposal: per-user temporary directories on by default?

Tollef Fog Heen <tfheen@hardware.no> writes:
> ATM, TMPDIR is defined using #define in libpam-tmpdir's source.
> Patches for having that as a run-time configuration are accepted.

Attached is a patch to allow you to specify TMPDIR in the relevent
pam.d file, like so:

session   optional   pam_tmpdir.so tmpdir=/tmp/users

It does not (yet) expand ~, $HOME, or the like.  I'd like someone to
look it over to make sure I didn't open any security holes or cause
any stupid bugs.  (I do realise that it trusts the contents of the
pam.d file... not sure how paranoid to be about that.)

Thanks,
Kevin

Index: pam-tmpdir-helper.c
===================================================================
--- pam-tmpdir-helper.c	(revision 1)
+++ pam-tmpdir-helper.c	(working copy)
@@ -27,6 +27,8 @@
 
 #define SYSUSRTMP "/tmp/user"
 
+char *tmpdir;
+
 /* some syslogging */
 
 static void _log_err(int err, const char *format, ...)
@@ -47,48 +49,48 @@
   struct stat statbuf;
   mode_t old_umask;
 
-  ret = lstat(SYSUSRTMP,&statbuf);
+  ret = lstat(tmpdir,&statbuf);
   if (ret == -1 && errno != ENOENT) {
-    snprintf(logbuf,sizeof logbuf,"lstat SYSUSRTMP failed: %s\n", strerror(errno));
+    snprintf(logbuf,sizeof logbuf,"lstat tmpdir failed: %s\n", strerror(errno));
     _log_err(LOG_NOTICE, "%s", logbuf);
     return 1;
   } else if (ret != -1 && statbuf.st_uid != 0) {
     /* Somebody else than root has grabbed /tmp/user.  Bad, bad, bad. */
     snprintf(logbuf,sizeof logbuf,"%s is owned by uid %d instead of root " 
-	    "(uid 0). Failed to create safe $TMPDIR\n", SYSUSRTMP, 
+	    "(uid 0). Failed to create safe $TMPDIR\n", tmpdir, 
              statbuf.st_uid);
     _log_err(LOG_ERR, "%s", logbuf);
     return 1;
   } else if (ret != -1 && !S_ISDIR(statbuf.st_mode)) {
-    snprintf(logbuf,sizeof logbuf,"%s is not a directory. Failed to create safe $TMPDIR\n", SYSUSRTMP);
+    snprintf(logbuf,sizeof logbuf,"%s is not a directory. Failed to create safe $TMPDIR\n", tmpdir);
     _log_err(LOG_NOTICE, "%s", logbuf);
     return 1;
   } else if (ret != -1 && ((statbuf.st_mode & S_IWGRP) || (statbuf.st_mode & S_IWOTH))) {
     snprintf(logbuf,sizeof logbuf,"%s is group or world writable. "
-	     "Failed to create safe $TMPDIR\n", SYSUSRTMP);
+	     "Failed to create safe $TMPDIR\n", tmpdir);
     _log_err(LOG_NOTICE, "%s", logbuf);
     return 1;
   } else if (ret != -1 && !(statbuf.st_mode & S_IXOTH)) {
     snprintf(logbuf,sizeof logbuf,"%s is not world searchable. "
-	     "Failed to create safe $TMPDIR\n", SYSUSRTMP); 
+	     "Failed to create safe $TMPDIR\n", tmpdir); 
     _log_err(LOG_NOTICE, "%s", logbuf);
     return 1;
   } else if (ret == -1 && errno == ENOENT) {
     old_umask = umask(0000);
-    if (mkdir(SYSUSRTMP,0711) == -1) {
-      snprintf(logbuf,sizeof logbuf,"mkdir SYSUSRTMP failed: %s\n", strerror(errno));
+    if (mkdir(tmpdir,0711) == -1) {
+      snprintf(logbuf,sizeof logbuf,"mkdir tmpdir failed: %s\n", strerror(errno));
       _log_err(LOG_NOTICE, "%s", logbuf);
       return 1;
     }
     umask(old_umask);
-    if (chown(SYSUSRTMP,0,0) == -1) {
-      snprintf(logbuf,sizeof logbuf,"chown 0:0 SYSUSRTMP failed: %s\n", strerror(errno));
+    if (chown(tmpdir,0,0) == -1) {
+      snprintf(logbuf,sizeof logbuf,"chown 0:0 tmpdir failed: %s\n", strerror(errno));
       _log_err(LOG_NOTICE, "%s", logbuf);
       return 1;
     }
   }
 
-  if (snprintf(buf, sizeof buf, "%s/%d",SYSUSRTMP,getuid()) == -1) {
+  if (snprintf(buf, sizeof buf, "%s/%d",tmpdir,getuid()) == -1) {
     return 1;
   }
   ret = lstat(buf,&statbuf);
@@ -131,5 +133,29 @@
 }
 
 int main(int argc, char **argv) {
+  /* Parse our command line arguments.  We assume that 
+   * we will either receive one argument (the tmpdir path),
+   * or none at all (in which case, we set tmpdir to be SYSUSRTMP).
+   */
+
+   if (argc == 2) {
+     if ((tmpdir = malloc(strlen(argv[1]) + 1)) == NULL) {
+       _log_err(LOG_ERR, "malloc failed.  Out of memory.");
+       return 1;
+     }
+     strcpy(tmpdir, argv[1]);
+   } else if (argc == 1) {
+     if ((tmpdir = malloc(strlen(SYSUSRTMP) + 1)) == NULL) {
+       _log_err(LOG_ERR, "malloc failed.  Out of memory.");
+       return 1;
+     }
+     strcpy(tmpdir, SYSUSRTMP);
+   } else {
+     _log_err(LOG_ERR, "Incorrect number of arguments.  Giving up.");
+     return 1;
+   }
+
+  /* At this point, tmpdir should contain a valid TMPDIR path. */
+  
   return make_tmp_directory();
 }
Index: pam_tmpdir.c
===================================================================
--- pam_tmpdir.c	(revision 1)
+++ pam_tmpdir.c	(working copy)
@@ -43,7 +43,10 @@
 
 #define SYSUSRTMP "/tmp/user"
 #define PAM_TMPDIR_HELPER "/sbin/pam-tmpdir-helper"
+#define TMPDIR_INTRO "tmpdir="
 
+char *tmpdir = NULL;
+
 static int set_environment(pam_handle_t *pamh);
 static int make_tmp_directory(pam_handle_t *pamh);
 
@@ -85,16 +88,45 @@
 #define PAM_ENV_SILENT      0x04
 #define PAM_NEW_ENV_FILE    0x10
 
+static int set_tmpdir(int argc, const char **argv) {
+
+  int i;
+
+  /* Don't set tmpdir if it's already been set. */
+  if (tmpdir == NULL) {
+
+    /* We see if they pass in a path for tmpdir.  If so, we remember it. */
+    for(i = 0; i < argc && tmpdir == NULL; ++i) {
+      if (strncmp(argv[i], TMPDIR_INTRO, strlen(TMPDIR_INTRO)) == 0) {
+	if ((tmpdir = malloc(strlen(argv[i]) - strlen(TMPDIR_INTRO) + 1)) == NULL) {
+	  _log_err(LOG_ERR, "malloc failed.  Out of memory.");
+	  return PAM_ABORT;
+	}
+	strcpy(tmpdir, argv[i] + strlen(TMPDIR_INTRO));
+      }
+    }
+
+    /* They didn't pass in a path, so we take the default. */
+    if (tmpdir == NULL) {
+      if ((tmpdir = malloc(strlen(SYSUSRTMP) + 1)) == NULL) {
+	_log_err(LOG_ERR, "malloc failed.  Out of memory.");
+	return PAM_ABORT;
+      }
+      strcpy(tmpdir, SYSUSRTMP);
+    }
+  }
+}
+
 static int set_environment(pam_handle_t *pamh) {
   char buf[512];
   char foo[24];
   char logbuf[1024];
   int ret;
 
-  snprintf(buf,512,"TMP=%s/%d",SYSUSRTMP,get_user_id(pamh));
+  snprintf(buf,512,"TMP=%s/%d",tmpdir,get_user_id(pamh));
   ret = pam_putenv(pamh, buf);
 
-  snprintf(buf,512,"TMPDIR=%s/%d",SYSUSRTMP,get_user_id(pamh));
+  snprintf(buf,512,"TMPDIR=%s/%d",tmpdir,get_user_id(pamh));
   ret = pam_putenv(pamh, buf);
   return 0;
 }
@@ -111,7 +143,7 @@
     /* child */
     if (getuid() == 0)
       setuid(get_user_id(pamh));
-    execl(PAM_TMPDIR_HELPER,PAM_TMPDIR_HELPER,NULL);
+    execl(PAM_TMPDIR_HELPER,PAM_TMPDIR_HELPER,tmpdir,NULL); 
     }
   wait(&status);
   return WEXITSTATUS(status);
@@ -129,6 +161,7 @@
 int pam_sm_setcred(pam_handle_t *pamh, int flags, int argc, 
 		   const char **argv)
 {
+  set_tmpdir(argc, argv);
 
   if (make_tmp_directory(pamh) == 0) {
     set_environment(pamh);
@@ -149,6 +182,7 @@
 PAM_EXTERN
 int pam_sm_open_session(pam_handle_t *pamh,int flags,int argc, const char **argv)
 {
+  set_tmpdir(argc, argv);
 
   if (make_tmp_directory(pamh) == 0) {
     set_environment(pamh);

Attachment: pgpd0O7kxS39W.pgp
Description: PGP signature

Reply to: