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

[Kshitij Gupta]

Hello,

On Wed, May 21, 2014 at 12:54 PM, Kshitij Gupta <kgupta8592 at gmail.com> wrote:
> 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:
>>
>>

Tested with Python 2.7 and Python 3.3, seems to be working okay.

Acked-by: Kshitij Gupta <kgupta8592 at gmail.com>

Please commit the patch along with thesuggested change for the -f option.

Regards,

Kshitij Gupta

>>
>> 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