From bdd5e4208fab07013301ce3e996f31643075765f Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Thu, 18 Feb 2010 19:09:56 +0100
Subject: [PATCH] Use OpenSSL module instead of binary to generate certs

This saves us one dependency and saves us from complicated handling of
external files if we need key and certificate separated from each other.

At the same time, the number of bits used for RSA keys is increased from
1024 to 2048.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 Makefile.am                   |  1 -
 configure.ac                  |  8 ----
 lib/constants.py              |  9 ++++-
 lib/utils.py                  | 73 +++++++++++++++++++++--------------
 test/ganeti.utils_unittest.py | 40 ++++++++++++++-----
 5 files changed, 82 insertions(+), 49 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 4057001bf..8480c2b2a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -458,7 +458,6 @@ lib/_autoconf.py: Makefile stamp-directories
 	  echo "PKGLIBDIR = '$(pkglibdir)'"; \
 	  echo "DRBD_BARRIERS = $(DRBD_BARRIERS)"; \
 	  echo "SYSLOG_USAGE = '$(SYSLOG_USAGE)'"; \
-	  echo "OPENSSL_PATH = '$(OPENSSL)'"; \
 	} > $@
 
 $(REPLACE_VARS_SED): Makefile
diff --git a/configure.ac b/configure.ac
index 3bc5992de..9836008bf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -201,14 +201,6 @@ then
   AC_MSG_WARN([pylint not found, checking code will not be possible])
 fi
 
-# Check for openssl
-AC_ARG_VAR(OPENSSL, [openssl path])
-AC_PATH_PROG(OPENSSL, [openssl], [])
-if test -z "$OPENSSL"
-then
-  AC_MSG_ERROR([openssl not found])
-fi
-
 # Check for socat
 AC_ARG_VAR(SOCAT, [socat path])
 AC_PATH_PROG(SOCAT, [socat], [])
diff --git a/lib/constants.py b/lib/constants.py
index 61ed6ed79..7087989ba 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -112,7 +112,6 @@ DEFAULT_FILE_STORAGE_DIR = _autoconf.FILE_STORAGE_DIR
 SYSCONFDIR = _autoconf.SYSCONFDIR
 TOOLSDIR = _autoconf.TOOLSDIR
 CONF_DIR = SYSCONFDIR + "/ganeti"
-OPENSSL_PATH = _autoconf.OPENSSL_PATH
 
 MASTER_SOCKET = SOCKET_DIR + "/ganeti-master"
 
@@ -171,6 +170,14 @@ SOCAT_PATH = _autoconf.SOCAT_PATH
 SOCAT_USE_ESCAPE = _autoconf.SOCAT_USE_ESCAPE
 SOCAT_ESCAPE_CODE = "0x1d"
 
+# For RSA keys more bits are better, but they also make operations more
+# expensive. NIST SP 800-131 recommends a minimum of 2048 bits from the year
+# 2010 on.
+RSA_KEY_BITS = 2048
+
+# Digest used to sign certificates ("openssl x509" uses SHA1 by default)
+X509_CERT_SIGN_DIGEST = "SHA1"
+
 VALUE_DEFAULT = "default"
 VALUE_AUTO = "auto"
 VALUE_GENERATE = "generate"
diff --git a/lib/utils.py b/lib/utils.py
index fdedebf71..57036f200 100644
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -43,6 +43,7 @@ import resource
 import logging
 import logging.handlers
 import signal
+import OpenSSL
 
 from cStringIO import StringIO
 
