security: protect legacy apps from insufficient privilege
Subsystems (3)
PATCH 1/1 security: protect legacy apps from insufficient privilege
-----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 <[email protected]>
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.
--- include/linux/binfmts.h | 2 +- security/commoncap.c | 109 ++++++++++++++++++++++++++--------------------- 2 files changed, 62 insertions(+), 49 deletions(-)
include/linux/binfmts.h +1 -1
@@ -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;
security/commoncap.c +61 -48
171 lines not on this page · raw message
Signature · 1 line
1.5.3.7
Quoted 33 lines · Andrew G. Morgan
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1This 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
916b252d3b631214acea6df6c61e94ce6770fdf7Mon Sep 17 00:00:00 2001
From: Andrew G. Morgan <[email protected]>
Date: Thu, 15 May 2008 23:17:13 -0700
Subject: [PATCH] Protect legacy applications from executing with insufficient privilege.When
cap_bsetsuppresses 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:
Quoted 8 lines
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 <[email protected]>
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
I'll try to give it a good test-run next week.
thanks,
-serge
Quoted 205 lines
--- include/linux/binfmts.h | 2 +- security/commoncap.c | 109 ++++++++++++++++++++++++++--------------------- 2 files changed, 62 insertions(+), 49 deletions(-)include/linux/binfmts.h +1
-1@@ -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;security/commoncap.c +61
-48@@ -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 ; }Signature · 1 line
1.5.3.7
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Serge E. Hallyn wrote:
| Quoting Andrew G. Morgan ([email protected]): | This is a fail-safe additional feature for filesystem capability support. | | Cheers | | Andrew
~From 916b252d3b631214acea6df6c61e94ce6770fdf7 Mon Sep 17 00:00:00 2001
From: Andrew G. Morgan <[email protected]>
Date: Thu, 15 May 2008 23:17:13 -0700
Subject: [PATCH] Protect legacy applications from executing with insufficient privilege.
[..]
| 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
Yes. That's the only material change. I also added a comment explaining the "strange" (since it caused some concern last time around) cap_bset | cap_inheritable bit...
| Acked-by: Serge Hallyn <[email protected]>
| I'll try to give it a good test-run next week.
Thanks
Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)
iD8DBQFINQIr+bHCR3gb8jsRAjUtAJ0dX67kT3AAtR7gUZgCPiXS2t+nzQCgghmv GSgqeiwwbZXcc6tSh9957Fo=
=dmv/
-----END PGP SIGNATURE-----
Quoted 17 lines · On Wed, 21 May 2008 08:50:25 -0700 "Andrew G. Morgan" <[email protected]> wrote:
security/commoncap.c +17
-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)
security/commoncap.c static inline int cap_from_disk(struct vfs_cap_data *caps, · line 249
Quoted 10 lines
security/commoncap.c +9
-0+ ); +if (value_cpu & + ~bprm->cap_post_exec_permitted.cap[i]) { +/* +* insufficient to execute correctly +*/ + ret = -EPERM ; + } + } }
That makes my eyes say ow. The 80-col thing is a pain.
With a judiciously placed `continue' we can do this:
CAP_FOR_EACH_U32(i) {
__u32 value_cpu;
if (i >= tocopy) {
/*
* Legacy capability sets have no upper bits
*/
bprm->cap_post_exec_permitted.cap[i] = 0;
continue;
}
/*
* 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;
}
}OK?
Andrew G. Morgan
Acked-by-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Andrew Morton wrote:
| With a judiciously placed `continue' we can do this:
|
| CAP_FOR_EACH_U32(i) {
| __u32 value_cpu;
|
| if (i >= tocopy) {
| /*
| * Legacy capability sets have no upper bits
| */
| bprm->cap_post_exec_permitted.cap[i] = 0;
| continue;
| }
| /*
| * 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;
| }
| }
|
| OK?Yes.
Cheers
Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)
iD8DBQFINXLL+bHCR3gb8jsRAiGUAJ49d61n7+uhc3M5vJjT5398w9tRGgCgwLRe vXHOIIJAoHvlGNACagKSPes=
=oPN0
-----END PGP SIGNATURE-----