APPLIED/cmt: [PATCH][SRU][xenial] UBUNTU: SAUCE: apparmor: fix nnp subset check failure, when stacking

[John Johansen]

On 8/13/19 2:51 AM, Kleber Souza wrote:
> On 8/13/19 6:02 AM, Khaled Elmously wrote:
>> Sigh..It's because there's a dependency patch, which I just saw.
>>
>> Pleaase ignore my previous comment.
> 
> Hi John,
> 
> When a submitted patch was not based on the master-next branch
> and cannot be applied cleanly because it's based on other
> changes, please send all the patches in a single patch set
> or state the dependency in the patch submission so we know in
> which order they need to be applied.
> 
oops sorry, I make sure to use git send-email on the set next time



> 
> Thank you,
> Kleber
> 
>>
>>
>>
>> On 2019-08-13 00:00:11 , Khaled Elmously wrote:
>>> Applied to Xenial, but note: This patch didn't apply cleanly to xenial/master-next.
>>>
>>> The first 2 delta chunks remove code that is expected to contain this line:
>>>
>>> perms.allow &= ~AA_MAY_ONEXEC
>>>
>>> but in xenial/master-next that line doesn't exist.
>>>
>>>
>>> In any case, I applied the patch by removing those 2 chunks of code anyway.
>>>
>>> The third delta chunk had no issues.
>>>
>>> Khaled
>>>
>>>
>>> On 2019-08-05 16:31:33 , John Johansen wrote:
>>>> This is a backport of a fix that landed as part of a larger patch
>>>> in 4.17 commit 9fcf78cca1986 ("apparmor: update domain transitions that are subsets of confinement at nnp")
>>>>
>>>> Domain transitions that add a new profile to the confinement stack
>>>> when under NO NEW PRIVS is allowed as it can not expand privileges.
>>>>
>>>> However such transitions are failing due to how/where the subset
>>>> test is being applied. Applying the test per profile in the
>>>> profile transition and profile_onexec call backs is incorrect as
>>>> it disregards the other profiles in the stack so it can not
>>>> correctly determine if the old confinement stack is a subset of
>>>> the new confinement stack.
>>>>
>>>> Move the test to after the new confinement stack is constructed.
>>>>
>>>> BugLink: http://bugs.launchpad.net/bugs/1839037
>>>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>>>> ---
>>>>  security/apparmor/domain.c | 47 ++++++++++++--------------------------
>>>>  1 file changed, 14 insertions(+), 33 deletions(-)
>>>>
>>>> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
>>>> index d9e29b2fdda2..38595ca7e642 100644
>>>> --- a/security/apparmor/domain.c
>>>> +++ b/security/apparmor/domain.c
>>>> @@ -560,22 +560,6 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
>>>>  	if (!new)
>>>>  		goto audit;
>>>>  
>>>> -	/* Policy has specified a domain transitions. if no_new_privs and
>>>> -	 * confined and not transitioning to the current domain fail.
>>>> -	 *
>>>> -	 * NOTE: Domain transitions from unconfined and to stritly stacked
>>>> -	 * subsets are allowed even when no_new_privs is set because this
>>>> -	 * aways results in a further reduction of permissions.
>>>> -	 */
>>>> -	if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
>>>> -	    !profile_unconfined(profile) &&
>>>> -	    !aa_label_is_subset(new, &profile->label)) {
>>>> -		error = -EPERM;
>>>> -		info = "no new privs";
>>>> -		nonewprivs = true;
>>>> -		perms.allow &= ~MAY_EXEC;
>>>> -		goto audit;
>>>> -	}
>>>>  
>>>>  	if (!(perms.xindex & AA_X_UNSAFE)) {
>>>>  		if (DEBUG_ON) {
>>>> @@ -653,22 +637,6 @@ static int profile_onexec(struct aa_profile *profile, struct aa_label *onexec,
>>>>  		goto audit;
>>>>  	}
>>>>  
>>>> -	/* Policy has specified a domain transitions. if no_new_privs and
>>>> -	 * confined and not transitioning to the current domain fail.
>>>> -	 *
>>>> -	 * NOTE: Domain transitions from unconfined and to stritly stacked
>>>> -	 * subsets are allowed even when no_new_privs is set because this
>>>> -	 * aways results in a further reduction of permissions.
>>>> -	 */
>>>> -	if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
>>>> -	    !profile_unconfined(profile) &&
>>>> -	    !aa_label_is_subset(onexec, &profile->label)) {
>>>> -		error = -EPERM;
>>>> -		info = "no new privs";
>>>> -		perms.allow &= ~AA_MAY_ONEXEC;
>>>> -		goto audit;
>>>> -	}
>>>> -
>>>>  	if (!(perms.xindex & AA_X_UNSAFE)) {
>>>>  		if (DEBUG_ON) {
>>>>  			dbg_printk("appaarmor: scrubbing environment "
>>>> @@ -788,7 +756,20 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>>>>  		goto done;
>>>>  	}
>>>>  
>>>> -	/* TODO: Add ns level no_new_privs subset test */
>>>> +	/* Policy has specified a domain transitions. If no_new_privs and
>>>> +	 * confined ensure the transition is to confinement that is subset
>>>> +	 * of the confinement when the task entered no new privs.
>>>> +	 *
>>>> +	 * NOTE: Domain transitions from unconfined and to stacked
>>>> +	 * subsets are allowed even when no_new_privs is set because this
>>>> +	 * aways results in a further reduction of permissions.
>>>> +	 */
>>>> +	if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
>>>> +	    !unconfined(label) && !aa_label_is_subset(new, label)) {
>>>> +		error = -EPERM;
>>>> +		info = "no new privs";
>>>> +		goto audit;
>>>> +	}
>>>>  
>>>>  	if (bprm->unsafe & LSM_UNSAFE_SHARE) {
>>>>  		/* FIXME: currently don't mediate shared state */
>>>> -- 
>>>> 2.17.1
>>>>
>>>>
>>>> -- 
>>>> kernel-team mailing list
>>>> kernel-team at lists.ubuntu.com
>>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>>
>