@@ -2371,39 +2372,53 @@ def Retry(fn, delay, timeout, args=None, wait_fn=time.sleep,
         wait_fn(current_delay)
 
 
-def GenerateSelfSignedSslCert(file_name, validity=(365 * 5)):
-  """Generates a self-signed SSL certificate.
+def GetClosedTempfile(*args, **kwargs):
+  """Creates a temporary file and returns its path.
 
-  @type file_name: str
-  @param file_name: Path to output file
+  """
+  (fd, path) = tempfile.mkstemp(*args, **kwargs)
+  _CloseFDNoErr(fd)
+  return path
+
+
+def GenerateSelfSignedX509Cert(common_name, validity):
+  """Generates a self-signed X509 certificate.
+
+  @type common_name: string
+  @param common_name: commonName value
   @type validity: int
-  @param validity: Validity for certificate in days
+  @param validity: Validity for certificate in seconds
 
   """
-  (fd, tmp_file_name) = tempfile.mkstemp(dir=os.path.dirname(file_name))
-  try:
-    try:
-      # Set permissions before writing key
-      os.chmod(tmp_file_name, 0600)
-
-      result = RunCmd([constants.OPENSSL_PATH, "req",
-                       "-new", "-newkey", "rsa:1024",
-                       "-days", str(validity), "-nodes", "-x509",
-                       "-keyout", tmp_file_name, "-out", tmp_file_name,
-                       "-batch"])
-      if result.failed:
-        raise errors.OpExecError("Could not generate SSL certificate, command"
-                                 " %s had exitcode %s and error message %s" %
-                                 (result.cmd, result.exit_code, result.output))
-
-      # Make read-only
-      os.chmod(tmp_file_name, 0400)
-
-      os.rename(tmp_file_name, file_name)
-    finally:
-      RemoveFile(tmp_file_name)
-  finally:
-    os.close(fd)
+  # Create private and public key
+  key = OpenSSL.crypto.PKey()
+  key.generate_key(OpenSSL.crypto.TYPE_RSA, constants.RSA_KEY_BITS)
+
+  # Create self-signed certificate
+  cert = OpenSSL.crypto.X509()
+  if common_name:
+    cert.get_subject().CN = common_name
+  cert.set_serial_number(1)
+  cert.gmtime_adj_notBefore(0)
+  cert.gmtime_adj_notAfter(validity)
+  cert.set_issuer(cert.get_subject())
+  cert.set_pubkey(key)
+  cert.sign(key, constants.X509_CERT_SIGN_DIGEST)
+
+  key_pem = OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key)
+  cert_pem = OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM, cert)
+
+  return (key_pem, cert_pem)
+
+
+def GenerateSelfSignedSslCert(filename, validity=(5 * 365)):
+  """Legacy function to generate self-signed X509 certificate.
+
+  """
+  (key_pem, cert_pem) = GenerateSelfSignedX509Cert(None,
+                                                   validity * 24 * 60 * 60)
+
+  WriteFile(filename, mode=0400, data=key_pem + cert_pem)
 
 
 class FileLock(object):
diff --git a/test/ganeti.utils_unittest.py b/test/ganeti.utils_unittest.py
index 3ddd8e956..535f3d23e 100755
--- a/test/ganeti.utils_unittest.py
+++ b/test/ganeti.utils_unittest.py
@@ -35,6 +35,7 @@ import re
 import select
 import string
 import fcntl
+import OpenSSL
 
 import ganeti
 import testutils
@@ -1158,32 +1159,51 @@ class TestUnescapeAndSplit(unittest.TestCase):
       self.failUnlessEqual(UnescapeAndSplit(sep.join(a), sep=sep), b)
 
 
-class TestGenerateSelfSignedSslCert(unittest.TestCase):
+class TestGenerateSelfSignedX509Cert(unittest.TestCase):
   def setUp(self):
     self.tmpdir = tempfile.mkdtemp()
 
   def tearDown(self):
     shutil.rmtree(self.tmpdir)
 
-  def _checkPrivateRsaKey(self, key):
+  def _checkRsaPrivateKey(self, key):
     lines = key.splitlines()
-    self.assert_("-----BEGIN RSA PRIVATE KEY-----" in lines)
-    self.assert_("-----END RSA PRIVATE KEY-----" in lines)
+    return ("-----BEGIN RSA PRIVATE KEY-----" in lines and
+            "-----END RSA PRIVATE KEY-----" in lines)
 
-  def _checkRsaCertificate(self, cert):
+  def _checkCertificate(self, cert):
     lines = cert.splitlines()
-    self.assert_("-----BEGIN CERTIFICATE-----" in lines)
-    self.assert_("-----END CERTIFICATE-----" in lines)
+    return ("-----BEGIN CERTIFICATE-----" in lines and
+            "-----END CERTIFICATE-----" in lines)
 
-  def testSingleFile(self):
+  def test(self):
+    for common_name in [None, ".", "Ganeti", "node1.example.com"]:
+      (key_pem, cert_pem) = utils.GenerateSelfSignedX509Cert(common_name, 300)
+      self._checkRsaPrivateKey(key_pem)
+      self._checkCertificate(cert_pem)
+
+      key = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM,
+                                           key_pem)
+      self.assert_(key.bits() >= 1024)
+      self.assertEqual(key.bits(), constants.RSA_KEY_BITS)
+      self.assertEqual(key.type(), OpenSSL.crypto.TYPE_RSA)
+
+      x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM,
+                                             cert_pem)
+      self.failIf(x509.has_expired())
+      self.assertEqual(x509.get_issuer().CN, common_name)
+      self.assertEqual(x509.get_subject().CN, common_name)
+      self.assertEqual(x509.get_pubkey().bits(), constants.RSA_KEY_BITS)
+
+  def testLegacy(self):
     cert1_filename = os.path.join(self.tmpdir, "cert1.pem")
 
     utils.GenerateSelfSignedSslCert(cert1_filename, validity=1)
 
     cert1 = utils.ReadFile(cert1_filename)
 
-    self._checkPrivateRsaKey(cert1)
-    self._checkRsaCertificate(cert1)
+    self.assert_(self._checkRsaPrivateKey(cert1))
+    self.assert_(self._checkCertificate(cert1))
 
 
 if __name__ == '__main__':
-- 
GitLab