Subject: SSH and sticky mode in directories
To: None <tech-userlevel@NetBSD.org>
From: Julio M. Merino Vidal <jmerino@ac.upc.edu>
List: tech-userlevel
Date: 12/20/2007 18:26:06
Hi,

First of all, I'm not sure if this message really belongs to this  
list, but I think it can be properly discussed here.  (OK, surely  
not, it should be put in the OpenSSH mailing list, whatever it is,  
but I first would like to hear some comments here.)

I'm trying to set up some automated tests for psshfs, and to do that  
I automatically configure a new SSH server (running as an  
unprivileged user or root, it does not matter).  The test uses a  
secure subdirectory in /tmp (such as /tmp/atf.123456) to store all of  
its files, which include the configuration files for the SSH server  
as well as all the user's keys and authorized_keys files (generated  
at run time to do a temporary password-less login).

The problem I'm having is that the server refuses to open the  
authorized_keys file because one of its path components is a group/ 
other-writable directory (that is, /tmp).  And I think that's  
incorrect, because it should also take into account the fact that the  
directory has the sticky bit set.  If that bit is set, I don't see  
how being group/other-writable is a problem.  Can anybody see any  
security implications of relaxing this permission check to make sure  
that the directory is not group/other-writable or, if it is, it is  
also marked as sticky?

If you think this is OK, can we have this fix committed to our copy  
of SSH, or should it be first passed through the OpenSSH developers?

While looking at the code, I've found that many different files  
opened by it can suffer from this problem, so I've mechanically fixed  
all the occurrences of similar code.  Patch below, just for review  
for now.

Index: auth-rhosts.c
===================================================================
RCS file: /cvsroot/src/crypto/dist/ssh/auth-rhosts.c,v
retrieving revision 1.17
diff -u -p -r1.17 auth-rhosts.c
--- auth-rhosts.c	28 Sep 2006 21:22:14 -0000	1.17
+++ auth-rhosts.c	20 Dec 2007 17:14:39 -0000
@@ -238,7 +238,7 @@ auth_rhosts2_raw(struct passwd *pw, cons
  	}
  	if (options.strict_modes &&
  	    ((st.st_uid != 0 && st.st_uid != pw->pw_uid) ||
-	    (st.st_mode & 022) != 0)) {
+	    ((st.st_mode & 022) != 0 && !(st.st_mode & 01000)))) {
  		logit("Rhosts authentication refused for %.100s: "
  		    "bad ownership or modes for home directory.", pw->pw_name);
  		auth_debug_add("Rhosts authentication refused for %.100s: "
@@ -265,7 +265,7 @@ auth_rhosts2_raw(struct passwd *pw, cons
  		 */
  		if (options.strict_modes &&
  		    ((st.st_uid != 0 && st.st_uid != pw->pw_uid) ||
-		    (st.st_mode & 022) != 0)) {
+		    ((st.st_mode & 022) != 0 && !(st.st_mode & 01000)))) {
  			logit("Rhosts authentication refused for %.100s: bad modes for %. 
200s",
  			    pw->pw_name, buf);
  			auth_debug_add("Bad file modes for %.200s", buf);
Index: auth.c
===================================================================
RCS file: /cvsroot/src/crypto/dist/ssh/auth.c,v
retrieving revision 1.26
diff -u -p -r1.26 auth.c
--- auth.c	10 Jul 2007 15:48:56 -0000	1.26
+++ auth.c	20 Dec 2007 17:14:39 -0000
@@ -416,7 +416,7 @@ check_key_in_hostfiles(struct passwd *pw
  		if (options.strict_modes &&
  		    (stat(user_hostfile, &st) == 0) &&
  		    ((st.st_uid != 0 && st.st_uid != pw->pw_uid) ||
-		    (st.st_mode & 022) != 0)) {
+		    ((st.st_mode & 022) != 0 && !(st.st_mode & 01000)))) {
  			logit("Authentication refused for %.100s: "
  			    "bad owner or modes for %.200s",
  			    pw->pw_name, user_hostfile);
@@ -469,7 +469,7 @@ secure_filename(FILE *f, const char *fil
  	/* check the open file to avoid races */
  	if (fstat(fileno(f), &st) < 0 ||
  	    (st.st_uid != 0 && st.st_uid != uid) ||
-	    (st.st_mode & 022) != 0) {
+	    ((st.st_mode & 022) != 0 && !(st.st_mode & 01000))) {
  		snprintf(err, errlen, "bad ownership or modes for file %s",
  		    buf);
  		return -1;
@@ -486,7 +486,7 @@ secure_filename(FILE *f, const char *fil
  		debug3("secure_filename: checking '%s'", buf);
  		if (stat(buf, &st) < 0 ||
  		    (st.st_uid != 0 && st.st_uid != uid) ||
-		    (st.st_mode & 022) != 0) {
+		    ((st.st_mode & 022) != 0 && !(st.st_mode & 01000))) {
  			snprintf(err, errlen,
  			    "bad ownership or modes for directory %s", buf);
  			return -1;
Index: readconf.c
===================================================================
RCS file: /cvsroot/src/crypto/dist/ssh/readconf.c,v
retrieving revision 1.32
diff -u -p -r1.32 readconf.c
--- readconf.c	10 Mar 2007 23:05:25 -0000	1.32
+++ readconf.c	20 Dec 2007 17:14:40 -0000
@@ -999,7 +999,7 @@ read_config_file(const char *filename, c
  		if (fstat(fileno(f), &sb) == -1)
  			fatal("fstat %s: %s", filename, strerror(errno));
  		if (((sb.st_uid != 0 && sb.st_uid != getuid()) ||
-		    (sb.st_mode & 022) != 0))
+		    ((sb.st_mode & 022) != 0 && !(sb.st_mode & 01000))))
  			fatal("Bad owner or permissions on %s", filename);
  	}

Thanks,

-- 
Julio M. Merino Vidal <jmerino@ac.upc.edu>