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=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 4AB47C82292 for ; Mon, 27 Apr 2020 22:18:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 29A2F2078C for ; Mon, 27 Apr 2020 22:18:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=paul-moore-com.20150623.gappssmtp.com header.i=@paul-moore-com.20150623.gappssmtp.com header.b="oo3D+/91" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726335AbgD0WSB (ORCPT ); Mon, 27 Apr 2020 18:18:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51514 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1726328AbgD0WSA (ORCPT ); Mon, 27 Apr 2020 18:18:00 -0400 Received: from mail-ej1-x643.google.com (mail-ej1-x643.google.com [IPv6:2a00:1450:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 46DCEC0610D5 for ; Mon, 27 Apr 2020 15:17:59 -0700 (PDT) Received: by mail-ej1-x643.google.com with SMTP id s9so15587826eju.1 for ; Mon, 27 Apr 2020 15:17:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=UL/KOG9jWEcCXLI+hyOZaTUXJMue/WeQvcbigSn5Ae8=; b=oo3D+/91hLZqWrB4onHqCW6rJJEyOlyHLO7bvwoLVFRudtP0fAigCrjLn7Z9f9n+nm hEj3u9hPHsKPnEOAK5Tuw7pSoz/aW6wlUwqvWWuKQicUWWuwIcCJLqvP581ltLbjJwXq OZCg+m65Ct4APQ+nG0JRjkiyAdNiBxEySxSDLi2sQBCxdRozmkvueShw0fcCe6b+l4on ME874FzMO6JJiq/29jNwepCF8CUFQ/0oH++U2iexahGpYeFxeF5ESFrEC7zY1Y/k6FJS 35UXfU5LyfP/zCORQLzV1NRM36Verhzjd0JYwN0SKQ+mzdTKei/rMyoGin1IsrPTnGn5 VDlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=UL/KOG9jWEcCXLI+hyOZaTUXJMue/WeQvcbigSn5Ae8=; b=dmuDexemy/agI8dyRJtpwSnruyChd/tfz3G3HkkrLUpSO/qYrQLRlF0fIcITBszJhI r2oKswAH6TQnDIzE08WSklrzJBC5BSKng/fzSoYpuFFe8rXbz2LguHXHt/CtCJjQgXoP mJYdXe68wBmjS9FXGJ8fpWfoRE1kPxjkgBk84tQx0faEJhxPGYUm27QfRVbODnZPvokM jEyxgbhLv5qPMeeGMuFLJ0nqmxclvmeDnQzIqTiuruM07w2Fa6vHTpyJt9nWtXvgXvsD t0rpMkHKWXRjyCnbfV1jvhcZ30fQkeUvfvWg1w6UvgZNEbTDHtAwJowgKMARDKLLg+TF JGjQ== X-Gm-Message-State: AGi0PuaFG3QgsMsCL7+iD3iRylNhZdmMTH7KHC4lKGZQo60yhfciMAkr 6bK01qTaQU1yBEivX6lI1WTafTb2j2O2SJNtgKj+ X-Google-Smtp-Source: APiQypKcaWzr+dqSzZiHdrMq3hOsjQyRzRtPnEHn5+cEMPnQPkQJKf6XD2ptG29hXT9B03+/SChoZBwUAP+jLwKJmYI= X-Received: by 2002:a17:906:31da:: with SMTP id f26mr21210369ejf.308.1588025877826; Mon, 27 Apr 2020 15:17:57 -0700 (PDT) MIME-Version: 1.0 References: <2136640.1587472186@warthog.procyon.org.uk> <3834193.1587771802@warthog.procyon.org.uk> <355576.1587996734@warthog.procyon.org.uk> In-Reply-To: From: Paul Moore Date: Mon, 27 Apr 2020 18:17:46 -0400 Message-ID: Subject: Re: [PATCH] selinux: Fix use of KEY_NEED_* instead of KEY__* perms To: Stephen Smalley Cc: David Howells , keyrings@vger.kernel.org, SElinux list , LSM List Content-Type: text/plain; charset="UTF-8" Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: On Mon, Apr 27, 2020 at 1:02 PM Stephen Smalley wrote: > On Mon, Apr 27, 2020 at 10:13 AM David Howells wrote: ... > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 0b4e32161b77..6087955b49d8 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -6539,20 +6539,38 @@ static void selinux_key_free(struct key *k) > > kfree(ksec); > > } > > > > +static unsigned int selinux_keyperm_to_av(unsigned int need_perm) > > +{ > > + switch (need_perm) { > > + case KEY_NEED_VIEW: return KEY__VIEW; > > + case KEY_NEED_READ: return KEY__READ; > > + case KEY_NEED_WRITE: return KEY__WRITE; > > + case KEY_NEED_SEARCH: return KEY__SEARCH; > > + case KEY_NEED_LINK: return KEY__LINK; > > + case KEY_NEED_SETATTR: return KEY__SETATTR; > > + default: > > Possibly WARN() or BUG() here? Or BUILD_BUG_ON(KEY_NEED_ALL != 0x3f) > to force an update here > whenever a new key permission is defined? My vote is for a build time check via BUILD_BUG_ON() or similar mechanism. It's worked really well in other places, I'm thinking specifically of the SELinux netlink mapping. > > + return 0; > > + } > > +} > > + > > static int selinux_key_permission(key_ref_t key_ref, > > const struct cred *cred, > > - unsigned perm) > > + unsigned need_perm) > > { > > struct key *key; > > struct key_security_struct *ksec; > > + unsigned int perm; > > u32 sid; > > > > /* if no specific permissions are requested, we skip the > > permission check. No serious, additional covert channels > > appear to be created. */ > > - if (perm == 0) > > + if (need_perm == 0) > > return 0; > > > > + perm = selinux_keyperm_to_av(need_perm); > > + if (perm == 0) > > + return -EACCES; > > We should log or audit some kind of message here, whether via WARN(), > audit_log(), or something, to avoid silent denials. Assuming we add a build time check (see above), I think a WARN here is okay. -- paul moore www.paul-moore.com