[pkg-apparmor] Bug#771978: Patch: apparmor profile for ps

[intrigeri]

Hi,

Craig Small wrote (06 Dec 2014 20:46:29 GMT) :
> I have tested this with ps and it seems that all the flags are working
> OK. I couldn't break it with the usual combination of ps options.

Thanks for testing!

> On Sat, Dec 06, 2014 at 11:17:02AM +0100, intrigeri wrote:
>> > #include <tunables/kernelvars>
>> > #include <tunables/sys>
>> These two last lines require AppArmor from Jessie, so the "Suggests:
>> apparmor" that will be added to the ps package needs to be versioned
>> for better partial upgrades support.
> Is that required, or is it a case of if you want apparmor you load it.
> I would of thought this would be the same as, say, logcheck where you
> ship logcheck files and its up to the admin to install logcheck
> themselves.

It's merely a wide-spread convention in Ubuntu and Debian, and *kind
of* matches the definition of Suggests:

    This is used to declare that one package may be more useful with
    one or more others. Using this field tells the packaging system
    and the user that the listed packages are related to this one and
    can perhaps enhance its usefulness, but that installing this one
    without them is perfectly reasonable.

But it's not required.

> I needed to remove the kernelvars include for it to work, otherwise
> I get a duplicate definition of pid errors. 

Yes, I expected that. Not sure how the profile's author managed to
have the parser compile it.

>> I'm not sure if it makes sense, long-term wise, to whitelist what we
>> want to allow in @{PROC}@{pid}. My concern is about the maintenance
>> costs, when the kernel adds new interfaces in there, and ps learns how
>> to use it, and then this profile has to learn about it.
> We (upstream procps) would certainly be changing this around from time
> to time.  I couldn't really tell you how often we do change the type of
> files we access.  An added complexity is that a lot of the file loading
> happens in the libprocps* library package and not the procps binary
> package.

> This is one of those balancing acts. The apparmor side of things should
> have a better idea of where the usual balance is.

OK, then I would simply replace all @{PROC}@{pid} lines with:

  @{PROC}@{pid}/** r,

> I also saw things like:
> # for hostdev
> /sys/devices/ r,
> /sys/devices/** r,

Where did you see that? I can't find it in the proposed patch.

> Should these use the @{sys} just like the @{PROC} ?

No, I don't think we have any such thing (nor why it would be needed,
but I must say I don't remember why we have @{PROC} either -- probably
for chroots or containers).

Cheers,
--
intrigeri