ACK: [PATCH][SRU][bionic] UBUNTU: SAUCE: apparmor: fix nnp subset check failure when, stacking

[Stefan Bader]

On 06.08.19 01:39, 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>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  security/apparmor/domain.c | 46 ++++++++++++--------------------------
>  1 file changed, 14 insertions(+), 32 deletions(-)
> 
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index 6a54d2ffa840..e596d2d425bc 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -592,22 +592,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) {
> @@ -684,21 +668,6 @@ static int profile_onexec(struct aa_profile *profile, struct aa_label *onexec,
>  		perms.allow &= ~AA_MAY_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) {
> @@ -819,7 +788,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 */
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190812/aebd40ed/attachment.sig>