From: Eric W. Biederman on
"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
> 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
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
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
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/