diff options
author | Eric W. Biederman <ebiederm@xmission.com> | 2020-05-29 22:00:54 -0500 |
---|---|---|
committer | Eric W. Biederman <ebiederm@xmission.com> | 2020-05-29 22:00:54 -0500 |
commit | 56305aa9b6fab91a5555a45796b79c1b0a6353d1 (patch) | |
tree | 378ea4452668d448b0834fd08008a5f81619f1fd /security | |
parent | a7868323c2638a7c6c5b30b37831b73cbdf0dc15 (diff) | |
download | lwn-56305aa9b6fab91a5555a45796b79c1b0a6353d1.tar.gz lwn-56305aa9b6fab91a5555a45796b79c1b0a6353d1.zip |
exec: Compute file based creds only once
Move the computation of creds from prepare_binfmt into begin_new_exec
so that the creds need only be computed once. This is just code
reorganization no semantic changes of any kind are made.
Moving the computation is safe. I have looked through the kernel and
verified none of the binfmts look at bprm->cred directly, and that
there are no helpers that look at bprm->cred indirectly. Which means
that it is not a problem to compute the bprm->cred later in the
execution flow as it is not used until it becomes current->cred.
A new function bprm_creds_from_file is added to contain the work that
needs to be done. bprm_creds_from_file first computes which file
bprm->executable or most likely bprm->file that the bprm->creds
will be computed from.
The funciton bprm_fill_uid is updated to receive the file instead of
accessing bprm->file. The now unnecessary work needed to reset the
bprm->cred->euid, and bprm->cred->egid is removed from brpm_fill_uid.
A small comment to document that bprm_fill_uid now only deals with the
work to handle suid and sgid files. The default case is already
heandled by prepare_exec_creds.
The function security_bprm_repopulate_creds is renamed
security_bprm_creds_from_file and now is explicitly passed the file
from which to compute the creds. The documentation of the
bprm_creds_from_file security hook is updated to explain when the hook
is called and what it needs to do. The file is passed from
cap_bprm_creds_from_file into get_file_caps so that the caps are
computed for the appropriate file. The now unnecessary work in
cap_bprm_creds_from_file to reset the ambient capabilites has been
removed. A small comment to document that the work of
cap_bprm_creds_from_file is to read capabilities from the files
secureity attribute and derive capabilities from the fact the
user had uid 0 has been added.
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Diffstat (limited to 'security')
-rw-r--r-- | security/commoncap.c | 24 | ||||
-rw-r--r-- | security/security.c | 4 |
2 files changed, 15 insertions, 13 deletions
diff --git a/security/commoncap.c b/security/commoncap.c index 6de72d22dc6c..59bf3c1674c8 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -647,7 +647,8 @@ 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_fcap) +static int get_file_caps(struct linux_binprm *bprm, struct file *file, + bool *effective, bool *has_fcap) { int rc = 0; struct cpu_vfs_cap_data vcaps; @@ -657,7 +658,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_f if (!file_caps_enabled) return 0; - if (!mnt_may_suid(bprm->file->f_path.mnt)) + if (!mnt_may_suid(file->f_path.mnt)) return 0; /* @@ -665,10 +666,10 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_f * explicit that capability bits are limited to s_user_ns and its * descendants. */ - if (!current_in_userns(bprm->file->f_path.mnt->mnt_sb->s_user_ns)) + if (!current_in_userns(file->f_path.mnt->mnt_sb->s_user_ns)) return 0; - rc = get_vfs_caps_from_disk(bprm->file->f_path.dentry, &vcaps); + rc = get_vfs_caps_from_disk(file->f_path.dentry, &vcaps); if (rc < 0) { if (rc == -EINVAL) printk(KERN_NOTICE "Invalid argument reading file caps for %s\n", @@ -797,26 +798,27 @@ static inline bool nonroot_raised_pE(struct cred *new, const struct cred *old, } /** - * cap_bprm_repopulate_creds - Set up the proposed credentials for execve(). + * cap_bprm_creds_from_file - Set up the proposed credentials for execve(). * @bprm: The execution parameters, including the proposed creds + * @file: The file to pull the credentials from * * Set up the proposed credentials for a new execution context being * constructed by execve(). The proposed creds in @bprm->cred is altered, * which won't take effect immediately. Returns 0 if successful, -ve on error. */ -int cap_bprm_repopulate_creds(struct linux_binprm *bprm) +int cap_bprm_creds_from_file(struct linux_binprm *bprm, struct file *file) { + /* Process setpcap binaries and capabilities for uid 0 */ const struct cred *old = current_cred(); struct cred *new = bprm->cred; bool effective = false, has_fcap = false, is_setid; int ret; kuid_t root_uid; - new->cap_ambient = old->cap_ambient; if (WARN_ON(!cap_ambient_invariant_ok(old))) return -EPERM; - ret = get_file_caps(bprm, &effective, &has_fcap); + ret = get_file_caps(bprm, file, &effective, &has_fcap); if (ret < 0) return ret; @@ -826,7 +828,7 @@ int cap_bprm_repopulate_creds(struct linux_binprm *bprm) /* if we have fs caps, clear dangerous personality flags */ if (__cap_gained(permitted, new, old)) - bprm->pf_per_clear |= PER_CLEAR_ON_SETID; + 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. @@ -889,7 +891,7 @@ int cap_bprm_repopulate_creds(struct linux_binprm *bprm) (!__is_real(root_uid, new) && (effective || __cap_grew(permitted, ambient, new)))) - bprm->active_secureexec = 1; + bprm->secureexec = 1; return 0; } @@ -1346,7 +1348,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_traceme, cap_ptrace_traceme), LSM_HOOK_INIT(capget, cap_capget), LSM_HOOK_INIT(capset, cap_capset), - LSM_HOOK_INIT(bprm_repopulate_creds, cap_bprm_repopulate_creds), + LSM_HOOK_INIT(bprm_creds_from_file, cap_bprm_creds_from_file), LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv), LSM_HOOK_INIT(inode_killpriv, cap_inode_killpriv), LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity), diff --git a/security/security.c b/security/security.c index b890b7e2a765..259b8e750aa2 100644 --- a/security/security.c +++ b/security/security.c @@ -828,9 +828,9 @@ int security_bprm_creds_for_exec(struct linux_binprm *bprm) return call_int_hook(bprm_creds_for_exec, 0, bprm); } -int security_bprm_repopulate_creds(struct linux_binprm *bprm) +int security_bprm_creds_from_file(struct linux_binprm *bprm, struct file *file) { - return call_int_hook(bprm_repopulate_creds, 0, bprm); + return call_int_hook(bprm_creds_from_file, 0, bprm, file); } int security_bprm_check(struct linux_binprm *bprm) |