Merge branch 'master' into ecdsa

Signed-off-by: Osiris Inferi <github@flut.demon.nl>

certbot-nginx/certbot_nginx/configurator.py

@@ -476,8 +476,9 @@ class NginxConfigurator(common.Installer):
     def _get_snakeoil_paths(self):
         # TODO: generate only once
         tmp_dir = os.path.join(self.config.work_dir, "snakeoil")
-        le_key = crypto_util.init_save_key(
+        le_pem = crypto_util.make_key_rsa(1024)
-            key_size=1024, key_dir=tmp_dir, keyname="key.pem")
+        le_key = crypto_util.save_key(
+            key_pem=le_pem, key_dir=tmp_dir, keyname="key.pem")
         key = OpenSSL.crypto.load_privatekey(
             OpenSSL.crypto.FILETYPE_PEM, le_key.pem)
         cert = acme_crypto_util.gen_ss_cert(key, domains=[socket.gethostname()])

certbot/cli.py

@@ -1073,6 +1073,12 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
     helpful.add(
         "security", "--rsa-key-size", type=int, metavar="N",
         default=flag_default("rsa_key_size"), help=config_help("rsa_key_size"))
+    helpful.add(
+        "security", "--ecdsa-curve", default=flag_default("ecdsa_curve"),
+        help=config_help("ecdsa_curve"))
+    helpful.add(
+        "security", "--key-types", default=flag_default("key_types"),
+        help=config_help("key_types"))
     helpful.add(
         "security", "--must-staple", action="store_true",
         dest="must_staple", default=flag_default("must_staple"),

certbot/client.py

@@ -312,6 +312,8 @@ class Client(object):
             (`.util.CSR`).
         :rtype: tuple
 
+        :raises ValueError: If unable to generate the key.
+
         """
         authzr = self.auth_handler.get_authorizations(
                 domains,
@@ -321,15 +323,46 @@ class Client(object):
         domains = [d for d in domains if d in auth_domains]
 
         # Create CSR from names
+
+        # Validate self.config.key_types:
+
+        # TODO Get list of supported key types from Boulder?
+        valid_key_types = ["rsa", "ecdsa"]
+
+        key_types = self.config.key_types.split()
+
+        if len(key_types) != 1:
+            raise errors.Error("Currently, only one key type is supported.")
+
+        for key in key_types:
+            if key.lower() not in valid_key_types:
+                raise errors.Error("Key algorithm not valid, try \"RSA\" or \"ECDSA\".")
+
+        # TODO: Implement the issuance of multiple certificates with different keys
+        key_algo = key_types[0]
+
+        # Envoke the proper function for the requested key type
+        try:
+            if key_algo.lower() == "rsa":
+                logger.info("Generating %d bits RSA key", self.config.rsa_key_size)
+                key_pem = crypto_util.make_key_rsa(self.config.rsa_key_size)
+            elif key_algo.lower() == "ecdsa":
+                logger.info("Generating ECDSA key with curve %s", self.config.ecdsa_curve)
+                key_pem = crypto_util.make_key_ecdsa(self.config.ecdsa_curve)
+            else:
+                raise errors.Error("Key algorithm not valid, try \"RSA\" or \"ECDSA\".")
+        except ValueError as err:
+            logger.exception(err)
+            raise err
+
         if self.config.dry_run:
-            key = util.Key(file=None,
+            key = util.Key(file=None, pem=key_pem)
-                           pem=crypto_util.make_key(self.config.rsa_key_size))
             csr = util.CSR(file=None, form="pem",
                            data=acme_crypto_util.make_csr(
-                               key.pem, domains, self.config.must_staple))
+                           key.pem, domains, self.config.must_staple))
         else:
-            key = crypto_util.init_save_key(
+            key = crypto_util.save_key(
-                self.config.rsa_key_size, self.config.key_dir)
+                key_pem, self.config.key_dir)
             csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir)
 
         certr, chain = self.obtain_certificate_from_csr(

certbot/constants.py

@@ -55,6 +55,8 @@ CLI_DEFAULTS = dict(
     http01_address="",
     break_my_certs=False,
     rsa_key_size=2048,
+    ecdsa_curve="P-256",
+    key_types="rsa",
     must_staple=False,
     redirect=None,
     hsts=None,

certbot/crypto_util.py

@@ -22,44 +22,41 @@ from certbot import errors
 from certbot import interfaces
 from certbot import util
 
+from cryptography.hazmat.backends import default_backend
+from cryptography.hazmat.primitives import serialization
+from cryptography.hazmat.primitives.asymmetric import ec
 
 logger = logging.getLogger(__name__)
 
 
 # High level functions
-def init_save_key(key_size, key_dir, keyname="key-certbot.pem"):
+def save_key(key_pem, key_dir, keyname="key-certbot.pem"):
     """Initializes and saves a privkey.
 
-    Inits key and saves it in PEM format on the filesystem.
+    Saves a generated private key in PEM format on the filesystem.
 
     .. note:: keyname is the attempted filename, it may be different if a file
         already exists at the path.
 
-    :param int key_size: RSA key size in bits
+    :param str key_pem: The PEM encoded private key to save
     :param str key_dir: Key save directory.
     :param str keyname: Filename of key
 
     :returns: Key
     :rtype: :class:`certbot.util.Key`
 
-    :raises ValueError: If unable to generate the key given key_size.
-
     """
-    try:
-        key_pem = make_key(key_size)
-    except ValueError as err:
-        logger.exception(err)
-        raise err
 
     config = zope.component.getUtility(interfaces.IConfig)
     # Save file
     util.make_or_verify_dir(key_dir, 0o700, os.geteuid(),
                             config.strict_permissions)
+
     key_f, key_path = util.unique_file(
         os.path.join(key_dir, keyname), 0o600, "wb")
     with key_f:
         key_f.write(key_pem)
-    logger.debug("Generating key (%d bits): %s", key_size, key_path)
+    logger.debug("Saving private key: %s", key_path)
 
     return util.Key(key_path, key_pem)
 
@@ -169,7 +166,7 @@ def import_csr_file(csrfile, data):
     return PEM, util.CSR(file=csrfile, data=data_pem, form="pem"), domains
 
 
-def make_key(bits):
+def make_key_rsa(bits):
     """Generate PEM encoded RSA key.
 
     :param int bits: Number of bits, at least 1024.
@@ -184,6 +181,30 @@ def make_key(bits):
     return OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key)
 
 
