[apparmor] [patch] Fix aa-logprof crash on ptrace garbage log events

[Seth Arnold]

On Fri, May 19, 2017 at 09:21:42PM +0200, Christian Boltz wrote:
> Hello,
> 
> (garbage) ptrace events like
>     ... apparmor="DENIED" operation="ptrace" profile="/bin/netstat" pid=1962 comm="netstat" target=""
> cause an empty name2 field, which leads to a crash in the tools.
> 
> This patch lets logparser.py ignore such garbage log events, which also
> avoids the crash.
> 
> As usual, add some testcases.
> 
> test-libapparmor-test_multi.py needs some special handling to ignore the
> empty name2 field in one of the testcases.
> 
> 
> References: https://bugs.launchpad.net/apparmor/+bug/1689667
> 
> 
> I propose this patch for trunk and 2.11.

Acked-by: Seth Arnold <seth.arnold at canonical.com>
Acked for both, thanks!

> Older releases can't handle ptrace log events and therefore can't crash ;-)
> 
> 
> As a sidenote - it would be nice if libapparmor would translate ptrace
> events with target="" to AA_RECORD_INVALID ;-)
> 
> 
> 
> [ 01-ptrace-garbage-lp1689667.diff ]
> 
> === added file 'libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.err'
> === added file 'libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.in'
> --- libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.in    1970-01-01 00:00:00 +0000
> +++ libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.in    2017-05-19 18:28:48 +0000
> @@ -0,0 +1,1 @@
> +type=AVC msg=audit(1494272099.261:3455): apparmor="DENIED" operation="ptrace" profile="/bin/netstat" pid=1962 comm="netstat" target=""
> 
> === added file 'libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.out'
> --- libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.out   1970-01-01 00:00:00 +0000
> +++ libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.out   2017-05-19 19:00:53 +0000
> @@ -0,0 +1,11 @@
> +START
> +File: ptrace_garbage_lp1689667_1.in
> +Event type: AA_RECORD_DENIED
> +Audit ID: 1494272099.261:3455
> +Operation: ptrace
> +Profile: /bin/netstat
> +Command: netstat
> +Name2: 
> +PID: 1962
> +Epoch: 1494272099
> +Audit subid: 3455
> 
> === added file 'libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.profile'
> --- libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.profile       1970-01-01 00:00:00 +0000
> +++ libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_1.profile       2017-05-19 18:35:42 +0000
> @@ -0,0 +1,2 @@
> +/bin/netstat {
> +}
> 
> === added file 'libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_2.err'
> === added file 'libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_2.in'
> --- libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_2.in    1970-01-01 00:00:00 +0000
> +++ libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_2.in    2017-05-19 18:42:19 +0000
> @@ -0,0 +1,1 @@
> +type=AVC msg=audit(1494272099.261:3455): apparmor="DENIED" operation="ptrace" profile="/bin/netstat" pid=1962 comm="netstat" target=8022C0FF81A0FFFF8022C0FF81A0FFFF1080CBFF81A0FFFF1080CBFF81A0FFFF2080CBFF81A0FFFF2080CBFF81A0FFFF9E03
> 
> === added file 'libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_2.out'
> --- libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_2.out   1970-01-01 00:00:00 +0000
> +++ libraries/libapparmor/testsuite/test_multi/ptrace_garbage_lp1689667_2.out   2017-05-19 18:42:37 +0000
> @@ -0,0 +1,10 @@
> +START
> +File: ptrace_garbage_lp1689667_2.in
> +Event type: AA_RECORD_INVALID
> +Audit ID: 1494272099.261:3455
> +Operation: ptrace
> +Profile: /bin/netstat
> +Command: netstat
> +PID: 1962
> +Epoch: 1494272099
> +Audit subid: 3455
> 
> === modified file 'utils/apparmor/logparser.py'
> --- utils/apparmor/logparser.py 2017-02-23 00:00:36 +0000
> +++ utils/apparmor/logparser.py 2017-05-19 18:41:31 +0000
> @@ -338,6 +338,10 @@
>              return(e['pid'], e['parent'], 'unknown_hat',
>                               [profile, hat, aamode, hat])
>          elif e['operation'] == 'ptrace':
> +            if not e['peer']:
> +                self.debug_logger.debug('ignored garbage ptrace event with empty peer')
> +                return None
> +
>              return(e['pid'], e['parent'], 'ptrace',
>                               [profile, hat, prog, aamode, e['denied_mask'], e['peer']])
>          elif e['operation'] == 'signal':
> 
> === modified file 'utils/test/test-libapparmor-test_multi.py'
> --- utils/test/test-libapparmor-test_multi.py   2017-03-02 21:21:53 +0000
> +++ utils/test/test-libapparmor-test_multi.py   2017-05-19 19:00:14 +0000
> @@ -66,6 +66,8 @@
>                      pass
>                  elif parsed_items['operation'] == 'exec' and label in ['sock_type', 'family', 'protocol']:
>                      pass  # XXX 'exec' + network? really?
> +                elif parsed_items['operation'] == 'ptrace' and label == 'name2' and params.endswith('/ptrace_garbage_lp1689667_1'):
> +                    pass  # libapparmor would better qualify this case as invalid event
>                  elif not parsed_items.get(label, None):
>                      raise Exception('parsed_items[%s] not set' % label)
>                  elif not expected.get(label, None):
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170519/b64bf9e0/attachment.pgp>