Interdiff:
diff --git a/lib/utils/security.py b/lib/utils/security.py
index 221b587..10d515a 100644
--- a/lib/utils/security.py
+++ b/lib/utils/security.py
@@ -48,7 +48,7 @@ def AddNodeToCandidateCerts(node_uuid, cert_digest,
candidate_certs,
old_cert_digest = candidate_certs[node_uuid]
if old_cert_digest == cert_digest:
info_fn("Certificate digest for node %s already in config."
- "Not doing anything.")
+ "Not doing anything." % node_uuid)
return
else:
warn_fn("Overriding differing certificate digest for node %s"
@@ -71,6 +71,6 @@ def RemoveNodeFromCandidateCerts(node_uuid,
candidate_certs,
"""
if node_uuid not in candidate_certs:
warn_fn("Cannot remove certifcate for node %s, because it's not in the"
- "candidate map.")
+ "candidate map." % node_uuid)
return
del candidate_certs[node_uuid]
On Thu, Dec 19, 2013 at 4:44 PM, Helga Velroyen <[email protected]> wrote:
>
>
>
> On Thu, Dec 19, 2013 at 4:23 PM, Hrvoje Ribicic <[email protected]> wrote:
>
>>
>>
>> On Thu, Dec 19, 2013 at 3:49 PM, Helga Velroyen <[email protected]>wrote:
>>
>>> This patch adds a couple of utility functions to manipulate
>>> the map of master candidate SSL certificate digests.
>>>
>>> Signed-off-by: Helga Velroyen <[email protected]>
>>> ---
>>> Makefile.am | 2 +
>>> lib/utils/security.py | 76
>>> +++++++++++++++++++++++++++++++
>>> test/py/ganeti.utils.security_unittest.py | 74
>>> ++++++++++++++++++++++++++++++
>>> 3 files changed, 152 insertions(+)
>>> create mode 100644 lib/utils/security.py
>>> create mode 100755 test/py/ganeti.utils.security_unittest.py
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index e74f58b..96e1284 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -490,6 +490,7 @@ utils_PYTHON = \
>>> lib/utils/nodesetup.py \
>>> lib/utils/process.py \
>>> lib/utils/retry.py \
>>> + lib/utils/security.py \
>>> lib/utils/storage.py \
>>> lib/utils/text.py \
>>> lib/utils/version.py \
>>> @@ -1473,6 +1474,7 @@ python_tests = \
>>> test/py/ganeti.utils.nodesetup_unittest.py \
>>> test/py/ganeti.utils.process_unittest.py \
>>> test/py/ganeti.utils.retry_unittest.py \
>>> + test/py/ganeti.utils.security_unittest.py \
>>> test/py/ganeti.utils.storage_unittest.py \
>>> test/py/ganeti.utils.text_unittest.py \
>>> test/py/ganeti.utils.version_unittest.py \
>>> diff --git a/lib/utils/security.py b/lib/utils/security.py
>>> new file mode 100644
>>> index 0000000..221b587
>>> --- /dev/null
>>> +++ b/lib/utils/security.py
>>> @@ -0,0 +1,76 @@
>>> +#
>>> +#
>>> +
>>> +# Copyright (C) 2013 Google Inc.
>>> +#
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 2 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# 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.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program; if not, write to the Free Software
>>> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> +# 02110-1301, USA.
>>> +
>>> +"""Utility functions for security features of Ganeti.
>>> +
>>> +"""
>>> +
>>> +import logging
>>> +
>>> +
>>> +def AddNodeToCandidateCerts(node_uuid, cert_digest, candidate_certs,
>>> + info_fn=logging.info,
>>> warn_fn=logging.warn):
>>> + """Adds an entry to the candidate certificate map.
>>> +
>>> + @type node_uuid: string
>>> + @param node_uuid: the node's UUID
>>> + @type cert_digest: string
>>> + @param cert_digest: the digest of the node's client SSL certificate
>>> + @type candidate_certs: dict of strings to strings
>>> + @param candidate_certs: map of node UUIDs to the digests of their
>>> client
>>> + SSL certificates, will be manipulated in this function
>>> + @type info_fn: function
>>> + @param info_fn: logging function for information messages
>>> + @type warn_fn: function
>>> + @param warn_fn: logging function for warning messages
>>> +
>>> + """
>>> + assert candidate_certs is not None
>>> +
>>> + if node_uuid in candidate_certs:
>>> + old_cert_digest = candidate_certs[node_uuid]
>>> + if old_cert_digest == cert_digest:
>>> + info_fn("Certificate digest for node %s already in config."
>>> + "Not doing anything.")
>>>
>>
>> The node identifier is not injected into the logging message here.
>>
>
> Right, thanks for noticing.
>
>
>>
>>
>>> + return
>>> + else:
>>> + warn_fn("Overriding differing certificate digest for node %s"
>>> + % node_uuid)
>>>
>>
>> Regarding the node_uuid, a more general question:
>> Do we have some standards regarding what goes into a log?
>> A node uuid will identify the node, but I guess a node name is more
>> useful to the user.
>> If this is what we do elsewhere, then let it be :)
>>
>
> I don't know what the standard is in general, but while working on it, I
> found it highly practical to have the node UUIDs instead of the name,
> because when debugging this it is kind of crucial to distinguish two nodes
> that have the same host name at some point in time, for example a node can
> be removed and a different node can be added using the same name and there
> it should actually be the case that the UUID differs. Hence related to this
> SSL certificate handling, I found it highly beneficial to have the UUID,
> because otherwise you don't know which host of that name has the
> certificate.
>
>
>>
>>
>>> + candidate_certs[node_uuid] = cert_digest
>>> +
>>> +
>>> +def RemoveNodeFromCandidateCerts(node_uuid, candidate_certs,
>>> + warn_fn=logging.warn):
>>> + """Removes the entry of the given node in the certificate map.
>>> +
>>> + @type node_uuid: string
>>> + @param node_uuid: the node's UUID
>>> + @type candidate_certs: dict of strings to strings
>>> + @param candidate_certs: map of node UUIDs to the digests of their
>>> client
>>> + SSL certificates, will be manipulated in this function
>>> + @type warn_fn: function
>>> + @param warn_fn: logging function for warning messages
>>> +
>>> + """
>>> + if node_uuid not in candidate_certs:
>>> + warn_fn("Cannot remove certifcate for node %s, because it's not in
>>> the"
>>> + "candidate map.")
>>>
>>
>> Again, missing identifier, and a typo: s/certifcate/certificate/
>>
>
> Thx, will send an interdiff.
>
>
>>
>>
>>> + return
>>> + del candidate_certs[node_uuid]
>>> diff --git a/test/py/ganeti.utils.security_unittest.py b/test/py/
>>> ganeti.utils.security_unittest.py
>>> new file mode 100755
>>> index 0000000..08c6b58
>>> --- /dev/null
>>> +++ b/test/py/ganeti.utils.security_unittest.py
>>> @@ -0,0 +1,74 @@
>>> +#!/usr/bin/python
>>> +#
>>> +
>>> +# Copyright (C) 2013 Google Inc.
>>> +#
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 2 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# 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.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program; if not, write to the Free Software
>>> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> +# 02110-1301, USA.
>>> +
>>> +
>>> +"""Script for unittesting the ganeti.utils.storage module"""
>>> +
>>> +import mock
>>> +import unittest
>>> +
>>> +from ganeti.utils import security
>>> +
>>> +import testutils
>>> +
>>> +
>>> +class TestCandidateCerts(unittest.TestCase):
>>> +
>>> + def setUp(self):
>>> + self._warn_fn = mock.Mock()
>>> + self._info_fn = mock.Mock()
>>> + self._candidate_certs = {}
>>> +
>>> + def testAddAndRemoveCerts(self):
>>> + self.assertEqual(0, len(self._candidate_certs))
>>> +
>>> + node_uuid = "1234"
>>> + cert_digest = "foobar"
>>> + security.AddNodeToCandidateCerts(node_uuid, cert_digest,
>>> + self._candidate_certs, warn_fn=self._warn_fn,
>>> info_fn=self._info_fn)
>>> + self.assertEqual(1, len(self._candidate_certs))
>>> +
>>> + # Try adding the same cert again
>>> + security.AddNodeToCandidateCerts(node_uuid, cert_digest,
>>> + self._candidate_certs, warn_fn=self._warn_fn,
>>> info_fn=self._info_fn)
>>> + self.assertEqual(1, len(self._candidate_certs))
>>> + self.assertTrue(self._candidate_certs[node_uuid] == cert_digest)
>>> +
>>> + # Overriding cert
>>> + other_digest = "barfoo"
>>> + security.AddNodeToCandidateCerts(node_uuid, other_digest,
>>> + self._candidate_certs, warn_fn=self._warn_fn,
>>> info_fn=self._info_fn)
>>> + self.assertEqual(1, len(self._candidate_certs))
>>> + self.assertTrue(self._candidate_certs[node_uuid] == other_digest)
>>> +
>>> + # Try removing a certificate from a node that is not in the list
>>> + other_node_uuid = "5678"
>>> + security.RemoveNodeFromCandidateCerts(
>>> + other_node_uuid, self._candidate_certs, warn_fn=self._warn_fn)
>>> + self.assertEqual(1, len(self._candidate_certs))
>>> +
>>> + # Remove a certificate from a node that is in the list
>>> + security.RemoveNodeFromCandidateCerts(
>>> + node_uuid, self._candidate_certs, warn_fn=self._warn_fn)
>>> + self.assertEqual(0, len(self._candidate_certs))
>>> +
>>> +
>>> +if __name__ == "__main__":
>>> + testutils.GanetiTestProgram()
>>> --
>>> 1.8.5.1
>>>
>>>
>>
>
>
> --
> --
> Helga Velroyen | Software Engineer | [email protected] |
>
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
>
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
>
--
--
Helga Velroyen | Software Engineer | [email protected] |
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
