Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932993AbaLBU1x (ORCPT ); Tue, 2 Dec 2014 15:27:53 -0500 Received: from out03.mta.xmission.com ([166.70.13.233]:36927 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932551AbaLBU1s (ORCPT ); Tue, 2 Dec 2014 15:27:48 -0500 From: ebiederm@xmission.com (Eric W. Biederman) To: Andy Lutomirski Cc: Linux Containers , Josh Triplett , Andrew Morton , Kees Cook , Michael Kerrisk-manpages , Linux API , linux-man , "linux-kernel\@vger.kernel.org" , LSM , Casey Schaufler , "Serge E. Hallyn" , Richard Weinberger , Kenton Varda , stable References: <52e0643bd47b1e5c65921d6e00aea1f724bb510a.1417281801.git.luto@amacapital.net> <87h9xez20g.fsf@x220.int.ebiederm.org> <87mw75ygwp.fsf@x220.int.ebiederm.org> Date: Tue, 02 Dec 2014 14:25:35 -0600 In-Reply-To: (Andy Lutomirski's message of "Tue, 2 Dec 2014 12:13:32 -0800") Message-ID: <87fvcxyf28.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1+pqtHkBRshrnzSVv5Utrk2f87RaaTibAc= X-SA-Exim-Connect-IP: 97.121.92.161 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 1.5 XMNoVowels Alpha-numberic number with no vowels * 0.7 XMSubLong Long Subject * 1.5 TR_Symld_Words too many words that have symbols inside * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] * 1.0 T_XMDrugObfuBody_08 obfuscated drug references X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ****;Andy Lutomirski X-Spam-Relay-Country: X-Spam-Timing: total 749 ms - load_scoreonly_sql: 0.07 (0.0%), signal_user_changed: 3.7 (0.5%), b_tie_ro: 2.5 (0.3%), parse: 1.07 (0.1%), extract_message_metadata: 16 (2.1%), get_uri_detail_list: 4.1 (0.6%), tests_pri_-1000: 7 (0.9%), tests_pri_-950: 1.38 (0.2%), tests_pri_-900: 1.20 (0.2%), tests_pri_-400: 34 (4.5%), check_bayes: 33 (4.4%), b_tokenize: 13 (1.7%), b_tok_get_all: 11 (1.5%), b_comp_prob: 2.7 (0.4%), b_tok_touch_all: 3.3 (0.4%), b_finish: 0.66 (0.1%), tests_pri_0: 677 (90.3%), tests_pri_500: 6 (0.7%), rewrite_mail: 0.00 (0.0%) Subject: [CFT][PATCH 1/3] userns: Avoid problems with negative groups X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Classic unix permission checks have an interesting feature, the group permissions for a file can be set to less than the other permissions on a file. Occassionally this is used deliberately to give a certain group of users fewer permissions than the default. Overlooking negative groups has resulted in the permission checks for setting up a group mapping in a user namespace to be too lax. Tighten the permission checks in new_idmap_permitted to ensure that mapping uids and gids into user namespaces without privilege will not result in new combinations of credentials being available to the users. When setting mappings without privilege only the creator of the user namespace is interesting as all other users that have CAP_SETUID over the user namespace will also have CAP_SETUID over the user namespaces parent. So the scope of the unprivileged check is reduced to just the case where cred->euid is the namespace creator. For setting a uid mapping without privilege only euid is considered as setresuid can set uid, suid and fsuid from euid without privielege making any combination of uids possible with user namespaces already possible without them. For now seeting a gid mapping without privilege is removed. The only possible set of credentials that would be safe without a gid mapping (egid without any supplementary groups) just doesn't happen in practice so would simply lead to unused untested code. setgroups is modified to fail not only when the group ids do not map but also when there are no gid mappings at all, preventing setgroups(0, NULL) from succeeding when gid mappings have not been established. For a small class of applications this change breaks userspace and removes useful functionality. This small class of applications includes tools/testing/selftests/mount/unprivileged-remount-test.c Most of the removed functionality will be added back with the addition of a one way knob to disable setgroups. Once setgroups is disabled setting the gid_map becomes as safe as setting the uid_map. For more common applications that set the uid_map and the gid_map with privilege this change will have no effect on them. This should fix CVE-2014-8989. Cc: stable@vger.kernel.org Signed-off-by: "Eric W. Biederman" --- arch/s390/kernel/compat_linux.c | 5 ++++- include/linux/user_namespace.h | 10 ++++++++++ kernel/groups.c | 5 ++++- kernel/uid16.c | 5 ++++- kernel/user_namespace.c | 17 ++++++++++------- 5 files changed, 32 insertions(+), 10 deletions(-) diff --git a/arch/s390/kernel/compat_linux.c b/arch/s390/kernel/compat_linux.c index ca38139423ae..21c91feeca2d 100644 --- a/arch/s390/kernel/compat_linux.c +++ b/arch/s390/kernel/compat_linux.c @@ -49,6 +49,7 @@ #include #include #include +#include #include #include @@ -246,10 +247,12 @@ out: COMPAT_SYSCALL_DEFINE2(s390_setgroups16, int, gidsetsize, u16 __user *, grouplist) { + struct user_namespace *user_ns = current_user_ns(); struct group_info *group_info; int retval; - if (!capable(CAP_SETGID)) + if (!gid_mapping_possible(user_ns) || + !capable(CAP_SETGID)) return -EPERM; if ((unsigned)gidsetsize > NGROUPS_MAX) return -EINVAL; diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h index e95372654f09..26d5e8f5db97 100644 --- a/include/linux/user_namespace.h +++ b/include/linux/user_namespace.h @@ -46,6 +46,11 @@ static inline struct user_namespace *get_user_ns(struct user_namespace *ns) return ns; } +static inline bool gid_mapping_possible(const struct user_namespace *ns) +{ + return ns->gid_map.nr_extents != 0; +} + extern int create_user_ns(struct cred *new); extern int unshare_userns(unsigned long unshare_flags, struct cred **new_cred); extern void free_user_ns(struct user_namespace *ns); @@ -70,6 +75,11 @@ static inline struct user_namespace *get_user_ns(struct user_namespace *ns) return &init_user_ns; } +static inline bool gid_mapping_possible(const struct user_namespace *ns) +{ + return true; +} + static inline int create_user_ns(struct cred *new) { return -EINVAL; diff --git a/kernel/groups.c b/kernel/groups.c index 451698f86cfa..b9a6a5c7e100 100644 --- a/kernel/groups.c +++ b/kernel/groups.c @@ -6,6 +6,7 @@ #include #include #include +#include #include /* init to 2 - one for init_task, one to ensure it is never freed */ @@ -220,10 +221,12 @@ out: SYSCALL_DEFINE2(setgroups, int, gidsetsize, gid_t __user *, grouplist) { + struct user_namespace *user_ns = current_user_ns(); struct group_info *group_info; int retval; - if (!ns_capable(current_user_ns(), CAP_SETGID)) + if (!gid_mapping_possible(user_ns) || + !ns_capable(user_ns, CAP_SETGID)) return -EPERM; if ((unsigned)gidsetsize > NGROUPS_MAX) return -EINVAL; diff --git a/kernel/uid16.c b/kernel/uid16.c index 602e5bbbceff..602c7de2aa11 100644 --- a/kernel/uid16.c +++ b/kernel/uid16.c @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -173,10 +174,12 @@ out: SYSCALL_DEFINE2(setgroups16, int, gidsetsize, old_gid_t __user *, grouplist) { + struct user_namespace *user_ns = current_user_ns(); struct group_info *group_info; int retval; - if (!ns_capable(current_user_ns(), CAP_SETGID)) + if (!gid_mapping_possible(user_ns) || + !ns_capable(user_ns, CAP_SETGID)) return -EPERM; if ((unsigned)gidsetsize > NGROUPS_MAX) return -EINVAL; diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index aa312b0dc3ec..51d65b444951 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -812,16 +812,19 @@ static bool new_idmap_permitted(const struct file *file, struct user_namespace *ns, int cap_setid, struct uid_gid_map *new_map) { - /* Allow mapping to your own filesystem ids */ - if ((new_map->nr_extents == 1) && (new_map->extent[0].count == 1)) { + const struct cred *cred = file->f_cred; + + /* Allow a mapping without capabilities when allowing the root + * of the user namespace capabilities restricted to that id + * will not change the set of credentials available to that + * user. + */ + if ((new_map->nr_extents == 1) && (new_map->extent[0].count == 1) && + uid_eq(ns->owner, cred->euid)) { u32 id = new_map->extent[0].lower_first; if (cap_setid == CAP_SETUID) { kuid_t uid = make_kuid(ns->parent, id); - if (uid_eq(uid, file->f_cred->fsuid)) - return true; - } else if (cap_setid == CAP_SETGID) { - kgid_t gid = make_kgid(ns->parent, id); - if (gid_eq(gid, file->f_cred->fsgid)) + if (uid_eq(uid, cred->euid)) return true; } } -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/