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

[Tyler Hicks]

On 2019-08-06 15:29:11, Tyler Hicks wrote:
> On 2019-08-06 13:19:34, John Johansen wrote:
> > On 8/6/19 8:04 AM, Tyler Hicks wrote:
> > > [+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.
> > > 
> > 
> > Its the wrong approach that leaves us with a technical debt that has to
> > be remembered/rediscovered every few months when we update the tests,
> > or bring patches back to Xenial.
> > 
> > Initial testing isn't done against UCT its done against the upstream
> > test suite, where patch and test development is done.
> 
> I'm not hard nack'ing this patch. I brought up the policy cache
> regeneration topic in May as a, "did you all think about this?" type of
> thing and then the answer I got back was that it wasn't worth causing
> every Xenial system out there to regenerate its policy cache (both from
> a CPU resources and risk aspect).
> 
> I personally don't think it is worth it but if the security team
> collectively thinks it should be merged, we can merge it. Please discuss
> it internally.

Friendly ping. A final answer today would be ideal if this is going to
make it into the next SRU cycle.

Tyler

> 
> Tyler
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team