From 5363e6d9a53146333da0d109aae170befc1b9481 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 12 Feb 2013 07:48:33 -0500 Subject: Added client ACLs: * IP and CIDR-based ACLs * Metadata (group/hostname)-based ACLs * Documentation * Unit tests --- doc/server/plugins/misc/acl.txt | 202 +++++++++++++++++++ doc/server/xml-common.txt | 5 + schemas/acl-ip.xsd | 56 ++++++ schemas/acl-metadata.xsd | 79 ++++++++ src/lib/Bcfg2/Server/CherryPyCore.py | 9 +- src/lib/Bcfg2/Server/Core.py | 72 +++++-- src/lib/Bcfg2/Server/Plugin/helpers.py | 36 ++-- src/lib/Bcfg2/Server/Plugin/interfaces.py | 30 +++ src/lib/Bcfg2/Server/Plugins/ACL.py | 145 ++++++++++++++ src/lib/Bcfg2/Server/SSLServer.py | 15 +- .../Testlib/TestServer/TestPlugin/Testhelpers.py | 161 ++++++++++----- .../TestServer/TestPlugin/Testinterfaces.py | 18 ++ .../Testlib/TestServer/TestPlugins/TestACL.py | 222 +++++++++++++++++++++ 13 files changed, 959 insertions(+), 91 deletions(-) create mode 100644 doc/server/plugins/misc/acl.txt create mode 100644 schemas/acl-ip.xsd create mode 100644 schemas/acl-metadata.xsd create mode 100644 src/lib/Bcfg2/Server/Plugins/ACL.py create mode 100644 testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestACL.py diff --git a/doc/server/plugins/misc/acl.txt b/doc/server/plugins/misc/acl.txt new file mode 100644 index 000000000..73f99bf85 --- /dev/null +++ b/doc/server/plugins/misc/acl.txt @@ -0,0 +1,202 @@ +.. -*- mode: rst -*- + +.. _server-plugins-misc-acl: + +=== +ACL +=== + +The ACL plugin lets you set client communication ACLs to prevent +clients from accessing the full range of exposed XML-RPC methods. + +You can get a list of all exposed methods by running:: + + bcfg2-admin xcmd listMethods + +Note that this will only list methods that are available to the client +this is run from; that is, if the ACL plugin is in place, +``listMethods`` will reflect the ACLs. + +ACLs can be set in two different ways: + +* IP-based ACLs allow you to set ACLs based on client IP address or + CIDR range. +* Metadata-based ACLs allow you to set ACLs based on client hostname, + group membership, or complex combinations thereof. + +IP-based ACLs are much faster, but metadata-based ACLs are often +easier and better. + +If you are not going to use any ACLs, it is recommended that you +disable this plugin because using it can incur a slight performance +hit. If you are using IP-based ACLs but *not* metadata-based ACLs, it +is similarly recommended that you ensure that your IP-based ACL file +ends with an explicit Deny for all clients; this will ensure that +metadata-based ACLs are never checked. If you are using +metadata-based ACLs, :ref:`server-caching` can alleviate most of the +performance penalty. + +Enabling the ACL plugin +======================= + +First, create ``/var/lib/bcfg2/ACL/``. Then, add ``ACL`` to your +``plugins`` list in ``bcfg2.conf``:: + + plugins = Bundler, Cfg, ..., Packages, ACL + +Finally, create ``/var/lib/bcfg2/ACL/ip.xml`` (for `IP-based ACLs`_), +``/var/lib/bcfg2/ACL/metadata.xml`` (for `Metadata-based ACLs`_), or +both. + +IP-based ACLs +============= + +IP-based ACLs allow you to set ACLs based on client IP address or CIDR +range. IP-based ACLs are very fast. If you are using IP-based ACLs +but *not* metadata-based ACLs, it is recommended that you ensure that +your IP-based ACL file ends with an explicit Deny for all clients; +this will ensure that metadata-based ACLs are never checked. + +IP-based ACLs are defined in ``ACL/ip.xml``. The file is parsed +sequentially; the first matching rule applies. Each rule is either +Allow (to allow the client access), Deny (to deny the client access), +or Defer (to defer to `Metadata-based ACLs`_). The last rule in +``ip.xml`` is an implicit default allow for 127.0.0.1, and an implicit +default defer for all other machines. + +If no ``ip.xml`` file exists, then ACL checking will be deferred to +metadata-based ACLs. + +Example +------- + +.. code-block:: xml + + + + + + + + +In this example: + +* The machine at 192.168.1.10 (perhaps the Bcfg2 server) can call all + plugin XML-RPC methods; +* Machines in the 192.168.2.0/24 network cannot assert their own + profiles; +* The machine at 192.168.1.12 (perhaps the Git server) can call the + Git.Update method; +* All machines can call core methods (except 192.168.2.0/24, which can + call all core methods except AssertProfile). + +Implicitly, all machines (except localhost) except 192.168.1.10 are +disallowed access to the plugin methods. + +You can also provide a minimal configuration to try to weed out some +obvious bad requests before doing the more expensive `Metadata-based +ACLs`_. For instance: + +.. code-block:: xml + + + + + + + +In this example: + +* All machines can call all core methods without checking metadata + ACLs; +* Plugin method calls from machines in 192.168.1.0/24 are deferred to + metadata ACLs; and +* All other plugin method calls are denied. + +The only time metadata ACLs would be checked in this example would be +plugin method calls by machines in 192.168.1.0/24. + +Reference +--------- + +.. xml:type: IPACLContainerType + +Metadata-based ACLs +=================== + +Metadata-based ACLs let you set ACLs based on client hostname or group +membership, which is much more flexible and maintainable than +`IP-based ACLs`_. The downside is that it is slower, because it +requires generating client metadata for each machine that tries to +authenticate. Without :ref:`server-caching`, using metadata-based +ACLs will double the number of client metadata builds per client run, +which could be a sizeable performance penalty. + +In order to limit the performance penalty, it's highly recommended +to: + +* Enable :ref:`server-caching` in ``cautious`` or ``aggressive`` mode; + and +* Deny as many clients as possible with `IP-based ACLs`_. + +Metadata-based ACLs are defined in ``ACL/metadata.xml``. Only Allow +and Deny rules are supported, not Defer rules. The file is parsed +sequentially; the first matching rule applies. The last rule in +``metadata.xml`` is an implicit default allow for machines called +``localhost`` or ``localhost.localdomain``, and an implicit default +deny for all other machines. + +If no ``metadata.xml`` file exists, then all requests are implicitly +allowed. + +Example +------- + +This example is functionally identical to the `IP-based ACLs` example +above, but more maintainable in several ways: + +.. code-block:: xml + + + + + + + + + + + + + + +In this case, if you add a Bcfg2 server or Git server, or one of those +servers changes IP address, you don't need to rewrite your ACLs. +Similarly, you could add a new subnet of user workstations. + +Reference +--------- + +.. xml:type: MetadataACLContainerType + +.. _server-plugins-misc-acl-wildcards: + +Wildcards +========= + +The ACL descriptions allow you to use '*' as a wildcard for any number +of characters *other than* ``.``. That is: + +* ``*`` would match ``DeclareVersion`` and ``GetProbes``, but would + *not* match ``Git.Update`. +* ``*.*`` would match ``Git.Update``, but not ``DeclareVersion`` or + ``GetProbes``. + +Since all plugin methods are scoped to their plugin (i.e., they are +all ``.``), and all core methods have no +scope, this lets you easily allow or deny core or plugin methods. You +could also do something like ``*.toggle_debug`` to allow a host to +enable or disable debugging for all plugins. + +No other bash globbing is supported. diff --git a/doc/server/xml-common.txt b/doc/server/xml-common.txt index 96644571b..f35c1bd23 100644 --- a/doc/server/xml-common.txt +++ b/doc/server/xml-common.txt @@ -289,6 +289,11 @@ Feature Matrix +-------------------------------------------------+--------------+--------+------------+------------+ | File | Group/Client | Genshi | Encryption | XInclude | +=================================================+==============+========+============+============+ +| :ref:`ACL ip.xml ' | No | No | No | Yes | ++-------------------------------------------------+--------------+--------+------------+------------+ +| :ref:`ACL metadata.xml | Yes | Yes | Yes | Yes | +| ' | | | | | ++-------------------------------------------------+--------------+--------+------------+------------+ | :ref:`Bundler | Yes | Yes | Yes | Yes | | ` | | | | | +-------------------------------------------------+--------------+--------+------------+------------+ diff --git a/schemas/acl-ip.xsd b/schemas/acl-ip.xsd new file mode 100644 index 000000000..1d6106c05 --- /dev/null +++ b/schemas/acl-ip.xsd @@ -0,0 +1,56 @@ + + + + Schema for IP-based client ACLs: + :ref:`server-plugins-misc-acl` ``ip.xml`` + + + + + + + + The name of the XML-RPC method to allow or deny. Limited + wildcards are supported. + + + + + + + The IP address to match against. This is an exact match + unless :xml:attribute:`IPACLType:netmask` is defined. If + this is not defined, all addresses match the given rule. + + + + + + + If this is defined, then it is combined with + :xml:attribute:`IPACLType:address` to produce a CIDR range, + which is used for matching instead of exact matching based + only on IP address. This can be either an integer netmask + (e.g., ``netmask="24"``) or a dotted-quad (e.g., + ``netmask="255.255.255.0"``). + + + + + + + + + Top-level tag for describing metadata-based client ACLs. + + + + + + + + + + + + diff --git a/schemas/acl-metadata.xsd b/schemas/acl-metadata.xsd new file mode 100644 index 000000000..7d996fb87 --- /dev/null +++ b/schemas/acl-metadata.xsd @@ -0,0 +1,79 @@ + + + + Schema for metadata-based client ACLs: + :ref:`server-plugins-misc-acl` ``metadata.xml`` + + + + + + + + + An **MetadataACLGroupType** is a tag used to provide logic. + Child entries of a MetadataACLGroupType tag only apply to + machines that match the condition specified -- either + membership in a group, or a matching client name. + :xml:attribute:`MetadataACLGroupType:negate` can be set to + negate the sense of the match. + + + + + + + The name of the client or group to match on. Child entries + will only apply to this client or group (unless + :xml:attribute:`MetadataACLGroupType:negate` is set). + + + + + + + Negate the sense of the match, so that child entries only + apply to a client if it is not a member of the given group + or does not have the given name. + + + + + + + + + + + The name of the XML-RPC method to allow or deny. Limited + wildcards are supported. + + + + + + + + + + Top-level tag for describing metadata-based client ACLs. + + + + + + + + + + + + + + + + + + diff --git a/src/lib/Bcfg2/Server/CherryPyCore.py b/src/lib/Bcfg2/Server/CherryPyCore.py index fa66abce9..bf3be72f9 100644 --- a/src/lib/Bcfg2/Server/CherryPyCore.py +++ b/src/lib/Bcfg2/Server/CherryPyCore.py @@ -67,10 +67,13 @@ class Core(BaseCore): cert = None address = (cherrypy.request.remote.ip, cherrypy.request.remote.port) - if not self.check_acls(address[0]): - raise cherrypy.HTTPError(401) + rpcmethod = xmlrpcutil.process_body()[1] + if rpcmethod == 'ERRORMETHOD': + raise Exception("Unknown error processing XML-RPC request body") - return self.authenticate(cert, username, password, address) + if (not self.check_acls(address[0], rpcmethod) or + not self.authenticate(cert, username, password, address)): + raise cherrypy.HTTPError(401) @cherrypy.expose def default(self, *args, **params): # pylint: disable=W0613 diff --git a/src/lib/Bcfg2/Server/Core.py b/src/lib/Bcfg2/Server/Core.py index c01b493de..f93ca0a7b 100644 --- a/src/lib/Bcfg2/Server/Core.py +++ b/src/lib/Bcfg2/Server/Core.py @@ -743,6 +743,48 @@ class BaseCore(object): % plugin.name, exc_info=1) return result + @Bcfg2.Server.Statistics.track_statistics() + def check_acls(self, address, rmi): + """ Check client IP address and metadata object against all + :class:`Bcfg2.Server.Plugin.interfaces.ClientACLs` plugins. + If any ACL plugin denies access, then access is denied. ACLs + are checked in two phases: First, with the client IP address; + and second, with the client metadata object. This lets an ACL + interface do a quick rejection based on IP before metadata is + ever built. + + :param address: The address pair of the client to check ACLs for + :type address: tuple of (, ) + :param rmi: The fully-qualified name of the RPC call + :param rmi: string + :returns: bool + """ + plugins = self.plugins_by_type(Bcfg2.Server.Plugin.ClientACLs) + try: + ip_checks = [p.check_acl_ip(address, rmi) for p in plugins] + except: + self.logger.error("Unexpected error checking ACLs for %s for %s: " + "%s" % (address[0], rmi, sys.exc_info()[1])) + return False # failsafe + + if all(ip_checks): + # if all ACL plugins return True (allow), then allow + return True + elif False in ip_checks: + # if any ACL plugin returned False (deny), then deny + return False + # else, no plugins returned False, but not all plugins + # returned True, so some plugin returned None (defer), so + # defer. + + client, metadata = self.resolve_client(address) + try: + return all(p.check_acl_metadata(metadata, rmi) for p in plugins) + except: + self.logger.error("Unexpected error checking ACLs for %s for %s: " + "%s" % (client, rmi, sys.exc_info()[1])) + return False # failsafe + @Bcfg2.Server.Statistics.track_statistics() def build_metadata(self, client_name): """ Build initial client metadata for a client @@ -804,7 +846,7 @@ class BaseCore(object): :param address: The address pair of the client to get the canonical hostname for. - :type address: tuple of (, ) + :type address: tuple of (, ) :param cleanup_cache: Tell the :class:`Bcfg2.Server.Plugin.interfaces.Metadata` plugin in :attr:`metadata` to clean up @@ -881,21 +923,23 @@ class BaseCore(object): def listMethods(self, address): # pylint: disable=W0613 """ List all exposed methods, including plugin RMI. - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :returns: list of exposed method names """ methods = [name for name, func in inspect.getmembers(self, callable) - if getattr(func, "exposed", False)] - methods.extend(self._get_rmi().keys()) + if (getattr(func, "exposed", False) and + self.check_acls(address, name))] + methods.extend([m for m in self._get_rmi().keys() + if self.check_acls(address, m)]) return methods @exposed def methodHelp(self, address, method_name): # pylint: disable=W0613 """ Get help from the docstring of an exposed method - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :param method_name: The name of the method to get help on :type method_name: string @@ -911,7 +955,7 @@ class BaseCore(object): def DeclareVersion(self, address, version): """ Declare the client version. - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :param version: The client's declared version :type version: string @@ -932,7 +976,7 @@ class BaseCore(object): def GetProbes(self, address): """ Fetch probes for the client. - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :returns: lxml.etree._Element - XML tree describing probes for this client @@ -955,7 +999,7 @@ class BaseCore(object): def RecvProbeData(self, address, probedata): """ Receive probe data from clients. - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :returns: bool - True on success :raises: :exc:`xmlrpclib.Fault` @@ -1001,7 +1045,7 @@ class BaseCore(object): def AssertProfile(self, address, profile): """ Set profile for a client. - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :returns: bool - True on success :raises: :exc:`xmlrpclib.Fault` @@ -1021,7 +1065,7 @@ class BaseCore(object): """ Build config for a client by calling :func:`BuildConfiguration`. - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :returns: lxml.etree._Element - The full configuration document for the client @@ -1039,7 +1083,7 @@ class BaseCore(object): def RecvStats(self, address, stats): """ Act on statistics upload with :func:`process_statistics`. - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :returns: bool - True on success :raises: :exc:`xmlrpclib.Fault` @@ -1060,7 +1104,7 @@ class BaseCore(object): :type user: string :param password: The password supplied by the client :type password: string - :param address: An address pair of ``(, )`` + :param address: An address pair of ``(, )`` :type address: tuple :return: bool - True if the authenticate succeeds, False otherwise """ @@ -1084,7 +1128,7 @@ class BaseCore(object): def GetDecisionList(self, address, mode): """ Get the decision list for the client with :func:`GetDecisions`. - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :returns: list of decision tuples :raises: :exc:`xmlrpclib.Fault` @@ -1111,7 +1155,7 @@ class BaseCore(object): def toggle_debug(self, address): """ Toggle debug status of the FAM and all plugins - :param address: Client (address, hostname) pair + :param address: Client (address, port) pair :type address: tuple :returns: bool - The new debug state of the FAM """ diff --git a/src/lib/Bcfg2/Server/Plugin/helpers.py b/src/lib/Bcfg2/Server/Plugin/helpers.py index 9bdfe347f..ae3b84fc2 100644 --- a/src/lib/Bcfg2/Server/Plugin/helpers.py +++ b/src/lib/Bcfg2/Server/Plugin/helpers.py @@ -137,7 +137,7 @@ class PluginDatabaseModel(object): app_label = "Server" -class FileBacked(object): +class FileBacked(Debuggable): """ This object caches file data in memory. FileBacked objects are principally meant to be used as a part of :class:`Bcfg2.Server.Plugin.helpers.DirectoryBacked`. """ @@ -147,7 +147,7 @@ class FileBacked(object): :param name: The full path to the file to cache and monitor :type name: string """ - object.__init__(self) + Debuggable.__init__(self) #: A string containing the raw data in this file self.data = '' @@ -172,10 +172,10 @@ class FileBacked(object): self.Index() except IOError: err = sys.exc_info()[1] - LOGGER.error("Failed to read file %s: %s" % (self.name, err)) + self.logger.error("Failed to read file %s: %s" % (self.name, err)) except: err = sys.exc_info()[1] - LOGGER.error("Failed to parse file %s: %s" % (self.name, err)) + self.logger.error("Failed to parse file %s: %s" % (self.name, err)) def Index(self): """ Index() is called by :func:`HandleEvent` every time the @@ -462,9 +462,9 @@ class XMLFileBacked(FileBacked): else: msg = "%s: %s does not exist, skipping" % (self.name, name) if el.findall('./%sfallback' % Bcfg2.Server.XI_NAMESPACE): - LOGGER.debug(msg) + self.logger.debug(msg) else: - LOGGER.warning(msg) + self.logger.warning(msg) def Index(self): self.xdata = lxml.etree.XML(self.data, base_url=self.name, @@ -475,7 +475,8 @@ class XMLFileBacked(FileBacked): self.xdata.getroottree().xinclude() except lxml.etree.XIncludeError: err = sys.exc_info()[1] - LOGGER.error("XInclude failed on %s: %s" % (self.name, err)) + self.logger.error("XInclude failed on %s: %s" % (self.name, + err)) self.entries = self.xdata.getchildren() if self.__identifier__ is not None: @@ -502,7 +503,7 @@ class XMLFileBacked(FileBacked): return "%s at %s" % (self.__class__.__name__, self.name) -class StructFile(XMLFileBacked, Debuggable): +class StructFile(XMLFileBacked): """ StructFiles are XML files that contain a set of structure file formatting logic for handling ```` and ```` tags. @@ -533,7 +534,6 @@ class StructFile(XMLFileBacked, Debuggable): def __init__(self, filename, should_monitor=False): XMLFileBacked.__init__(self, filename, should_monitor=should_monitor) - Debuggable.__init__(self) self.setup = Bcfg2.Options.get_option_parser() self.encoding = self.setup['encoding'] self.template = None @@ -684,6 +684,8 @@ class StructFile(XMLFileBacked, Debuggable): Match() (and *not* their descendents) should be considered to match the metadata. + Match() returns matching fragments in document order. + :param metadata: Client metadata to match against. :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata :returns: list of lxml.etree._Element objects """ @@ -734,11 +736,15 @@ class StructFile(XMLFileBacked, Debuggable): All ```` and ```` tags will have been stripped out. + The new document produced by XMLMatch() is not necessarily in + the same order as the original document. + :param metadata: Client metadata to match against. :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata :returns: lxml.etree._Element """ return self._do_xmlmatch(metadata) + class INode(object): """ INodes provide lists of things available at a particular group intersection. INodes are deprecated; new plugins should use @@ -834,7 +840,7 @@ class XMLSrc(XMLFileBacked): data = open(self.name).read() except IOError: msg = "Failed to read file %s: %s" % (self.name, sys.exc_info()[1]) - LOGGER.error(msg) + self.logger.error(msg) raise PluginExecutionError(msg) self.items = {} try: @@ -842,7 +848,7 @@ class XMLSrc(XMLFileBacked): except lxml.etree.XMLSyntaxError: msg = "Failed to parse file %s: %s" % (self.name, sys.exc_info()[1]) - LOGGER.error(msg) + self.logger.error(msg) raise PluginExecutionError(msg) self.pnode = self.__node__(xdata, self.items) self.cache = None @@ -852,7 +858,7 @@ class XMLSrc(XMLFileBacked): if self.__priority_required__: msg = "Got bogus priority %s for file %s" % \ (xdata.get('priority'), self.name) - LOGGER.error(msg) + self.logger.error(msg) raise PluginExecutionError(msg) del xdata, data @@ -862,8 +868,8 @@ class XMLSrc(XMLFileBacked): if self.cache is None or self.cache[0] != metadata: cache = (metadata, self.__cacheobj__()) if self.pnode is None: - LOGGER.error("Cache method called early for %s; " - "forcing data load" % self.name) + self.logger.error("Cache method called early for %s; " + "forcing data load" % self.name) self.HandleEvent() return self.pnode.Match(metadata, cache[1]) @@ -1165,7 +1171,7 @@ class SpecificData(object): except UnicodeDecodeError: self.data = open(self.name, mode='rb').read() except: # pylint: disable=W0201 - LOGGER.error("Failed to read file %s" % self.name) + self.logger.error("Failed to read file %s" % self.name) class EntrySet(Debuggable): diff --git a/src/lib/Bcfg2/Server/Plugin/interfaces.py b/src/lib/Bcfg2/Server/Plugin/interfaces.py index fcd342b33..c1dbb1578 100644 --- a/src/lib/Bcfg2/Server/Plugin/interfaces.py +++ b/src/lib/Bcfg2/Server/Plugin/interfaces.py @@ -596,3 +596,33 @@ class ClientRunHooks(object): :returns: None """ pass + + +class ClientACLs(object): + """ ClientACLs are used to grant or deny access to different + XML-RPC calls based on client IP or metadata. """ + + def check_acl_ip(self, address, rmi): + """ Check if the given IP address is authorized to make the + named XML-RPC call. + + :param address: The address pair of the client to check ACLs for + :type address: tuple of (, ) + :param rmi: The fully-qualified name of the RPC call + :param rmi: string + :returns: bool or None - True to allow, False to deny, None to + defer to metadata ACLs + """ + return True + + def check_acl_metadata(self, metadata, rmi): + """ Check if the given client is authorized to make the named + XML-RPC call. + + :param metadata: The client metadata + :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata + :param rmi: The fully-qualified name of the RPC call + :param rmi: string + :returns: bool + """ + return True diff --git a/src/lib/Bcfg2/Server/Plugins/ACL.py b/src/lib/Bcfg2/Server/Plugins/ACL.py new file mode 100644 index 000000000..3de3f767c --- /dev/null +++ b/src/lib/Bcfg2/Server/Plugins/ACL.py @@ -0,0 +1,145 @@ +""" Support for client ACLs based on IP address and client metadata """ + +import os +import struct +import socket +import Bcfg2.Server.Plugin + + +def rmi_names_equal(first, second): + """ Compare two XML-RPC method names and see if they match. + Resolves some limited wildcards; see + :ref:`server-plugins-misc-acl-wildcards` for details. + + :param first: One of the ACLs to compare + :type first: string + :param second: The other ACL to compare + :type second: string + :returns: bool """ + if first == second: + # single wildcard is special, and matches everything + return True + if first is None or second is None: + return False + if '*' not in first + second: + # no wildcards, and not exactly equal + return False + first_parts = first.split('.') + second_parts = second.split('.') + if len(first_parts) != len(second_parts): + return False + for i in range(len(first_parts)): + if (first_parts[i] != second_parts[i] and first_parts[i] != '*' and + second_parts[i] != '*'): + return False + return True + + +def ip2int(ip): + """ convert a dotted-quad IP address into an integer + representation of the same """ + return struct.unpack('>L', socket.inet_pton(socket.AF_INET, ip))[0] + + +def ip_matches(ip, entry): + """ Return True if the given IP matches the IP or IP and netmask + in the given ACL entry; False otherwise """ + if entry.get("netmask"): + try: + mask = int("1" * int(entry.get("netmask")) + + "0" * (32 - int(entry.get("netmask"))), 2) + except ValueError: + mask = ip2int(entry.get("netmask")) + return ip2int(ip) & mask == ip2int(entry.get("address")) & mask + elif entry.get("address") is None: + # no address, no netmask -- match all + return True + elif ip == entry.get("address"): + # just a plain ip address + return True + return False + + +class IPACLFile(Bcfg2.Server.Plugin.XMLFileBacked): + """ representation of ACL ip.xml, for IP-based ACLs """ + actions = dict(Allow=True, + Deny=False, + Defer=None) + + def check_acl(self, address, rmi): + """ Check a client address against the ACL list """ + if not len(self.entries): + # default defer if no ACLs are defined. + self.debug_log("ACL: %s requests %s: No IP ACLs, defer" % + (address, rmi)) + return self.actions["Defer"] + for entry in self.entries: + if (ip_matches(address, entry) and + rmi_names_equal(entry.get("method"), rmi)): + self.debug_log("ACL: %s requests %s: Found matching IP ACL, " + "%s" % (address, rmi, entry.tag.lower())) + return self.actions[entry.tag] + if address == "127.0.0.1": + self.debug_log("ACL: %s requests %s: No matching IP ACLs, " + "localhost allowed" % (address, rmi)) + return self.actions['Allow'] # default allow for localhost + + self.debug_log("ACL: %s requests %s: No matching IP ACLs, defer" % + (address, rmi)) + return self.actions["Defer"] # default defer for other machines + + +class MetadataACLFile(Bcfg2.Server.Plugin.StructFile): + """ representation of ACL metadata.xml, for metadata-based ACLs """ + def check_acl(self, metadata, rmi): + """ check client metadata against the ACL list """ + if not len(self.entries): + # default allow if no ACLs are defined. + self.debug_log("ACL: %s requests %s: No metadata ACLs, allow" % + (metadata.hostname, rmi)) + return True + for el in self.Match(metadata): + if rmi_names_equal(el.get("method"), rmi): + self.debug_log("ACL: %s requests %s: Found matching metadata " + "ACL, %s" % (metadata.hostname, rmi, + el.tag.lower())) + return el.tag == "Allow" + if metadata.hostname in ['localhost', 'localhost.localdomain']: + # default allow for localhost + self.debug_log("ACL: %s requests %s: No matching metadata ACLs, " + "localhost allowed" % (metadata.hostname, rmi)) + return True + self.debug_log("ACL: %s requests %s: No matching metadata ACLs, deny" % + (metadata.hostname, rmi)) + return False # default deny for other machines + + +class ACL(Bcfg2.Server.Plugin.Plugin, + Bcfg2.Server.Plugin.ClientACLs): + """ allow connections to bcfg-server based on IP address """ + + def __init__(self, core, datastore): + Bcfg2.Server.Plugin.Plugin.__init__(self, core, datastore) + Bcfg2.Server.Plugin.ClientACLs.__init__(self) + self.ip_acls = IPACLFile(os.path.join(self.data, 'ip.xml'), + should_monitor=True) + self.metadata_acls = MetadataACLFile(os.path.join(self.data, + 'metadata.xml'), + should_monitor=True) + + def check_acl_ip(self, address, rmi): + self.debug_log("ACL: %s requests %s: Checking IP ACLs" % + (address[0], rmi)) + return self.ip_acls.check_acl(address[0], rmi) + + def check_acl_metadata(self, metadata, rmi): + self.debug_log("ACL: %s requests %s: Checking metadata ACLs" % + (metadata.hostname, rmi)) + return self.metadata_acls.check_acl(metadata, rmi) + + def set_debug(self, debug): + rv = Bcfg2.Server.Plugin.Plugin.set_debug(self, debug) + self.ip_acls.set_debug(debug) + self.metadata_acls.set_debug(debug) + return rv + set_debug.__doc__ = Bcfg2.Server.Plugin.Plugin.set_debug.__doc__ diff --git a/src/lib/Bcfg2/Server/SSLServer.py b/src/lib/Bcfg2/Server/SSLServer.py index f2fb4913a..0d1246d85 100644 --- a/src/lib/Bcfg2/Server/SSLServer.py +++ b/src/lib/Bcfg2/Server/SSLServer.py @@ -27,8 +27,7 @@ class XMLRPCDispatcher(SimpleXMLRPCServer.SimpleXMLRPCDispatcher): # Python 2.4? SimpleXMLRPCServer.SimpleXMLRPCDispatcher.__init__(self) - self.logger = logging.getLogger("%s.%s" % (self.__class__.__module__, - self.__class__.__name__)) + self.logger = logging.getLogger(self.__class__.__name__) self.allow_none = allow_none self.encoding = encoding @@ -95,8 +94,7 @@ class SSLServer(SocketServer.TCPServer, object): if ':' in server_address[0]: self.address_family = socket.AF_INET6 - self.logger = logging.getLogger("%s.%s" % (self.__class__.__module__, - self.__class__.__name__)) + self.logger = logging.getLogger(self.__class__.__name__) try: SocketServer.TCPServer.__init__(self, listen_address, @@ -185,10 +183,9 @@ class XMLRPCRequestHandler(SimpleXMLRPCServer.SimpleXMLRPCRequestHandler): """ def __init__(self, *args, **kwargs): + self.logger = logging.getLogger(self.__class__.__name__) SimpleXMLRPCServer.SimpleXMLRPCRequestHandler.__init__(self, *args, **kwargs) - self.logger = logging.getLogger("%s.%s" % (self.__class__.__module__, - self.__class__.__name__)) def authenticate(self): try: @@ -262,6 +259,12 @@ class XMLRPCRequestHandler(SimpleXMLRPCServer.SimpleXMLRPCRequestHandler): raise else: # got a valid XML RPC response + # first, check ACLs + client_address = self.request.getpeername() + method = xmlrpclib.loads(data)[1] + if not self.server.instance.check_acls(client_address, method): + self.send_error(401, self.responses[401][0]) + self.end_headers() try: self.send_response(200) self.send_header("Content-type", "text/xml") diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py index ba837f0c9..4cb148bb0 100644 --- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py +++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py @@ -812,32 +812,44 @@ class TestStructFile(TestXMLFileBacked): sf._include_element = Mock() metadata = Mock() - (xdata, groups, subgroups, children, subchildren, standalone) = \ - self._get_test_data() - sf._include_element.side_effect = \ lambda x, _: (x.tag not in sf._include_tests.keys() or x.get("include") == "true") - for i, group in groups.items(): - actual = sf._match(group, metadata) - expected = children[i] + subchildren[i] - self.assertEqual(len(actual), len(expected)) - # easiest way to compare the values is actually to make - # them into an XML document and let assertXMLEqual compare - # them - xactual = lxml.etree.Element("Container") - xactual.extend(actual) - xexpected = lxml.etree.Element("Container") - xexpected.extend(expected) - self.assertXMLEqual(xactual, xexpected) + for test_data in [self._get_test_data(), + self._get_template_test_data()]: + (xdata, groups, subgroups, children, subchildren, standalone) = \ + test_data - for el in standalone: - self.assertXMLEqual(el, sf._match(el, metadata)[0]) + for i, group in groups.items(): + actual = sf._match(group, metadata) + expected = children[i] + subchildren[i] + self.assertEqual(len(actual), len(expected)) + # easiest way to compare the values is actually to make + # them into an XML document and let assertXMLEqual compare + # them + xactual = lxml.etree.Element("Container") + xactual.extend(actual) + xexpected = lxml.etree.Element("Container") + xexpected.extend(expected) + self.assertXMLEqual(xactual, xexpected) + + for el in standalone: + self.assertXMLEqual(el, sf._match(el, metadata)[0]) def test_do_match(self): sf = self.get_obj() sf._match = Mock() + + def match_rv(el, _): + if el.tag not in sf._include_tests.keys(): + return [el] + elif el.get("include") == "true": + return el.getchildren() + else: + return [] + sf._match.side_effect = match_rv + metadata = Mock() for test_data in [self._get_test_data(), @@ -847,14 +859,6 @@ class TestStructFile(TestXMLFileBacked): sf.data = lxml.etree.tostring(xdata) sf.Index() - def match_rv(el, _): - if el.tag not in sf._include_tests.keys(): - return [el] - elif el.get("include") == "true": - return el.getchildren() - else: - return [] - sf._match.side_effect = match_rv actual = sf._do_match(metadata) expected = reduce(lambda x, y: x + y, list(children.values()) + \ @@ -874,45 +878,96 @@ class TestStructFile(TestXMLFileBacked): sf._include_element = Mock() metadata = Mock() - (xdata, groups, subgroups, children, subchildren, standalone) = \ - self._get_test_data() - sf._include_element.side_effect = \ lambda x, _: (x.tag not in sf._include_tests.keys() or x.get("include") == "true") - actual = copy.deepcopy(xdata) - for el in actual.getchildren(): - sf._xml_match(el, metadata) - expected = lxml.etree.Element(xdata.tag, **dict(xdata.attrib)) - expected.text = xdata.text - expected.extend(reduce(lambda x, y: x + y, - list(children.values()) + list(subchildren.values()))) - expected.extend(standalone) - self.assertXMLEqual(actual, expected) + for test_data in [self._get_test_data(), + self._get_template_test_data()]: + (xdata, groups, subgroups, children, subchildren, standalone) = \ + test_data + + actual = copy.deepcopy(xdata) + for el in actual.getchildren(): + sf._xml_match(el, metadata) + expected = lxml.etree.Element(xdata.tag, **dict(xdata.attrib)) + expected.text = xdata.text + expected.extend(reduce(lambda x, y: x + y, + list(children.values()) + \ + list(subchildren.values()))) + expected.extend(standalone) + self.assertXMLEqual(actual, expected) def test_do_xmlmatch(self): sf = self.get_obj() sf._xml_match = Mock() metadata = Mock() - (sf.xdata, groups, subgroups, children, subchildren, standalone) = \ - self._get_test_data() + for data_type, test_data in \ + [("", self._get_test_data()), + ("templated ", self._get_template_test_data())]: + (xdata, groups, subgroups, children, subchildren, standalone) = \ + test_data + sf.xdata = xdata + sf._xml_match.reset_mock() + + sf._do_xmlmatch(metadata) + actual = [] + for call in sf._xml_match.call_args_list: + actual.append(call[0][0]) + self.assertEqual(call[0][1], metadata) + expected = list(groups.values()) + standalone + # easiest way to compare the values is actually to make + # them into an XML document and let assertXMLEqual compare + # them + xactual = lxml.etree.Element("Container") + xactual.extend(actual) + xexpected = lxml.etree.Element("Container") + xexpected.extend(expected) + self.assertXMLEqual(xactual, xexpected, + "XMLMatch() calls were incorrect for " + "%stest data" % data_type) + + def test_match_ordering(self): + """ Match() returns elements in document order """ + sf = self.get_obj() + sf._match = Mock() + + def match_rv(el, _): + if el.tag not in sf._include_tests.keys(): + return [el] + elif el.get("include") == "true": + return el.getchildren() + else: + return [] + sf._match.side_effect = match_rv - sf._do_xmlmatch(metadata) - actual = [] - for call in sf._xml_match.call_args_list: - actual.append(call[0][0]) - self.assertEqual(call[0][1], metadata) - expected = list(groups.values()) + standalone - # easiest way to compare the values is actually to make - # them into an XML document and let assertXMLEqual compare - # them - xactual = lxml.etree.Element("Container") - xactual.extend(actual) - xexpected = lxml.etree.Element("Container") - xexpected.extend(expected) - self.assertXMLEqual(xactual, xexpected) + metadata = Mock() + + test_data = lxml.etree.Element("Test") + group = lxml.etree.SubElement(test_data, "Group", name="group", + include="true") + first = lxml.etree.SubElement(group, "Element", name="first") + second = lxml.etree.SubElement(test_data, "Element", name="second") + + # sanity check to ensure that first and second are in the + # correct document order + if test_data.xpath("//Element") != [first, second]: + skip("lxml.etree does not construct documents in a reliable order") + + sf.data = lxml.etree.tostring(test_data) + sf.Index() + rv = sf._do_match(metadata) + self.assertEqual(len(rv), 2, + "Match() seems to be broken, cannot test ordering") + msg = "Match() does not return elements in document order:\n" + \ + "Expected: [%s, %s]\n" % (first, second) + \ + "Actual: %s" % rv + self.assertXMLEqual(rv[0], first, msg) + self.assertXMLEqual(rv[1], second, msg) + + # TODO: add tests to ensure that XMLMatch() returns elements + # in document order class TestINode(Bcfg2TestCase): diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py index 35f4e0700..6effe05de 100644 --- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py +++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py @@ -368,3 +368,21 @@ class TestVersion(TestPlugin): class TestClientRunHooks(Bcfg2TestCase): """ placeholder for future tests """ pass + + +class TestClientACLs(Bcfg2TestCase): + test_obj = ClientACLs + + def get_obj(self): + return self.test_obj() + + def test_check_acl_ip(self): + ca = self.get_obj() + self.assertIn(ca.check_acl_ip(Mock(), Mock()), + [True, False, None]) + + def test_check_acl_metadata(self): + ca = self.get_obj() + self.assertIn(ca.check_acl_metadata(Mock(), Mock()), + [True, False]) + diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestACL.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestACL.py new file mode 100644 index 000000000..e457ca7c1 --- /dev/null +++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestACL.py @@ -0,0 +1,222 @@ +import os +import sys +import lxml.etree +import Bcfg2.Server.Plugin +from mock import Mock, MagicMock, patch +from Bcfg2.Server.Plugins.ACL import * + +# add all parent testsuite directories to sys.path to allow (most) +# relative imports in python 2.4 +path = os.path.dirname(__file__) +while path != "/": + if os.path.basename(path).lower().startswith("test"): + sys.path.append(path) + if os.path.basename(path) == "testsuite": + break + path = os.path.dirname(path) +from common import * +from TestPlugin import TestXMLFileBacked, TestStructFile, TestPlugin, \ + TestClientACLs + + +class TestFunctions(Bcfg2TestCase): + def test_rmi_names_equal(self): + good_cases = [('*', 'foo'), + ('foo', 'foo'), + ('foo.*', 'foo.bar'), + ('*.*', 'foo.bar'), + ('foo.bar', 'foo.bar'), + ('*.bar', 'foo.bar'), + ('foo.*.bar', 'foo.baz.bar')] + bad_cases = [('foo', 'bar'), + ('*', 'foo.bar'), + ('*.*', 'foo'), + ('*.*', 'foo.bar.baz'), + ('foo.*', 'bar.foo'), + ('*.bar', 'bar.foo'), + ('foo.*', 'foobar')] + for first, second in good_cases: + self.assertTrue(rmi_names_equal(first, second), + "rmi_names_equal(%s, %s) unexpectedly False" % + (first, second)) + self.assertTrue(rmi_names_equal(second, first), + "rmi_names_equal(%s, %s) unexpectedly False" % + (second, first)) + for first, second in bad_cases: + self.assertFalse(rmi_names_equal(first, second), + "rmi_names_equal(%s, %s) unexpectedly True" % + (first, second)) + self.assertFalse(rmi_names_equal(second, first), + "rmi_names_equal(%s, %s) unexpectedly True" % + (second, first)) + + def test_ip_matches(self): + good_cases = [ + ("192.168.1.1", lxml.etree.Element("test", address="192.168.1.1")), + ("192.168.1.17", lxml.etree.Element("test", address="192.168.1.0", + netmask="24")), + ("192.168.1.17", lxml.etree.Element("test", address="192.168.1.0", + netmask="255.255.255.0")), + ("192.168.1.31", lxml.etree.Element("test", address="192.168.1.0", + netmask="255.255.255.224")), + ("192.168.1.31", lxml.etree.Element("test", address="192.168.1.0", + netmask="27")), + ("10.55.67.191", lxml.etree.Element("test", address="10.55.0.0", + netmask="16"))] + bad_cases = [ + ("192.168.1.1", lxml.etree.Element("test", address="192.168.1.2")), + ("192.168.2.17", lxml.etree.Element("test", address="192.168.1.0", + netmask="24")), + ("192.168.2.17", lxml.etree.Element("test", address="192.168.1.0", + netmask="255.255.255.0")), + ("192.168.1.35", lxml.etree.Element("test", address="192.168.1.0", + netmask="255.255.255.224")), + ("192.168.1.35", lxml.etree.Element("test", address="192.168.1.0", + netmask="27")), + ("10.56.67.191", lxml.etree.Element("test", address="10.55.0.0", + netmask="16"))] + for ip, entry in good_cases: + self.assertTrue(ip_matches(ip, entry), + "ip_matches(%s, %s) unexpectedly False" % + (ip, lxml.etree.tostring(entry))) + for ip, entry in bad_cases: + self.assertFalse(ip_matches(ip, entry), + "ip_matches(%s, %s) unexpectedly True" % + (ip, lxml.etree.tostring(entry))) + + +class TestIPACLFile(TestXMLFileBacked): + test_obj = IPACLFile + + @patch("Bcfg2.Server.Plugins.ACL.ip_matches") + @patch("Bcfg2.Server.Plugins.ACL.rmi_names_equal") + def test_check_acl(self, mock_rmi_names_equal, mock_ip_matches): + af = self.get_obj() + ip = "10.0.0.8" + rmi = "ACL.test" + + def reset(): + mock_rmi_names_equal.reset_mock() + mock_ip_matches.reset_mock() + + # test default defer with no entries + af.entries = [] + self.assertIsNone(af.check_acl(ip, rmi)) + + # test explicit allow, deny, and defer + entries = dict(Allow=lxml.etree.Element("Allow", method=rmi), + Deny=lxml.etree.Element("Deny", method=rmi), + Defer=lxml.etree.Element("Defer", method=rmi)) + af.entries = list(entries.values()) + + def get_ip_matches(tag): + def ip_matches(ip, entry): + return entry.tag == tag + + return ip_matches + + mock_rmi_names_equal.return_value = True + + reset() + mock_ip_matches.side_effect = get_ip_matches("Allow") + self.assertTrue(af.check_acl(ip, rmi)) + mock_ip_matches.assert_called_with(ip, entries['Allow']) + mock_rmi_names_equal.assert_called_with(rmi, rmi) + + reset() + mock_ip_matches.side_effect = get_ip_matches("Deny") + self.assertFalse(af.check_acl(ip, rmi)) + mock_ip_matches.assert_called_with(ip, entries['Deny']) + mock_rmi_names_equal.assert_called_with(rmi, rmi) + + reset() + mock_ip_matches.side_effect = get_ip_matches("Defer") + self.assertIsNone(af.check_acl(ip, rmi)) + mock_ip_matches.assert_called_with(ip, entries['Defer']) + mock_rmi_names_equal.assert_called_with(rmi, rmi) + + # test matching RMI names + reset() + mock_ip_matches.side_effect = lambda i, e: True + mock_rmi_names_equal.side_effect = lambda a, b: a == b + rmi = "ACL.test2" + matching = lxml.etree.Element("Allow", method=rmi) + af.entries.append(matching) + self.assertTrue(af.check_acl(ip, rmi)) + mock_ip_matches.assert_called_with(ip, matching) + self.assertTrue( + call('ACL.test', rmi) in mock_rmi_names_equal.call_args_list or + call(rmi, 'ACL.test') in mock_rmi_names_equal.call_args_list) + + # test implicit allow for localhost, defer for others + reset() + mock_ip_matches.side_effect = lambda i, e: False + self.assertIsNone(af.check_acl(ip, rmi)) + + reset() + self.assertTrue(af.check_acl("127.0.0.1", rmi)) + + +class TestMetadataACLFile(TestStructFile): + test_obj = MetadataACLFile + + @patch("Bcfg2.Server.Plugins.ACL.rmi_names_equal") + def test_check_acl(self, mock_rmi_names_equal): + af = self.get_obj() + af.Match = Mock() + metadata = Mock() + mock_rmi_names_equal.side_effect = lambda a, b: a == b + + def reset(): + af.Match.reset_mock() + mock_rmi_names_equal.reset_mock() + + # test default allow + af.entries = [] + self.assertTrue(af.check_acl(metadata, 'ACL.test')) + + # test explicit allow and deny + reset() + af.entries = [lxml.etree.Element("Allow", method='ACL.test'), + lxml.etree.Element("Deny", method='ACL.test2')] + af.Match.return_value = af.entries + self.assertTrue(af.check_acl(metadata, 'ACL.test')) + af.Match.assert_called_with(metadata) + self.assertIn(call('ACL.test', 'ACL.test'), + mock_rmi_names_equal.call_args_list) + + reset() + self.assertFalse(af.check_acl(metadata, 'ACL.test2')) + af.Match.assert_called_with(metadata) + self.assertIn(call('ACL.test2', 'ACL.test2'), + mock_rmi_names_equal.call_args_list) + + # test default deny for non-localhost + reset() + self.assertFalse(af.check_acl(metadata, 'ACL.test3')) + af.Match.assert_called_with(metadata) + + # test default allow for localhost + reset() + metadata.hostname = 'localhost' + self.assertTrue(af.check_acl(metadata, 'ACL.test3')) + af.Match.assert_called_with(metadata) + + +class TestACL(TestPlugin, TestClientACLs): + test_obj = ACL + + def test_check_acl_ip(self): + acl = self.get_obj() + acl.ip_acls = Mock() + self.assertEqual(acl.check_acl_ip("192.168.1.10", "ACL.test"), + acl.ip_acls.check_acl.return_value) + acl.ip_acls.check_acl.assert_called_with("192.168.1.10", "ACL.test") + + def test_check_acl_metadata(self): + acl = self.get_obj() + acl.metadata_acls = Mock() + metadata = Mock() + self.assertEqual(acl.check_acl_metadata(metadata, "ACL.test"), + acl.metadata_acls.check_acl.return_value) + acl.metadata_acls.check_acl.assert_called_with(metadata, "ACL.test") -- cgit v1.2.3-1-g7c22