diff options
author | Jann Horn <jannh@google.com> | 2019-04-11 23:12:43 +0300 |
---|---|---|
committer | Micah Morton <mortonm@chromium.org> | 2019-07-15 18:07:51 +0300 |
commit | 4f72123da579655855301b591535a1415224f123 (patch) | |
tree | 6b9ca3a8a23eb20b41591819ee7fef3b04f207b4 /security/safesetid | |
parent | fbd9acb2dc2aa55902c48a83f157082849209fba (diff) |
LSM: SafeSetID: verify transitive constrainedness
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 <jannh@google.com>
Signed-off-by: Micah Morton <mortonm@chromium.org>
Diffstat (limited to 'security/safesetid')
-rw-r--r-- | security/safesetid/securityfs.c | 38 |
1 files changed, 37 insertions, 1 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 @@ out_free_rule: 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 |