[apparmor] [patch] aa-genprof: fix byte vs. string and wrong filename

[Kshitij Gupta]

Hello,

(Long time no see ;-) )

On Mon, May 19, 2014 at 5:47 AM, Christian Boltz <apparmor at cboltz.de> wrote:
> Hello,
>
> aa-genprof crashes instantly after creating the basic profile in
> /etc/apparmor.d and before asking any questions:
>
> # LANG=C python3 ~cb/apparmor/HEAD-CLEAN/utils/aa-genprof  /bin/true
> [...]
> For each AppArmor event, you will be given the
> opportunity to choose whether the access should be
> allowed or denied.
> Traceback (most recent call last):
>   File "/home/cb/apparmor/HEAD-CLEAN/utils/aa-genprof", line 141, in <module>
>     logmark = last_audit_entry_time()
>   File "/home/cb/apparmor/HEAD-CLEAN/utils/aa-genprof", line 44, in last_audit_entry_time
>     if re.search('^.*msg\=audit\((\d+\.\d+\:\d+).*\).*$', out):
>   File "/usr/lib64/python3.4/re.py", line 166, in search
>     return _compile(pattern, flags).search(string)
> TypeError: can't use a string pattern on a bytes-like object
>
>
> Besides that, I noticed that aa-genprof is using hardcoded
> /var/log/audit/audit.log in last_audit_entry_time().
>
>
> This patch fixes those issues in aa-genprof's last_audit_entry_time():
> - convert "tail" result from byte to string to avoid TypeError
> - use apparmor.filename instead of hardcoded /var/log/audit/audit.log
>
> Note: I'm using python 3.4.0 - maybe there's a nice[tm] change in this
> version...
> (I didn't test with older python versions.)
>

Encoding issues probably need to be checked with previous versions
(2.7.* and maybe even <3.4). Encoding is something they keep changing.
I'll test out the patch with these versions.

The patch looks good., however.

>
> BTW: there's another hardcoded /var/log/audit.log in aa-genprof:
>
> if os.path.exists('/var/log/audit/audit.log'):
>     syslog = False
>
> Does this also need a change to honor the -f parameter?
>

Yes, you're right the -f param is not being honored. The code
basically is supposed to default to the system logger
(/usr/bin/logger) incase the /var/log/audit/audit.log did not exist,
which would be silly if the audit.log did not exist in the system when
user gave a custom log source.

In short, feel free to patch that meanwhile I'll test this patch when
I get out of office.


Regards,

Kshitij Gupta
>
> That all said, here's the patch:
>
> === modified file 'utils/aa-genprof'
> --- utils/aa-genprof    2014-03-19 23:10:13 +0000
> +++ utils/aa-genprof    2014-05-19 00:10:18 +0000
> @@ -39,8 +39,9 @@
>          f_out.write(str(value))
>
>  def last_audit_entry_time():
> -    out = subprocess.check_output(['tail', '-1', '/var/log/audit/audit.log'])
> +    out = subprocess.check_output(['tail', '-1', apparmor.filename])
>      logmark = None
> +    out = out.decode('ascii')
>      if re.search('^.*msg\=audit\((\d+\.\d+\:\d+).*\).*$', out):
>          logmark = re.search('^.*msg\=audit\((\d+\.\d+\:\d+).*\).*$', out).groups()[0]
>      else:
>
>
>
> Regards,
>
> Christian Boltz
> --
> I'm root - if you see me laughing you better have a backup!
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor