| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20080521203405.GA27901@sergelap.wowway.com>
Date: Wed, 21 May 2008 15:34:05 -0500
From: "Serge E. Hallyn" <serue@...ibm.com>
To: "Andrew G. Morgan" <morgan@...nel.org>
Cc: "Serge E. Hallyn" <serue@...ibm.com>,
Andrew Morton <akpm@...ux-foundation.org>,
lkml <linux-kernel@...r.kernel.org>,
Linux Security Modules List
<linux-security-module@...r.kernel.org>
Subject: Re: [PATCH] security: protect legacy apps from insufficient
privilege
Quoting Andrew G. Morgan (morgan@...nel.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> This is a fail-safe additional feature for filesystem capability support.
>
> Cheers
>
> Andrew
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.6 (GNU/Linux)
>
> iD8DBQFINES/+bHCR3gb8jsRAmPdAJsG8fKTNBXYmUb3CJDchLQ1MCYgDQCg2uQl
> ltlVX1O9D6BtDK/4+gsoq8U=
> =AUc0
> -----END PGP SIGNATURE-----
> From 916b252d3b631214acea6df6c61e94ce6770fdf7 Mon Sep 17 00:00:00 2001
> From: Andrew G. Morgan <morgan@...nel.org>
> Date: Thu, 15 May 2008 23:17:13 -0700
> Subject: [PATCH] Protect legacy applications from executing with insufficient privilege.
>
> When cap_bset suppresses some of the forced (fP) capabilities of a
> file, it is generally only safe to execute the program if it
> understands how to recognize it doesn't have enough privilege to work
> correctly. For legacy applications (fE!=0), which have no
> non-destructive way to determine that they are missing privilege, we
> fail to execute (EPERM) any executable that requires fP capabilities,
> but would otherwise get pP' < fP. This is a fail-safe permission check.
>
> For some discussion of why it is problematic for (legacy) privileged
> applications to run with less than the set of capabilities requested
> for them, see:
>
> http://userweb.kernel.org/~morgan/sendmail-capabilities-war-story.html
>
> With this iteration of this support, we do not include setuid-0 based
> privilege protection from the bounding set. That is, the admin can still
> (ab)use the bounding set to suppress the privileges of a setuid-0
> program.
>
> Signed-off-by: Andrew G. Morgan <morgan@...nel.org>
Assuming (as it appears) the only change from last time is that you
dropped the part changing cap_bprm_apply_creds() contraints for a
ptraced process, then
Acked-by: Serge Hallyn <serue@...ibm.com>
I'll try to give it a good test-run next week.
thanks,
-serge
> ---
> include/linux/binfmts.h | 2 +-
> security/commoncap.c | 109 ++++++++++++++++++++++++++---------------------
> 2 files changed, 62 insertions(+), 49 deletions(-)
>
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index ee0ed48..826f623 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -38,7 +38,7 @@ struct linux_binprm{
> misc_bang:1;
> struct file * file;
> int e_uid, e_gid;
> - kernel_cap_t cap_inheritable, cap_permitted;
> + kernel_cap_t cap_post_exec_permitted;
> bool cap_effective;
> void *security;
> int argc, envc;
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 5edabc7..84f1ab5 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -155,8 +155,7 @@ void cap_capset_set (struct task_struct *target, kernel_cap_t *effective,
>
> static inline void bprm_clear_caps(struct linux_binprm *bprm)
> {
> - cap_clear(bprm->cap_inheritable);
> - cap_clear(bprm->cap_permitted);
> + cap_clear(bprm->cap_post_exec_permitted);
> bprm->cap_effective = false;
> }
>
> @@ -191,6 +190,7 @@ static inline int cap_from_disk(struct vfs_cap_data *caps,
> {
> __u32 magic_etc;
> unsigned tocopy, i;
> + int ret;
>
> if (size < sizeof(magic_etc))
> return -EINVAL;
> @@ -218,19 +218,42 @@ static inline int cap_from_disk(struct vfs_cap_data *caps,
> bprm->cap_effective = false;
> }
>
> - for (i = 0; i < tocopy; ++i) {
> - bprm->cap_permitted.cap[i] =
> - le32_to_cpu(caps->data[i].permitted);
> - bprm->cap_inheritable.cap[i] =
> - le32_to_cpu(caps->data[i].inheritable);
> - }
> - while (i < VFS_CAP_U32) {
> - bprm->cap_permitted.cap[i] = 0;
> - bprm->cap_inheritable.cap[i] = 0;
> - i++;
> + ret = 0;
> +
> + CAP_FOR_EACH_U32(i) {
> + if (i >= tocopy) {
> + /*
> + * Legacy capability sets have no upper bits
> + */
> + bprm->cap_post_exec_permitted.cap[i] = 0;
> + } else {
> + __u32 value_cpu;
> + /*
> + * pP' = (X & fP) | (pI & fI)
> + */
> + value_cpu = le32_to_cpu(caps->data[i].permitted);
> + bprm->cap_post_exec_permitted.cap[i] = (
> + current->cap_bset.cap[i] & value_cpu
> + ) | (
> + current->cap_inheritable.cap[i] &
> + le32_to_cpu(caps->data[i].inheritable)
> + );
> + if (value_cpu &
> + ~bprm->cap_post_exec_permitted.cap[i]) {
> + /*
> + * insufficient to execute correctly
> + */
> + ret = -EPERM;
> + }
> + }
> }
>
> - return 0;
> + /*
> + * For legacy apps, with no internal support for recognizing they
> + * do not have enough capabilities, we return an error if they are
> + * missing some "forced" (aka file-permitted) capabilities.
> + */
> + return (bprm->cap_effective ? ret : 0);
> }
>
> /* Locate any VFS capabilities: */
> @@ -262,9 +285,9 @@ static int get_file_caps(struct linux_binprm *bprm)
> goto out;
>
> rc = cap_from_disk(&vcaps, bprm, rc);
> - if (rc)
> + if (rc == -EINVAL)
> printk(KERN_NOTICE "%s: cap_from_disk returned %d for %s\n",
> - __func__, rc, bprm->filename);
> + __func__, rc, bprm->filename);
>
> out:
> dput(dentry);
> @@ -297,25 +320,24 @@ int cap_bprm_set_security (struct linux_binprm *bprm)
> int ret;
>
> ret = get_file_caps(bprm);
> - if (ret)
> - printk(KERN_NOTICE "%s: get_file_caps returned %d for %s\n",
> - __func__, ret, bprm->filename);
> -
> - /* To support inheritance of root-permissions and suid-root
> - * executables under compatibility mode, we raise all three
> - * capability sets for the file.
> - *
> - * If only the real uid is 0, we only raise the inheritable
> - * and permitted sets of the executable file.
> - */
>
> - if (!issecure (SECURE_NOROOT)) {
> + if (!issecure(SECURE_NOROOT)) {
> + /*
> + * 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 (bprm->e_uid == 0 || current->uid == 0) {
> - cap_set_full (bprm->cap_inheritable);
> - cap_set_full (bprm->cap_permitted);
> + /* pP' = (cap_bset & ~0) | (pI & ~0) */
> + bprm->cap_post_exec_permitted = cap_combine(
> + current->cap_bset, current->cap_inheritable
> + );
> + bprm->cap_effective = (bprm->e_uid == 0);
> + ret = 0;
> }
> - if (bprm->e_uid == 0)
> - bprm->cap_effective = true;
> }
>
> return ret;
> @@ -323,17 +345,9 @@ int cap_bprm_set_security (struct linux_binprm *bprm)
>
> void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
> {
> - /* Derived from fs/exec.c:compute_creds. */
> - kernel_cap_t new_permitted, working;
> -
> - new_permitted = cap_intersect(bprm->cap_permitted,
> - current->cap_bset);
> - working = cap_intersect(bprm->cap_inheritable,
> - current->cap_inheritable);
> - new_permitted = cap_combine(new_permitted, working);
> -
> if (bprm->e_uid != current->uid || bprm->e_gid != current->gid ||
> - !cap_issubset (new_permitted, current->cap_permitted)) {
> + !cap_issubset(bprm->cap_post_exec_permitted,
> + current->cap_permitted)) {
> set_dumpable(current->mm, suid_dumpable);
> current->pdeath_signal = 0;
>
> @@ -343,8 +357,9 @@ void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
> bprm->e_gid = current->gid;
> }
> if (!capable (CAP_SETPCAP)) {
> - new_permitted = cap_intersect (new_permitted,
> - current->cap_permitted);
> + bprm->cap_post_exec_permitted = cap_intersect(
> + bprm->cap_post_exec_permitted,
> + current->cap_permitted);
> }
> }
> }
> @@ -356,9 +371,9 @@ void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
> * in the init_task struct. Thus we skip the usual
> * capability rules */
> if (!is_global_init(current)) {
> - current->cap_permitted = new_permitted;
> + current->cap_permitted = bprm->cap_post_exec_permitted;
> if (bprm->cap_effective)
> - current->cap_effective = new_permitted;
> + current->cap_effective = bprm->cap_post_exec_permitted;
> else
> cap_clear(current->cap_effective);
> }
> @@ -373,9 +388,7 @@ int cap_bprm_secureexec (struct linux_binprm *bprm)
> if (current->uid != 0) {
> if (bprm->cap_effective)
> return 1;
> - if (!cap_isclear(bprm->cap_permitted))
> - return 1;
> - if (!cap_isclear(bprm->cap_inheritable))
> + if (!cap_isclear(bprm->cap_post_exec_permitted))
> return 1;
> }
>
> --
> 1.5.3.7
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists