From d61e316833eb7d05b0b5c937bfce8ee0f19dc7cb Mon Sep 17 00:00:00 2001 From: Colin Watson Date: Sat, 22 May 2010 22:43:47 +0100 Subject: Allow ~/.ssh/authorized_keys and other secure files to be group-writable, provided that the group in question contains only the file's owner; this extends a patch previously applied to ~/.ssh/config (closes: #581919). --- debian/patches/debian-config.patch | 2 +- debian/patches/user-group-modes.patch | 155 +++++++++++++++++++++++++++------- 2 files changed, 125 insertions(+), 32 deletions(-) (limited to 'debian/patches') diff --git a/debian/patches/debian-config.patch b/debian/patches/debian-config.patch index a395d43a0..ac77919e6 100644 --- a/debian/patches/debian-config.patch +++ b/debian/patches/debian-config.patch @@ -24,7 +24,7 @@ Index: b/readconf.c =================================================================== --- a/readconf.c +++ b/readconf.c -@@ -1152,7 +1152,7 @@ +@@ -1132,7 +1132,7 @@ if (options->forward_x11 == -1) options->forward_x11 = 0; if (options->forward_x11_trusted == -1) diff --git a/debian/patches/user-group-modes.patch b/debian/patches/user-group-modes.patch index 4d7ebe566..8cf862049 100644 --- a/debian/patches/user-group-modes.patch +++ b/debian/patches/user-group-modes.patch @@ -1,10 +1,11 @@ Description: Allow harmless group-writability - Allow ~/.ssh/config to be group-writable, provided that the group in - question contains only the file's owner. Rejected upstream for IMO - incorrect reasons (e.g. a misunderstanding about the contents of - gr->gr_mem). Given that per-user groups and umask 002 are the default - setup in Debian (for good reasons - this makes operating in setgid - directories with other groups much easier), we need to permit this. + Allow secure files (~/.ssh/config, ~/.ssh/authorized_keys, etc.) to be + group-writable, provided that the group in question contains only the + file's owner. Rejected upstream for IMO incorrect reasons (e.g. a + misunderstanding about the contents of gr->gr_mem). Given that + per-user groups and umask 002 are the default setup in Debian (for good + reasons - this makes operating in setgid directories with other groups + much easier), we need to permit this by default. Author: Colin Watson Bug: https://bugzilla.mindrot.org/show_bug.cgi?id=1060 Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=314347 @@ -23,36 +24,13 @@ Index: b/readconf.c #include "xmalloc.h" #include "ssh.h" -@@ -1000,11 +1002,30 @@ - - if (checkperm) { - struct stat sb; -+ int bad_modes = 0; +@@ -1003,8 +1005,7 @@ 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)) -+ if (sb.st_uid != 0 && sb.st_uid != getuid()) -+ bad_modes = 1; -+ if ((sb.st_mode & 020) != 0) { -+ /* If the file is group-writable, the group in -+ * question must have at most one member, namely the -+ * file's owner. -+ */ -+ struct passwd *pw = getpwuid(sb.st_uid); -+ struct group *gr = getgrgid(sb.st_gid); -+ if (!pw || !gr) -+ bad_modes = 1; -+ else if (gr->gr_mem[0]) { -+ if (strcmp(pw->pw_name, gr->gr_mem[0]) || -+ gr->gr_mem[1]) -+ bad_modes = 1; -+ } -+ } -+ if ((sb.st_mode & 002) != 0) -+ bad_modes = 1; -+ if (bad_modes) ++ if (!secure_permissions(&sb, getuid())) fatal("Bad owner or permissions on %s", filename); } @@ -82,3 +60,118 @@ Index: b/ssh_config.5 .It Pa /etc/ssh/ssh_config Systemwide configuration file. This file provides defaults for those +Index: b/auth.c +=================================================================== +--- a/auth.c ++++ b/auth.c +@@ -385,8 +385,7 @@ + user_hostfile = tilde_expand_filename(userfile, pw->pw_uid); + if (options.strict_modes && + (stat(user_hostfile, &st) == 0) && +- ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || +- (st.st_mode & 022) != 0)) { ++ !secure_permissions(&st, pw->pw_uid)) { + logit("Authentication refused for %.100s: " + "bad owner or modes for %.200s", + pw->pw_name, user_hostfile); +@@ -438,8 +437,7 @@ + + /* 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) { ++ !secure_permissions(&st, uid)) { + snprintf(err, errlen, "bad ownership or modes for file %s", + buf); + return -1; +@@ -455,8 +453,7 @@ + + debug3("secure_filename: checking '%s'", buf); + if (stat(buf, &st) < 0 || +- (st.st_uid != 0 && st.st_uid != uid) || +- (st.st_mode & 022) != 0) { ++ !secure_permissions(&st, uid)) { + snprintf(err, errlen, + "bad ownership or modes for directory %s", buf); + return -1; +Index: b/misc.c +=================================================================== +--- a/misc.c ++++ b/misc.c +@@ -45,8 +45,9 @@ + #include + #ifdef HAVE_PATHS_H + # include +-#include + #endif ++#include ++#include + #ifdef SSH_TUN_OPENBSD + #include + #endif +@@ -638,6 +639,30 @@ + } + + int ++secure_permissions(struct stat *st, uid_t uid) ++{ ++ if (st->st_uid != 0 && st->st_uid != uid) ++ return 0; ++ if ((st->st_mode & 020) != 0) { ++ /* If the file is group-writable, the group in question must ++ * have at most one member, namely the file's owner. ++ */ ++ struct passwd *pw = getpwuid(st->st_uid); ++ struct group *gr = getgrgid(st->st_gid); ++ if (!pw || !gr) ++ return 0; ++ else if (gr->gr_mem[0]) { ++ if (strcmp(pw->pw_name, gr->gr_mem[0]) || ++ gr->gr_mem[1]) ++ return 0; ++ } ++ } ++ if ((st->st_mode & 002) != 0) ++ return 0; ++ return 1; ++} ++ ++int + tun_open(int tun, int mode) + { + #if defined(CUSTOM_SYS_TUN_OPEN) +Index: b/misc.h +=================================================================== +--- a/misc.h ++++ b/misc.h +@@ -91,4 +91,6 @@ + int ask_permission(const char *, ...) __attribute__((format(printf, 1, 2))); + int read_keyfile_line(FILE *, const char *, char *, size_t, u_long *); + ++int secure_permissions(struct stat *st, uid_t uid); ++ + #endif /* _MISC_H */ +Index: b/auth-rhosts.c +=================================================================== +--- a/auth-rhosts.c ++++ b/auth-rhosts.c +@@ -256,8 +256,7 @@ + return 0; + } + if (options.strict_modes && +- ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || +- (st.st_mode & 022) != 0)) { ++ !secure_permissions(&st, pw->pw_uid)) { + logit("Rhosts authentication refused for %.100s: " + "bad ownership or modes for home directory.", pw->pw_name); + auth_debug_add("Rhosts authentication refused for %.100s: " +@@ -283,8 +282,7 @@ + * allowing access to their account by anyone. + */ + if (options.strict_modes && +- ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || +- (st.st_mode & 022) != 0)) { ++ !secure_permissions(&st, pw->pw_uid)) { + logit("Rhosts authentication refused for %.100s: bad modes for %.200s", + pw->pw_name, buf); + auth_debug_add("Bad file modes for %.200s", buf); -- cgit v1.2.3