[apparmor] [patch 3/4] utils/aa-unconfined: allow specifying ss/netstat binary locations

[Christian Boltz]

Hello,

Am Freitag, 30. Dezember 2016, 09:47:53 CET schrieb Steve Beattie:
> On Fri, Dec 30, 2016 at 03:16:04PM +0100, Christian Boltz wrote:
> > Am Donnerstag, 29. Dezember 2016 schrieb Steve Beattie:
> > > This patch allows a user to specify a specific location for ss or
> > > netstat for use in aa-unconfined, allowing a user to work around a
> > > tool that's buggy, uninstalled, or installed in an unexpected
> > > location. Note this option in the manpage.
> > 
> > How likely is it that someone really needs this?
> > 
> > I mean, if I have a broken ss or netstat installed, I'd want to fix
> > it for everything - but I wouldn't think about overriding it just
> > for one application, and leave everything else broken ;-)
> 
> Well, I was more thinking in terms of "broken in that aa-unconfined
> couldn't parse it, and netstat isn't available".

In such cases, I want a bugreport instead of people working around the 
issue ;-)

Thinking about this - would it make sense to print a warning if 
aa-unconfined finds _no_ processes listening to the network? I know this 
can in theory happen, but it's not too likely nowadays.

> > So even without the security concerns, I tend to dislike this patch
> > simply because I think these options are superfluous IMHO.
> 
> But yeah, I kind of agree with that.
> 
> > However, parts of the patch make sense. I'm talking about handing
> > over the command name as parameter to the function - this will
> > allow to write tests using a fake_ss and fake_netstat command.
> > (We might need to move out those functions to a python module when
> > we
> > write those tests to avoid running the global code in aa-unconfined,
> > but that can be easily done once someone writes such a test.)
> 
> I did wonder if get_pids_ss() and get_pids_netstat() ought to move to
> apparmor/common.py, but didn't see a need to do so for the time being.

We'll probably need it once we write tests for those functions - or we
wrap the global code in aa-unconfined with
    if __name__ == '__main__':

Anyway, as long as we don't have tests, keeping the functions in 
aa-unconfined isn't a problem. (But: please don't abuse this as an
argument not to write tests ;-)

> > To sum it up:
> >     Acked-by: Christian Boltz <apparmor at cboltz.de>
> > 
> > for adding the function parameters that make testing possible with a
> > fake_ss or fake_netstat script
> > 
> > The Ack does _not_ cover allowing users to specify the ss or netstat
> > to use via commandline parameters - if you really want that, use
> > John's ack for this part of the patch ;-)
> 
> Okay to apply?
> 
> Subject: utils/aa-unconfined: allow specifying ss/netstat binary
> locations
> 
> This patch allows a user to specify a specific location for ss or
> netstat in the invocations of get_pids_ss() or get_pids_netstat().
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>
> ---
>  utils/aa-unconfined |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Index: b/utils/aa-unconfined
> ===================================================================
> --- a/utils/aa-unconfined
> +++ b/utils/aa-unconfined
> @@ -50,7 +50,7 @@ def get_all_pids():
>      return set(filter(lambda x: re.search(r"^\d+$", x),
> aa.get_subdirectories("/proc")))
> 
> 
> -def get_pids_ss():
> +def get_pids_ss(ss='ss'):
>      '''Get a set of pids listening on network sockets via ss(8)'''
>      regex_lines =
> re.compile(r"^(tcp|udp|raw|p_dgr)\s.+\s+users:(?P<users>\(\(.*\)\))$"
> ) regex_users_pids = re.compile(r'(\("[^"]+",(pid=)?(\d+),[^)]+\))')
> @@ -60,7 +60,7 @@ def get_pids_ss():
>      my_env['LANG'] = 'C'
>      my_env['PATH'] = '/bin:/usr/bin:/sbin:/usr/sbin'
>      for family in ['inet', 'inet6', 'link']:
> -        cmd = ['ss', '-nlp', '--family', family]
> +        cmd = [ss, '-nlp', '--family', family]
>          if sys.version_info < (3, 0):
>              output = subprocess.check_output(cmd, shell=False,
> env=my_env).split("\n") else:
> @@ -76,11 +76,11 @@ def get_pids_ss():
>      return pids
> 
> 
> -def get_pids_netstat():
> +def get_pids_netstat(netstat='netstat'):
>      '''Get a set of pids listening on network sockets via
> netstat(8)''' regex_tcp_udp =
> re.compile(r"^(tcp|udp|raw)6?\s+\d+\s+\d+\s+\S+\:(\d+)\s+\S+\:(\*|\d+
> )\s+(LISTEN|\d+|\s+)\s+(?P<pid>\d+)\/(\S+)")
> 
> -    cmd = ['netstat', '-nlp', '--protocol', 'inet,inet6']
> +    cmd = [netstat, '-nlp', '--protocol', 'inet,inet6']
>      my_env = os.environ.copy()
>      my_env['LANG'] = 'C'
>      my_env['PATH'] = '/bin:/usr/bin:/sbin:/usr/sbin'

Thanks, looks good :-)

Acked-by: Christian Boltz <apparmor at cboltz.de>

And now, please commit this patch series before the copyright line from 
1/4 gets outdated ;-)


Regards,

Christian Boltz
-- 
> All cats purr at 28hz.
I think your cats need tuning - according to a couple of quick measure-
ments on a recently calibrated reference cat, the dominant frequency of
a correctly adjusted cat should be 12Hz +/-20%.          [Lionel Lauer]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20161230/7dcf556b/attachment.pgp>