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/
>

Changed it to "needs".


>  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/
>

ACK


> +  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?
>
>
>> +      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/
>

ACK


>    ]
>>
>>  _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?
>
>
>> +
>> +-- 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
>>
>>
>
Interdiff:

diff --git a/lib/backend.py b/lib/backend.py
index e2e3e28..da556fd 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -1165,7 +1165,7 @@ def VerifyNode(what, cluster_name, all_hvparams,
node_groups, groups_cfg):
 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
+  This can be the public ssh key of the node or the certificate digest of
   the node's public client SSL certificate.

   Note: so far, only retrieval of the SSL digest is implemented
@@ -1181,7 +1181,7 @@ def GetCryptoTokens(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:
+    if token_type == constants.CRYPTO_TYPE_SSL_DIGEST:
       tokens.append((token_type, utils.GetClientCertificateDigest()))
   return tokens


-cryptoTypeSsl :: String
-cryptoTypeSsl = "ssl"
+cryptoTypeSslDigest :: String
+cryptoTypeSslDigest = "ssl"

 cryptoTypeSsh :: String
 cryptoTypeSsh = "ssh"

 -- So far only ssl keys are used in the context of this constant
 cryptoTypes :: FrozenSet String
-cryptoTypes = ConstantUtils.mkSet [cryptoTypeSsl]
+cryptoTypes = ConstantUtils.mkSet [cryptoTypeSslDigest]


 -- * SSH key types
diff --git a/test/py/ganeti.backend_unittest.py b/test/py/
ganeti.backend_unittest.py
index 5c30bc2..81f3217 100755
--- a/test/py/ganeti.backend_unittest.py
+++ b/test/py/ganeti.backend_unittest.py
@@ -85,8 +85,9 @@ class TestGetCryptoTokens(testutils.GanetiTestCase):
     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)
+    result = backend.GetCryptoTokens([constants.CRYPTO_TYPE_SSL_DIGEST])
+    self.assertTrue((constants.CRYPTO_TYPE_SSL_DIGEST, self._ssl_digest)
+                    in result)diff --git a/lib/backend.py b/lib/backend.py
index e2e3e28..da556fd 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -1165,7 +1165,7 @@ def VerifyNode(what, cluster_name, all_hvparams,
node_groups, groups_cfg):
 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
+  This can be the public ssh key of the node or the certificate digest of
   the node's public client SSL certificate.

   Note: so far, only retrieval of the SSL digest is implemented
@@ -1181,7 +1181,7 @@ def GetCryptoTokens(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:
+    if token_type == constants.CRYPTO_TYPE_SSL_DIGEST:
       tokens.append((token_type, utils.GetClientCertificateDigest()))
   return tokens


   def testUnknownToken(self):
     self.assertRaises(errors.ProgrammerError,



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

Reply via email to