Re: [PATCH] virt-aa-helper: delete dynamic files
On Tue, Apr 01, 2025 at 03:24:10PM +0100, Daniel P. Berrangé wrote:
On Tue, Apr 01, 2025 at 07:15:46AM -0700, Andrea Bolognani via Devel
wrote:
> More importantly, I'm not convinced that you can just start deleting
> that file unconditionally. Since, as you note, it's explicitly
> documented that the user might add local customizations to the
> profile, deleting it might result in the loss of those local changes.
>
> So I think you need something more like:>
> expected = generate_default_profile();
> actual = read_existing_profile();
> if (STREQ(actual, expected)) {
> delete_profile();
> }
>
> In other words, only delete the existing profile if you can prove
> that it contains no valuable information.
Not sure its that's simple as we've changed what's in the profile
over time, especially if the host in question upgraded from
apparmor 2.x to 3.x, so such a comparison is likely to get false
results a decent amount of the time.
You're right about the AppArmor 2/3 part. And of course the admin
might have made local modifications to the template over time too,
which a simplistic approach like the one I've described above would
not capture correctly.
Still, false positives (i.e. the profiles incorrectly being detected
as containing local modifications) are not that harmful, as they only
result in lack of cleanup, which is the previous status quo. So
perhaps that's tolerable.
It might need to be more of a config file setting to allow people
turn off, and/or a build time default if a distro really wants
to preserve old behaviour a bit longer
I'm not a big fan of having to introduce yet another tunable. If it
really must exist, I think it should be a "force cleanup ignoring
local modification detection logic" bool.
--
Andrea Bolognani / Red Hat / Virtualization