+def make_key_ecdsa(curve):
+    """Generate PEM encoded ECDSA key.
+
+    :param str curve: The ECDSA curve used (currently P-256 [prime256v1] or P-384 [secp384r1])
+
+    :returns: new ECDSA key in PEM form with the specified curve
+    :rtype: str
+
+    """
+
+    if curve.lower() == "p-256":
+        private_key = ec.generate_private_key(ec.SECP256R1(), default_backend())
+    elif curve.lower() == "p-384":
+        private_key = ec.generate_private_key(ec.SECP384R1(), default_backend())
+    else:
+        raise errors.Error(
+            "Elliptic curve for ECDSA keypair generation not recognised. Current allowed curves "
+            "are \"P-256\" or \"P-384\".")
+
+    return private_key.private_bytes(
+        encoding=serialization.Encoding.PEM, format=serialization.PrivateFormat.TraditionalOpenSSL,
+        encryption_algorithm=serialization.NoEncryption())
+
+
 def valid_privkey(privkey):
     """Is valid RSA private key?
 

certbot/interfaces.py

@@ -203,6 +203,14 @@ class IConfig(zope.interface.Interface):
     email = zope.interface.Attribute(
         "Email used for registration and recovery contact. (default: Ask)")
     rsa_key_size = zope.interface.Attribute("Size of the RSA key.")
+    ecdsa_curve = zope.interface.Attribute(
+        "Set the certificate ECDSA curve. Current possible curves: "
+        "P-256 (default) or P-384.")
+    key_types = zope.interface.Attribute(
+        "Whitespace seperated list of key types for which certificates will be "
+        "issued. Possible values are: \"RSA\" and/or \"ECDSA\". Default: \"RSA\". "
+        "Note: at the moment it's only possible to request one of the key "
+        "types listed above. This is set to change in the future.")
     must_staple = zope.interface.Attribute(
         "Adds the OCSP Must Staple extension to the certificate. "
         "Autoconfigures OCSP Stapling for supported setups "

certbot/renewal.py

@@ -27,11 +27,12 @@ logger = logging.getLogger(__name__)
 # file's renewalparams and actually used in the client configuration
 # during the renewal process. We have to record their types here because
 # the renewal configuration process loses this information.
+
 STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
                     "server", "account", "authenticator", "installer",
                     "standalone_supported_challenges", "renew_hook",
                     "pre_hook", "post_hook", "tls_sni_01_address",
-                    "http01_address"]
+                    "http01_address", "ecdsa_curve", "key_types"]
 INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
 BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names"]
 

certbot/tests/client_test.py

@@ -117,6 +117,7 @@ class ClientTestCommon(test_util.ConfigTestCase):
         super(ClientTestCommon, self).setUp()
         self.config.no_verify_ssl = False
         self.config.allow_subset_of_names = False
+        self.config.key_types = "rsa"
 
         # pylint: disable=star-args
         self.account = mock.MagicMock(**{"key.pem": KEY})
@@ -131,6 +132,7 @@ class ClientTestCommon(test_util.ConfigTestCase):
 
 
 class ClientTest(ClientTestCommon):
+    # pylint: disable=too-many-public-methods
     """Tests for certbot.client.Client."""
     def setUp(self):
         super(ClientTest, self).setUp()
@@ -234,6 +236,91 @@ class ClientTest(ClientTestCommon):
         self.assertEqual(1, mock_get_utility().notification.call_count)
 
     @mock.patch("certbot.client.crypto_util")
+<<<<<<< HEAD
+    @test_util.patch_get_utility()
+    def test_obtain_certificate_rsa(self, unused_mock_get_utility,
+                                    mock_crypto_util):
+        self._mock_obtain_certificate()
+
+        csr = util.CSR(form="pem", file=None, data=CSR_SAN)
+        mock_crypto_util.init_save_csr.return_value = csr
+        mock_crypto_util.save_key.return_value = mock.sentinel.key
+        domains = ["example.com", "www.example.com"]
+
+        # return_value is essentially set to (None, None) in
+        # _mock_obtain_certificate(), which breaks this test.
+        # Thus fixed by the next line.
+
+        authzr = []
+
+        for domain in domains:
+            authzr.append(
+                mock.MagicMock(
+                    body=mock.MagicMock(
+                        identifier=mock.MagicMock(
+                            value=domain))))
+
+        self.client.auth_handler.get_authorizations.return_value = authzr
+
+        self.assertEqual(
+            self.client.obtain_certificate(domains),
+            (mock.sentinel.certr, mock.sentinel.chain, mock.sentinel.key, csr))
+
+        mock_crypto_util.save_key.assert_called_once_with(
+            mock_crypto_util.make_key_rsa(), self.config.key_dir)
+        mock_crypto_util.init_save_csr.assert_called_once_with(
+            mock.sentinel.key, domains, self.config.csr_dir)
+        self._check_obtain_certificate()
+
+    @mock.patch("certbot.client.crypto_util")
+    @test_util.patch_get_utility()
+    def test_obtain_certificate_ecdsa_p256(self, unused_mock_get_utility,
+                                           mock_crypto_util):
+        self._mock_obtain_certificate()
+
+        self.config.key_types = "ecdsa"
+        self.config.ecdsa_curve = "p-256"
+        csr = util.CSR(form="pem", file=None, data=CSR_SAN)
+        mock_crypto_util.init_save_csr.return_value = csr
+        mock_crypto_util.save_key.return_value = mock.sentinel.key
+        domains = ["example.com", "www.example.com"]
+
+        # return_value is essentially set to (None, None) in
+        # _mock_obtain_certificate(), which breaks this test.
+        # Thus fixed by the next line.
+
+        authzr = []
+
+        for domain in domains:
+            authzr.append(
+                mock.MagicMock(
+                    body=mock.MagicMock(
+                        identifier=mock.MagicMock(
+                            value=domain))))
+
+        self.client.auth_handler.get_authorizations.return_value = authzr
+
+        self.assertEqual(
+            self.client.obtain_certificate(domains),
+            (mock.sentinel.certr, mock.sentinel.chain, mock.sentinel.key, csr))
+
+        mock_crypto_util.save_key.assert_called_once_with(
+            mock_crypto_util.make_key_ecdsa(), self.config.key_dir)
+        mock_crypto_util.init_save_csr.assert_called_once_with(
+            mock.sentinel.key, domains, self.config.csr_dir)
+        self._check_obtain_certificate()
+
+    @mock.patch("certbot.client.crypto_util")
+    def test_obtain_certificate_ecdsa_p384(self, mock_crypto_util):
+        self._mock_obtain_certificate()
+
+        self.config.key_types = "ecdsa"
+        self.config.ecdsa_curve = "p-384"
+        csr = util.CSR(form="der", file=None, data=CSR_SAN)
+        mock_crypto_util.init_save_csr.return_value = csr
+        mock_crypto_util.save_key.return_value = mock.sentinel.key
+        domains = ["example.com", "www.example.com"]
+=======
     def test_obtain_certificate(self, mock_crypto_util):
         csr = util.CSR(form="pem", file=None, data=CSR_SAN)
         mock_crypto_util.init_save_csr.return_value = csr
@@ -265,6 +352,7 @@ class ClientTest(ClientTestCommon):
 
     def _test_obtain_certificate_common(self, key, csr):
         self._mock_obtain_certificate()
+>>>>>>> master
 
         # return_value is essentially set to (None, None) in
         # _mock_obtain_certificate(), which breaks this test.
@@ -285,11 +373,74 @@ class ClientTest(ClientTestCommon):
         with test_util.patch_get_utility():
             result = self.client.obtain_certificate(self.eg_domains)
 
+<<<<<<< HEAD
+        mock_crypto_util.save_key.assert_called_once_with(
+            mock_crypto_util.make_key_ecdsa(), self.config.key_dir)
+        mock_crypto_util.init_save_csr.assert_called_once_with(
+            mock.sentinel.key, domains, self.config.csr_dir)
+=======
         self.assertEqual(
             result,
             (mock.sentinel.certr, mock.sentinel.chain, key, csr))
+>>>>>>> master
         self._check_obtain_certificate()
 
+    @mock.patch("certbot.client.crypto_util")
+    def test_obtain_certificate_multi_alg(self, mock_crypto_util):
+        self._mock_obtain_certificate()
+
+        self.config.key_types = "ecdsa rsa"
+        csr = util.CSR(form="der", file=None, data=CSR_SAN)
+        mock_crypto_util.init_save_csr.return_value = csr
+        mock_crypto_util.save_key.return_value = mock.sentinel.key
+        domains = ["example.com", "www.example.com"]
+
+        # return_value is essentially set to (None, None) in
+        # _mock_obtain_certificate(), which breaks this test.
+        # Thus fixed by the next line.
+
+        authzr = []
+
+        for domain in domains:
+            authzr.append(
+                mock.MagicMock(
+                    body=mock.MagicMock(
+                        identifier=mock.MagicMock(
+                            value=domain))))
+
+        self.client.auth_handler.get_authorizations.return_value = authzr
+
+        self.assertRaises(errors.Error,
+            self.client.obtain_certificate, domains)
+
+    @mock.patch("certbot.client.crypto_util")
+    def test_obtain_certificate_unsupported_alg(self, mock_crypto_util):
+        self._mock_obtain_certificate()
+
+        self.config.key_types = "nonexistingalgo"
+        csr = util.CSR(form="der", file=None, data=CSR_SAN)
+        mock_crypto_util.init_save_csr.return_value = csr
+        mock_crypto_util.save_key.return_value = mock.sentinel.key
+        domains = ["example.com", "www.example.com"]
+
+        # return_value is essentially set to (None, None) in
+        # _mock_obtain_certificate(), which breaks this test.
+        # Thus fixed by the next line.
+
+        authzr = []
+
+        for domain in domains:
+            authzr.append(
+                mock.MagicMock(
+                    body=mock.MagicMock(
+                        identifier=mock.MagicMock(
+                            value=domain))))
+
+        self.client.auth_handler.get_authorizations.return_value = authzr
+
+        self.assertRaises(errors.Error,
+            self.client.obtain_certificate, domains)
+
     @mock.patch('certbot.client.Client.obtain_certificate')
     @mock.patch('certbot.storage.RenewableCert.new_lineage')
     @mock.patch('OpenSSL.crypto.dump_certificate')

certbot/tests/crypto_util_test.py

@@ -12,7 +12,6 @@ from certbot import interfaces
 from certbot import util
 import certbot.tests.util as test_util
 
-
 RSA256_KEY = test_util.load_vector('rsa256_key.pem')
 RSA256_KEY_PATH = test_util.vector_path('rsa256_key.pem')
 RSA512_KEY = test_util.load_vector('rsa512_key.pem')
@@ -22,38 +21,31 @@ CERT = test_util.load_vector('cert_512.pem')
 SS_CERT_PATH = test_util.vector_path('cert_2048.pem')
 SS_CERT = test_util.load_vector('cert_2048.pem')
 
-class InitSaveKeyTest(test_util.TempDirTestCase):
+class SaveKeyTest(test_util.TempDirTestCase):
-    """Tests for certbot.crypto_util.init_save_key."""
+    """Tests for certbot.crypto_util.save_key."""
     def setUp(self):
-        super(InitSaveKeyTest, self).setUp()
+        super(SaveKeyTest, self).setUp()
 
         logging.disable(logging.CRITICAL)
         zope.component.provideUtility(
             mock.Mock(strict_permissions=True), interfaces.IConfig)
 
     def tearDown(self):
-        super(InitSaveKeyTest, self).tearDown()
+        super(SaveKeyTest, self).tearDown()
 
         logging.disable(logging.NOTSET)
 
     @classmethod
-    def _call(cls, key_size, key_dir):
+    def _call(cls, key_pem, key_dir):
-        from certbot.crypto_util import init_save_key
+        from certbot.crypto_util import save_key
-        return init_save_key(key_size, key_dir, 'key-certbot.pem')
+        return save_key(key_pem, key_dir, 'key-certbot.pem')
 
-    @mock.patch('certbot.crypto_util.make_key')
+    def test_success(self):
-    def test_success(self, mock_make):
+        key = self._call(RSA512_KEY, self.tempdir)
-        mock_make.return_value = b'key_pem'
+        self.assertEqual(key.pem, RSA512_KEY)
-        key = self._call(1024, self.tempdir)
-        self.assertEqual(key.pem, b'key_pem')
         self.assertTrue('key-certbot.pem' in key.file)
         self.assertTrue(os.path.exists(os.path.join(self.tempdir, key.file)))
 
-    @mock.patch('certbot.crypto_util.make_key')
-    def test_key_failure(self, mock_make):
-        mock_make.side_effect = ValueError
-        self.assertRaises(ValueError, self._call, 431, self.tempdir)
-
 
 class InitSaveCSRTest(test_util.TempDirTestCase):
     """Tests for certbot.crypto_util.init_save_csr."""
@@ -158,14 +150,31 @@ class ImportCSRFileTest(unittest.TestCase):
                           test_util.load_vector('cert_512.pem'))
 
 
-class MakeKeyTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
+class MakeKeyRSATest(unittest.TestCase):  # pylint: disable=too-few-public-methods
-    """Tests for certbot.crypto_util.make_key."""
+    """Tests for certbot.crypto_util.make_key_rsa."""
 
     def test_it(self):  # pylint: disable=no-self-use
-        from certbot.crypto_util import make_key
+        from certbot.crypto_util import make_key_rsa
         # Do not test larger keys as it takes too long.
         OpenSSL.crypto.load_privatekey(
-            OpenSSL.crypto.FILETYPE_PEM, make_key(1024))
+            OpenSSL.crypto.FILETYPE_PEM, make_key_rsa(1024))
+
+
+class MakeKeyECDSATest(unittest.TestCase):
+    """Tests for certbot.crypto_util.make_key_ecdsa."""
+
+    def test_it(self):
+        from certbot.crypto_util import make_key_ecdsa
+        from cryptography.hazmat.backends import default_backend
+        from cryptography.hazmat.primitives.serialization import load_pem_private_key
+        from cryptography.hazmat.primitives.asymmetric import ec
+        self.assertTrue(isinstance(load_pem_private_key(
+            make_key_ecdsa(curve="P-256"), password=None, backend=default_backend()),
+            ec.EllipticCurvePrivateKey))
+        self.assertTrue(isinstance(load_pem_private_key(
+            make_key_ecdsa(curve="P-384"), password=None, backend=default_backend()),
+            ec.EllipticCurvePrivateKey))
+        self.assertRaises(Exception, make_key_ecdsa, curve="P-123")
 
 
 class VerifyCertSetup(unittest.TestCase):