[apparmor] [patch] aamode.py - fix LOG_MODE_RE
Peter Maloney
peter.maloney at brockmann-consult.de
Tue Dec 2 09:17:31 UTC 2014
Oops didn't notice my test exception there until I read it in my own
email (if there is an invalid mode without that, the trace doesn't have
validate_log_mode in it... so I tested with that. Not sure which you
prefer). Here is without that.
FYI Here's the slightly less helpful exception and trace:
Reading log entries from /var/log/audit/audit.log.
Updating AppArmor profiles in /etc/apparmor.d.
Traceback (most recent call last):
File "/usr/sbin/aa-logprof", line 52, in <module>
apparmor.do_logprof_pass(logmark)
File "/usr/lib/python3.4/site-packages/apparmor/aa.py", line 2265, in
do_logprof_pass
log = log_reader.read_log(logmark)
File "/usr/lib/python3.4/site-packages/apparmor/logparser.py", line
351, in read_log
event = self.parse_log_record(line)
File "/usr/lib/python3.4/site-packages/apparmor/logparser.py", line
88, in parse_log_record
record_event = self.parse_event(record)
File "/usr/lib/python3.4/site-packages/apparmor/logparser.py", line
127, in parse_event
raise AppArmorException(_('Log contains unknown mode %s') % rmask)
apparmor.common.AppArmorException: 'Log contains unknown mode a'
And the punchline, here was the cause (mix of ' and " :D)
LOG_MODE_SET = {'r", "w", "l", "m", "k", "a", "x", "ix", "ux", "px",
"pux", "cx", "nx", "pix", "cix", "Ux", "Px", "PUx", "Cx", "Nx", "Pix",
"Cix'}
Peter
On 2014-12-02 10:11, Peter Maloney wrote:
> Howdy,
>
> Oh if that's the way it is supposed to be (each part of the regex has to
> be the whole string), using a set instead of a regex there is also very
> easy, and should be faster just like the other regex to set change.
>
>
> Patch attached.
>
> Peter
>
>
>
> PS / FYI. this patch is from openSUSE apparmor-utils 2.9.0-3.1 rather
> than bzr, which has a duplicated line that is just the same as the
> previous but is a comment... So if the patch looks weird, you know why.
>
> def validate_log_mode(mode):
> if LOG_MODE_RE.search(mode):
> #if LOG_MODE_RE.search(mode):
> return True
> else:
> return False
>
>
> On 2014-12-01 21:09, Christian Boltz wrote:
>> Hello,
>>
>> Am Montag, 1. Dezember 2014 schrieb Steve Beattie:
>>> On Sat, Nov 29, 2014 at 08:10:38PM +0100, Christian Boltz wrote:
>>>> LOG_MODE_RE (used in validate_log_mode() in aamode.py) just checked
>>>> if the given parameter contains one of the possible matches. This
>>>> resulted in "invalid" [1] being a valid log mode (from audit.log
>>>> requested_mask or denied_mask) because it contains 'a', which is a
>>>> valid file mode.
>>>>
>>>> This patch wraps the regex into ^(...)*$ to make sure the full
>>>> string contains only allowed file modes.
>>>>
>>>> (The question is if the log (!) really contains things like Pix -
>>>> but if not, that's worth another patch ;-)
>>>>
>>>> The patch also adds some tests for validate_log_mode().
>>>>
>>>> (Just in case you wonder about the order - I first noticed the regex
>>>> bug, then wrote the tests.)
>>>> + def test_validate_log_mode_5(self):
>>>> + self.assertTrue(validate_log_mode(''))
>>> Is this the correct result? I don't think the old version would have
>>> returned True in that situation, and it seems fishy to do so. I think
>>> you want '+' instead of '*' (Kleene star).
>> Indeed, that's the only sanity check that worked in the old code ;-)
>> Good catch!
>>
>> Here's the updated patch with two changes:
>> - change the regex to use "+" instead of "*" (which means an empty
>> string will return False)
>> - rename test_validate_log_mode_5() to test_validate_log_mode_invalid_4()
>> and expect False for ''
>>
>>
>> [ aamode-validate_log_mode-v2.diff ]
>>
>> === modified file 'utils/apparmor/aamode.py'
>> --- utils/apparmor/aamode.py 2014-12-01 19:56:31 +0000
>> +++ utils/apparmor/aamode.py 2014-12-01 20:05:41 +0000
>> @@ -68,7 +68,7 @@
>> 'N': AA_EXEC_NT
>> }
>>
>> -LOG_MODE_RE = re.compile('(r|w|l|m|k|a|x|ix|ux|px|pux|cx|nx|pix|cix|Ux|Px|PUx|Cx|Nx|Pix|Cix)')
>> +LOG_MODE_RE = re.compile('^(r|w|l|m|k|a|x|ix|ux|px|pux|cx|nx|pix|cix|Ux|Px|PUx|Cx|Nx|Pix|Cix)+$')
>> MODE_MAP_SET = {"r", "w", "l", "m", "k", "a", "x", "i", "u", "p", "c", "n", "I", "U", "P", "C", "N"}
>>
>> def str_to_mode(string):
>>
>> === modified file 'utils/test/test-aamode.py'
>> --- utils/test/test-aamode.py 2014-11-29 08:15:17 +0000
>> +++ utils/test/test-aamode.py 2014-12-01 20:06:12 +0000
>> @@ -11,7 +11,7 @@
>>
>> import unittest
>>
>> -from apparmor.aamode import split_log_mode, sub_str_to_mode
>> +from apparmor.aamode import split_log_mode, sub_str_to_mode, validate_log_mode
>> from apparmor.common import AppArmorBug
>>
>> class AamodeTest_split_log_mode(unittest.TestCase):
>> @@ -51,6 +51,24 @@
>> def test_sub_str_to_mode_dupes(self):
>> self.assertEqual(sub_str_to_mode('rwrwrw'), {'r', 'w'})
>>
>> +class AamodeTest_validate_log_mode(unittest.TestCase):
>> + def test_validate_log_mode_1(self):
>> + self.assertTrue(validate_log_mode('a'))
>> + def test_validate_log_mode_2(self):
>> + self.assertTrue(validate_log_mode('rw'))
>> + def test_validate_log_mode_3(self):
>> + self.assertTrue(validate_log_mode('Pixrw'))
>> + def test_validate_log_mode_4(self):
>> + self.assertTrue(validate_log_mode('rrrr'))
>> +
>> + def test_validate_log_mode_invalid_1(self):
>> + self.assertFalse(validate_log_mode('c')) # 'c' (create) must be converted to 'a' before calling validate_log_mode()
>> + def test_validate_log_mode_invalid_2(self):
>> + self.assertFalse(validate_log_mode('R')) # only lowercase 'r' is valid
>> + def test_validate_log_mode_invalid_3(self):
>> + self.assertFalse(validate_log_mode('foo'))
>> + def test_validate_log_mode_invalid_4(self):
>> + self.assertFalse(validate_log_mode(''))
>>
>> if __name__ == '__main__':
>> unittest.main(verbosity=2)
>>
>>
>>
>> Regards,
>>
>> Christian Boltz
>
>
>
--
--------------------------------------------
Peter Maloney
Brockmann Consult
Max-Planck-Str. 2
21502 Geesthacht
Germany
Tel: +49 4152 889 300
Fax: +49 4152 889 333
E-mail: peter.maloney at brockmann-consult.de
Internet: http://www.brockmann-consult.de
--------------------------------------------
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141202/970852d1/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: p4-aamode-log-mode-set.patch
Type: text/x-patch
Size: 1115 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141202/970852d1/attachment-0001.bin>
More information about the AppArmor
mailing list