On 06/23/2014 03:48 PM, Daniel J Walsh wrote:
> We can use it to fix sandbox.
>
> sandbox -X xterm can do the following
>
> unconfined_t -> seunshare_t -> sandbox_x_t Then I allow sandbox_x_t to
> setcur to sandbox_x_client_t. sandbox_x_t can run the xserver, and the
> confined app will run as sandbox_x_client_t. If I am allowed to
> transition to a tighter domain, I can get the scripts to work.
Which of those transitions are on exec and which ones are via explicit
setcon() call?
Would it work if you couldn't allow it via dyntransition but had to
specify typebounds <callingdomain> <newdomain>;
instead?
>
> On 06/23/2014 02:25 PM, Andy Lutomirski wrote:
>> On Mon, Jun 23, 2014 at 10:23 AM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
>>> On 06/19/2014 04:51 PM, Paul Moore wrote:
>>>> On Thursday, June 19, 2014 04:04:18 PM Paul Moore wrote:
>>>>> On Thursday, June 12, 2014 03:29:04 PM Stephen Smalley wrote:
>>>>>> v2 - fix patch description to match the code.
>>>>> Okay Stephen, I suppose you should get some special consideration, but is
>>>>> posting your patches inline really that hard :)
>>>>>
>>>>> ---
>>>>> From c1fa21950c5c3eb0dd97ae5145fa3d3f04adc5c4 Mon Sep 17 00:00:00 2001
>>>>> From: Stephen Smalley <sds@xxxxxxxxxxxxx>
>>>>> Date: Thu, 12 Jun 2014 08:17:48 -0400
>>>>> Subject: [PATCH] selinux: Permit exec transitions under NO_NEW_PRIVS or
>>>>> NOSUID under certain circumstances.
>>>>>
>>>>> If the callee SID is bounded by the caller SID or if the caller SID is
>>>>> allowed to perform a dynamic transition (setcon) to the callee SID, then
>>>>> allowing the transition to occur poses no risk of privilege escalation and
>>>>> we can therefore safely allow the transition to occur. Add these two
>>>>> exemptions for both the case where a transition was explicitly requested by
>>>>> the application and the case where an automatic transition is defined in
>>>>> policy.
>>>>>
>>>>> Signed-off-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
>>>>> Acked-by: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
>>>> Comments below ...
>>>>
>>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>>> index 83d06db..d5e8dc5 100644
>>>>> --- a/security/selinux/hooks.c
>>>>> +++ b/security/selinux/hooks.c
>>>>> @@ -2086,6 +2086,36 @@ static int selinux_vm_enough_memory(struct mm_struct
>>>>> *mm, long pages)
>>>>>
>>>>> /* binprm security operations */
>>>>>
>>>>> +static int check_nnp_nosuid(const struct linux_binprm *bprm,
>>>>> + const struct task_security_struct *old_tsec,
>>>>> + const struct task_security_struct *new_tsec)
>>>>> +{
>>>>> + int nnp = (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS);
>>>>> + int nosuid = (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID);
>>>>> + int rc;
>>>>> +
>>>>> + if (!nnp && !nosuid)
>>>>> + return 0; /* neither NNP nor nosuid */
>>>>> +
>>>>> + if (new_tsec->sid == old_tsec->sid)
>>>>> + return 0; /* No change in credentials */
>>>>> +
>>>>> + rc = security_bounded_transition(old_tsec->sid, new_tsec->sid);
>>>>> + if (rc == 0)
>>>>> + return 0; /* allowed via bounded transition */
>>>> I think there might be an audit issue here; security_bounded_transition() will
>>>> generate an audit record on failure which probably isn't what we want in this
>>>> case.
>>>>
>>>> Other than that, this seems reasonable, even in the face of NNP, and as others
>>>> point out, standard DAC capabilities do something similar.
>>>>
>>>>> + /* Only allow if dyntransition permission aka setcon() is allowed. */
>>>>> + rc = avc_has_perm(old_tsec->sid, new_tsec->sid, SECCLASS_PROCESS,
>>>>> + PROCESS__DYNTRANSITION, NULL);
>>>>> + if (rc) {
>>>>> + if (nnp)
>>>>> + return -EPERM;
>>>>> + else
>>>>> + return -EACCES;
>>>>> + }
>>>>> + return 0;
>>>> I know this dyntransition/NNP/NOSUID interaction has been discussed quite a
>>>> bit off-list (mostly while I was away, my apologies), but I haven't seen a lot
>>>> on-list and while the descriptions hints at it the code itself doesn't
>>>> elaborate on why this is "OK". I'd like to see some comments about why it is
>>>> okay to allow the transition, regardless of NNP/NOSUID, if the dyntransition
>>>> is allowed. I'd also like to see a quick comment about why we return -EPERM/-
>>>> EACCES.
>>>>
>>>> There was a lot of discussion around these points and I want to make sure the
>>>> ideas aren't lost.
>>> The claim was that dyntransition is sufficient since it would allow the
>>> caller to setcon() to the new domain directly and therefore perform any
>>> action in that domain. Thus, allowing it to transition to that domain
>>> upon exec under NNP or on a file in a nosuid mount does not enable it to
>>> do anything it could not already do directly.
>>>
>>> However, this presumes that the policy writer does not merely add
>>> dyntransition to the caller domain upon encountering the avc denial in
>>> this situation without thinking about the implications and deciding
>>> whether to truly trust the caller domain in this way. Which is likely a
>>> dangerous assumption, especially for people who write policy via
>>> audit2allow.
>>>
>>> At least with the bounded transition, you have to explicitly declare
>>> that the new domain is bounded by the caller domain and the kernel will
>>> then enforce the restriction that the new domain is not granted any
>>> permission not allowed to the caller domain.
>>>
>> How about making the change just for bounded transitions, then? No
>> one will write the policy if the kernel doesn't support it.
>>
>>> I'm also unclear as to whether this in fact solves the actual problem
>>> for sandbox -X.
>> Dunno. dwalsh, does it?
>>
>>> So I'd drop this patch for now at least.
>> --Andy
>> _______________________________________________
>> Selinux mailing list
>> Selinux@xxxxxxxxxxxxx
>> To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
>> To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
>>
>>
>
>
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
