
[Please CC me, I'm not subscribed to the mailinglist] Hello, regarding the initial patch in this thread: The patch looks good and should go upstream IMHO. (Maybe except creating the dummy local/* files for AppArmor 3.x - see below for details.) A note about what you mentioned in the patch comment: If someone uses aa-logprof to update a profile, it will modify the profile, _not_ the local/ file. (Changing that is on the TODO list, but so far nobody did it.) Therefore I'm not sure if switching from %config(noreplace) to %config is a good idea. Am Montag, 26. Juni 2023, 18:29:11 CEST schrieb Andrea Bolognani:
On Mon, Jun 26, 2023 at 09:42:32AM -0600, Jim Fehlig wrote: [...]
Specifying which copy to use via a build time option is also an option :-). Does your idea include preserving commit 9b743ee19053 and adjusting the 'include if exists' to 'include'?
The modern (3.x) version would install the profile as it is currently, while the legacy (2.x) version would have the "include if exists" replaced with "include".
Sounds like a good idea. Note that you'll have to install dummy files for "include", while they don't have to exist when using "include if exists".
We could also decide to leave the include out altogether for the legacy version, and simply keep it as the less-featureful version it has been until now. In fact, I wonder if we should install placeholder files for the profiles at all?
For AppArmor 3.x, I'd tend to "no". And I say that despite knowing that upstream still creates all the local/ dummy files. (Which reminds me that this should maybe stop doig this in the upcoming 4.0 release. If you want to follow the discussion: https://gitlab.com/apparmor/apparmor/-/issues/337 ) These local sniplets are mostly "noise" (empty/comment-only) files, and it's harder than needed to find files that were actually modified. (Besides that, I'm currently testing a set of ~1000 AppArmor profiles, and having 1000 empty local/ files would make things much harder.)
For abstractions on 3.x, the abstractions/foo.d approach means that such placeholders do not exist. On the other hand, you could argue that the empty abstractions/foo.d directory and the empty local/foo file both serve the same purpose of providing a hint for the user that they can customize things using that mechanism...
If all abstractions would create empty *.d directories by default, that would be quite some noise and useless directories. So please don't do that ;-)
FWIW, Debian seems to consistently create placeholders for profiles. I think this is done automatically by the dh-apparmor utility that's used as standard for packaging. But that feels like a decision better left to each downstream... Maybe we should look into what upstream AppArmor does for its own profiles and abstractions.
See above - IMHO the current upstream behaviour is not perfect, and will hopefully change to not creating the local/ files by default in 4.0. Regards, Christian Boltz -- Social Media News: Instagram is down Science News: Scientists baffled as average human IQ increases over 25% in less than an hour [https://twitter.com/PaulRigbywrites/status/1146505629491781632]