#934735 dh-apparmor: please improve dh integration

#934735#5
Date:
2019-08-14 07:37:12 UTC
From:
To:
Dear Maintainer,

It would be great if the integration with dh could be improved.

The first step would be to enable --with=apparmor support with a script
like this (/usr/share/perl5/Debian/Debhelper/Sequence/apparmor.pm):

    #! /usr/bin/perl
    # debhelper sequence file for dh_apparmor

    use warnings;
    use strict;
    use Debian::Debhelper::Dh_Lib;

    insert_after("dh_install", "dh_apparmor");

    1

It would also require dh_apparmor to do some guesswork and not create
postinst scripts for binary packages not shipping anything
apparmor-related.

Please consider implementing this.

Thanks!

- --
Cheers,
  Andrej
-----BEGIN PGP SIGNATURE-----

iQFIBAEBCAAyFiEEeuS9ZL8A0js0NGiOXkCM2RzYOdIFAl1TuicUHGFuZHJld3No
QGRlYmlhbi5vcmcACgkQXkCM2RzYOdJh0Qf/WJSl0gC+6DG9kMEwZxvX5shXGbXZ
n5RzwjOxzu8II3+otK+OzpZZIFjd7gjCX6SOd9ZAa/54mxtodhTameN5rOkJwXIE
5qfMxX3bwhFp+OhFHW2vSfj73UjMWqlc2wds3dDcg9qWDjNUGu2Wux8BVmwGXq6v
5CB0WKBq66h5UshyW+ZzPaRrYdAzbUchBmk4sdzzPb0tC+VLI8IK1CNNa0jZ2Htd
2coDjqqFiLH9IyKF0LdqH8r5uT6H2cq8aeZc0QKxgDNnO3l1uMKK4o4D5zUcGDYN
hodvihCEbrr/PQqMLfmn0+tT8N2LNMiiZzbCeLAIpdnYhvWOFXN8zbvhcQ==
=ar7M
-----END PGP SIGNATURE-----

#934735#10
Date:
2019-08-17 08:12:00 UTC
From:
To:
Hi AppArmor team,

I am happy to assist with technical bits (from a debhelper side PoV) if
you have questions or best practises in this area. :)

Thanks,
~Niels

#934735#15
Date:
2019-12-11 06:54:07 UTC
From:
To:
Hi Andrej!

Andrej Shadura:

I'm glad you care :)

Could you please describe what problem this solution would solve?
One way to explain this could be to show us how you envision this
improved integration would be used in a package, compared to the
status quo.

Cheers,

#934735#22
Date:
2021-02-05 16:45:18 UTC
From:
To:
Hi,

intrigeri (2019-12-11):

Ping? :)

#934735#27
Date:
2021-02-05 16:54:42 UTC
From:
To:
Hi,

I was thinking I should actually find time for this :D

I might do this weekend.

#934735#32
Date:
2021-02-05 17:07:10 UTC
From:
To:
Hi,

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

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.

#934735#37
Date:
2021-02-06 08:07:31 UTC
From:
To:
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 :)

Andrej Shadura (2021-02-05):

Awesome, thanks, it helps a lot!

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).
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?

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.

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

Cheers!

#934735#44
Date:
2021-02-06 11:11:38 UTC
From:
To:
intrigeri:

Hi

Ok, having a look at it. :)

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 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.

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.

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

#934735#49
Date:
2021-02-06 11:37:15 UTC
From:
To:
Hi,

thanks for the quick answer. For now I'll focus on Andrej's plans
(not potential future improvements) and the bits I know best,
leaving it to Andrej to reply about the other aspects :)

Niels Thykier (2021-02-06):

Right.

FTR, I've not found any trace of a package in the archive using
the --manifest=manifestfile facility.

Fully agreed, this was merely food for thought for potential future
iterations :)