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/

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


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

Reply via email to