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

[Kleber Souza]

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.


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
>