diff options
author | Colin Watson <cjwatson@debian.org> | 2014-02-09 16:09:58 +0000 |
---|---|---|
committer | Colin Watson <cjwatson@debian.org> | 2016-01-04 15:07:00 +0000 |
commit | c60b1066b877429b723b351f44efb9e84bc64252 (patch) | |
tree | a9ff77b802c536867b18e60a386c54e18155b02a | |
parent | 86d7bcd53809aacc75344386bd8b88bf5fcb2fce (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: 2013-09-14
Patch-Name: user-group-modes.patch
-rw-r--r-- | auth-rhosts.c | 6 | ||||
-rw-r--r-- | auth.c | 9 | ||||
-rw-r--r-- | misc.c | 69 | ||||
-rw-r--r-- | misc.h | 2 | ||||
-rw-r--r-- | platform.c | 16 | ||||
-rw-r--r-- | readconf.c | 5 | ||||
-rw-r--r-- | ssh.1 | 2 | ||||
-rw-r--r-- | ssh_config.5 | 2 |
8 files changed, 82 insertions, 29 deletions
diff --git a/auth-rhosts.c b/auth-rhosts.c index ee9e827af..2ff2cffa9 100644 --- a/auth-rhosts.c +++ b/auth-rhosts.c | |||
@@ -271,8 +271,7 @@ auth_rhosts2_raw(struct passwd *pw, const char *client_user, const char *hostnam | |||
271 | return 0; | 271 | return 0; |
272 | } | 272 | } |
273 | if (options.strict_modes && | 273 | if (options.strict_modes && |
274 | ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || | 274 | !secure_permissions(&st, pw->pw_uid)) { |
275 | (st.st_mode & 022) != 0)) { | ||
276 | logit("Rhosts authentication refused for %.100s: " | 275 | logit("Rhosts authentication refused for %.100s: " |
277 | "bad ownership or modes for home directory.", pw->pw_name); | 276 | "bad ownership or modes for home directory.", pw->pw_name); |
278 | auth_debug_add("Rhosts authentication refused for %.100s: " | 277 | auth_debug_add("Rhosts authentication refused for %.100s: " |
@@ -298,8 +297,7 @@ auth_rhosts2_raw(struct passwd *pw, const char *client_user, const char *hostnam | |||
298 | * allowing access to their account by anyone. | 297 | * allowing access to their account by anyone. |
299 | */ | 298 | */ |
300 | if (options.strict_modes && | 299 | if (options.strict_modes && |
301 | ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || | 300 | !secure_permissions(&st, pw->pw_uid)) { |
302 | (st.st_mode & 022) != 0)) { | ||
303 | logit("Rhosts authentication refused for %.100s: bad modes for %.200s", | 301 | logit("Rhosts authentication refused for %.100s: bad modes for %.200s", |
304 | pw->pw_name, buf); | 302 | pw->pw_name, buf); |
305 | auth_debug_add("Bad file modes for %.200s", buf); | 303 | auth_debug_add("Bad file modes for %.200s", buf); |
@@ -425,8 +425,7 @@ check_key_in_hostfiles(struct passwd *pw, Key *key, const char *host, | |||
425 | user_hostfile = tilde_expand_filename(userfile, pw->pw_uid); | 425 | user_hostfile = tilde_expand_filename(userfile, pw->pw_uid); |
426 | if (options.strict_modes && | 426 | if (options.strict_modes && |
427 | (stat(user_hostfile, &st) == 0) && | 427 | (stat(user_hostfile, &st) == 0) && |
428 | ((st.st_uid != 0 && st.st_uid != pw->pw_uid) || | 428 | !secure_permissions(&st, pw->pw_uid)) { |
429 | (st.st_mode & 022) != 0)) { | ||
430 | logit("Authentication refused for %.100s: " | 429 | logit("Authentication refused for %.100s: " |
431 | "bad owner or modes for %.200s", | 430 | "bad owner or modes for %.200s", |
432 | pw->pw_name, user_hostfile); | 431 | pw->pw_name, user_hostfile); |
@@ -488,8 +487,7 @@ auth_secure_path(const char *name, struct stat *stp, const char *pw_dir, | |||
488 | snprintf(err, errlen, "%s is not a regular file", buf); | 487 | snprintf(err, errlen, "%s is not a regular file", buf); |
489 | return -1; | 488 | return -1; |
490 | } | 489 | } |
491 | if ((!platform_sys_dir_uid(stp->st_uid) && stp->st_uid != uid) || | 490 | if (!secure_permissions(stp, uid)) { |
492 | (stp->st_mode & 022) != 0) { | ||
493 | snprintf(err, errlen, "bad ownership or modes for file %s", | 491 | snprintf(err, errlen, "bad ownership or modes for file %s", |
494 | buf); | 492 | buf); |
495 | return -1; | 493 | return -1; |
@@ -504,8 +502,7 @@ auth_secure_path(const char *name, struct stat *stp, const char *pw_dir, | |||
504 | strlcpy(buf, cp, sizeof(buf)); | 502 | strlcpy(buf, cp, sizeof(buf)); |
505 | 503 | ||
506 | if (stat(buf, &st) < 0 || | 504 | if (stat(buf, &st) < 0 || |
507 | (!platform_sys_dir_uid(st.st_uid) && st.st_uid != uid) || | 505 | !secure_permissions(&st, uid)) { |
508 | (st.st_mode & 022) != 0) { | ||
509 | snprintf(err, errlen, | 506 | snprintf(err, errlen, |
510 | "bad ownership or modes for directory %s", buf); | 507 | "bad ownership or modes for directory %s", buf); |
511 | return -1; | 508 | return -1; |
@@ -50,8 +50,9 @@ | |||
50 | #include <netdb.h> | 50 | #include <netdb.h> |
51 | #ifdef HAVE_PATHS_H | 51 | #ifdef HAVE_PATHS_H |
52 | # include <paths.h> | 52 | # include <paths.h> |
53 | #include <pwd.h> | ||
54 | #endif | 53 | #endif |
54 | #include <pwd.h> | ||
55 | #include <grp.h> | ||
55 | #ifdef SSH_TUN_OPENBSD | 56 | #ifdef SSH_TUN_OPENBSD |
56 | #include <net/if.h> | 57 | #include <net/if.h> |
57 | #endif | 58 | #endif |
@@ -60,6 +61,7 @@ | |||
60 | #include "misc.h" | 61 | #include "misc.h" |
61 | #include "log.h" | 62 | #include "log.h" |
62 | #include "ssh.h" | 63 | #include "ssh.h" |
64 | #include "platform.h" | ||
63 | 65 | ||
64 | /* remove newline at end of string */ | 66 | /* remove newline at end of string */ |
65 | char * | 67 | char * |
@@ -644,6 +646,71 @@ read_keyfile_line(FILE *f, const char *filename, char *buf, size_t bufsz, | |||
644 | return -1; | 646 | return -1; |
645 | } | 647 | } |
646 | 648 | ||
649 | /* | ||
650 | * return 1 if the specified uid is a uid that may own a system directory | ||
651 | * otherwise 0. | ||
652 | */ | ||
653 | int | ||
654 | platform_sys_dir_uid(uid_t uid) | ||
655 | { | ||
656 | if (uid == 0) | ||
657 | return 1; | ||
658 | #ifdef PLATFORM_SYS_DIR_UID | ||
659 | if (uid == PLATFORM_SYS_DIR_UID) | ||
660 | return 1; | ||
661 | #endif | ||
662 | return 0; | ||
663 | } | ||
664 | |||
665 | int | ||
666 | secure_permissions(struct stat *st, uid_t uid) | ||
667 | { | ||
668 | if (!platform_sys_dir_uid(st->st_uid) && st->st_uid != uid) | ||
669 | return 0; | ||
670 | if ((st->st_mode & 002) != 0) | ||
671 | return 0; | ||
672 | if ((st->st_mode & 020) != 0) { | ||
673 | /* If the file is group-writable, the group in question must | ||
674 | * have exactly one member, namely the file's owner. | ||
675 | * (Zero-member groups are typically used by setgid | ||
676 | * binaries, and are unlikely to be suitable.) | ||
677 | */ | ||
678 | struct passwd *pw; | ||
679 | struct group *gr; | ||
680 | int members = 0; | ||
681 | |||
682 | gr = getgrgid(st->st_gid); | ||
683 | if (!gr) | ||
684 | return 0; | ||
685 | |||
686 | /* Check primary group memberships. */ | ||
687 | while ((pw = getpwent()) != NULL) { | ||
688 | if (pw->pw_gid == gr->gr_gid) { | ||
689 | ++members; | ||
690 | if (pw->pw_uid != uid) | ||
691 | return 0; | ||
692 | } | ||
693 | } | ||
694 | endpwent(); | ||
695 | |||
696 | pw = getpwuid(st->st_uid); | ||
697 | if (!pw) | ||
698 | return 0; | ||
699 | |||
700 | /* Check supplementary group memberships. */ | ||
701 | if (gr->gr_mem[0]) { | ||
702 | ++members; | ||
703 | if (strcmp(pw->pw_name, gr->gr_mem[0]) || | ||
704 | gr->gr_mem[1]) | ||
705 | return 0; | ||
706 | } | ||
707 | |||
708 | if (!members) | ||
709 | return 0; | ||
710 | } | ||
711 | return 1; | ||
712 | } | ||
713 | |||
647 | int | 714 | int |
648 | tun_open(int tun, int mode) | 715 | tun_open(int tun, int mode) |
649 | { | 716 | { |
@@ -135,4 +135,6 @@ char *read_passphrase(const char *, int); | |||
135 | int ask_permission(const char *, ...) __attribute__((format(printf, 1, 2))); | 135 | int ask_permission(const char *, ...) __attribute__((format(printf, 1, 2))); |
136 | int read_keyfile_line(FILE *, const char *, char *, size_t, u_long *); | 136 | int read_keyfile_line(FILE *, const char *, char *, size_t, u_long *); |
137 | 137 | ||
138 | int secure_permissions(struct stat *st, uid_t uid); | ||
139 | |||
138 | #endif /* _MISC_H */ | 140 | #endif /* _MISC_H */ |
diff --git a/platform.c b/platform.c index f35ec39a8..9a23e6e3e 100644 --- a/platform.c +++ b/platform.c | |||
@@ -197,19 +197,3 @@ platform_krb5_get_principal_name(const char *pw_name) | |||
197 | return NULL; | 197 | return NULL; |
198 | #endif | 198 | #endif |
199 | } | 199 | } |
200 | |||
201 | /* | ||
202 | * return 1 if the specified uid is a uid that may own a system directory | ||
203 | * otherwise 0. | ||
204 | */ | ||
205 | int | ||
206 | platform_sys_dir_uid(uid_t uid) | ||
207 | { | ||
208 | if (uid == 0) | ||
209 | return 1; | ||
210 | #ifdef PLATFORM_SYS_DIR_UID | ||
211 | if (uid == PLATFORM_SYS_DIR_UID) | ||
212 | return 1; | ||
213 | #endif | ||
214 | return 0; | ||
215 | } | ||
diff --git a/readconf.c b/readconf.c index 46c343f30..c0ba5a72c 100644 --- a/readconf.c +++ b/readconf.c | |||
@@ -39,6 +39,8 @@ | |||
39 | #include <stdio.h> | 39 | #include <stdio.h> |
40 | #include <string.h> | 40 | #include <string.h> |
41 | #include <unistd.h> | 41 | #include <unistd.h> |
42 | #include <pwd.h> | ||
43 | #include <grp.h> | ||
42 | #ifdef HAVE_UTIL_H | 44 | #ifdef HAVE_UTIL_H |
43 | #include <util.h> | 45 | #include <util.h> |
44 | #endif | 46 | #endif |
@@ -1579,8 +1581,7 @@ read_config_file(const char *filename, struct passwd *pw, const char *host, | |||
1579 | 1581 | ||
1580 | if (fstat(fileno(f), &sb) == -1) | 1582 | if (fstat(fileno(f), &sb) == -1) |
1581 | fatal("fstat %s: %s", filename, strerror(errno)); | 1583 | fatal("fstat %s: %s", filename, strerror(errno)); |
1582 | if (((sb.st_uid != 0 && sb.st_uid != getuid()) || | 1584 | if (!secure_permissions(&sb, getuid())) |
1583 | (sb.st_mode & 022) != 0)) | ||
1584 | fatal("Bad owner or permissions on %s", filename); | 1585 | fatal("Bad owner or permissions on %s", filename); |
1585 | } | 1586 | } |
1586 | 1587 | ||
@@ -1458,6 +1458,8 @@ The file format and configuration options are described in | |||
1458 | .Xr ssh_config 5 . | 1458 | .Xr ssh_config 5 . |
1459 | Because of the potential for abuse, this file must have strict permissions: | 1459 | Because of the potential for abuse, this file must have strict permissions: |
1460 | read/write for the user, and not writable by others. | 1460 | read/write for the user, and not writable by others. |
1461 | It may be group-writable provided that the group in question contains only | ||
1462 | the user. | ||
1461 | .Pp | 1463 | .Pp |
1462 | .It Pa ~/.ssh/environment | 1464 | .It Pa ~/.ssh/environment |
1463 | Contains additional definitions for environment variables; see | 1465 | Contains additional definitions for environment variables; see |
diff --git a/ssh_config.5 b/ssh_config.5 index 78e918a0e..1e9c058ff 100644 --- a/ssh_config.5 +++ b/ssh_config.5 | |||
@@ -1757,6 +1757,8 @@ The format of this file is described above. | |||
1757 | This file is used by the SSH client. | 1757 | This file is used by the SSH client. |
1758 | Because of the potential for abuse, this file must have strict permissions: | 1758 | Because of the potential for abuse, this file must have strict permissions: |
1759 | read/write for the user, and not accessible by others. | 1759 | read/write for the user, and not accessible by others. |
1760 | It may be group-writable provided that the group in question contains only | ||
1761 | the user. | ||
1760 | .It Pa /etc/ssh/ssh_config | 1762 | .It Pa /etc/ssh/ssh_config |
1761 | Systemwide configuration file. | 1763 | Systemwide configuration file. |
1762 | This file provides defaults for those | 1764 | This file provides defaults for those |