NACK: [PATCH][SRU][xenial] apparmor: provide userspace flag indicating binfmt_elf_mmap change

[Tyler Hicks]

[+sbeattie]

On 2019-08-05 16:25:28, John Johansen wrote:
> Commit 7b8d468e60d6 ("binfmt_elf: switch to new creds when switching to new mm") brought upstream
> Commit 9f834ec18def ("binfmt_elf: switch to new creds when switching to new mm")
> back to Xenial without the required apparmor flag to indicate the change.
> 
> This is a backport of the apparmor fix
> 
> Commit 9f834ec18def ("binfmt_elf: switch to new creds when switching to new mm")
> changed when the creds are installed by the binfmt_elf handler. This
> affects which creds are used to mmap the executable into the address
> space. Which can have an affect on apparmor policy.
> 
> Add a flag to apparmor at
> /sys/kernel/security/apparmor/features/domain/fix_binfmt_elf_mmap
> 
> to make it possible to detect this semantic change so that the userspace
> tools and the regression test suite can correctly deal with the change.
> 
> BugLink: http://bugs.launchpad.net/bugs/1630069
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> (backported from commit 34c426acb75cc21bdf84685e106db0c1a3565057)

Steve proposed this patch in late May:

 https://lists.ubuntu.com/archives/kernel-team/2019-May/100977.html

The security team discussed it and it was decided that causing a full
policy cache recompilation across all Xenial devices wasn't worth it:

 https://lists.ubuntu.com/archives/kernel-team/2019-May/101031.html

Steve then worked around this in QRT:

 https://git.launchpad.net/qa-regression-testing/commit/?id=7631da6c8025e657732cb65bcaaf11ad8be162ab

If the previous decision has changed, we should reconsider the patch but
I'm nack'ing for now so that it doesn't get merged without more
discussion.

Tyler

> ---
>  security/apparmor/apparmorfs.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 6950d27105e6..f63fa9a4f25d 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -1533,6 +1533,7 @@ static struct aa_fs_entry aa_fs_entry_domain[] = {
>  	AA_FS_FILE_BOOLEAN("change_profile",	1),
>  	AA_FS_FILE_BOOLEAN("stack",		1),
>  	AA_FS_FILE_STRING("version", "1.2"),
> +	AA_FS_FILE_BOOLEAN("fix_binfmt_elf_mmap",	1),
>  	{ }
>  };
>  
> -- 
> 2.17.1
>