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,'



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

Reply via email to