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.


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

Reply via email to