2018-06-05 16:34 GMT+02:00 Jan Zarsky <jzarsky@xxxxxxxxxx>:
> Add support for extended permissions to audit2allow. Extend AuditParser
> to parse the 'ioctlcmd' field in AVC message. Extend PolicyGenerator to
> generate allowxperm rules. Add the '-x'/'--xperms' option to audit2allow
> to turn on generating of extended permission AV rules.
>
> AVCMessage parses the ioctlcmd field in AVC messages. AuditParser
> converts the ioctlcmd values into generic representation of extended
> permissions that is stored in access vectors.
>
> Extended permissions are represented by operations (currently only
> 'ioctl') and values associated to the operations. Values (for example
> '~{ 0x42 1234 23-34 }') are stored in the XpermSet class.
>
> PolicyGenerator contains new method to turn on generating of xperms.
> When turned on, for each access vector, standard AV rule and possibly
> several xperm AV rules are generated. Xperm AV rules are represented by
> the AVExtRule class.
>
> With xperm generating turned off, PolicyGenerator provides comments
> about extended permissions in certain situations. When the AVC message
> contains the ioctlcmd field and the access would be allowed according to
> the policy, PolicyGenerator warns about xperm rules being the possible
> cause of the denial.
>
> Signed-off-by: Jan Zarsky <jzarsky@xxxxxxxxxx>
Hello,
Thanks for these patches. The first two look good to me and I will
merge them if nobody else has a comment on them. On the third one, it
is really nice it introduces more tests :) I have successfully run
them.
On the code itself, please find some comments below (one Python syntax
and two minor whitespace issues).
> diff --git a/python/sepolgen/src/sepolgen/audit.py b/python/sepolgen/src/sepolgen/audit.py
> index 26ce6c92..aae41e89 100644
> --- a/python/sepolgen/src/sepolgen/audit.py
> +++ b/python/sepolgen/src/sepolgen/audit.py
> @@ -237,6 +239,11 @@ class AVCMessage(AuditMessage):
> self.exe = fields[1][1:-1]
> elif fields[0] == "name":
> self.name = fields[1][1:-1]
> + elif fields[0] == "ioctlcmd":
> + try:
> + self.ioctlcmd = int(fields[1], 16)
> + except:
> + pass
Please do not introduce new bare "except". It also intercepts
KeyboardInterrupt which may cause issues when the command takes too
much time. It is quite difficult to change the existing ones in the
code because there may be some tricky exceptions which are
legitimately caught, but here, a "except ValueError" is better.
> diff --git a/python/sepolgen/tests/test_access.py b/python/sepolgen/tests/test_access.py
> index d45a823e..39424539 100644
> --- a/python/sepolgen/tests/test_access.py
> +++ b/python/sepolgen/tests/test_access.py
[...]
> + s.add_av = test_add_av
> +
> + s.add("foo", "bar", "file", refpolicy.IdSet(["read"]),
> + audit_msg='test message', avc_type=42, data='test data')
> +
There is a stray end of line here. My git reports:
<stdin>:159: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
> diff --git a/python/sepolgen/tests/test_refpolicy.py b/python/sepolgen/tests/test_refpolicy.py
> index 16e66806..0bcbd8e0 100644
> --- a/python/sepolgen/tests/test_refpolicy.py
> +++ b/python/sepolgen/tests/test_refpolicy.py
[...]
> +class TestAVExtRule(unittest.TestCase):
> + def test_init(self):
> + """ Test initialization of attributes """
> + a = refpolicy.AVExtRule()
> + self.assertEqual(a.rule_type, a.ALLOWXPERM)
> + self.assertIsInstance(a.src_types, set)
> + self.assertIsInstance(a.tgt_types, set)
> + self.assertIsInstance(a.obj_classes, set)
> + self.assertIsNone(a.operation)
> + self.assertIsInstance(a.xperms, refpolicy.XpermSet)
> +
> + def test_rule_type_str(self):
There is a empty line with whitespaces right before this function. My
git reports:
<stdin>:102: trailing whitespace.
warning: 1 line adds whitespace errors.
If you send a new patch which fixes these comments, I will gladly merge it.
Thanks!
Nicolas
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
