From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 80485C433B4 for ; Wed, 21 Apr 2021 19:18:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4AF1861453 for ; Wed, 21 Apr 2021 19:18:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242914AbhDUTSd (ORCPT ); Wed, 21 Apr 2021 15:18:33 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:34098 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241659AbhDUTSb (ORCPT ); Wed, 21 Apr 2021 15:18:31 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out03.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1lZIME-004DZE-Pl; Wed, 21 Apr 2021 13:17:55 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=fess.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1lZIMD-00AtxS-Q9; Wed, 21 Apr 2021 13:17:54 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: "Serge E. Hallyn" Cc: Christian Brauner , lkml , Linus Torvalds , Kees Cook , "Andrew G. Morgan" , Greg Kroah-Hartman , security@kernel.org, Tycho Andersen , Andy Lutomirski References: <20210416045851.GA13811@mail.hallyn.com> <20210416150501.zam55gschpn2w56i@wittgenstein> <20210416213453.GA29094@mail.hallyn.com> <20210417021945.GA687@mail.hallyn.com> <20210417200434.GA17430@mail.hallyn.com> <20210419122514.GA20598@mail.hallyn.com> <20210419160911.5pguvpj7kfuj6rnr@wittgenstein> <20210420034208.GA2830@mail.hallyn.com> <20210420083129.exyn7ptahx2fg72e@wittgenstein> <20210420134334.GA11582@mail.hallyn.com> Date: Wed, 21 Apr 2021 14:16:34 -0500 In-Reply-To: <20210420134334.GA11582@mail.hallyn.com> (Serge E. Hallyn's message of "Tue, 20 Apr 2021 08:43:34 -0500") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1lZIMD-00AtxS-Q9;;;mid=;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX18G0AOfV6rwW+2m4+vJWzKrMcLT6wolZpc= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v3.4] capabilities: require CAP_SETFCAP to map uid 0 X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org "Serge E. Hallyn" writes: > +/** > + * verify_root_map() - check the uid 0 mapping > + * @file: idmapping file > + * @map_ns: user namespace of the target process > + * @new_map: requested idmap > + * > + * If a process requests mapping parent uid 0 into the new ns, verify that the > + * process writing the map had the CAP_SETFCAP capability as the target process > + * will be able to write fscaps that are valid in ancestor user namespaces. > + * > + * Return: true if the mapping is allowed, false if not. > + */ > +static bool verify_root_map(const struct file *file, > + struct user_namespace *map_ns, > + struct uid_gid_map *new_map) > +{ > + int idx; > + const struct user_namespace *file_ns = file->f_cred->user_ns; > + struct uid_gid_extent *extent0 = NULL; > + > + for (idx = 0; idx < new_map->nr_extents; idx++) { > + if (new_map->nr_extents <= UID_GID_MAP_MAX_BASE_EXTENTS) > + extent0 = &new_map->extent[idx]; > + else > + extent0 = &new_map->forward[idx]; > + if (extent0->lower_first == 0) > + break; > + > + extent0 = NULL; > + } > + > + if (!extent0) > + return true; > + > + if (map_ns == file_ns) { > + /* The process unshared its ns and is writing to its own > + * /proc/self/uid_map. User already has full capabilites in > + * the new namespace. Verify that the parent had CAP_SETFCAP > + * when it unshared. > + * */ > + if (!file_ns->parent_could_setfcap) > + return false; > + } else { > + /* Process p1 is writing to uid_map of p2, who is in a child > + * user namespace to p1's. Verify that the opener of the map > + * file has CAP_SETFCAP against the parent of the new map > + * namespace */ > + if (!file_ns_capable(file, map_ns->parent, CAP_SETFCAP)) > + return false; > + } Is there any reason this permission check is not simply: return map_ns->parent_could_setfcap || file_ns_capable(file, map_ns->parent, CAP_SETFCAP); That is why don't we allow any mapping (that is otherwise valid) in user namespaces whose creator had the permission to call CAP_SETFCAP? Why limit the case of using the creators permissions to only the case of mapping just a single uid (that happens to be the current euid) in the user namespace? I don't see any safety reasons for the map_ns == file_ns test. Is the file_ns_capable check for CAP_SETFCAP actually needed? AKA could the permission check be simplified to: return map_ns->parent_could_setfcap; That would be a much easier rule to explain to people. I seem to remember distributions at least trying to make newuidmap have just CAP_SETUID and newgidmap have just CAP_SETGID. Such a simplified check would facilitate that. Eric