[apparmor] [PATCH] various aa-notify fixes

Jamie Strandboge jamie at canonical.com
Wed Aug 17 14:59:13 UTC 2011


On Wed, 2011-08-17 at 01:21 +0200, Christian Boltz wrote:
> > 0003-check-dirname-with-auditd.patch:
> I'd like to reject that patch.
> 
> This might surprise you because it fixes the bug I reported (tested 
> successfully). The reason for the recect is that it introduces lots of 
> duplicated code to raise and drop privileges. That's a guarantee for a 
> future maintenance hell [1].
> 
> Please move this code into a "sub raise_privileges" and another "sub 
> drop_privileges", and I'll happily say that the patch is OK ;-)
> (If you want to keep the name of the calling sub in the debug message, 
> add it as parameter when calling raise_privileges/drop_privileges.)

Attached 0003-check-dirname-with-auditd_v2.patch:
utils/aa-notify:
  
aa-notify would abort if it could not stat the logfile, as can happen
when using auditd and the directory perms for the logfile do not allow access
(x). Add raise_privileges() and drop_privileges() helper functions and adjust
get_logfile_size() and get_logfile_inode() to raise then drop privileges if the
logfile parent directory is not executable. Also adjust reopen_logfile() to use
these helpers.
  
When error checking, use '$> == ...' instead of '$> = ... or die...' since perl
always dies when raising privs in this manner even though the euid did change
(and $!, $@, $^E, and $? are all the same). Not sure why this is happening but
the '==' check should be sufficient.


> >   Interestingly, this issue was masked on Ubuntu because of the
> > improper
> >   dropping of supplemental groups fixed in 0001, above.
> 
> Bad Ubuntu, they have set /var/log/audit too permissive ;-)

Well, they aren't that bad at root:root 750...

-- 
Jamie Strandboge             | http://www.canonical.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-check-dirname-with-auditd_v2.patch
Type: text/x-patch
Size: 4023 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20110817/e221132d/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20110817/e221132d/attachment.pgp>


More information about the AppArmor mailing list