On Fri, Dec 20, 2013 at 12:35 PM, Hrvoje Ribicic <[email protected]> wrote:
> On Thu, Dec 19, 2013 at 3:49 PM, Helga Velroyen <[email protected]> wrote: > >> This patch makes Ganeti create a client SSL certificate for >> the master node on cluster initialization. Note that some of >> the code in this patch is later moved into an LU to serve >> requirements for crypto renewal and updates, but for this >> point in the patch series it makes sense to add it here. >> >> Signed-off-by: Helga Velroyen <[email protected]> >> --- >> lib/bootstrap.py | 37 ++++++++++++++++++++----------------- >> lib/client/gnt_cluster.py | 2 ++ >> lib/pathutils.py | 1 + >> lib/utils/security.py | 22 ++++++++++++++++++++++ >> tools/cfgupgrade | 3 ++- >> 5 files changed, 47 insertions(+), 18 deletions(-) >> >> diff --git a/lib/bootstrap.py b/lib/bootstrap.py >> index f122b7f..80a2520 100644 >> --- a/lib/bootstrap.py >> +++ b/lib/bootstrap.py >> @@ -92,10 +92,11 @@ def GenerateHmacKey(file_name): >> >> >> def GenerateClusterCrypto(new_cluster_cert, new_rapi_cert, >> new_spice_cert, >> - new_confd_hmac_key, new_cds, >> + new_confd_hmac_key, new_cds, >> new_node_client_cert, >> rapi_cert_pem=None, spice_cert_pem=None, >> spice_cacert_pem=None, cds=None, >> nodecert_file=pathutils.NODED_CERT_FILE, >> + >> nodecert_client_file=pathutils.NODED_CLIENT_CERT_FILE, >> rapicert_file=pathutils.RAPI_CERT_FILE, >> spicecert_file=pathutils.SPICE_CERT_FILE, >> spicecacert_file=pathutils.SPICE_CACERT_FILE, >> @@ -113,6 +114,9 @@ def GenerateClusterCrypto(new_cluster_cert, >> new_rapi_cert, new_spice_cert, >> @param new_confd_hmac_key: Whether to generate a new HMAC key >> @type new_cds: bool >> @param new_cds: Whether to generate a new cluster domain secret >> + @type new_node_client_cert: bool >> + @param new_node_client_cert: Whether to generate a new node (SSL) >> + client certificate >> @type rapi_cert_pem: string >> @param rapi_cert_pem: New RAPI certificate in PEM format >> @type spice_cert_pem: string >> @@ -124,6 +128,9 @@ def GenerateClusterCrypto(new_cluster_cert, >> new_rapi_cert, new_spice_cert, >> @param cds: New cluster domain secret >> @type nodecert_file: string >> @param nodecert_file: optional override of the node cert file path >> + @type nodecert_client_file: string >> + @param nodecert_client_file: optiona override of the node client >> certificate >> > s/optiona/optional/ > Interdiff: diff --git a/lib/bootstrap.py b/lib/bootstrap.py index 80a2520..6f42c3c 100644 --- a/lib/bootstrap.py +++ b/lib/bootstrap.py @@ -129,7 +129,7 @@ def GenerateClusterCrypto(new_cluster_cert, new_rapi_cert, new_spice_cert, @type nodecert_file: string @param nodecert_file: optional override of the node cert file path @type nodecert_client_file: string - @param nodecert_client_file: optiona override of the node client certificate + @param nodecert_client_file: optional override of the node client certificate file path @type rapicert_file: string @param rapicert_file: optional override of the rapi cert file path > + file path >> @type rapicert_file: string >> @param rapicert_file: optional override of the rapi cert file path >> @type spicecert_file: string >> @@ -135,33 +142,29 @@ def GenerateClusterCrypto(new_cluster_cert, >> new_rapi_cert, new_spice_cert, >> >> """ >> # noded SSL certificate >> - cluster_cert_exists = os.path.exists(nodecert_file) >> - if new_cluster_cert or not cluster_cert_exists: >> - if cluster_cert_exists: >> - utils.CreateBackup(nodecert_file) >> + utils.GenerateNewSslCert( >> + new_cluster_cert, nodecert_file, >> + "Generating new cluster certificate at %s" % nodecert_file) >> >> - logging.debug("Generating new cluster certificate at %s", >> nodecert_file) >> - utils.GenerateSelfSignedSslCert(nodecert_file) >> + # noded client SSL certificate (to be used only by this very node) >> + utils.GenerateNewSslCert( >> + new_node_client_cert, nodecert_client_file, >> + "Generating new node client certificate at %s" % >> nodecert_client_file) >> >> # confd HMAC key >> if new_confd_hmac_key or not os.path.exists(hmackey_file): >> logging.debug("Writing new confd HMAC key to %s", hmackey_file) >> GenerateHmacKey(hmackey_file) >> >> - # RAPI >> - rapi_cert_exists = os.path.exists(rapicert_file) >> - >> if rapi_cert_pem: >> # Assume rapi_pem contains a valid PEM-formatted certificate and key >> logging.debug("Writing RAPI certificate at %s", rapicert_file) >> utils.WriteFile(rapicert_file, data=rapi_cert_pem, backup=True) >> >> - elif new_rapi_cert or not rapi_cert_exists: >> - if rapi_cert_exists: >> - utils.CreateBackup(rapicert_file) >> - >> - logging.debug("Generating new RAPI certificate at %s", rapicert_file) >> - utils.GenerateSelfSignedSslCert(rapicert_file) >> + else: >> + utils.GenerateNewSslCert( >> + new_rapi_cert, rapicert_file, >> + "Generating new RAPI certificate at %s" % rapicert_file) >> >> # SPICE >> spice_cert_exists = os.path.exists(spicecert_file) >> @@ -209,7 +212,7 @@ def _InitGanetiServerSetup(master_name): >> >> """ >> # Generate cluster secrets >> - GenerateClusterCrypto(True, False, False, False, False) >> + GenerateClusterCrypto(True, False, False, False, False, True) >> > > The invocation is a bit obfuscated here. What could help is explicitly > naming parameters despite the fact they are not optional? Your choice > though. > I agree about this invokation, though this will be changed in a later patch again anyway, so I won't do anything about it. > > >> >> result = utils.RunCmd([pathutils.DAEMON_UTIL, "start", >> constants.NODED]) >> if result.failed: >> diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py >> index fb57568..ea538f0 100644 >> --- a/lib/client/gnt_cluster.py >> +++ b/lib/client/gnt_cluster.py >> @@ -961,11 +961,13 @@ def _RenewCrypto(new_cluster_cert, new_rapi_cert, # >> pylint: disable=R0911 >> >> def _RenewCryptoInner(ctx): >> ctx.feedback_fn("Updating certificates and keys") >> + # FIXME: add separate option for client certs >> bootstrap.GenerateClusterCrypto(new_cluster_cert, >> new_rapi_cert, >> new_spice_cert, >> new_confd_hmac_key, >> new_cds, >> + new_cluster_cert, >> rapi_cert_pem=rapi_cert_pem, >> spice_cert_pem=spice_cert_pem, >> spice_cacert_pem=spice_cacert_pem, >> diff --git a/lib/pathutils.py b/lib/pathutils.py >> index 30a4817..adc418d 100644 >> --- a/lib/pathutils.py >> +++ b/lib/pathutils.py >> @@ -106,6 +106,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" >> >> #: Node daemon certificate file permissions >> NODED_CERT_MODE = 0440 >> diff --git a/lib/utils/security.py b/lib/utils/security.py >> index fe1f21c..61d0490 100644 >> --- a/lib/utils/security.py >> +++ b/lib/utils/security.py >> @@ -24,8 +24,10 @@ >> >> import logging >> import OpenSSL >> +import os >> >> from ganeti.utils import io >> +from ganeti.utils import x509 >> from ganeti import pathutils >> >> >> @@ -92,3 +94,23 @@ def >> GetClientCertificateDigest(cert_filename=pathutils.NODED_CERT_FILE): >> cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, >> cert_plain) >> return cert.digest("sha1") >> + >> + >> +def GenerateNewSslCert(new_cert, cert_filename, log_msg): >> + """Creates a new SSL certificate and backups the old one. >> + >> + @type new_cert: boolean >> + @param new_cert: whether a new certificate should be created >> + @type cert_filename: string >> + @param cert_filename: filename of the certificate file >> + @type log_msg: string >> + @param log_msg: log message to be written on certificate creation >> + >> + """ >> + cert_exists = os.path.exists(cert_filename) >> + if new_cert or not cert_exists: >> + if cert_exists: >> + io.CreateBackup(cert_filename) >> + >> + logging.debug(log_msg) >> + x509.GenerateSelfSignedSslCert(cert_filename) >> diff --git a/tools/cfgupgrade b/tools/cfgupgrade >> index 8be0d7d..038d200 100755 >> --- a/tools/cfgupgrade >> +++ b/tools/cfgupgrade >> @@ -571,8 +571,9 @@ def main(): >> backup=True) >> >> if not options.dry_run: >> + # FIXME: fix node client certificate >> bootstrap.GenerateClusterCrypto( >> - False, False, False, False, False, >> + False, False, False, False, False, False, >> nodecert_file=options.SERVER_PEM_PATH, >> rapicert_file=options.RAPI_CERT_FILE, >> spicecert_file=options.SPICE_CERT_FILE, >> -- >> 1.8.5.1 >> >> > Barring the typo and the optional suggestion, LGTM, thanks. > Thx. -- -- 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
