[PATCH][SRU][bionic] UBUNTU: SAUCE: apparmor: fix nnp subset test for unconfined

[John Johansen]

On 10/3/19 2:13 PM, Tyler Hicks wrote:
> On 2019-10-03 12:14:58, John Johansen wrote:
>> The subset test is not taking into account the unconfined exception
>> which will cause profile transitions in the stacked confinement
>> case to fail when no_new_privs is applied.
>>
>> This fixes a regression introduced in the fix for
>> https://bugs.launchpad.net/bugs/1839037
>>
>> Fixes: d9b32413d82a ("UBUNTU: SAUCE: apparmor: fix nnp subset check failure when, stacking")
>> BugLink: https://bugs.launchpad.net/bugs/1844186
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>> ---
>>  security/apparmor/domain.c        |  4 ++--
>>  security/apparmor/include/label.h |  1 +
>>  security/apparmor/label.c         | 33 +++++++++++++++++++++++++++++++
>>  3 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
>> index 76cebad829fa..442550b13775 100644
>> --- a/security/apparmor/domain.c
>> +++ b/security/apparmor/domain.c
>> @@ -797,7 +797,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>>  	 * aways results in a further reduction of permissions.
>>  	 */
>>  	if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
>> -	    !unconfined(label) && !aa_label_is_subset(new, label)) {
>> +	    !unconfined(label) && !aa_label_is_unconfined_subset(new, label)) {
>>  		error = -EPERM;
>>  		info = "no new privs";
>>  		goto audit;
>> @@ -1136,7 +1136,7 @@ static int change_profile_perms_wrapper(const char *op, const char *name,
>>  	 */
>>  	if (task_no_new_privs(current) && !stack &&
>>  	    !profile_unconfined(profile) &&
>> -	    !aa_label_is_subset(target, &profile->label)) {
>> +	    !aa_label_is_unconfined_subset(target, &profile->label)) {
>>  		info = "no new privs";
>>  		error = -EPERM;
>>  	}
> 
> aa_change_hat() in Bionic is still affected by 1839037 which is why this
> regression fix doesn't include a hunk for aa_change_hat(). Is it worth
> sending a second revision for Bionic that has a patch that fixes 1839037
> for aa_change_hat() and then revise this patch to fix the
> aa_change_hat() fix?
> 

yeah, probably. I get that together and resend

> Tyler
> 
>> diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h
>> index af22dcbbcb8a..95075bd908f3 100644
>> --- a/security/apparmor/include/label.h
>> +++ b/security/apparmor/include/label.h
>> @@ -285,6 +285,7 @@ bool aa_label_init(struct aa_label *label, int size);
>>  struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp);
>>  
>>  bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub);
>> +bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub);
>>  struct aa_profile *__aa_label_next_not_in_set(struct label_it *I,
>>  					     struct aa_label *set,
>>  					     struct aa_label *sub);
>> diff --git a/security/apparmor/label.c b/security/apparmor/label.c
>> index 324fe5c60f87..0d9741f6d860 100644
>> --- a/security/apparmor/label.c
>> +++ b/security/apparmor/label.c
>> @@ -555,6 +555,39 @@ bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub)
>>  	return __aa_label_next_not_in_set(&i, set, sub) == NULL;
>>  }
>>  
>> +/**
>> + * aa_label_is_unconfined_subset - test if @sub is a subset of @set
>> + * @set: label to test against
>> + * @sub: label to test if is subset of @set
>> + *
>> + * This checks for subset but taking into account unconfined. IF
>> + * @sub contains an unconfined profile that does not have a matching
>> + * unconfined in @set then this will not cause the test to fail.
>> + * Conversely we don't care about an unconfined in @set that is not in
>> + * @sub
>> + *
>> + * Returns: true if @sub is special_subset of @set
>> + *     else false
>> + */
>> +bool aa_label_is_unconfined_subset(struct aa_label *set, struct aa_label *sub)
>> +{
>> +	struct label_it i = { };
>> +	struct aa_profile *p;
>> +
>> +	AA_BUG(!set);
>> +	AA_BUG(!sub);
>> +
>> +	if (sub == set)
>> +		return true;
>> +
>> +	do {
>> +		p = __aa_label_next_not_in_set(&i, set, sub);
>> +		if (p && !profile_unconfined(p))
>> +			break;
>> +	} while (p);
>> +
>> +	return p == NULL;
>> +}
>>  
>>  
>>  /**
>> -- 
>> 2.17.1
>>
>>
>> -- 
>> kernel-team mailing list
>> kernel-team at lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team