diff options
author | Colin Watson <cjwatson@debian.org> | 2014-02-09 16:09:58 +0000 |
---|---|---|
committer | Colin Watson <cjwatson@debian.org> | 2018-04-03 08:21:37 +0100 |
commit | 9e45701c5d6105444cc2f4f5d6c44b0f69969479 (patch) | |
tree | 9df3ccd6a8eee5c69d410a69d06be927bc7284fd | |
parent | 027619c6b05713e3f08a51e7232389383900e5d8 (diff) |
Allow harmless group-writability
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.
Bug: https://bugzilla.mindrot.org/show_bug.cgi?id=1060
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=314347
Last-Update: 2017-10-04
Patch-Name: user-group-modes.patch
-rw-r--r-- | auth-rhosts.c | 6 | ||||
-rw-r--r-- | auth.c | 3 | ||||
-rw-r--r-- | misc.c | 58 | ||||
-rw-r--r-- | misc.h | 2 | ||||
-rw-r--r-- | readconf.c | 3 | ||||
-rw-r--r-- | ssh.1 | 2 | ||||
-rw-r--r-- | ssh_config.5 | 2 |
7 files changed, 63 insertions, 13 deletions
diff --git a/auth-rhosts.c b/auth-rhosts.c index ecf956f06..4dccd5e6a 100644 --- a/auth-rhosts.c +++ b/auth-rhosts.c | |||
@@ -261,8 +261,7 @@ auth_rhosts2(struct passwd *pw, const char *client_user, const char *hostname, | |||
261 | return 0; | 261 | return 0; |
262 | } | 262 | } |
263 | if (options.strict_modes && | 263 | if (options.strict_modes && |
264 | ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || | 264 | !secure_permissions(&st, pw->pw_uid)) { |
265 | (st.st_mode & 022) != 0)) { | ||
266 | logit("Rhosts authentication refused for %.100s: " | 265 | logit("Rhosts authentication refused for %.100s: " |
267 | "bad ownership or modes for home directory.", pw->pw_name); | 266 | "bad ownership or modes for home directory.", pw->pw_name); |
268 | auth_debug_add("Rhosts authentication refused for %.100s: " | 267 | auth_debug_add("Rhosts authentication refused for %.100s: " |
@@ -288,8 +287,7 @@ auth_rhosts2(struct passwd *pw, const char *client_user, const char *hostname, | |||
288 | * allowing access to their account by anyone. | 287 | * allowing access to their account by anyone. |
289 | */ | 288 | */ |
290 | if (options.strict_modes && | 289 | if (options.strict_modes && |
291 | ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || | 290 | !secure_permissions(&st, pw->pw_uid)) { |
292 | (st.st_mode & 022) != 0)) { | ||
293 | logit("Rhosts authentication refused for %.100s: bad modes for %.200s", | 291 | logit("Rhosts authentication refused for %.100s: bad modes for %.200s", |
294 | pw->pw_name, buf); | 292 | pw->pw_name, buf); |
295 | auth_debug_add("Bad file modes for %.200s", buf); | 293 | auth_debug_add("Bad file modes for %.200s", buf); |
@@ -468,8 +468,7 @@ check_key_in_hostfiles(struct passwd *pw, struct sshkey *key, const char *host, | |||
468 | user_hostfile = tilde_expand_filename(userfile, pw->pw_uid); | 468 | user_hostfile = tilde_expand_filename(userfile, pw->pw_uid); |
469 | if (options.strict_modes && | 469 | if (options.strict_modes && |
470 | (stat(user_hostfile, &st) == 0) && | 470 | (stat(user_hostfile, &st) == 0) && |
471 | ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || | 471 | !secure_permissions(&st, pw->pw_uid)) { |
472 | (st.st_mode & 022) != 0)) { | ||
473 | logit("Authentication refused for %.100s: " | 472 | logit("Authentication refused for %.100s: " |
474 | "bad owner or modes for %.200s", | 473 | "bad owner or modes for %.200s", |
475 | pw->pw_name, user_hostfile); | 474 | pw->pw_name, user_hostfile); |
@@ -57,8 +57,9 @@ | |||
57 | #include <netdb.h> | 57 | #include <netdb.h> |
58 | #ifdef HAVE_PATHS_H | 58 | #ifdef HAVE_PATHS_H |
59 | # include <paths.h> | 59 | # include <paths.h> |
60 | #include <pwd.h> | ||
61 | #endif | 60 | #endif |
61 | #include <pwd.h> | ||
62 | #include <grp.h> | ||
62 | #ifdef SSH_TUN_OPENBSD | 63 | #ifdef SSH_TUN_OPENBSD |
63 | #include <net/if.h> | 64 | #include <net/if.h> |
64 | #endif | 65 | #endif |
@@ -1031,6 +1032,55 @@ read_keyfile_line(FILE *f, const char *filename, char *buf, size_t bufsz, | |||
1031 | } | 1032 | } |
1032 | 1033 | ||
1033 | int | 1034 | int |
1035 | secure_permissions(struct stat *st, uid_t uid) | ||
1036 | { | ||
1037 | if (!platform_sys_dir_uid(st->st_uid) && st->st_uid != uid) | ||
1038 | return 0; | ||
1039 | if ((st->st_mode & 002) != 0) | ||
1040 | return 0; | ||
1041 | if ((st->st_mode & 020) != 0) { | ||
1042 | /* If the file is group-writable, the group in question must | ||
1043 | * have exactly one member, namely the file's owner. | ||
1044 | * (Zero-member groups are typically used by setgid | ||
1045 | * binaries, and are unlikely to be suitable.) | ||
1046 | */ | ||
1047 | struct passwd *pw; | ||
1048 | struct group *gr; | ||
1049 | int members = 0; | ||
1050 | |||
1051 | gr = getgrgid(st->st_gid); | ||
1052 | if (!gr) | ||
1053 | return 0; | ||
1054 | |||
1055 | /* Check primary group memberships. */ | ||
1056 | while ((pw = getpwent()) != NULL) { | ||
1057 | if (pw->pw_gid == gr->gr_gid) { | ||
1058 | ++members; | ||
1059 | if (pw->pw_uid != uid) | ||
1060 | return 0; | ||
1061 | } | ||
1062 | } | ||
1063 | endpwent(); | ||
1064 | |||
1065 | pw = getpwuid(st->st_uid); | ||
1066 | if (!pw) | ||
1067 | return 0; | ||
1068 | |||
1069 | /* Check supplementary group memberships. */ | ||
1070 | if (gr->gr_mem[0]) { | ||
1071 | ++members; | ||
1072 | if (strcmp(pw->pw_name, gr->gr_mem[0]) || | ||
1073 | gr->gr_mem[1]) | ||
1074 | return 0; | ||
1075 | } | ||
1076 | |||
1077 | if (!members) | ||
1078 | return 0; | ||
1079 | } | ||
1080 | return 1; | ||
1081 | } | ||
1082 | |||
1083 | int | ||
1034 | tun_open(int tun, int mode, char **ifname) | 1084 | tun_open(int tun, int mode, char **ifname) |
1035 | { | 1085 | { |
1036 | #if defined(CUSTOM_SYS_TUN_OPEN) | 1086 | #if defined(CUSTOM_SYS_TUN_OPEN) |
@@ -1797,8 +1847,7 @@ safe_path(const char *name, struct stat *stp, const char *pw_dir, | |||
1797 | snprintf(err, errlen, "%s is not a regular file", buf); | 1847 | snprintf(err, errlen, "%s is not a regular file", buf); |
1798 | return -1; | 1848 | return -1; |
1799 | } | 1849 | } |
1800 | if ((!platform_sys_dir_uid(stp->st_uid) && stp->st_uid != uid) || | 1850 | if (!secure_permissions(stp, uid)) { |
1801 | (stp->st_mode & 022) != 0) { | ||
1802 | snprintf(err, errlen, "bad ownership or modes for file %s", | 1851 | snprintf(err, errlen, "bad ownership or modes for file %s", |
1803 | buf); | 1852 | buf); |
1804 | return -1; | 1853 | return -1; |
@@ -1813,8 +1862,7 @@ safe_path(const char *name, struct stat *stp, const char *pw_dir, | |||
1813 | strlcpy(buf, cp, sizeof(buf)); | 1862 | strlcpy(buf, cp, sizeof(buf)); |
1814 | 1863 | ||
1815 | if (stat(buf, &st) < 0 || | 1864 | if (stat(buf, &st) < 0 || |
1816 | (!platform_sys_dir_uid(st.st_uid) && st.st_uid != uid) || | 1865 | !secure_permissions(&st, uid)) { |
1817 | (st.st_mode & 022) != 0) { | ||
1818 | snprintf(err, errlen, | 1866 | snprintf(err, errlen, |
1819 | "bad ownership or modes for directory %s", buf); | 1867 | "bad ownership or modes for directory %s", buf); |
1820 | return -1; | 1868 | return -1; |
@@ -168,6 +168,8 @@ char *read_passphrase(const char *, int); | |||
168 | int ask_permission(const char *, ...) __attribute__((format(printf, 1, 2))); | 168 | int ask_permission(const char *, ...) __attribute__((format(printf, 1, 2))); |
169 | int read_keyfile_line(FILE *, const char *, char *, size_t, u_long *); | 169 | int read_keyfile_line(FILE *, const char *, char *, size_t, u_long *); |
170 | 170 | ||
171 | int secure_permissions(struct stat *st, uid_t uid); | ||
172 | |||
171 | #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b)) | 173 | #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b)) |
172 | #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b)) | 174 | #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b)) |
173 | #define ROUNDUP(x, y) ((((x)+((y)-1))/(y))*(y)) | 175 | #define ROUNDUP(x, y) ((((x)+((y)-1))/(y))*(y)) |
diff --git a/readconf.c b/readconf.c index 7f2b5c172..50349e238 100644 --- a/readconf.c +++ b/readconf.c | |||
@@ -1741,8 +1741,7 @@ read_config_file_depth(const char *filename, struct passwd *pw, | |||
1741 | 1741 | ||
1742 | if (fstat(fileno(f), &sb) == -1) | 1742 | if (fstat(fileno(f), &sb) == -1) |
1743 | fatal("fstat %s: %s", filename, strerror(errno)); | 1743 | fatal("fstat %s: %s", filename, strerror(errno)); |
1744 | if (((sb.st_uid != 0 && sb.st_uid != getuid()) || | 1744 | if (!secure_permissions(&sb, getuid())) |
1745 | (sb.st_mode & 022) != 0)) | ||
1746 | fatal("Bad owner or permissions on %s", filename); | 1745 | fatal("Bad owner or permissions on %s", filename); |
1747 | } | 1746 | } |
1748 | 1747 | ||
@@ -1471,6 +1471,8 @@ The file format and configuration options are described in | |||
1471 | .Xr ssh_config 5 . | 1471 | .Xr ssh_config 5 . |
1472 | Because of the potential for abuse, this file must have strict permissions: | 1472 | Because of the potential for abuse, this file must have strict permissions: |
1473 | read/write for the user, and not writable by others. | 1473 | read/write for the user, and not writable by others. |
1474 | It may be group-writable provided that the group in question contains only | ||
1475 | the user. | ||
1474 | .Pp | 1476 | .Pp |
1475 | .It Pa ~/.ssh/environment | 1477 | .It Pa ~/.ssh/environment |
1476 | Contains additional definitions for environment variables; see | 1478 | Contains additional definitions for environment variables; see |
diff --git a/ssh_config.5 b/ssh_config.5 index 32c3632c7..84dcd52cc 100644 --- a/ssh_config.5 +++ b/ssh_config.5 | |||
@@ -1818,6 +1818,8 @@ The format of this file is described above. | |||
1818 | This file is used by the SSH client. | 1818 | This file is used by the SSH client. |
1819 | Because of the potential for abuse, this file must have strict permissions: | 1819 | Because of the potential for abuse, this file must have strict permissions: |
1820 | read/write for the user, and not accessible by others. | 1820 | read/write for the user, and not accessible by others. |
1821 | It may be group-writable provided that the group in question contains only | ||
1822 | the user. | ||
1821 | .It Pa /etc/ssh/ssh_config | 1823 | .It Pa /etc/ssh/ssh_config |
1822 | Systemwide configuration file. | 1824 | Systemwide configuration file. |
1823 | This file provides defaults for those | 1825 | This file provides defaults for those |