Hello,
On Sat, Apr 18, 2015 at 2:49 AM, Christian Boltz <appar...@cboltz.de> wrote:
> Hello,
>
> Am Mittwoch, 15. April 2015 schrieb Kshitij Gupta:
> > On Wed, Apr 15, 2015 at 3:07 AM, Christian Boltz wrote:
> > > this patch adds utils/apparmor/rule/network.py with the NetworkRule
> > > and NetworkRuleset classes. These classes are meant to handle
> > > network rules.
> > >
> > > In comparison to the existing code in aa.py, relevant news are:
> > > - the keywords are checked against a list of allowed domains, types
> > > and>
> > > protocols (these lists are based on what the utils/vim/Makefile
> > > generates - on the long term an autogenerated file with the
> > > keywords
> > > for all rule types would be nice ;-)
> >
> > In theory you can have a script generate a python module that solely
> > contains these keywords, which can be imported in here. Sounds pretty
> > trivial ;-)
>
> We have something like that already for creating apparmor.vim (see
> utils/vim/Makefile) - but I'd describe the code that does that as
> "scary" ;-)
>
> Yes, it can be done, and it can even be done in a more sane way ;-)
>
> > > - there are variables for domain and type_or_protocol instead of
> > >
> > > first_param and second_param. (If someone is bored enough to map
> > > the
> > > protocol "shortcuts" to their expanded meaning, that shouldn't be
> > > too
> > > hard.)
> > >
> > > - (obviously) more readable code because we have everything at one
> > > place>
> > > now
> >
> > yay!
> >
> > > - some bugs are fixed along the way (for example, "network foo" will
> > > now>
> > > be kept, not "network foo bar" - see my last mail about
> > > write_net_rules() for details)
> > >
> > > [ 44-add-NetworkRule-and-NetworkRuleset-classes.diff ]
> > >
> > > === added file 'utils/apparmor/rule/network.py'
> > > --- utils/apparmor/rule/network.py 1970-01-01 00:00:00 +0000
> > > +++ utils/apparmor/rule/network.py 2015-04-14 20:47:40 +0000
>
> > > + # missing in manpage: 'unix', 'rds', 'llc', 'can', 'tipc',
> > > 'iucv', 'rxrpc', 'isdn', 'phonet', 'ieee802154', 'caif', 'alg',
> > > 'nfc', 'vsock'
> > > +
> >
> > comments from others on the missing types?
>
> That line should have gone into a bugreport or a patch for
> parser/apparmor.d.pod ;-)
>
> > > +network_type_keywords = ['stream', 'dgram', 'seqpacket', 'rdm',
> > 'raw',
> > > 'packet']
> > > +network_protocol_keywords = ['tcp', 'udp', 'icmp']
> > > +
> > > +
> > > +RE_NETWORK_DOMAIN = '(' + '|'.join(network_domain_keywords) + ')'
> > > +RE_NETWORK_TYPE = '(' + '|'.join(network_type_keywords) + ')'
> > > +RE_NETWORK_PROTOCOL = '(' + '|'.join(network_protocol_keywords) +
> > > ')'
> > neat!
>
> If we need it more often, it might even be worth a little function.
> We'll see...
>
> > > +RE_NETWORK_DETAILS = re.compile(
> > > + '^\s*(' +
> > > + '(?P<domain>' + RE_NETWORK_DOMAIN +
> > > ')(\s+(?P<type_or_protocol>' + RE_NETWORK_TYPE + '|' +
> > > RE_NETWORK_PROTOCOL + '))?' + # domain, optional type or protocol
> >
> > exercising new/larger monitor privileges? ;-)
>
> Not really, it even fits on my laptop screen ;-)
>
> > can be split into two lines I think.
>
> Good idea.
>
> > + '|' + # or
> >
> > > + '(?P<protocol>' + RE_NETWORK_PROTOCOL + ')' + # protocol
> > > only + ')\s*$')
> >
> > The leading and trailing \s* become superfluous as __parse has:
> > rule_details = matches.group('details').strip()
>
> I know, but it doesn't hurt and helps in case someone uses that regex
> in another way one day ;-)
>
> > > +
> > > +
> > > +
> > > +
> > > +
> >
> > what is the standard for whitespace separation?
>
> I don't know if there's a standard ;-)
>
> > 7 seems a bit much.
>
> Agreed, I trimmed some of them.
>
> > > + else:
> > > + raise AppArmorBug('Passed unknown domain to
> > > NetworkRule: %s' % str(domain))
> >
> > str(domain) is unnecessary, as type of domain is known to be str.
>
> Indeed, it's superfluous inside an "elif type(domain) == str:" ;-)
>
> > > + else:
> > > + raise AppArmorBug('Passed unknown object to
> > > NetworkRule: %s' % str(domain))
>
> (but still needed here)
>
> > > + self.type_or_protocol = None
> > > + self.all_type_or_protocols = False
> > > + if type_or_protocol == NetworkRule.ALL:
> > > + self.all_type_or_protocols = True
> > > + elif type(type_or_protocol) == str:
> > > + if type_or_protocol in network_protocol_keywords:
> > > + self.type_or_protocol = type_or_protocol
> > > + elif type_or_protocol in network_type_keywords:
> > > + if self.all_domains:
> > > + raise AppArmorException('Passing type %s to
> > > NetworkRule without specifying a domain keyword is not allowed' %
> > > str(type_or_protocol))
> >
> > str(type_or_protocol) not required.
>
> Right.
>
> > + self.type_or_protocol = type_or_protocol
> >
> > > + else:
> > > + raise AppArmorBug('Passed unknown type_or_protocol
> > > to NetworkRule: %s' % str(type_or_protocol))
> >
> > str(type_or_protocol) not required. #copy paste reviews
>
> ;-)
>
> > > + def is_equal_localvars(self, rule_obj):
> > > + '''compare if rule-specific variables are equal'''
> > > +
> > > + if not type(rule_obj) == NetworkRule:
> > > + raise AppArmorBug('Passes non-network rule: %s' %
> > > str(rule_obj))
> >
> > Passes or Passed?
>
> Passed
> (That's a copy&paste bug - rule/capability.py has the same. I sent a
> separate patch for that.)
>
>
> Here's the updated patch:
>
>
> [ 44-add-NetworkRule-and-NetworkRuleset-classes.diff ]
>
> === modified file utils/apparmor/rule/network.py
> --- utils/apparmor/rule/network.py 2015-04-17 22:50:11.062009564 +0200
> +++ utils/apparmor/rule/network.py 2015-04-17 23:05:06.804416579 +0200
> @@ -0,0 +1,205 @@
> +#!/usr/bin/env python
> +# ----------------------------------------------------------------------
> +# Copyright (C) 2013 Kshitij Gupta <kgupta8...@gmail.com>
> +# Copyright (C) 2015 Christian Boltz <appar...@cboltz.de>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of version 2 of the GNU General Public
> +# License as published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# ----------------------------------------------------------------------
> +
> +import re
> +
> +from apparmor.regex import RE_PROFILE_NETWORK
> +from apparmor.common import AppArmorBug, AppArmorException
> +from apparmor.rule import BaseRule, BaseRuleset, parse_modifiers
> +
> +# setup module translations
> +from apparmor.translations import init_translation
> +_ = init_translation()
> +
> +
> +network_domain_keywords = [ 'unix', 'inet', 'ax25', 'ipx', 'appletalk',
> 'netrom', 'bridge', 'atmpvc', 'x25', 'inet6',
> + 'rose', 'netbeui', 'security', 'key',
> 'netlink', 'packet', 'ash', 'econet', 'atmsvc', 'rds', 'sna',
> + 'irda', 'pppox', 'wanpipe', 'llc', 'can',
> 'tipc', 'bluetooth', 'iucv', 'rxrpc', 'isdn', 'phonet',
> + 'ieee802154', 'caif', 'alg', 'nfc', 'vsock'
> ]
> +
> +network_type_keywords = ['stream', 'dgram', 'seqpacket', 'rdm',
> 'raw', 'packet']
> +network_protocol_keywords = ['tcp', 'udp', 'icmp']
> +
> +
> +RE_NETWORK_DOMAIN = '(' + '|'.join(network_domain_keywords) + ')'
> +RE_NETWORK_TYPE = '(' + '|'.join(network_type_keywords) + ')'
> +RE_NETWORK_PROTOCOL = '(' + '|'.join(network_protocol_keywords) + ')'
> +
> +RE_NETWORK_DETAILS = re.compile(
> + '^\s*(' +
> + '(?P<domain>' + RE_NETWORK_DOMAIN + ')' + # domain and ...
> + '(\s+(?P<type_or_protocol>' + RE_NETWORK_TYPE + '|' +
> RE_NETWORK_PROTOCOL + '))?' + # ... optional type or protocol
> + '|' + # or
> + '(?P<protocol>' + RE_NETWORK_PROTOCOL + ')' + # protocol only
> + ')\s*$')
> +
> +
> +class NetworkRule(BaseRule):
> + '''Class to handle and store a single network rule'''
> +
> + # Nothing external should reference this class, all external users
> + # should reference the class field NetworkRule.ALL
> + class __NetworkAll(object):
> + pass
> +
> + ALL = __NetworkAll
> +
> + def __init__(self, domain, type_or_protocol, audit=False, deny=False,
> allow_keyword=False,
> + comment='', log_event=None):
> +
> + '''
> + NETWORK RULE = 'network' [ [ DOMAIN [ TYPE | PROTOCOL ] ] | [
> PROTOCOL ] ] ','
> + '''
> +
> + super(NetworkRule, self).__init__(audit=audit, deny=deny,
> + allow_keyword=allow_keyword,
> + comment=comment,
> + log_event=log_event)
> +
> + self.domain = None
> + self.all_domains = False
> + if domain == NetworkRule.ALL:
> + self.all_domains = True
> + elif type(domain) == str:
> + if domain in network_domain_keywords:
> + self.domain = domain
> + else:
> + raise AppArmorBug('Passed unknown domain to NetworkRule:
> %s' % domain)
> + else:
> + raise AppArmorBug('Passed unknown object to NetworkRule: %s'
> % str(domain))
> +
> + self.type_or_protocol = None
> + self.all_type_or_protocols = False
> + if type_or_protocol == NetworkRule.ALL:
> + self.all_type_or_protocols = True
> + elif type(type_or_protocol) == str:
> + if type_or_protocol in network_protocol_keywords:
> + self.type_or_protocol = type_or_protocol
> + elif type_or_protocol in network_type_keywords:
> + if self.all_domains:
> + raise AppArmorException('Passing type %s to
> NetworkRule without specifying a domain keyword is not allowed' %
> type_or_protocol)
> + self.type_or_protocol = type_or_protocol
> + else:
> + raise AppArmorBug('Passed unknown type_or_protocol to
> NetworkRule: %s' % type_or_protocol)
> + else:
> + raise AppArmorBug('Passed unknown object to NetworkRule: %s'
> % str(type_or_protocol))
> +
> + @classmethod
> + def _parse(cls, raw_rule):
> + '''parse raw_rule and return NetworkRule'''
> +
> + matches = RE_PROFILE_NETWORK.search(raw_rule)
> + if not matches:
> + raise AppArmorException(_("Invalid network rule '%s'") %
> raw_rule)
> +
> + audit, deny, allow_keyword, comment = parse_modifiers(matches)
> +
> + rule_details = ''
> + if matches.group('details'):
> + rule_details = matches.group('details').strip()
> +
> + if rule_details:
> + details = RE_NETWORK_DETAILS.search(rule_details)
> + if not details:
> + raise AppArmorException(_("Invalid or unknown keywords in
> 'network %s" % rule_details))
> +
> + if details.group('domain'):
> + domain = details.group('domain')
> + else:
> + domain = NetworkRule.ALL
> +
> + if details.group('type_or_protocol'):
> + type_or_protocol = details.group('type_or_protocol')
> + elif details.group('protocol'):
> + type_or_protocol = details.group('protocol')
> + else:
> + type_or_protocol = NetworkRule.ALL
> + else:
> + domain = NetworkRule.ALL
> + type_or_protocol = NetworkRule.ALL
> +
> + return NetworkRule(domain, type_or_protocol,
> + audit=audit, deny=deny,
> allow_keyword=allow_keyword, comment=comment)
> +
> + def get_clean(self, depth=0):
> + '''return rule (in clean/default formatting)'''
> +
> + space = ' ' * depth
> +
> + if self.all_domains:
> + domain = ''
> + elif self.domain:
> + domain = ' %s' % self.domain
> + else:
> + raise AppArmorBug('Empty domain in network rule')
> +
> + if self.all_type_or_protocols:
> + type_or_protocol = ''
> + elif self.type_or_protocol:
> + type_or_protocol = ' %s' % self.type_or_protocol
> + else:
> + raise AppArmorBug('Empty type or protocol in network rule')
> +
> + return('%s%snetwork%s%s,%s' % (space, self.modifiers_str(),
> domain, type_or_protocol, self.comment))
> +
> + def is_covered_localvars(self, other_rule):
> + '''check if other_rule is covered by this rule object'''
> +
> + if not other_rule.domain and not other_rule.all_domains:
> + raise AppArmorBug('No domain specified in other network rule')
> +
> + if not other_rule.type_or_protocol and not
> other_rule.all_type_or_protocols:
> + raise AppArmorBug('No type or protocol specified in other
> network rule')
> +
> + if not self.all_domains:
> + if other_rule.all_domains:
> + return False
> + if other_rule.domain != self.domain:
> + return False
> +
> + if not self.all_type_or_protocols:
> + if other_rule.all_type_or_protocols:
> + return False
> + if other_rule.type_or_protocol != self.type_or_protocol:
> + return False
> +
> + # still here? -> then it is covered
> + return True
> +
> + def is_equal_localvars(self, rule_obj):
> + '''compare if rule-specific variables are equal'''
> +
> + if not type(rule_obj) == NetworkRule:
> + raise AppArmorBug('Passed non-network rule: %s' %
> str(rule_obj))
> +
> + if (self.domain != rule_obj.domain
> + or self.all_domains != rule_obj.all_domains):
> + return False
> +
> + if (self.type_or_protocol != rule_obj.type_or_protocol
> + or self.all_type_or_protocols !=
> rule_obj.all_type_or_protocols):
> + return False
> +
> + return True
> +
> +
> +class NetworkRuleset(BaseRuleset):
> + '''Class to handle and store a collection of network rules'''
> +
> + def get_glob(self, path_or_rule):
> + '''Return the next possible glob. For network rules, that's
> "network DOMAIN," or "network," (all network)'''
> + # XXX return 'network DOMAIN,' if 'network DOMAIN
> TYPE_OR_PROTOCOL' was given
> + return 'network,'
>
>
> Thanks for the patch.
Acked-by: Kshitij Gupta <kgupta8...@gmail.com>.
Regards,
Kshitij Gupta
>
> Regards,
>
> Christian Boltz
> --
> Die Wahrscheinlichkeit, daß sich:
> Message-Id: <1093808348.5574.1.camel@ratti>
> wiederholt, ist so gering, daß ich mich vorher lieber um eine
> unterbrechnungsfreie Stromversorgung, einen Wachdienst vor der Haustür
> und einen Atombunker kümmern sollte, weil die Wahrscheinlichkeit eines
> Mailverlusts durch diese Ereignisse deutlich größer ist.
> [Ratti in suse-linux]
>
>
> --
> AppArmor mailing list
> AppArmor@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
