For both this patch and future ones: I'm okay with that
On Fri, Dec 20, 2013 at 12:58 PM, Helga Velroyen <[email protected]> wrote: > > > > 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 >
