From patchwork Thu Apr 11 20:11:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Micah Morton X-Patchwork-Id: 10896763 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8B10417E0 for ; Thu, 11 Apr 2019 20:12:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7019428B0D for ; Thu, 11 Apr 2019 20:12:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6182028DD8; Thu, 11 Apr 2019 20:12:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DDD2428B0D for ; Thu, 11 Apr 2019 20:12:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726713AbfDKUL7 (ORCPT ); Thu, 11 Apr 2019 16:11:59 -0400 Received: from mail-pl1-f195.google.com ([209.85.214.195]:38279 "EHLO mail-pl1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726615AbfDKUL6 (ORCPT ); Thu, 11 Apr 2019 16:11:58 -0400 Received: by mail-pl1-f195.google.com with SMTP id f36so3938683plb.5 for ; Thu, 11 Apr 2019 13:11:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=JjTPkoM9UPI3NoiT+mhvmo5CzoCwUgsBiLiwGNSQjFw=; b=GaXT2vsJT94J3pPv3yA9qJxYucSpJs9F8a+lM3LJ1pG5YQmTv8cxUcxDlGGOdaTkTH Yy+wKIH8+H0UzFdZr61/ubMMlIy7EJQKrAHgcVlvsMsenIF8CeGeO9DoQK468rp8eJOY ZRML9TgrM5d0GVRw2nOd5EWVy7i7Ww1aQNmPw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=JjTPkoM9UPI3NoiT+mhvmo5CzoCwUgsBiLiwGNSQjFw=; b=eeH7PjdmrrKsV/B336aAYXkEdYralDvJrH6Oeo418Bttqco1TzTin1lxQ0xvAdHvm8 8FCuK5MDwx3srRJDs75mgegmjCy7G6XNGSV9sQl64Xits9XEO+mSSssc4fPHyN3LfDb2 BIjnVwMcPHUTgBDeHil8E2J8ZUXEIWQPa8sBZlGMgXs5lV4t6TIvUlm2BWBMaQpmB9w9 wZO88633dqK7k4j6MpLu30k5k/iT5PGukodzZ7rm6nFyoCP46XJTQX/q0w18nHIZwA8a 8wN4HV1Mrwpqe79WD40TuI2YhWekMmLhQhvSG+7OEAzcTRmAux2vTVdeMh5AJqi2gDjb yP6Q== X-Gm-Message-State: APjAAAVyPw/u7f/uS06xvNF1Al1ZuouR9iubn7cyn6qAiqN/2QSjOPMA ap/GrvawH2rBTgqHyVAzXSDhu0OWL8+Ggw== X-Google-Smtp-Source: APXvYqyySK0ojZTSut90f4gRQjkQrrS7vnikwBlY8tjg3wPsq4rBCtK0WBPvCMpqIit2u2jk/T0DEQ== X-Received: by 2002:a17:902:b597:: with SMTP id a23mr3619147pls.284.1555013517206; Thu, 11 Apr 2019 13:11:57 -0700 (PDT) Received: from localhost ([2620:15c:202:201:9e10:971c:f11c:a814]) by smtp.gmail.com with ESMTPSA id t13sm34937228pgo.14.2019.04.11.13.11.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Apr 2019 13:11:56 -0700 (PDT) From: Micah Morton X-Google-Original-From: Micah Morton To: jmorris@namei.org, keescook@chromium.org, casey@schaufler-ca.com, linux-security-module@vger.kernel.org Cc: Jann Horn , Micah Morton Subject: [PATCH v2 08/10] LSM: SafeSetID: add read handler Date: Thu, 11 Apr 2019 13:11:54 -0700 Message-Id: <20190411201154.167617-1-mortonm@chromium.org> X-Mailer: git-send-email 2.21.0.392.gf8f6787159e-goog MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP From: Jann Horn For debugging a running system, it is very helpful to be able to see what policy the system is using. Add a read handler that can dump out a copy of the loaded policy. Signed-off-by: Jann Horn Signed-off-by: Micah Morton Reviewed-by: Kees Cook --- Changes since the last patch set: Instead of doing refcounting, change policy_update_lock to a mutex and hold the mutex across the policy read. security/safesetid/lsm.h | 1 + security/safesetid/securityfs.c | 35 +++++++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/security/safesetid/lsm.h b/security/safesetid/lsm.h index 4a34f558d964..db6d16e6bbc3 100644 --- a/security/safesetid/lsm.h +++ b/security/safesetid/lsm.h @@ -41,6 +41,7 @@ struct setuid_rule { struct setuid_ruleset { DECLARE_HASHTABLE(rules, SETID_HASH_BITS); + char *policy_str; struct rcu_head rcu; }; diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c index 250d59e046c1..997b403c6255 100644 --- a/security/safesetid/securityfs.c +++ b/security/safesetid/securityfs.c @@ -19,7 +19,7 @@ #include "lsm.h" -static DEFINE_SPINLOCK(policy_update_lock); +static DEFINE_MUTEX(policy_update_lock); /* * In the case the input buffer contains one or more invalid UIDs, the kuid_t @@ -67,6 +67,7 @@ static void __release_ruleset(struct rcu_head *rcu) hash_for_each_safe(pol->rules, bucket, tmp, rule, next) kfree(rule); + kfree(pol->policy_str); kfree(pol); } @@ -85,6 +86,7 @@ static ssize_t handle_policy_update(struct file *file, pol = kmalloc(sizeof(struct setuid_ruleset), GFP_KERNEL); if (!pol) return -ENOMEM; + pol->policy_str = NULL; hash_init(pol->rules); p = buf = memdup_user_nul(ubuf, len); @@ -92,6 +94,11 @@ static ssize_t handle_policy_update(struct file *file, err = PTR_ERR(buf); goto out_free_pol; } + pol->policy_str = kstrdup(buf, GFP_KERNEL); + if (pol->policy_str == NULL) { + err = -ENOMEM; + goto out_free_buf; + } /* policy lines, including the last one, end with \n */ while (*p != '\0') { @@ -135,10 +142,10 @@ static ssize_t handle_policy_update(struct file *file, * What we really want here is an xchg() wrapper for RCU, but since that * doesn't currently exist, just use a spinlock for now. */ - spin_lock(&policy_update_lock); + mutex_lock(&policy_update_lock); rcu_swap_protected(safesetid_setuid_rules, pol, lockdep_is_held(&policy_update_lock)); - spin_unlock(&policy_update_lock); + mutex_unlock(&policy_update_lock); err = len; out_free_buf: @@ -162,7 +169,27 @@ static ssize_t safesetid_file_write(struct file *file, return handle_policy_update(file, buf, len); } +static ssize_t safesetid_file_read(struct file *file, char __user *buf, + size_t len, loff_t *ppos) +{ + ssize_t res = 0; + struct setuid_ruleset *pol; + const char *kbuf; + + mutex_lock(&policy_update_lock); + pol = rcu_dereference_protected(safesetid_setuid_rules, + lockdep_is_held(&policy_update_lock)); + if (pol) { + kbuf = pol->policy_str; + res = simple_read_from_buffer(buf, len, ppos, + kbuf, strlen(kbuf)); + } + mutex_unlock(&policy_update_lock); + return res; +} + static const struct file_operations safesetid_file_fops = { + .read = safesetid_file_read, .write = safesetid_file_write, }; @@ -181,7 +208,7 @@ static int __init safesetid_init_securityfs(void) goto error; } - policy_file = securityfs_create_file("whitelist_policy", 0200, + policy_file = securityfs_create_file("whitelist_policy", 0600, policy_dir, NULL, &safesetid_file_fops); if (IS_ERR(policy_file)) { ret = PTR_ERR(policy_file); From patchwork Thu Apr 11 20:12:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Micah Morton X-Patchwork-Id: 10896765 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EBD1417E0 for ; Thu, 11 Apr 2019 20:13:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D2CF328B0D for ; Thu, 11 Apr 2019 20:13:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C6A8928DD8; Thu, 11 Apr 2019 20:13:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EC92728B0D for ; Thu, 11 Apr 2019 20:12:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726829AbfDKUMr (ORCPT ); Thu, 11 Apr 2019 16:12:47 -0400 Received: from mail-pf1-f195.google.com ([209.85.210.195]:44496 "EHLO mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726538AbfDKUMq (ORCPT ); Thu, 11 Apr 2019 16:12:46 -0400 Received: by mail-pf1-f195.google.com with SMTP id y13so3963679pfm.11 for ; Thu, 11 Apr 2019 13:12:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=eMTDDO7hg7vcj82VEGutxpZ2D4xMOKcVJ8qdeJ+R0i8=; b=AxDaVDX/PQoJOKDnb35CeZhh+ck0OWMenkrj+wA6tUvZ+xUgFmwzklEhI6Pysy4IR/ Rkly4s20ORdEIkhOddBc3+TjXJk8BqeV5q3Jgf1OlbFwLiSa0gU104V+R1TS8P8Dhs1D NF948+weo4AOB2CpzvgklerAC0R5VEvZDHnkQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=eMTDDO7hg7vcj82VEGutxpZ2D4xMOKcVJ8qdeJ+R0i8=; b=mugTcvQGhbqAm1o2ANdBFNjCdr02jn30NdkYyhc7P9iCQ/+g8Sz/D8fUdeaRRI51Tw lLwSaoFFS7vaGdrvZkPO4zeEdbNUy/BmnQLG4m4mEx2euhoN1C8mDgcDfFcVZrKhHuGf PgQb2bNHCIwxtYa+eJWqO1QEfFo+OzznCGpXhawioOhF1YgKLA6y41SG4irTjzJq38cn E53E+2XzT0wVbQGeD5abcgud6aSc80TclvzxJLSLZxERSzrjx/TTxxnqG5On0LTn5WHl 9mLYu+Fo8bmy2rzYURbaimPnDjK7kyvY0B9J9O+MQ/fwRRdRRXUOlBJsKjYoILEtxaDb z0Zg== X-Gm-Message-State: APjAAAU1vyBHwRu09zcBsC355RZ4SG8FMZ2Ki9Wae+3sFCxa90fFXE5Z 5Tqtr7sJy8D8b2XYFaQ16xEAOw== X-Google-Smtp-Source: APXvYqwqY0g8USD1U0NsGUXfLqXZYT7GxUHAXPZ8zlfWQbUpFDaHEEjFPNb/zSc1waN2/afQtHfqYw== X-Received: by 2002:a63:cf0d:: with SMTP id j13mr48970331pgg.34.1555013566262; Thu, 11 Apr 2019 13:12:46 -0700 (PDT) Received: from localhost ([2620:15c:202:201:9e10:971c:f11c:a814]) by smtp.gmail.com with ESMTPSA id v6sm48291662pgv.92.2019.04.11.13.12.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Apr 2019 13:12:45 -0700 (PDT) From: Micah Morton X-Google-Original-From: Micah Morton To: jmorris@namei.org, keescook@chromium.org, casey@schaufler-ca.com, linux-security-module@vger.kernel.org Cc: Jann Horn , Micah Morton Subject: [PATCH v2 09/10] LSM: SafeSetID: verify transitive constrainedness Date: Thu, 11 Apr 2019 13:12:43 -0700 Message-Id: <20190411201243.167800-1-mortonm@chromium.org> X-Mailer: git-send-email 2.21.0.392.gf8f6787159e-goog MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP From: Jann Horn Someone might write a ruleset like the following, expecting that it securely constrains UID 1 to UIDs 1, 2 and 3: 1:2 1:3 However, because no constraints are applied to UIDs 2 and 3, an attacker with UID 1 can simply first switch to UID 2, then switch to any UID from there. The secure way to write this ruleset would be: 1:2 1:3 2:2 3:3 , which uses "transition to self" as a way to inhibit the default-allow policy without allowing anything specific. This is somewhat unintuitive. To make sure that policy authors don't accidentally write insecure policies because of this, let the kernel verify that a new ruleset does not contain any entries that are constrained, but transitively unconstrained. Signed-off-by: Jann Horn Signed-off-by: Micah Morton Reviewed-by: Kees Cook --- Changes since the last patch: Instead of failing open when userspace configures an unconstrained (and vulnerable) policy, fix up the policy to make sure it is safe by restricting the un-constrained UIDs. Return EINVAL from the policy write in the case that userspace writes an unconstrained policy. Also move hash_add() into a small helper function. security/safesetid/securityfs.c | 38 ++++++++++++++++++- .../selftests/safesetid/safesetid-test.c | 4 +- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c index 997b403c6255..d568e17dd773 100644 --- a/security/safesetid/securityfs.c +++ b/security/safesetid/securityfs.c @@ -76,6 +76,37 @@ static void release_ruleset(struct setuid_ruleset *pol) call_rcu(&pol->rcu, __release_ruleset); } +static void insert_rule(struct setuid_ruleset *pol, struct setuid_rule *rule) +{ + hash_add(pol->rules, &rule->next, __kuid_val(rule->src_uid)); +} + +static int verify_ruleset(struct setuid_ruleset *pol) +{ + int bucket; + struct setuid_rule *rule, *nrule; + int res = 0; + + hash_for_each(pol->rules, bucket, rule, next) { + if (_setuid_policy_lookup(pol, rule->dst_uid, INVALID_UID) == + SIDPOL_DEFAULT) { + pr_warn("insecure policy detected: uid %d is constrained but transitively unconstrained through uid %d\n", + __kuid_val(rule->src_uid), + __kuid_val(rule->dst_uid)); + res = -EINVAL; + + /* fix it up */ + nrule = kmalloc(sizeof(struct setuid_rule), GFP_KERNEL); + if (!nrule) + return -ENOMEM; + nrule->src_uid = rule->dst_uid; + nrule->dst_uid = rule->dst_uid; + insert_rule(pol, nrule); + } + } + return res; +} + static ssize_t handle_policy_update(struct file *file, const char __user *ubuf, size_t len) { @@ -128,7 +159,7 @@ static ssize_t handle_policy_update(struct file *file, goto out_free_rule; } - hash_add(pol->rules, &rule->next, __kuid_val(rule->src_uid)); + insert_rule(pol, rule); p = end + 1; continue; @@ -137,6 +168,11 @@ static ssize_t handle_policy_update(struct file *file, goto out_free_buf; } + err = verify_ruleset(pol); + /* bogus policy falls through after fixing it up */ + if (err && err != -EINVAL) + goto out_free_buf; + /* * Everything looks good, apply the policy and release the old one. * What we really want here is an xchg() wrapper for RCU, but since that diff --git a/tools/testing/selftests/safesetid/safesetid-test.c b/tools/testing/selftests/safesetid/safesetid-test.c index 4f03813d1911..8f40c6ecdad1 100644 --- a/tools/testing/selftests/safesetid/safesetid-test.c +++ b/tools/testing/selftests/safesetid/safesetid-test.c @@ -144,7 +144,9 @@ static void write_policies(void) { static char *policy_str = "1:2\n" - "1:3\n"; + "1:3\n" + "2:2\n" + "3:3\n"; ssize_t written; int fd;