[pkg-apparmor] Bug#934735: dh-apparmor: please improve dh integration

[Niels Thykier]

intrigeri:
> Control: tag -1 - moreinfo
> 
> Heya,
> 
> High-level note: I lack knowledge to evaluate this proposal in terms
> of debhelper integration. I'd be grateful if Niels could take a look :)
> 

Hi

Ok, having a look at it. :)

> Andrej Shadura (2021-02-05):
>> I’ll start with explaining the idea.
> 
> Awesome, thanks, it helps a lot!
> 
>> The status quo is:
>> * Each profile needs to be installed manually
>> * dh_apparmor needs to be told which profiles to use
>> * dh_apparmor needs to be told to only run on specific packages
>> * override_* or execute_after_* are mandatory
> 
> We're on the same page here.
> 
> More generally, it is seriously sub-optimal that using dh_apparmor
> requires programming, as opposed to using a declarative syntax (for
> non-obvious cases) and auto-detection (for obvious cases).
> 
>> My proposal is:
>> * For dh compat level <= 13:
>>   - allow running dh_apparmor without arguments;
>>   - without arguments, scan binary packages for apparmor profiles and use their names automatically
>>   - dh_apparmor can be enabled with --with=apparmor or B-D: dh-sequence-apparmor
>>   - without arguments, dh_apparmor only generates maintainer scripts for packages with apparmor profiles
>>   - with arguments, dh_apparmor does everything like it does now, no changes
>>
>> * For dh compat level 14:
>>   - as above, but with arguments, only generate maintainer scripts for the corresponding binary packages
> 
>> The above will allow processing apparmor profiles without extra
>> rules in d/rules, while maintaining compatibility with
>> existing packages.
> 

As I read dh_apparmor, it generates maintscript based on the
--profile-name parameter.  That name must match a file installed
in /etc/apparmor.d (of same name).  This implies that something else
have (or will) install the actual file into /etc/apparmor.d.

 => Is this correctly understood?

If so, then the proposed solution seems like it would mostly "just
work(tm)" for the general case with the proper implementation of "scan
binary packages for apparmor files".  I have assumed that it would just
"every /file/ in /etc/apparmor.d/".

In the assumed implementation, then we could also include a NOOP PROMISE
to assist debhelper with skipping dh_apparmor when there is nothing to
do (which could happen for -indep vs. -arch builds or even dh_apparmor
is run by default in a later version).


> I understand that at a higher level, for the simplest cases this
> translates into something like this:
> 
>   As a maintainer
>   When I ship an AppArmor profile in a binary package
>   And I enable the dh_apparmor debhelper sequence
>   Then the dh_apparmor machinery Does The Right Thing™ without further configuration
> 
> As a maintainer, I think it's awesome!
> 
> It may be worth specifying behavior for one corner case:
> what happens if a "-p PACKAGE" argument is passed?
> 

I do not think "-p" (or -i/-a/-N) matter here for backwards compat -
anything using dh_apparmor today would have --profile-name (combined
with -p), so --profile-name is a much better discriminator.
  Not to mention, implementing the check for -p would be difficult to
get right.

> Possible improvements for further iterations, definitely not blocking
> this plan IMO, i.e. food for future thought:
> 
>  - Either drop support for --profile-name or, if for some reason it's
>    still needed, support declarative syntax to configure it.
> 

What about manifests?  We can have them declarative by providing them in
a "guessible" location (e.g. debian/apparmor-manifests/<foo> would match
debian/.../etc/apparmor.d/<foo>).  But that implies that "omission"
(including accidental) is silently accepted as "no manifest".
  I do not know the consequence of that, so I cannot say if this
approach is good or not.

>  - Consider enabling dh_apparmor by default (and provide means to
>    disable it, either entirely or on a per-profile basis with
>    declarative syntax).
> 
> Cheers!
> 

I believe the whole idea behind this plan is to enable you to provide a
"apparmor" sequence (possibly via "dh-sequence-apparmor" Provides), so
it is much easier to use dh_apparmor.  This would handle
enabling/disabling dh_apparmor entirely.

For "per-profile exclusion", I think this is a question of supporting
-X/--exclude with the scanning if necessary (or calling dh_apparmor
manually with the exact files/profile names to be processed).

Though if you meant "by default" as in debhelper always running
dh_apparmor, then it is fine to design dh_apparmor after that principle,
but I would like to defer that decision of whether it happens for now.
As for concrete things that would be helpful here:

  * dh_aparmor should provide NOOP PROMISE support to enable dh to skip
    it when it will do nothing.  With the above plan, it looks mostly
    doable (without having considered all the edge cases though).

  * We would migrate towards triggers rather than maintscripts for
    processing the profiles on install.

But even then, the most important things are things that are hard for
you do directly as it is a question of a trade-off between how many
packages are using the feature vs. dh_apparmor being chiselled into
debhelper's external interface "forever" vs. debhelper maintenance team
capacity (etc.).
  Accordingly, I am not going to take a decision in the near future
about whether dh_apparmor should be enabled by default via debhelper
itself.

~Niels