LGTM, thanks!
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 > >
