Prev: BFS v0.313 CPU scheduler for 2.6.32
Next: ACPI warning from alloc_pages_nodemask on boot (2.6.33 regression)
From: Eric W. Biederman on 30 Dec 2009 15:50 "Serge E. Hallyn" <serue(a)us.ibm.com> writes: >> @@ -869,6 +869,18 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, >> new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS); >> goto changed; >> >> + case PR_SET_NOSUID: >> + { >> + const struct cred *cred = current->cred; >> + error = -EINVAL; > > Should this be -EPERM? not sure... I intended -EINVAL to say it is simply a set of initial conditions that are not supported today. But could be supported if someone does the audit, and found there are no security issues. >> + /* Perform the capabilities checks */ >> + if (!cap_isclear(cred->cap_permitted) || >> + !cap_isclear(cred->cap_effective)) > > No need to check cap_effective, as no bits can be there which are not > in cap_permitted. > > To be honest, I don't think there is much reason to not have this > check done in the main sys_prctl(0 - capabilities themselves are not > optional in the kernel, while cap_task_prctl() is. So you are setting > us up to have cases where say an apparmor user can call this with uid > 0 and/or active capabilities. Sounds fine to me. I had noticed all of the capabilities checks were off in their own file, so I had tried to maintain that. But you are right we can't remove capabilities so splitting the code like this only obfuscates it. >> @@ -2147,7 +2147,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) >> COMMON_AUDIT_DATA_INIT(&ad, FS); >> ad.u.fs.path = bprm->file->f_path; >> >> - if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID) >> + if (bprm->nosid) > > typo - nosuid? Yep. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 30 Dec 2009 16:30 > Added bprm->nosuid to make remove the need to add > duplicate error prone checks. This ensures that > the disabling of suid executables is exactly the > same as MNT_NOSUID. Another fine example of why we have security hooks so that we don't get a kernel full of other "random security idea of the day" hacks. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Eric W. Biederman on 30 Dec 2009 16:40 Alan Cox <alan(a)lxorguk.ukuu.org.uk> writes: >> Added bprm->nosuid to make remove the need to add >> duplicate error prone checks. This ensures that >> the disabling of suid executables is exactly the >> same as MNT_NOSUID. > > Another fine example of why we have security hooks so that we don't get a > kernel full of other "random security idea of the day" hacks. Well it comes from plan 9. Except there they just simply did not implement suid. What causes you to think dropping the ability to execute suid executables is a random security idea of the day? Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 30 Dec 2009 18:10 On Wed, 30 Dec 2009 13:36:57 -0800 ebiederm(a)xmission.com (Eric W. Biederman) wrote: > Alan Cox <alan(a)lxorguk.ukuu.org.uk> writes: > > >> Added bprm->nosuid to make remove the need to add > >> duplicate error prone checks. This ensures that > >> the disabling of suid executables is exactly the > >> same as MNT_NOSUID. > > > > Another fine example of why we have security hooks so that we don't get a > > kernel full of other "random security idea of the day" hacks. > > Well it comes from plan 9. Except there they just simply did not > implement suid. What causes you to think dropping the ability > to execute suid executables is a random security idea of the day? Well to be fair its random regurgitated security idea of every year or two. More to the point - we have security_* hooks so this kind of continuous security proposal turdstream can stay out of the main part of the kernel. Cleaning up the mechanism by which NOSUID is handled in kernel seems a good idea. Adding wacky new prctls and gunk for it doesn't, and belongs in whatever security model you are using via the security hooks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Bryan Donlan on 30 Dec 2009 21:50
On Wed, Dec 30, 2009 at 6:00 PM, Alan Cox <alan(a)lxorguk.ukuu.org.uk> wrote: > On Wed, 30 Dec 2009 13:36:57 -0800 > ebiederm(a)xmission.com (Eric W. Biederman) wrote: > >> Alan Cox <alan(a)lxorguk.ukuu.org.uk> writes: >> >> >> Added bprm->nosuid to make remove the need to add >> >> duplicate error prone checks. �This ensures that >> >> the disabling of suid executables is exactly the >> >> same as MNT_NOSUID. >> > >> > Another fine example of why we have security hooks so that we don't get a >> > kernel full of other "random security idea of the day" hacks. >> >> Well it comes from plan 9. �Except there they just simply did not >> implement suid. �What causes you to think dropping the ability >> to execute suid executables is a random security idea of the day? > > Well to be fair its random regurgitated security idea of every year or > two. > > More to the point - we have security_* hooks so this kind of continuous > security proposal turdstream can stay out of the main part of the kernel. > > Cleaning up the mechanism by which NOSUID is handled in kernel seems a > good idea. Adding wacky new prctls and gunk for it doesn't, and belongs > in whatever security model you are using via the security hooks. I see this as being a security-model agnostic API - the reason being, the application is specifying a policy for itself that has meaning in all existing security models, and which does not require administrator intervention to configure. Rather than reimplementing this for each security model, it's far better to do it just once. Moreover, by having a single, common API, the application can state the general policy "I will never need to gain priviliges over exec" without needing to know what LSM is in use. The future goal of this API is to allow us to relax restrictions on creating new namespaces, chrooting, and otherwise altering the task's environment in ways that may confuse privileged applications. Since security hooks are all about making the existing security restrictions _stricter_, it's not easy to later relax these using the security hook model. And once we put in the general requirement that "this task shall never gain privilege", it should be safe to relax these restrictions for _all_ security models. In short, this is something which is meaningful for all existing LSMs and should be implemented in a central point, it will make things easier for the namespace folks, and since it will lead to relaxing restrictions later, it doesn't make sense to put it in a LSM as they stand now. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |