On Fri, Dec 20, 2013 at 9:59 AM, Helga Velroyen <[email protected]> wrote:
> > > > On Thu, Dec 19, 2013 at 4:59 PM, Hrvoje Ribicic <[email protected]> wrote: > >> >> >> >> On Thu, Dec 19, 2013 at 3:49 PM, Helga Velroyen <[email protected]>wrote: >> >>> In various cluster operations, the master node need to >>> >> s/need/needs/ or s/need/may need/ >> >>> retrieve the digest of a node's SSL certificate. For this >>> purpose, we add an RPC call to retrieve the digest. The >>> function is designed in a general way to make it possible >>> to retrieve other (public) cryptographic tokens of a node >>> in the future as well (for example an SSH key). >>> >>> Signed-off-by: Helga Velroyen <[email protected]> >>> --- >>> lib/backend.py | 24 ++++++++++++++++++++++++ >>> lib/rpc_defs.py | 3 +++ >>> lib/server/noded.py | 8 ++++++++ >>> lib/utils/__init__.py | 1 + >>> lib/utils/security.py | 18 ++++++++++++++++++ >>> src/Ganeti/Constants.hs | 15 +++++++++++++++ >>> test/py/ganeti.backend_unittest.py | 20 ++++++++++++++++++++ >>> test/py/ganeti.utils.security_unittest.py | 18 ++++++++++++++++++ >>> 8 files changed, 107 insertions(+) >>> >>> diff --git a/lib/backend.py b/lib/backend.py >>> index b329864..e2e3e28 100644 >>> --- a/lib/backend.py >>> +++ b/lib/backend.py >>> @@ -1162,6 +1162,30 @@ def VerifyNode(what, cluster_name, all_hvparams, >>> node_groups, groups_cfg): >>> return result >>> >>> >>> +def GetCryptoTokens(token_types): >>> + """Get the node's public cryptographic tokens. >>> + >>> + This can be the public ssh key of the node or the certifciate digest >>> of >>> >> s/certifciate/certificate/ >> >>> + the node's public client SSL certificate. >>> + >>> + Note: so far, only retrieval of the SSL digest is implemented >>> + >>> + @type token_types: list of strings in constants.CRYPTO_TYPES >>> + @param token_types: list of types of requested cryptographic tokens >>> + @rtype: list of tuples (string, string) >>> + @return: list of tuples of the token type and the public crypto token >>> + >>> + """ >>> + tokens = [] >>> + for token_type in token_types: >>> + if token_type not in constants.CRYPTO_TYPES: >>> + raise errors.ProgrammerError("Token type %s not supported." % >>> + token_type) >>> + if token_type == constants.CRYPTO_TYPE_SSL: >>> >> >> This is quite a generic name; given that this call can retrieve any sort >> of crypto token, >> there might be confusion. >> >> Consider making it more descriptive, maybe by adding a _DIGEST to the >> name? >> > > Hm, sure, will do. > > >> >> >>> + tokens.append((token_type, utils.GetClientCertificateDigest())) >>> + return tokens >>> + >>> + >>> def GetBlockDevSizes(devices): >>> """Return the size of the given block devices >>> >>> diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py >>> index b9b1905..b92f720 100644 >>> --- a/lib/rpc_defs.py >>> +++ b/lib/rpc_defs.py >>> @@ -505,6 +505,9 @@ _NODE_CALLS = [ >>> ("ovs_name", None, "Name of the OpenvSwitch to create"), >>> ("ovs_link", None, "Link of the OpenvSwitch to the outside"), >>> ], None, None, "This will create and setup the OpenvSwitch"), >>> + ("node_crypto_tokens", SINGLE, None, constants.RPC_TMO_NORMAL, [ >>> + ("token_types", None, "List of requested crypto token types"), >>> + ], None, None, "Retrieve public crypto tokes from the node."), >>> >> s/tokes/tokens/ >> >>> ] >>> >>> _MISC_CALLS = [ >>> diff --git a/lib/server/noded.py b/lib/server/noded.py >>> index 74f300d..e5bf330 100644 >>> --- a/lib/server/noded.py >>> +++ b/lib/server/noded.py >>> @@ -864,6 +864,14 @@ class >>> NodeRequestHandler(http.server.HttpServerHandler): >>> (ovs_name, ovs_link) = params >>> return backend.ConfigureOVS(ovs_name, ovs_link) >>> >>> + @staticmethod >>> + def perspective_node_crypto_tokens(params): >>> + """Gets the node's public crypto tokens. >>> + >>> + """ >>> + token_types = params[0] >>> + return backend.GetCryptoTokens(token_types) >>> + >>> # cluster -------------------------- >>> >>> @staticmethod >>> diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py >>> index 23650ca..50d88c0 100644 >>> --- a/lib/utils/__init__.py >>> +++ b/lib/utils/__init__.py >>> @@ -53,6 +53,7 @@ from ganeti.utils.mlock import * >>> from ganeti.utils.nodesetup import * >>> from ganeti.utils.process import * >>> from ganeti.utils.retry import * >>> +from ganeti.utils.security import * >>> from ganeti.utils.storage import * >>> from ganeti.utils.text import * >>> from ganeti.utils.wrapper import * >>> diff --git a/lib/utils/security.py b/lib/utils/security.py >>> index 221b587..fe1f21c 100644 >>> --- a/lib/utils/security.py >>> +++ b/lib/utils/security.py >>> @@ -23,6 +23,10 @@ >>> """ >>> >>> import logging >>> +import OpenSSL >>> + >>> +from ganeti.utils import io >>> +from ganeti import pathutils >>> >>> >>> def AddNodeToCandidateCerts(node_uuid, cert_digest, candidate_certs, >>> @@ -74,3 +78,17 @@ def RemoveNodeFromCandidateCerts(node_uuid, >>> candidate_certs, >>> "candidate map.") >>> return >>> del candidate_certs[node_uuid] >>> + >>> + >>> +def GetClientCertificateDigest(cert_filename=pathutils.NODED_CERT_FILE): >>> + """Reads the SSL certificate and returns the sha1 digest. >>> + >>> + """ >>> + # FIXME: This is supposed to read the client certificate, but >>> + # in this stage of the patch series there is no client certificate >>> + # yet, so we return the digest of the server certificate to get >>> + # the rest of the key management infrastructure running. >>> + cert_plain = io.ReadFile(cert_filename) >>> + cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, >>> + cert_plain) >>> + return cert.digest("sha1") >>> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs >>> index b27657a..09ee7b8 100644 >>> --- a/src/Ganeti/Constants.hs >>> +++ b/src/Ganeti/Constants.hs >>> @@ -4140,6 +4140,21 @@ fakeOpMasterTurndown = "OP_CLUSTER_IP_TURNDOWN" >>> fakeOpMasterTurnup :: String >>> fakeOpMasterTurnup = "OP_CLUSTER_IP_TURNUP" >>> >>> + >>> +-- * Crypto Types >>> +-- Types of cryptographic tokens used in node communication >>> + >>> +cryptoTypeSsl :: String >>> +cryptoTypeSsl = "ssl" >>> + >>> +cryptoTypeSsh :: String >>> +cryptoTypeSsh = "ssh" >>> >> >> A quick search of this patch series did not show the constant being used >> afterwards. >> Was it left behind during testing or do you want it to remain as a >> reminder? >> > > I designed this function to be reused when the second part of the 'node > secrity' design doc is implemented. There we essentially do the same with > ssh keys that we do with ssl keys now, meaning that we get individual keys > for each node. Since this patch series already got quite long, I did not > include the ssh implementation yet, but designed this function already > having the ssh option in mind. If you insist, I can make it more > ssl-customized for now, but then I would have to change it again as soon as > I start with the ssh implementation. > > No problem with that - I was just wondering why it sat there unused. > >> >>> + >>> +-- So far only ssl keys are used in the context of this constant >>> +cryptoTypes :: FrozenSet String >>> +cryptoTypes = ConstantUtils.mkSet [cryptoTypeSsl] >>> + >>> + >>> -- * SSH key types >>> >>> sshkDsa :: String >>> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ >>> ganeti.backend_unittest.py >>> index 033419e..5c30bc2 100755 >>> --- a/test/py/ganeti.backend_unittest.py >>> +++ b/test/py/ganeti.backend_unittest.py >>> @@ -73,6 +73,26 @@ class TestX509Certificates(unittest.TestCase): >>> self.assertEqual(utils.ListVisibleFiles(self.tmpdir), [name]) >>> >>> >>> +class TestGetCryptoTokens(testutils.GanetiTestCase): >>> + >>> + def setUp(self): >>> + self._digest_fn_orig = utils.GetClientCertificateDigest >>> + self._ssl_digest = "12345" >>> + utils.GetClientCertificateDigest = mock.Mock( >>> + return_value=self._ssl_digest) >>> + >>> + def tearDown(self): >>> + utils.GetClientCertificateDigest = self._digest_fn_orig >>> + >>> + def testSslToken(self): >>> + result = backend.GetCryptoTokens([constants.CRYPTO_TYPE_SSL]) >>> + self.assertTrue((constants.CRYPTO_TYPE_SSL, self._ssl_digest) in >>> result) >>> + >>> + def testUnknownToken(self): >>> + self.assertRaises(errors.ProgrammerError, >>> + backend.GetCryptoTokens, ["pink_bunny"]) >>> + >>> + >>> class TestNodeVerify(testutils.GanetiTestCase): >>> >>> def setUp(self): >>> diff --git a/test/py/ganeti.utils.security_unittest.py b/test/py/ >>> ganeti.utils.security_unittest.py >>> index 08c6b58..f42dd22 100755 >>> --- a/test/py/ganeti.utils.security_unittest.py >>> +++ b/test/py/ganeti.utils.security_unittest.py >>> @@ -70,5 +70,23 @@ class TestCandidateCerts(unittest.TestCase): >>> self.assertEqual(0, len(self._candidate_certs)) >>> >>> >>> +class TestGetCertificateDigest(testutils.GanetiTestCase): >>> + >>> + def setUp(self): >>> + testutils.GanetiTestCase.setUp(self) >>> + # certificate file that contains the certificate only >>> + self._certfilename1 = testutils.TestDataFilename("cert1.pem") >>> + # (different) certificate file that contains both, certificate >>> + # and private key >>> + self._certfilename2 = testutils.TestDataFilename("cert2.pem") >>> + >>> + def testGetCertificateDigest(self): >>> + digest1 = security.GetClientCertificateDigest( >>> + cert_filename=self._certfilename1) >>> + digest2 = security.GetClientCertificateDigest( >>> + cert_filename=self._certfilename2) >>> + self.assertFalse(digest1 == digest2) >>> + >>> + >>> if __name__ == "__main__": >>> testutils.GanetiTestProgram() >>> -- >>> 1.8.5.1 >>> >>> >> > About the typos/smaller issues in this patch, I'll send an interdiff soon. > > Cheers, > Helga > > > > -- > -- > 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 >
