From db1a8922cf3f0b936595ba41774fe4b66adf091a Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:05 -0400 Subject: capabilities: factor out cap_bprm_set_creds privileged root Factor out the case of privileged root from the function cap_bprm_set_creds() to make the latter easier to read and analyse. Suggested-by: Serge Hallyn Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Okay-ished-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 76 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 28 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index c25e0d27537f..be9bca50c312 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -695,6 +695,52 @@ out: return rc; } +/* + * handle_privileged_root - Handle case of privileged root + * @bprm: The execution parameters, including the proposed creds + * @has_fcap: Are any file capabilities set? + * @effective: Do we have effective root privilege? + * @root_uid: This namespace' root UID WRT initial USER namespace + * + * Handle the case where root is privileged and hasn't been neutered by + * SECURE_NOROOT. If file capabilities are set, they won't be combined with + * set UID root and nothing is changed. If we are root, cap_permitted is + * updated. If we have become set UID root, the effective bit is set. + */ +static void handle_privileged_root(struct linux_binprm *bprm, bool has_cap, + bool *effective, kuid_t root_uid) +{ + const struct cred *old = current_cred(); + struct cred *new = bprm->cred; + + if (issecure(SECURE_NOROOT)) + return; + /* + * If the legacy file capability is set, then don't set privs + * for a setuid root binary run by a non-root user. Do set it + * for a root user just to cause least surprise to an admin. + */ + if (has_cap && !uid_eq(new->uid, root_uid) && uid_eq(new->euid, root_uid)) { + warn_setuid_and_fcaps_mixed(bprm->filename); + return; + } + /* + * To support inheritance of root-permissions and suid-root + * executables under compatibility mode, we override the + * capability sets for the file. + */ + if (uid_eq(new->euid, root_uid) || uid_eq(new->uid, root_uid)) { + /* pP' = (cap_bset & ~0) | (pI & ~0) */ + new->cap_permitted = cap_combine(old->cap_bset, + old->cap_inheritable); + } + /* + * If only the real uid is 0, we do not set the effective bit. + */ + if (uid_eq(new->euid, root_uid)) + *effective = true; +} + /** * cap_bprm_set_creds - Set up the proposed credentials for execve(). * @bprm: The execution parameters, including the proposed creds @@ -707,46 +753,20 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) { const struct cred *old = current_cred(); struct cred *new = bprm->cred; - bool effective, has_cap = false, is_setid; + bool effective = false, has_cap = false, is_setid; int ret; kuid_t root_uid; if (WARN_ON(!cap_ambient_invariant_ok(old))) return -EPERM; - effective = false; ret = get_file_caps(bprm, &effective, &has_cap); if (ret < 0) return ret; root_uid = make_kuid(new->user_ns, 0); - if (!issecure(SECURE_NOROOT)) { - /* - * If the legacy file capability is set, then don't set privs - * for a setuid root binary run by a non-root user. Do set it - * for a root user just to cause least surprise to an admin. - */ - if (has_cap && !uid_eq(new->uid, root_uid) && uid_eq(new->euid, root_uid)) { - warn_setuid_and_fcaps_mixed(bprm->filename); - goto skip; - } - /* - * To support inheritance of root-permissions and suid-root - * executables under compatibility mode, we override the - * capability sets for the file. - * - * If only the real uid is 0, we do not set the effective bit. - */ - if (uid_eq(new->euid, root_uid) || uid_eq(new->uid, root_uid)) { - /* pP' = (cap_bset & ~0) | (pI & ~0) */ - new->cap_permitted = cap_combine(old->cap_bset, - old->cap_inheritable); - } - if (uid_eq(new->euid, root_uid)) - effective = true; - } -skip: + handle_privileged_root(bprm, has_cap, &effective, root_uid); /* if we have fs caps, clear dangerous personality flags */ if (!cap_issubset(new->cap_permitted, old->cap_permitted)) -- cgit v1.2.3 From 4c7e715fc87b6f8b652363b3515b48b3822c5b5f Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:06 -0400 Subject: capabilities: intuitive names for cap gain status Introduce macros cap_gained, cap_grew, cap_full to make the use of the negation of is_subset() easier to read and analyse. Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Okay-ished-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index be9bca50c312..4c9af6ef24b6 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -741,6 +741,12 @@ static void handle_privileged_root(struct linux_binprm *bprm, bool has_cap, *effective = true; } +#define __cap_gained(field, target, source) \ + !cap_issubset(target->cap_##field, source->cap_##field) +#define __cap_grew(target, source, cred) \ + !cap_issubset(cred->cap_##target, cred->cap_##source) +#define __cap_full(field, cred) \ + cap_issubset(CAP_FULL_SET, cred->cap_##field) /** * cap_bprm_set_creds - Set up the proposed credentials for execve(). * @bprm: The execution parameters, including the proposed creds @@ -769,10 +775,9 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) handle_privileged_root(bprm, has_cap, &effective, root_uid); /* if we have fs caps, clear dangerous personality flags */ - if (!cap_issubset(new->cap_permitted, old->cap_permitted)) + if (__cap_gained(permitted, new, old)) bprm->per_clear |= PER_CLEAR_ON_SETID; - /* Don't let someone trace a set[ug]id/setpcap binary with the revised * credentials unless they have the appropriate permit. * @@ -780,8 +785,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) */ is_setid = !uid_eq(new->euid, old->uid) || !gid_eq(new->egid, old->gid); - if ((is_setid || - !cap_issubset(new->cap_permitted, old->cap_permitted)) && + if ((is_setid || __cap_gained(permitted, new, old)) && ((bprm->unsafe & ~LSM_UNSAFE_PTRACE) || !ptracer_capable(current, new->user_ns))) { /* downgrade; they get no more than they had, and maybe less */ @@ -831,8 +835,8 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) * Number 1 above might fail if you don't have a full bset, but I think * that is interesting information to audit. */ - if (!cap_issubset(new->cap_effective, new->cap_ambient)) { - if (!cap_issubset(CAP_FULL_SET, new->cap_effective) || + if (__cap_grew(effective, ambient, new)) { + if (!__cap_full(effective, new) || !uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) || issecure(SECURE_NOROOT)) { ret = audit_log_bprm_fcaps(bprm, new, old); @@ -852,7 +856,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) bprm->cap_elevated = 1; } else if (!uid_eq(new->uid, root_uid)) { if (effective || - !cap_issubset(new->cap_permitted, new->cap_ambient)) + __cap_grew(permitted, ambient, new)) bprm->cap_elevated = 1; } -- cgit v1.2.3 From fc7eadf768a3e2c062e52eea89b52a0076d53b0c Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:07 -0400 Subject: capabilities: rename has_cap to has_fcap Rename has_cap to has_fcap to clarify it applies to file capabilities since the entire source file is about capabilities. Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Okay-ished-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 4c9af6ef24b6..13661d34f842 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -536,7 +536,7 @@ int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t size) static inline int bprm_caps_from_vfs_caps(struct cpu_vfs_cap_data *caps, struct linux_binprm *bprm, bool *effective, - bool *has_cap) + bool *has_fcap) { struct cred *new = bprm->cred; unsigned i; @@ -546,7 +546,7 @@ static inline int bprm_caps_from_vfs_caps(struct cpu_vfs_cap_data *caps, *effective = true; if (caps->magic_etc & VFS_CAP_REVISION_MASK) - *has_cap = true; + *has_fcap = true; CAP_FOR_EACH_U32(i) { __u32 permitted = caps->permitted.cap[i]; @@ -652,7 +652,7 @@ int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data * its xattrs and, if present, apply them to the proposed credentials being * constructed by execve(). */ -static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_cap) +static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_fcap) { int rc = 0; struct cpu_vfs_cap_data vcaps; @@ -683,7 +683,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_c goto out; } - rc = bprm_caps_from_vfs_caps(&vcaps, bprm, effective, has_cap); + rc = bprm_caps_from_vfs_caps(&vcaps, bprm, effective, has_fcap); if (rc == -EINVAL) printk(KERN_NOTICE "%s: cap_from_disk returned %d for %s\n", __func__, rc, bprm->filename); @@ -707,7 +707,7 @@ out: * set UID root and nothing is changed. If we are root, cap_permitted is * updated. If we have become set UID root, the effective bit is set. */ -static void handle_privileged_root(struct linux_binprm *bprm, bool has_cap, +static void handle_privileged_root(struct linux_binprm *bprm, bool has_fcap, bool *effective, kuid_t root_uid) { const struct cred *old = current_cred(); @@ -720,7 +720,7 @@ static void handle_privileged_root(struct linux_binprm *bprm, bool has_cap, * for a setuid root binary run by a non-root user. Do set it * for a root user just to cause least surprise to an admin. */ - if (has_cap && !uid_eq(new->uid, root_uid) && uid_eq(new->euid, root_uid)) { + if (has_fcap && !uid_eq(new->uid, root_uid) && uid_eq(new->euid, root_uid)) { warn_setuid_and_fcaps_mixed(bprm->filename); return; } @@ -759,20 +759,20 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) { const struct cred *old = current_cred(); struct cred *new = bprm->cred; - bool effective = false, has_cap = false, is_setid; + bool effective = false, has_fcap = false, is_setid; int ret; kuid_t root_uid; if (WARN_ON(!cap_ambient_invariant_ok(old))) return -EPERM; - ret = get_file_caps(bprm, &effective, &has_cap); + ret = get_file_caps(bprm, &effective, &has_fcap); if (ret < 0) return ret; root_uid = make_kuid(new->user_ns, 0); - handle_privileged_root(bprm, has_cap, &effective, root_uid); + handle_privileged_root(bprm, has_fcap, &effective, root_uid); /* if we have fs caps, clear dangerous personality flags */ if (__cap_gained(permitted, new, old)) @@ -802,7 +802,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) new->sgid = new->fsgid = new->egid; /* File caps or setid cancels ambient. */ - if (has_cap || is_setid) + if (has_fcap || is_setid) cap_clear(new->cap_ambient); /* -- cgit v1.2.3 From 9304b46c912d65a103a68f093b456ba3c02dca3b Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:08 -0400 Subject: capabilities: use root_priveleged inline to clarify logic Introduce inline root_privileged() to make use of SECURE_NONROOT easier to read. Suggested-by: Serge Hallyn Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Okay-ished-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 13661d34f842..9b8a6e79d858 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -695,6 +695,8 @@ out: return rc; } +static inline bool root_privileged(void) { return !issecure(SECURE_NOROOT); } + /* * handle_privileged_root - Handle case of privileged root * @bprm: The execution parameters, including the proposed creds @@ -713,7 +715,7 @@ static void handle_privileged_root(struct linux_binprm *bprm, bool has_fcap, const struct cred *old = current_cred(); struct cred *new = bprm->cred; - if (issecure(SECURE_NOROOT)) + if (!root_privileged()) return; /* * If the legacy file capability is set, then don't set privs @@ -838,7 +840,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) if (__cap_grew(effective, ambient, new)) { if (!__cap_full(effective, new) || !uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) || - issecure(SECURE_NOROOT)) { + !root_privileged()) { ret = audit_log_bprm_fcaps(bprm, new, old); if (ret < 0) return ret; -- cgit v1.2.3 From 81a6a012996b3fd47608d87b16e79412dd73578e Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:09 -0400 Subject: capabilities: use intuitive names for id changes Introduce a number of inlines to make the use of the negation of uid_eq() easier to read and analyse. Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Okay-ished-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 9b8a6e79d858..421f7438d3c8 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -697,6 +697,15 @@ out: static inline bool root_privileged(void) { return !issecure(SECURE_NOROOT); } +static inline bool __is_real(kuid_t uid, struct cred *cred) +{ return uid_eq(cred->uid, uid); } + +static inline bool __is_eff(kuid_t uid, struct cred *cred) +{ return uid_eq(cred->euid, uid); } + +static inline bool __is_suid(kuid_t uid, struct cred *cred) +{ return !__is_real(uid, cred) && __is_eff(uid, cred); } + /* * handle_privileged_root - Handle case of privileged root * @bprm: The execution parameters, including the proposed creds @@ -722,7 +731,7 @@ static void handle_privileged_root(struct linux_binprm *bprm, bool has_fcap, * for a setuid root binary run by a non-root user. Do set it * for a root user just to cause least surprise to an admin. */ - if (has_fcap && !uid_eq(new->uid, root_uid) && uid_eq(new->euid, root_uid)) { + if (has_fcap && __is_suid(root_uid, new)) { warn_setuid_and_fcaps_mixed(bprm->filename); return; } @@ -731,7 +740,7 @@ static void handle_privileged_root(struct linux_binprm *bprm, bool has_fcap, * executables under compatibility mode, we override the * capability sets for the file. */ - if (uid_eq(new->euid, root_uid) || uid_eq(new->uid, root_uid)) { + if (__is_eff(root_uid, new) || __is_real(root_uid, new)) { /* pP' = (cap_bset & ~0) | (pI & ~0) */ new->cap_permitted = cap_combine(old->cap_bset, old->cap_inheritable); @@ -739,7 +748,7 @@ static void handle_privileged_root(struct linux_binprm *bprm, bool has_fcap, /* * If only the real uid is 0, we do not set the effective bit. */ - if (uid_eq(new->euid, root_uid)) + if (__is_eff(root_uid, new)) *effective = true; } @@ -749,6 +758,13 @@ static void handle_privileged_root(struct linux_binprm *bprm, bool has_fcap, !cap_issubset(cred->cap_##target, cred->cap_##source) #define __cap_full(field, cred) \ cap_issubset(CAP_FULL_SET, cred->cap_##field) + +static inline bool __is_setuid(struct cred *new, const struct cred *old) +{ return !uid_eq(new->euid, old->uid); } + +static inline bool __is_setgid(struct cred *new, const struct cred *old) +{ return !gid_eq(new->egid, old->gid); } + /** * cap_bprm_set_creds - Set up the proposed credentials for execve(). * @bprm: The execution parameters, including the proposed creds @@ -785,7 +801,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) * * In addition, if NO_NEW_PRIVS, then ensure we get no new privs. */ - is_setid = !uid_eq(new->euid, old->uid) || !gid_eq(new->egid, old->gid); + is_setid = __is_setuid(new, old) || __is_setgid(new, old); if ((is_setid || __cap_gained(permitted, new, old)) && ((bprm->unsafe & ~LSM_UNSAFE_PTRACE) || @@ -839,7 +855,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) */ if (__cap_grew(effective, ambient, new)) { if (!__cap_full(effective, new) || - !uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) || + !__is_eff(root_uid, new) || !__is_real(root_uid, new) || !root_privileged()) { ret = audit_log_bprm_fcaps(bprm, new, old); if (ret < 0) @@ -856,7 +872,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) bprm->cap_elevated = 0; if (is_setid) { bprm->cap_elevated = 1; - } else if (!uid_eq(new->uid, root_uid)) { + } else if (!__is_real(root_uid, new)) { if (effective || __cap_grew(permitted, ambient, new)) bprm->cap_elevated = 1; -- cgit v1.2.3 From 9fbc2c79644a88a1cc40a2628ccff1bbbbc9ecc5 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:10 -0400 Subject: capabilities: move audit log decision to function Move the audit log decision logic to its own function to isolate the complexity in one place. Suggested-by: Serge Hallyn Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Okay-ished-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 50 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 20 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 421f7438d3c8..d7f0cbdf04c4 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -765,6 +765,32 @@ static inline bool __is_setuid(struct cred *new, const struct cred *old) static inline bool __is_setgid(struct cred *new, const struct cred *old) { return !gid_eq(new->egid, old->gid); } +/* + * Audit candidate if current->cap_effective is set + * + * We do not bother to audit if 3 things are true: + * 1) cap_effective has all caps + * 2) we are root + * 3) root is supposed to have all caps (SECURE_NOROOT) + * Since this is just a normal root execing a process. + * + * Number 1 above might fail if you don't have a full bset, but I think + * that is interesting information to audit. + */ +static inline bool nonroot_raised_pE(struct cred *cred, kuid_t root) +{ + bool ret = false; + + if (__cap_grew(effective, ambient, cred)) { + if (!__cap_full(effective, cred) || + !__is_eff(root, cred) || !__is_real(root, cred) || + !root_privileged()) { + ret = true; + } + } + return ret; +} + /** * cap_bprm_set_creds - Set up the proposed credentials for execve(). * @bprm: The execution parameters, including the proposed creds @@ -841,26 +867,10 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) if (WARN_ON(!cap_ambient_invariant_ok(new))) return -EPERM; - /* - * Audit candidate if current->cap_effective is set - * - * We do not bother to audit if 3 things are true: - * 1) cap_effective has all caps - * 2) we are root - * 3) root is supposed to have all caps (SECURE_NOROOT) - * Since this is just a normal root execing a process. - * - * Number 1 above might fail if you don't have a full bset, but I think - * that is interesting information to audit. - */ - if (__cap_grew(effective, ambient, new)) { - if (!__cap_full(effective, new) || - !__is_eff(root_uid, new) || !__is_real(root_uid, new) || - !root_privileged()) { - ret = audit_log_bprm_fcaps(bprm, new, old); - if (ret < 0) - return ret; - } + if (nonroot_raised_pE(new, root_uid)) { + ret = audit_log_bprm_fcaps(bprm, new, old); + if (ret < 0) + return ret; } new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS); -- cgit v1.2.3 From 02ebbaf48cf211498a9bd2c6b65e7d1b0a901807 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:11 -0400 Subject: capabilities: remove a layer of conditional logic Remove a layer of conditional logic to make the use of conditions easier to read and analyse. Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Okay-ished-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index d7f0cbdf04c4..eac70e2b400b 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -781,13 +781,12 @@ static inline bool nonroot_raised_pE(struct cred *cred, kuid_t root) { bool ret = false; - if (__cap_grew(effective, ambient, cred)) { - if (!__cap_full(effective, cred) || - !__is_eff(root, cred) || !__is_real(root, cred) || - !root_privileged()) { - ret = true; - } - } + if (__cap_grew(effective, ambient, cred) && + (!__cap_full(effective, cred) || + !__is_eff(root, cred) || + !__is_real(root, cred) || + !root_privileged())) + ret = true; return ret; } @@ -880,13 +879,11 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) /* Check for privilege-elevated exec. */ bprm->cap_elevated = 0; - if (is_setid) { + if (is_setid || + (!__is_real(root_uid, new) && + (effective || + __cap_grew(permitted, ambient, new)))) bprm->cap_elevated = 1; - } else if (!__is_real(root_uid, new)) { - if (effective || - __cap_grew(permitted, ambient, new)) - bprm->cap_elevated = 1; - } return 0; } -- cgit v1.2.3 From c0d1adefe0a3775cc16374dc9ebdfd8504afa14b Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:12 -0400 Subject: capabilities: invert logic for clarity The way the logic was presented, it was awkward to read and verify. Invert the logic using DeMorgan's Law to be more easily able to read and understand. Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Okay-ished-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index eac70e2b400b..0bd94d36e635 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -782,10 +782,10 @@ static inline bool nonroot_raised_pE(struct cred *cred, kuid_t root) bool ret = false; if (__cap_grew(effective, ambient, cred) && - (!__cap_full(effective, cred) || - !__is_eff(root, cred) || - !__is_real(root, cred) || - !root_privileged())) + !(__cap_full(effective, cred) && + __is_eff(root, cred) && + __is_real(root, cred) && + root_privileged())) ret = true; return ret; } -- cgit v1.2.3 From 588fb2c7e294753d3090a1dc2e7c34e7e3ce5aff Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:13 -0400 Subject: capabilities: fix logic for effective root or real root Now that the logic is inverted, it is much easier to see that both real root and effective root conditions had to be met to avoid printing the BPRM_FCAPS record with audit syscalls. This meant that any setuid root applications would print a full BPRM_FCAPS record when it wasn't necessary, cluttering the event output, since the SYSCALL and PATH records indicated the presence of the setuid bit and effective root user id. Require only one of effective root or real root to avoid printing the unnecessary record. Ref: commit 3fc689e96c0c ("Add audit_log_bprm_fcaps/AUDIT_BPRM_FCAPS") See: https://github.com/linux-audit/audit-kernel/issues/16 Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Acked-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 0bd94d36e635..ad7536d76820 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -770,7 +770,7 @@ static inline bool __is_setgid(struct cred *new, const struct cred *old) * * We do not bother to audit if 3 things are true: * 1) cap_effective has all caps - * 2) we are root + * 2) we became root *OR* are were already root * 3) root is supposed to have all caps (SECURE_NOROOT) * Since this is just a normal root execing a process. * @@ -783,8 +783,7 @@ static inline bool nonroot_raised_pE(struct cred *cred, kuid_t root) if (__cap_grew(effective, ambient, cred) && !(__cap_full(effective, cred) && - __is_eff(root, cred) && - __is_real(root, cred) && + (__is_eff(root, cred) || __is_real(root, cred)) && root_privileged())) ret = true; return ret; -- cgit v1.2.3 From dbbbe1105ea6aa0c49d78a4ea0d924e0c02307eb Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 11 Oct 2017 20:57:14 -0400 Subject: capabilities: audit log other surprising conditions The existing condition tested for process effective capabilities set by file attributes but intended to ignore the change if the result was unsurprisingly an effective full set in the case root is special with a setuid root executable file and we are root. Stated again: - When you execute a setuid root application, it is no surprise and expected that it got all capabilities, so we do not want capabilities recorded. if (pE_grew && !(pE_fullset && (eff_root || real_root) && root_priveleged) ) Now make sure we cover other cases: - If something prevented a setuid root app getting all capabilities and it wound up with one capability only, then it is a surprise and should be logged. When it is a setuid root file, we only want capabilities when the process does not get full capabilities.. root_priveleged && setuid_root && !pE_fullset - Similarly if a non-setuid program does pick up capabilities due to file system based capabilities, then we want to know what capabilities were picked up. When it has file system based capabilities we want the capabilities. !is_setuid && (has_fcap && pP_gained) - If it is a non-setuid file and it gets ambient capabilities, we want the capabilities. !is_setuid && pA_gained - These last two are combined into one due to the common first parameter. Related: https://github.com/linux-audit/audit-kernel/issues/16 Signed-off-by: Richard Guy Briggs Reviewed-by: Serge Hallyn Acked-by: James Morris Acked-by: Kees Cook Acked-by: Paul Moore Signed-off-by: James Morris --- security/commoncap.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index ad7536d76820..5fa839c7fb3f 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -766,7 +766,7 @@ static inline bool __is_setgid(struct cred *new, const struct cred *old) { return !gid_eq(new->egid, old->gid); } /* - * Audit candidate if current->cap_effective is set + * 1) Audit candidate if current->cap_effective is set * * We do not bother to audit if 3 things are true: * 1) cap_effective has all caps @@ -776,16 +776,31 @@ static inline bool __is_setgid(struct cred *new, const struct cred *old) * * Number 1 above might fail if you don't have a full bset, but I think * that is interesting information to audit. + * + * A number of other conditions require logging: + * 2) something prevented setuid root getting all caps + * 3) non-setuid root gets fcaps + * 4) non-setuid root gets ambient */ -static inline bool nonroot_raised_pE(struct cred *cred, kuid_t root) +static inline bool nonroot_raised_pE(struct cred *new, const struct cred *old, + kuid_t root, bool has_fcap) { bool ret = false; - if (__cap_grew(effective, ambient, cred) && - !(__cap_full(effective, cred) && - (__is_eff(root, cred) || __is_real(root, cred)) && - root_privileged())) + if ((__cap_grew(effective, ambient, new) && + !(__cap_full(effective, new) && + (__is_eff(root, new) || __is_real(root, new)) && + root_privileged())) || + (root_privileged() && + __is_suid(root, new) && + !__cap_full(effective, new)) || + (!__is_setuid(new, old) && + ((has_fcap && + __cap_gained(permitted, new, old)) || + __cap_gained(ambient, new, old)))) + ret = true; + return ret; } @@ -865,7 +880,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) if (WARN_ON(!cap_ambient_invariant_ok(new))) return -EPERM; - if (nonroot_raised_pE(new, root_uid)) { + if (nonroot_raised_pE(new, old, root_uid, has_fcap)) { ret = audit_log_bprm_fcaps(bprm, new, old); if (ret < 0) return ret; -- cgit v1.2.3 From 927340926ed61477e34f960eec64b7532e35d2f0 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Thu, 19 Oct 2017 14:29:04 +0200 Subject: tomoyo: fix timestamping for y2038 Tomoyo uses an open-coded version of time_to_tm() to create a timestamp from the current time as read by get_seconds(). This will overflow and give wrong results on 32-bit systems in 2038. To correct this, this changes the code to use ktime_get_real_seconds() and the generic time64_to_tm() function that are both y2038-safe. Using the library function avoids adding an expensive 64-bit division in this code and can benefit from any optimizations we do in common code. Acked-by: Tetsuo Handa Signed-off-by: Arnd Bergmann Signed-off-by: James Morris --- security/tomoyo/audit.c | 2 +- security/tomoyo/common.c | 4 ++-- security/tomoyo/common.h | 2 +- security/tomoyo/util.c | 39 +++++++++------------------------------ 4 files changed, 13 insertions(+), 34 deletions(-) (limited to 'security') diff --git a/security/tomoyo/audit.c b/security/tomoyo/audit.c index 3ffa4f5509d8..a51edfbe593b 100644 --- a/security/tomoyo/audit.c +++ b/security/tomoyo/audit.c @@ -156,7 +156,7 @@ static char *tomoyo_print_header(struct tomoyo_request_info *r) if (!buffer) return NULL; - tomoyo_convert_time(get_seconds(), &stamp); + tomoyo_convert_time(ktime_get_real_seconds(), &stamp); pos = snprintf(buffer, tomoyo_buffer_len - 1, "#%04u/%02u/%02u %02u:%02u:%02u# profile=%u mode=%s " diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c index e0fb75052550..c19970db89c4 100644 --- a/security/tomoyo/common.c +++ b/security/tomoyo/common.c @@ -2256,7 +2256,7 @@ static const char * const tomoyo_memory_headers[TOMOYO_MAX_MEMORY_STAT] = { /* Timestamp counter for last updated. */ static unsigned int tomoyo_stat_updated[TOMOYO_MAX_POLICY_STAT]; /* Counter for number of updates. */ -static unsigned int tomoyo_stat_modified[TOMOYO_MAX_POLICY_STAT]; +static time64_t tomoyo_stat_modified[TOMOYO_MAX_POLICY_STAT]; /** * tomoyo_update_stat - Update statistic counters. @@ -2271,7 +2271,7 @@ void tomoyo_update_stat(const u8 index) * I don't use atomic operations because race condition is not fatal. */ tomoyo_stat_updated[index]++; - tomoyo_stat_modified[index] = get_seconds(); + tomoyo_stat_modified[index] = ktime_get_real_seconds(); } /** diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h index 361e7a284699..d9628d1635b2 100644 --- a/security/tomoyo/common.h +++ b/security/tomoyo/common.h @@ -1036,7 +1036,7 @@ void tomoyo_check_acl(struct tomoyo_request_info *r, bool (*check_entry) (struct tomoyo_request_info *, const struct tomoyo_acl_info *)); void tomoyo_check_profile(void); -void tomoyo_convert_time(time_t time, struct tomoyo_time *stamp); +void tomoyo_convert_time(time64_t time, struct tomoyo_time *stamp); void tomoyo_del_condition(struct list_head *element); void tomoyo_fill_path_info(struct tomoyo_path_info *ptr); void tomoyo_get_attributes(struct tomoyo_obj_info *obj); diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c index 848317fea704..cac431d381d2 100644 --- a/security/tomoyo/util.c +++ b/security/tomoyo/util.c @@ -86,38 +86,17 @@ const u8 tomoyo_index2category[TOMOYO_MAX_MAC_INDEX] = { * @stamp: Pointer to "struct tomoyo_time". * * Returns nothing. - * - * This function does not handle Y2038 problem. */ -void tomoyo_convert_time(time_t time, struct tomoyo_time *stamp) +void tomoyo_convert_time(time64_t time64, struct tomoyo_time *stamp) { - static const u16 tomoyo_eom[2][12] = { - { 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 }, - { 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366 } - }; - u16 y; - u8 m; - bool r; - stamp->sec = time % 60; - time /= 60; - stamp->min = time % 60; - time /= 60; - stamp->hour = time % 24; - time /= 24; - for (y = 1970; ; y++) { - const unsigned short days = (y & 3) ? 365 : 366; - if (time < days) - break; - time -= days; - } - r = (y & 3) == 0; - for (m = 0; m < 11 && time >= tomoyo_eom[r][m]; m++) - ; - if (m) - time -= tomoyo_eom[r][m - 1]; - stamp->year = y; - stamp->month = ++m; - stamp->day = ++time; + struct tm tm; + time64_to_tm(time64, 0, &tm); + stamp->sec = tm.tm_sec; + stamp->min = tm.tm_min; + stamp->hour = tm.tm_hour; + stamp->day = tm.tm_mday; + stamp->month = tm.tm_mon + 1; + stamp->year = tm.tm_year + 1900; } /** -- cgit v1.2.3 From d6d80cb57be45fc1a7d08c30526ab81ae9e7bc3d Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Thu, 28 Sep 2017 14:54:50 -0700 Subject: Smack: Base support for overlayfs Supply the Smack module hooks in support of overlayfs. Ensure that the Smack label of new files gets the correct value when a directory is transmuting. Original implementation by Romanini Daniele, with a few tweaks added. Signed-off-by: Romanini Daniele Signed-off-by: Casey Schaufler Signed-off-by: James Morris --- security/smack/smack_lsm.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) (limited to 'security') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 319add31b4a4..569f28034116 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4605,6 +4605,82 @@ static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) return 0; } +static int smack_inode_copy_up(struct dentry *dentry, struct cred **new) +{ + + struct task_smack *tsp; + struct smack_known *skp; + struct inode_smack *isp; + struct cred *new_creds = *new; + + if (new_creds == NULL) { + new_creds = prepare_creds(); + if (new_creds == NULL) + return -ENOMEM; + } + + tsp = new_creds->security; + + /* + * Get label from overlay inode and set it in create_sid + */ + isp = d_inode(dentry->d_parent)->i_security; + skp = isp->smk_inode; + tsp->smk_task = skp; + *new = new_creds; + return 0; +} + +static int smack_inode_copy_up_xattr(const char *name) +{ + /* + * Return 1 if this is the smack access Smack attribute. + */ + if (strcmp(name, XATTR_NAME_SMACK) == 0) + return 1; + + return -EOPNOTSUPP; +} + +static int smack_dentry_create_files_as(struct dentry *dentry, int mode, + struct qstr *name, + const struct cred *old, + struct cred *new) +{ + struct task_smack *otsp = old->security; + struct task_smack *ntsp = new->security; + struct inode_smack *isp; + int may; + + /* + * Use the process credential unless all of + * the transmuting criteria are met + */ + ntsp->smk_task = otsp->smk_task; + + /* + * the attribute of the containing directory + */ + isp = d_inode(dentry->d_parent)->i_security; + + if (isp->smk_flags & SMK_INODE_TRANSMUTE) { + rcu_read_lock(); + may = smk_access_entry(otsp->smk_task->smk_known, + isp->smk_inode->smk_known, + &otsp->smk_task->smk_rules); + rcu_read_unlock(); + + /* + * If the directory is transmuting and the rule + * providing access is transmuting use the containing + * directory label instead of the process label. + */ + if (may > 0 && (may & MAY_TRANSMUTE)) + ntsp->smk_task = isp->smk_inode; + } + return 0; +} + static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check), LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme), @@ -4740,6 +4816,9 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx), LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx), LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx), + LSM_HOOK_INIT(inode_copy_up, smack_inode_copy_up), + LSM_HOOK_INIT(inode_copy_up_xattr, smack_inode_copy_up_xattr), + LSM_HOOK_INIT(dentry_create_files_as, smack_dentry_create_files_as), }; -- cgit v1.2.3