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).

1 files changed, 124 insertions, 31 deletions

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 <cjwatson@debian.org>
 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 <netdb.h>
+ #ifdef HAVE_PATHS_H
+ # include <paths.h>
+-#include <pwd.h>
+ #endif
++#include <pwd.h>
++#include <grp.h>
+ #ifdef SSH_TUN_OPENBSD
+ #include <net/if.h>
+ #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);