From e51614dbcea65eee92bb1a726449a66be41de55b Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 11 Sep 2013 10:07:02 -0400 Subject: XMLFileBacked: Fixed multiple identical XIncludes in one file --- src/lib/Bcfg2/Server/Plugin/helpers.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/lib/Bcfg2/Server/Plugin/helpers.py b/src/lib/Bcfg2/Server/Plugin/helpers.py index 3a994606c..ea94f71de 100644 --- a/src/lib/Bcfg2/Server/Plugin/helpers.py +++ b/src/lib/Bcfg2/Server/Plugin/helpers.py @@ -555,16 +555,12 @@ class XMLFileBacked(FileBacked): xdata = self.xdata.getroottree() else: xdata = lxml.etree.parse(fname) - included = [el for el in xdata.findall('//' + xinclude)] - for el in included: + for el in xdata.findall('//' + xinclude): name = el.get("href") if name.startswith("/"): fpath = name else: - if fname: - rel = fname - else: - rel = self.name + rel = fname or self.name fpath = os.path.join(os.path.dirname(rel), name) # expand globs in xinclude, a bcfg2-specific extension @@ -579,12 +575,13 @@ class XMLFileBacked(FileBacked): parent = el.getparent() parent.remove(el) for extra in extras: - if extra != self.name and extra not in self.extras: - self.extras.append(extra) - lxml.etree.SubElement(parent, xinclude, href=extra) - self._follow_xincludes(fname=extra) - if extra not in self.extra_monitors: - self.add_monitor(extra) + if extra != self.name: + added = lxml.etree.SubElement(parent, xinclude, href=extra) + if extra not in self.extras: + self.extras.append(extra) + self._follow_xincludes(fname=extra) + if extra not in self.extra_monitors: + self.add_monitor(extra) def Index(self): self.xdata = lxml.etree.XML(self.data, base_url=self.name, -- cgit v1.2.3-1-g7c22 From 18701bbeb2858e08d3d6755b5335a756ed20c5ca Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 11 Sep 2013 10:43:49 -0400 Subject: XMLFileBacked: removed unused variable --- src/lib/Bcfg2/Server/Plugin/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Bcfg2/Server/Plugin/helpers.py b/src/lib/Bcfg2/Server/Plugin/helpers.py index ea94f71de..d9e208746 100644 --- a/src/lib/Bcfg2/Server/Plugin/helpers.py +++ b/src/lib/Bcfg2/Server/Plugin/helpers.py @@ -576,7 +576,7 @@ class XMLFileBacked(FileBacked): parent.remove(el) for extra in extras: if extra != self.name: - added = lxml.etree.SubElement(parent, xinclude, href=extra) + lxml.etree.SubElement(parent, xinclude, href=extra) if extra not in self.extras: self.extras.append(extra) self._follow_xincludes(fname=extra) -- cgit v1.2.3-1-g7c22 From 5f98fa9d7cf175d565905189018a758adc1431b5 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Thu, 12 Sep 2013 09:21:13 -0400 Subject: Remove all ACLs (including mask) from entries with no ACLs listed When installing an entry with no ACLs specified, but with ACLs on the file as it exists on the filesystem, the ACL mask was preserved, even as the ACLs are deleted. --- src/lib/Bcfg2/Client/Tools/POSIX/base.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lib/Bcfg2/Client/Tools/POSIX/base.py b/src/lib/Bcfg2/Client/Tools/POSIX/base.py index 1e73d4f11..85da3576b 100644 --- a/src/lib/Bcfg2/Client/Tools/POSIX/base.py +++ b/src/lib/Bcfg2/Client/Tools/POSIX/base.py @@ -232,6 +232,11 @@ class POSIXTool(Bcfg2.Client.Tools.Tool): else: defacl = None + if not acls: + self.logger.debug("POSIX: Removed ACLs from %s" % + entry.get("name")) + return True + for aclkey, perms in acls.items(): atype, scope, qualifier = aclkey if atype == "default": -- cgit v1.2.3-1-g7c22 From b03e1e47c9805332cd83dcc5cf3e68e0b3c8175a Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Fri, 13 Sep 2013 15:19:56 -0400 Subject: CfgPublicKeyCreator: properly handle case where only private key has been created Previously, only two cases were handled properly: both public and private keys had been created; or neither had been created. If the private key had been created (e.g., manually added to the repo), the public key would not be created from it. This fixes that. --- .../Server/Plugins/Cfg/CfgPrivateKeyCreator.py | 16 +--- .../Server/Plugins/Cfg/CfgPublicKeyCreator.py | 53 ++++++++--- .../TestCfg/TestCfgPrivateKeyCreator.py | 19 +--- .../TestPlugins/TestCfg/TestCfgPublicKeyCreator.py | 106 ++++++++++++++++++--- 4 files changed, 137 insertions(+), 57 deletions(-) diff --git a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py index c0a3036a9..e890fdecb 100644 --- a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py +++ b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py @@ -159,7 +159,7 @@ class CfgPrivateKeyCreator(CfgCreator, StructFile): return specificity # pylint: disable=W0221 - def create_data(self, entry, metadata, return_pair=False): + def create_data(self, entry, metadata): """ Create data for the given entry on the given client :param entry: The abstract entry to create data for. This @@ -167,15 +167,7 @@ class CfgPrivateKeyCreator(CfgCreator, StructFile): :type entry: lxml.etree._Element :param metadata: The client metadata to create data for :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata - :param return_pair: Return a tuple of ``(public key, private - key)`` instead of just the private key. - This is used by - :class:`Bcfg2.Server.Plugins.Cfg.CfgPublicKeyCreator.CfgPublicKeyCreator` - to create public keys as requested. - :type return_pair: bool :returns: string - The private key data - :returns: tuple - Tuple of ``(public key, private key)``, if - ``return_pair`` is set to True """ spec = self.XMLMatch(metadata) specificity = self.get_specificity(metadata, spec) @@ -201,11 +193,7 @@ class CfgPrivateKeyCreator(CfgCreator, StructFile): specificity['ext'] = '.crypt' self.write_data(privkey, **specificity) - - if return_pair: - return (pubkey, privkey) - else: - return privkey + return privkey finally: shutil.rmtree(os.path.dirname(filename)) # pylint: enable=W0221 diff --git a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py index 6be438462..4bd8690ed 100644 --- a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py +++ b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPublicKeyCreator.py @@ -2,7 +2,11 @@ :class:`Bcfg2.Server.Plugins.Cfg.CfgPrivateKeyCreator.CfgPrivateKeyCreator` to create SSH keys on the fly. """ +import os +import sys +import tempfile import lxml.etree +from Bcfg2.Utils import Executor from Bcfg2.Server.Plugin import StructFile, PluginExecutionError from Bcfg2.Server.Plugins.Cfg import CfgCreator, CfgCreationError, CFG @@ -27,7 +31,8 @@ class CfgPublicKeyCreator(CfgCreator, StructFile): CfgCreator.__init__(self, fname) StructFile.__init__(self, fname) self.cfg = CFG - __init__.__doc__ = CfgCreator.__init__.__doc__ + self.core = CFG.core + self.cmd = Executor() def create_data(self, entry, metadata): if entry.get("name").endswith(".pub"): @@ -37,25 +42,51 @@ class CfgPublicKeyCreator(CfgCreator, StructFile): "%s: Filename does not end in .pub" % entry.get("name")) - if privkey not in self.cfg.entries: - raise CfgCreationError("Cfg: Could not find Cfg entry for %s " - "(private key for %s)" % (privkey, - self.name)) - eset = self.cfg.entries[privkey] + privkey_entry = lxml.etree.Element("Path", name=privkey) try: + self.core.Bind(privkey_entry, metadata) + except PluginExecutionError: + raise CfgCreationError("Cfg: Could not bind %s (private key for " + "%s): %s" % (privkey, self.name, + sys.exc_info()[1])) + + try: + eset = self.cfg.entries[privkey] creator = eset.best_matching(metadata, eset.get_handlers(metadata, CfgCreator)) + except KeyError: + raise CfgCreationError("Cfg: No private key defined for %s (%s)" % + (self.name, privkey)) except PluginExecutionError: raise CfgCreationError("Cfg: No privkey.xml defined for %s " "(private key for %s)" % (privkey, self.name)) - privkey_entry = lxml.etree.Element("Path", name=privkey) - pubkey = creator.create_data(privkey_entry, metadata, - return_pair=True)[0] - return pubkey - create_data.__doc__ = CfgCreator.create_data.__doc__ + specificity = creator.get_specificity(metadata) + fname = self.get_filename(**specificity) + + # if the private key didn't exist, then creating it may have + # created the private key, too. check for it first. + if os.path.exists(fname): + return open(fname).read() + else: + # generate public key from private key + fd, privfile = tempfile.mkstemp() + try: + os.fdopen(fd, 'w').write(privkey_entry.text) + cmd = ["ssh-keygen", "-y", "-f", privfile] + self.debug_log("Cfg: Extracting SSH public key from %s: %s" % + (privkey, " ".join(cmd))) + result = self.cmd.run(cmd) + if not result.success: + raise CfgCreationError("Cfg: Failed to extract public key " + "from %s: %s" % (privkey, + result.error)) + self.write_data(result.stdout, **specificity) + return result.stdout + finally: + os.unlink(privfile) def handle_event(self, event): CfgCreator.handle_event(self, event) diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py index dc4b11241..e139a592b 100644 --- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py +++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py @@ -31,6 +31,7 @@ class TestCfgPrivateKeyCreator(TestCfgCreator, TestStructFile): should_monitor = False def get_obj(self, name=None, fam=None): + Bcfg2.Server.Plugins.Cfg.CfgPublicKeyCreator.CFG = Mock() return TestCfgCreator.get_obj(self, name=name) @patch("Bcfg2.Server.Plugins.Cfg.CfgCreator.handle_event") @@ -259,24 +260,6 @@ class TestCfgPrivateKeyCreator(TestCfgCreator, TestStructFile): pkc.write_data.assert_called_with("privatekey", group="foo") mock_rmtree.assert_called_with(datastore) - reset() - self.assertEqual(pkc.create_data(entry, metadata, return_pair=True), - ("ssh-rsa publickey pubkey.filename\n", - "privatekey")) - pkc.XMLMatch.assert_called_with(metadata) - pkc.get_specificity.assert_called_with(metadata, - pkc.XMLMatch.return_value) - pkc._gen_keypair.assert_called_with(metadata, - pkc.XMLMatch.return_value) - self.assertItemsEqual(mock_open.call_args_list, - [call(privkey + ".pub"), call(privkey)]) - pkc.pubkey_creator.get_filename.assert_called_with(group="foo") - pkc.pubkey_creator.write_data.assert_called_with( - "ssh-rsa publickey pubkey.filename\n", - group="foo") - pkc.write_data.assert_called_with("privatekey", group="foo") - mock_rmtree.assert_called_with(datastore) - inner() if HAS_CRYPTO: diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPublicKeyCreator.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPublicKeyCreator.py index 04772cf9a..ed529253b 100644 --- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPublicKeyCreator.py +++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPublicKeyCreator.py @@ -26,6 +26,7 @@ class TestCfgPublicKeyCreator(TestCfgCreator, TestStructFile): should_monitor = False def get_obj(self, name=None, fam=None): + Bcfg2.Server.Plugins.Cfg.CfgPublicKeyCreator.CFG = Mock() return TestCfgCreator.get_obj(self, name=name) @patch("Bcfg2.Server.Plugins.Cfg.CfgCreator.handle_event") @@ -37,41 +38,118 @@ class TestCfgPublicKeyCreator(TestCfgCreator, TestStructFile): mock_HandleEvent.assert_called_with(pkc, evt) mock_handle_event.assert_called_with(pkc, evt) - def test_create_data(self): + @patch("os.unlink") + @patch("os.path.exists") + @patch("tempfile.mkstemp") + @patch("os.fdopen", Mock()) + @patch("%s.open" % builtins) + def test_create_data(self, mock_open, mock_mkstemp, mock_exists, + mock_unlink): metadata = Mock() pkc = self.get_obj() pkc.cfg = Mock() + pkc.core = Mock() + pkc.cmd = Mock() + pkc.get_filename = Mock() + pkc.write_data = Mock() + pubkey = "public key data" privkey_entryset = Mock() privkey_creator = Mock() - pubkey = Mock() - privkey = Mock() - privkey_creator.create_data.return_value = (pubkey, privkey) - privkey_entryset.best_matching.return_value = privkey_creator + privkey_creator.get_specificity = Mock() + privkey_creator.get_specificity.return_value = MagicMock() pkc.cfg.entries = {"/home/foo/.ssh/id_rsa": privkey_entryset} + def reset(): + privkey_creator.reset_mock() + pkc.cmd.reset_mock() + pkc.core.reset_mock() + pkc.get_filename.reset_mock() + pkc.write_data.reset_mock() + mock_exists.reset_mock() + mock_unlink.reset_mock() + mock_mkstemp.reset_mock() + mock_open.reset_mock() + # public key doesn't end in .pub entry = lxml.etree.Element("Path", name="/home/bar/.ssh/bogus") self.assertRaises(CfgCreationError, pkc.create_data, entry, metadata) + self.assertFalse(pkc.write_data.called) + + # cannot bind private key + reset() + pkc.core.Bind.side_effect = PluginExecutionError + entry = lxml.etree.Element("Path", name="/home/foo/.ssh/id_rsa.pub") + self.assertRaises(CfgCreationError, + pkc.create_data, entry, metadata) + self.assertFalse(pkc.write_data.called) # private key not in cfg.entries + reset() + pkc.core.Bind.side_effect = None + pkc.core.Bind.return_value = "private key data" entry = lxml.etree.Element("Path", name="/home/bar/.ssh/id_rsa.pub") self.assertRaises(CfgCreationError, pkc.create_data, entry, metadata) + self.assertFalse(pkc.write_data.called) - # successful operation + # no privkey.xml defined + reset() + privkey_entryset.best_matching.side_effect = PluginExecutionError + entry = lxml.etree.Element("Path", name="/home/foo/.ssh/id_rsa.pub") + self.assertRaises(CfgCreationError, + pkc.create_data, entry, metadata) + self.assertFalse(pkc.write_data.called) + + # successful operation, create new key + reset() + pkc.cmd.run.return_value = Mock() + pkc.cmd.run.return_value.success = True + pkc.cmd.run.return_value.stdout = pubkey + mock_mkstemp.return_value = (Mock(), str(Mock())) + mock_exists.return_value = False + privkey_entryset.best_matching.side_effect = None + privkey_entryset.best_matching.return_value = privkey_creator entry = lxml.etree.Element("Path", name="/home/foo/.ssh/id_rsa.pub") self.assertEqual(pkc.create_data(entry, metadata), pubkey) + self.assertTrue(pkc.core.Bind.called) + (privkey_entry, md) = pkc.core.Bind.call_args[0] + self.assertXMLEqual(privkey_entry, + lxml.etree.Element("Path", + name="/home/foo/.ssh/id_rsa")) + self.assertEqual(md, metadata) + privkey_entryset.get_handlers.assert_called_with(metadata, CfgCreator) - privkey_entryset.best_matching.assert_called_with(metadata, - privkey_entryset.get_handlers.return_value) - self.assertXMLEqual(privkey_creator.create_data.call_args[0][0], + privkey_entryset.best_matching.assert_called_with( + metadata, + privkey_entryset.get_handlers.return_value) + mock_exists.assert_called_with(pkc.get_filename.return_value) + pkc.cmd.run.assert_called_with(["ssh-keygen", "-y", "-f", + mock_mkstemp.return_value[1]]) + self.assertEqual(pkc.write_data.call_args[0][0], pubkey) + mock_unlink.assert_called_with(mock_mkstemp.return_value[1]) + self.assertFalse(mock_open.called) + + # successful operation, no need to create new key + reset() + mock_exists.return_value = True + mock_open.return_value = Mock() + mock_open.return_value.read.return_value = pubkey + pkc.cmd.run.return_value.stdout = None + self.assertEqual(pkc.create_data(entry, metadata), pubkey) + self.assertTrue(pkc.core.Bind.called) + (privkey_entry, md) = pkc.core.Bind.call_args[0] + self.assertXMLEqual(privkey_entry, lxml.etree.Element("Path", name="/home/foo/.ssh/id_rsa")) - self.assertEqual(privkey_creator.create_data.call_args[0][1], metadata) + self.assertEqual(md, metadata) - # no privkey.xml - privkey_entryset.best_matching.side_effect = PluginExecutionError - self.assertRaises(CfgCreationError, - pkc.create_data, entry, metadata) + privkey_entryset.get_handlers.assert_called_with(metadata, CfgCreator) + privkey_entryset.best_matching.assert_called_with( + metadata, + privkey_entryset.get_handlers.return_value) + mock_exists.assert_called_with(pkc.get_filename.return_value) + mock_open.assert_called_with(pkc.get_filename.return_value) + self.assertFalse(mock_mkstemp.called) + self.assertFalse(pkc.write_data.called) -- cgit v1.2.3-1-g7c22