On Fri, Dec 20, 2013 at 12:47 PM, Hrvoje Ribicic <[email protected]> wrote:
> LGTM, thanks! > Thx. FYI: this patch is also affected by the _DIGEST renaming, but I kind of lost the diff and merged it directly. Hope you are okay without seeing the interdiff here. If you insist, I can resend the whole patch. > > > On Thu, Dec 19, 2013 at 3:49 PM, Helga Velroyen <[email protected]> wrote: > >> So far the RPC call 'node_crypto_tokens' did only retrieve >> the certificate digest of an existing certificate. This >> call is now enhanced to also create a new certificate and >> return the respective digest. This will be used in various >> operations, among those cluster init and renew-crypto. >> >> Signed-off-by: Helga Velroyen <[email protected]> >> --- >> lib/backend.py | 58 >> +++++++++++++++++++++++++++++--------- >> lib/cmdlib/common.py | 3 +- >> lib/pathutils.py | 1 + >> lib/rpc_defs.py | 5 ++-- >> lib/server/noded.py | 4 +-- >> src/Ganeti/Constants.hs | 18 ++++++++++++ >> test/py/ganeti.backend_unittest.py | 37 ++++++++++++++++++++---- >> 7 files changed, 102 insertions(+), 24 deletions(-) >> >> diff --git a/lib/backend.py b/lib/backend.py >> index e2e3e28..7046de7 100644 >> --- a/lib/backend.py >> +++ b/lib/backend.py >> @@ -1162,27 +1162,59 @@ 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 >> - 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 >> +def GetCryptoTokens(token_requests): >> + """Perform actions on the node's cryptographic tokens. >> + >> + Token types can be 'ssl' or 'ssh'. So far only some actions are >> implemented >> + for 'ssl'. Action 'get' returns the digest of the public client ssl >> + certificate. Action 'create' creates a new client certificate and >> private key >> + and also returns the digest of the certificate. The third parameter of >> a >> + token request are optional parameters for the actions, so far only the >> + filename is supported. >> + >> + @type token_requests: list of tuples of (string, string, dict), where >> the >> + first string is in constants.CRYPTO_TYPES, the second in >> + constants.CRYPTO_ACTIONS. The third parameter is a dictionary of >> string >> + to string. >> + @param token_requests: list of requests of cryptographic tokens and >> actions >> + to perform on them. The actions come with a dictionary of options. >> @rtype: list of tuples (string, string) >> @return: list of tuples of the token type and the public crypto token >> >> """ >> + _VALID_CERT_FILES = [pathutils.NODED_CERT_FILE, >> + pathutils.NODED_CLIENT_CERT_FILE, >> + pathutils.NODED_CLIENT_CERT_FILE_TMP] >> + _DEFAULT_CERT_FILE = pathutils.NODED_CLIENT_CERT_FILE >> tokens = [] >> - for token_type in token_types: >> + for (token_type, action, options) in token_requests: >> if token_type not in constants.CRYPTO_TYPES: >> - raise errors.ProgrammerError("Token type %s not supported." % >> + raise errors.ProgrammerError("Token type '%s' not supported." % >> token_type) >> + if action not in constants.CRYPTO_ACTIONS: >> + raise errors.ProgrammerError("Action '%s' is not supported." % >> + action) >> if token_type == constants.CRYPTO_TYPE_SSL: >> - tokens.append((token_type, utils.GetClientCertificateDigest())) >> + if action == constants.CRYPTO_ACTION_CREATE: >> + cert_filename = None >> + if options: >> + cert_filename = options.get(constants.CRYPTO_OPTION_CERT_FILE) >> + if not cert_filename: >> + cert_filename = _DEFAULT_CERT_FILE >> + # For security reason, we don't allow arbitrary filenames >> + if not cert_filename in _VALID_CERT_FILES: >> + raise errors.ProgrammerError( >> + "The certificate file name path '%s' is not allowed." % >> + cert_filename) >> + utils.GenerateNewSslCert( >> + True, cert_filename, >> + "Create new client SSL certificate in %s." % cert_filename) >> + tokens.append((token_type, >> + utils.GetClientCertificateDigest( >> + cert_filename=cert_filename))) >> + elif action == constants.CRYPTO_ACTION_GET: >> + tokens.append((token_type, >> + utils.GetClientCertificateDigest())) >> return tokens >> >> >> diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py >> index e501965..b8faee9 100644 >> --- a/lib/cmdlib/common.py >> +++ b/lib/cmdlib/common.py >> @@ -1228,7 +1228,8 @@ def AddNodeCertToCandidateCerts(lu, node_uuid, >> cluster): >> >> """ >> result = lu.rpc.call_node_crypto_tokens( >> - node_uuid, [constants.CRYPTO_TYPE_SSL]) >> + node_uuid, >> + [(constants.CRYPTO_TYPE_SSL, constants.CRYPTO_ACTION_GET, >> None)]) >> result.Raise("Could not retrieve the node's (uuid %s) SSL digest." >> % node_uuid) >> ((crypto_type, digest), ) = result.payload >> diff --git a/lib/pathutils.py b/lib/pathutils.py >> index adc418d..abf6470 100644 >> --- a/lib/pathutils.py >> +++ b/lib/pathutils.py >> @@ -107,6 +107,7 @@ RESTRICTED_COMMANDS_DIR = CONF_DIR + >> "/restricted-commands" >> #: Node daemon certificate path >> NODED_CERT_FILE = DATA_DIR + "/server.pem" >> NODED_CLIENT_CERT_FILE = DATA_DIR + "/client.pem" >> +NODED_CLIENT_CERT_FILE_TMP = DATA_DIR + "/client.pem.tmp" >> >> #: Node daemon certificate file permissions >> NODED_CERT_MODE = 0440 >> diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py >> index b92f720..73edeb4 100644 >> --- a/lib/rpc_defs.py >> +++ b/lib/rpc_defs.py >> @@ -506,8 +506,9 @@ _NODE_CALLS = [ >> ("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."), >> + ("token_request", None, >> + "List of tuples of requested crypto token types, actions"), >> + ], None, None, "Handle crypto tokens of the node."), >> ] >> >> _MISC_CALLS = [ >> diff --git a/lib/server/noded.py b/lib/server/noded.py >> index e5bf330..54d638e 100644 >> --- a/lib/server/noded.py >> +++ b/lib/server/noded.py >> @@ -869,8 +869,8 @@ class >> NodeRequestHandler(http.server.HttpServerHandler): >> """Gets the node's public crypto tokens. >> >> """ >> - token_types = params[0] >> - return backend.GetCryptoTokens(token_types) >> + token_requests = params[0] >> + return backend.GetCryptoTokens(token_requests) >> >> # cluster -------------------------- >> >> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs >> index c4ba9e2..a1bf23b 100644 >> --- a/src/Ganeti/Constants.hs >> +++ b/src/Ganeti/Constants.hs >> @@ -4157,6 +4157,24 @@ cryptoTypeSsh = "ssh" >> cryptoTypes :: FrozenSet String >> cryptoTypes = ConstantUtils.mkSet [cryptoTypeSsl] >> >> +-- * Crypto Actions >> +-- Actions that can be performed on crypto tokens >> + >> +cryptoActionGet :: String >> +cryptoActionGet = "get" >> + >> +-- This is 'create and get' >> +cryptoActionCreate :: String >> +cryptoActionCreate = "create" >> + >> +cryptoActions :: FrozenSet String >> +cryptoActions = ConstantUtils.mkSet [cryptoActionGet, cryptoActionCreate] >> + >> +-- * Options for CryptoActions >> + >> +-- Filename of the certificate >> +cryptoOptionCertFile :: String >> +cryptoOptionCertFile = "cert_file" >> >> -- * SSH key types >> >> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ >> ganeti.backend_unittest.py >> index 5c30bc2..f410cc1 100755 >> --- a/test/py/ganeti.backend_unittest.py >> +++ b/test/py/ganeti.backend_unittest.py >> @@ -34,6 +34,7 @@ from ganeti import errors >> from ganeti import hypervisor >> from ganeti import netutils >> from ganeti import objects >> +from ganeti import pathutils >> from ganeti import utils >> >> >> @@ -76,21 +77,45 @@ class TestX509Certificates(unittest.TestCase): >> class TestGetCryptoTokens(testutils.GanetiTestCase): >> >> def setUp(self): >> - self._digest_fn_orig = utils.GetClientCertificateDigest >> + self._get_digest_fn_orig = utils.GetClientCertificateDigest >> + self._create_digest_fn_orig = utils.GenerateNewSslCert >> self._ssl_digest = "12345" >> utils.GetClientCertificateDigest = mock.Mock( >> return_value=self._ssl_digest) >> + utils.GenerateNewSslCert = mock.Mock() >> >> def tearDown(self): >> - utils.GetClientCertificateDigest = self._digest_fn_orig >> + utils.GetClientCertificateDigest = self._get_digest_fn_orig >> + utils.GenerateNewSslCert = self._create_digest_fn_orig >> >> - def testSslToken(self): >> - result = backend.GetCryptoTokens([constants.CRYPTO_TYPE_SSL]) >> + def testGetSslToken(self): >> + result = backend.GetCryptoTokens( >> + [(constants.CRYPTO_TYPE_SSL, constants.CRYPTO_ACTION_GET, None)]) >> self.assertTrue((constants.CRYPTO_TYPE_SSL, self._ssl_digest) in >> result) >> >> - def testUnknownToken(self): >> + def testCreateSslToken(self): >> + result = backend.GetCryptoTokens( >> + [(constants.CRYPTO_TYPE_SSL, constants.CRYPTO_ACTION_CREATE, >> None)]) >> + self.assertTrue((constants.CRYPTO_TYPE_SSL, self._ssl_digest) in >> result) >> + self.assertTrue(utils.GenerateNewSslCert.assert_calls().once()) >> + >> + def testCreateSslTokenDifferentFilename(self): >> + result = backend.GetCryptoTokens( >> + [(constants.CRYPTO_TYPE_SSL, constants.CRYPTO_ACTION_CREATE, >> + {constants.CRYPTO_OPTION_CERT_FILE: >> + pathutils.NODED_CLIENT_CERT_FILE_TMP})]) >> + self.assertTrue((constants.CRYPTO_TYPE_SSL, self._ssl_digest) in >> result) >> + self.assertTrue(utils.GenerateNewSslCert.assert_calls().once()) >> + >> + def testUnknownTokenType(self): >> + self.assertRaises(errors.ProgrammerError, >> + backend.GetCryptoTokens, >> + [("pink_bunny", constants.CRYPTO_ACTION_GET, >> None)]) >> + >> + def testUnknownAction(self): >> self.assertRaises(errors.ProgrammerError, >> - backend.GetCryptoTokens, ["pink_bunny"]) >> + backend.GetCryptoTokens, >> + [(constants.CRYPTO_TYPE_SSL, "illuminate", None)]) >> >> >> class TestNodeVerify(testutils.GanetiTestCase): >> -- >> 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
