From 47c1045f6d467a18a30c2a8482a6bfb373706fb8 Mon Sep 17 00:00:00 2001 From: Mads Jensen Date: Mon, 31 Aug 2020 17:50:45 +0200 Subject: [PATCH 1/5] Implements support for ECDSA keys. Fixes #2163. Thanks to @pahrohfit and @Tomoyuki-GH for previous efforts to implement suport for this. Co-Authored-By: Robert Dailey Co-Authored-By: Tomoyuki-GH <55397638+Tomoyuki-GH@users.noreply.github.com> --- AUTHORS.md | 2 + .../archive/c.encryption-example.com/README | 14 ++++ .../archive/c.encryption-example.com/cert.pem | 18 +++++ .../c.encryption-example.com/chain.pem | 20 +++++ .../c.encryption-example.com/fullchain.pem | 38 +++++++++ .../c.encryption-example.com/privkey.pem | 5 ++ .../live/c.encryption-example.com/README | 14 ++++ .../live/c.encryption-example.com/cert.pem | 1 + .../live/c.encryption-example.com/chain.pem | 1 + .../c.encryption-example.com/fullchain.pem | 1 + .../live/c.encryption-example.com/privkey.pem | 1 + .../renewal/c.encryption-example.com.conf | 17 ++++ .../certbot_tests/assertions.py | 14 ++++ .../certbot_tests/test_main.py | 75 ++++++++++++++++-- .../certbot_integration_tests/utils/misc.py | 6 +- certbot/CHANGELOG.md | 11 ++- certbot/README.rst | 2 + certbot/certbot/_internal/cert_manager.py | 27 +++++-- certbot/certbot/_internal/cli/__init__.py | 10 +++ certbot/certbot/_internal/cli/helpful.py | 4 + certbot/certbot/_internal/client.py | 34 ++++++-- certbot/certbot/_internal/constants.py | 2 + certbot/certbot/_internal/main.py | 3 + certbot/certbot/_internal/renewal.py | 8 +- certbot/certbot/_internal/storage.py | 21 +++++ certbot/certbot/crypto_util.py | 68 ++++++++++++---- certbot/certbot/interfaces.py | 8 ++ certbot/certbot/tests/testdata/README | 8 +- .../tests/testdata/ec_prime256v1_key.pem | 8 ++ .../tests/testdata/ec_secp384r1_key.pem | 9 +++ .../tests/testdata/ec_secp521r1_key.pem | 10 +++ .../testdata/sample-archive-ec/cert1.pem | 18 +++++ .../testdata/sample-archive-ec/chain1.pem | 20 +++++ .../testdata/sample-archive-ec/fullchain1.pem | 38 +++++++++ .../testdata/sample-archive-ec/privkey1.pem | 5 ++ .../tests/testdata/sample-renewal-ec.conf | 79 +++++++++++++++++++ .../tests/testdata/sample-renewal.conf | 1 + certbot/certbot/tests/util.py | 4 +- certbot/docs/ciphers.rst | 6 +- certbot/docs/cli-help.txt | 8 +- certbot/docs/using.rst | 16 +++- certbot/examples/cli.ini | 4 + certbot/tests/cli_test.py | 15 ++++ certbot/tests/client_test.py | 12 ++- certbot/tests/crypto_util_test.py | 50 +++++++++++- certbot/tests/renewal_test.py | 24 ++++++ 46 files changed, 709 insertions(+), 51 deletions(-) create mode 100644 certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/README create mode 100644 certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/cert.pem create mode 100644 certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/chain.pem create mode 100644 certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/fullchain.pem create mode 100644 certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/privkey.pem create mode 100644 certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/README create mode 120000 certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/cert.pem create mode 120000 certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/chain.pem create mode 120000 certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/fullchain.pem create mode 120000 certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/privkey.pem create mode 100644 certbot-ci/certbot_integration_tests/assets/sample-config/renewal/c.encryption-example.com.conf create mode 100644 certbot/certbot/tests/testdata/ec_prime256v1_key.pem create mode 100644 certbot/certbot/tests/testdata/ec_secp384r1_key.pem create mode 100644 certbot/certbot/tests/testdata/ec_secp521r1_key.pem create mode 100644 certbot/certbot/tests/testdata/sample-archive-ec/cert1.pem create mode 100644 certbot/certbot/tests/testdata/sample-archive-ec/chain1.pem create mode 100644 certbot/certbot/tests/testdata/sample-archive-ec/fullchain1.pem create mode 100644 certbot/certbot/tests/testdata/sample-archive-ec/privkey1.pem create mode 100644 certbot/certbot/tests/testdata/sample-renewal-ec.conf diff --git a/AUTHORS.md b/AUTHORS.md index 56fe2a3d8..f76c323a5 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -154,6 +154,7 @@ Authors * [Luca Olivetti](https://github.com/olivluca) * [Luke Rogers](https://github.com/lukeroge) * [Maarten](https://github.com/mrtndwrd) +* [Mads Jensen](https://github.com/atombrella) * [Maikel Martens](https://github.com/krukas) * [Malte Janduda](https://github.com/MalteJ) * [Mantas Mikulėnas](https://github.com/grawity) @@ -213,6 +214,7 @@ Authors * [Richard Barnes](https://github.com/r-barnes) * [Richard Panek](https://github.com/kernelpanek) * [Robert Buchholz](https://github.com/rbu) +* [Robert Dailey](https://github.com/pahrohfit) * [Robert Habermann](https://github.com/frennkie) * [Robert Xiao](https://github.com/nneonneo) * [Roland Shoemaker](https://github.com/rolandshoemaker) diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/README b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/README new file mode 100644 index 000000000..5050078ff --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/README @@ -0,0 +1,14 @@ +This directory contains your keys and certificates. + +`privkey.pem` : the private key for your certificate. +`fullchain.pem`: the certificate file used in most server software. +`chain.pem` : used for OCSP stapling in Nginx >=1.3.7. +`cert.pem` : will break many server configurations, and should not be used + without reading further documentation (see link below). + +WARNING: DO NOT MOVE OR RENAME THESE FILES! + Certbot expects these files to remain in this location in order + to function properly! + +We recommend not moving these files. For more information, see the Certbot +User Guide at https://certbot.eff.org/docs/using.html#where-are-my-certificates. diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/cert.pem b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/cert.pem new file mode 100644 index 000000000..b07af2bf7 --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/cert.pem @@ -0,0 +1,18 @@ +-----BEGIN CERTIFICATE----- +MIIC2zCCAcOgAwIBAgIIBvrEnbPRYu8wDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UE +AxMdUGViYmxlIEludGVybWVkaWF0ZSBDQSAxMjZjNGIwHhcNMjAxMDEyMjEwNzQw +WhcNMjUxMDEyMjEwNzQwWjAjMSEwHwYDVQQDExhjLmVuY3J5cHRpb24tZXhhbXBs +ZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARjMhuW0ENPPC33PjB5XsYU +CRw640kPQENIDatcTJaENZIZdqKd6rI6jc+lpbmXot7Zi52clJlSJS+V6oDAt2Lh +o4HYMIHVMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB +BQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUj7Kd3ENqxlPf8B2bIGhsjydX +mPswHwYDVR0jBBgwFoAUEiGxlkRsi+VvcogH5dVD3h1laAcwMQYIKwYBBQUHAQEE +JTAjMCEGCCsGAQUFBzABhhVodHRwOi8vMTI3LjAuMC4xOjQwMDIwIwYDVR0RBBww +GoIYYy5lbmNyeXB0aW9uLWV4YW1wbGUuY29tMA0GCSqGSIb3DQEBCwUAA4IBAQCl +k0JXsa8y7fg41WWMDhw60bPW77O0FtOmTcnhdI5daYNemQVk+Q5EMaBLQ/oGjgXd +9QXFzXH1PL904YEnSLt+iTpXn++7rQSNzQsdYqw0neWk4f5pEBiN+WORpb6mwobV +ifMtBOkNEHvrJ2Pkci9U1lLwtKD/DSew6QtJU5DSkmH1XdGuMJiubygEIvELtvgq +cP9S368ZvPmPGmKaJQXBiuaR8MTjY/Bkr79aXQMjKbf+mpn7h0POCcePk1DY/rm6 +Da+X16lf0hHyQhSUa7Vgyim6rK1/hlw+Z00i+sQCKD9Ih7kXuuGqfSDC33cfO8Tj +o/MXO8lcxkrem5zU5QWP +-----END CERTIFICATE----- diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/chain.pem b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/chain.pem new file mode 100644 index 000000000..64a805c0f --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/chain.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDUDCCAjigAwIBAgIIbi787yVrcMAwDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE +AxMVUGViYmxlIFJvb3QgQ0EgMGM1MjI1MCAXDTIwMTAxMjIwMjI0NloYDzIwNTAx +MDEyMjEyMjQ2WjAoMSYwJAYDVQQDEx1QZWJibGUgSW50ZXJtZWRpYXRlIENBIDEy +NmM0YjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALGeVk1BMJraeqRq +mJ2+hgso8VOAv2s2CVxUJjIVcn7f2adE8NyTsSQ1brlsnKCUYUw7yLTQH0izLQRB +qKVIDFkUqo5/FuTJ2QlfA2EwBL8J7s/7L7vj3L0DiVpwgxPSyFEwdl/Y5y7ofsX5 +CIhCFcaMAmTIuKLiSfCJjGwkbEMuolm+lO8Mikxxc/JtDVUC479ugU7PU9O09bMH +nm+sD6Bgd+KMoPkCCCoeShJS9X3Ziq9HGc7Z6nhM/zirFARt2XkonEdAZ8br01zY +MRiY9txhlWQ7mUkOtzOSoEuYJNoUbvMUf0+tNzto26WRyF7dJmh7lTBsYrvAwUTx +PzNyst0CAwEAAaOBgzCBgDAOBgNVHQ8BAf8EBAMCAoQwHQYDVR0lBBYwFAYIKwYB +BQUHAwEGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFBIhsZZE +bIvlb3KIB+XVQ94dZWgHMB8GA1UdIwQYMBaAFOaKTaXg37vKgRt7d79YOjAoAtJT +MA0GCSqGSIb3DQEBCwUAA4IBAQAU2mZii7PH2pkw2lNM0QqPbcW/UYyvFoUeM8Aq +uCtsI2s+oxCJTqzfLsA0N8NY4nHLQ5wAlNJfJekngni8hbmJTKU4JFTMe7kLQO8P +fJbk0pTzhhHVQw7CVwB6Pwq3u2m/JV+d6xDIDc+AVkuEl19ZJU0rTWyooClfFLZV +EdZmEiUtA3PGlxoYwYhoGHYlhFxsoFONhCsBEdN7k7FKtFGVxN7oc5SKmKp0YZTW +fcrEtrdNThATO4ymhCC2zh33NI/MT1O74fpaAc2k6LcTl57MKiLfTYX4LTL6v9JG +9tlNqjFVRRmzEbtXTPcCb+w9g1VqoOGok7mGXYLTYtShCuvE +-----END CERTIFICATE----- diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/fullchain.pem b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/fullchain.pem new file mode 100644 index 000000000..1ba80ba4e --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/fullchain.pem @@ -0,0 +1,38 @@ +-----BEGIN CERTIFICATE----- +MIIC2zCCAcOgAwIBAgIILlmGtZhUFEwwDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UE +AxMdUGViYmxlIEludGVybWVkaWF0ZSBDQSAxMjZjNGIwHhcNMjAxMDEyMjA1MDM0 +WhcNMjUxMDEyMjA1MDM0WjAjMSEwHwYDVQQDExhjLmVuY3J5cHRpb24tZXhhbXBs +ZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARHEzR8JPWrEmpmgM+F2bk5 +9mT0u6CjzmJG0QpbaqprLiG5NGpW84VQ5TFCrmC4KxYfigCfMhfHRNfFYvNUK3V/ +o4HYMIHVMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB +BQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU1CsVL+bPnzaxxQ5jUENmQJIO +lKwwHwYDVR0jBBgwFoAUEiGxlkRsi+VvcogH5dVD3h1laAcwMQYIKwYBBQUHAQEE +JTAjMCEGCCsGAQUFBzABhhVodHRwOi8vMTI3LjAuMC4xOjQwMDIwIwYDVR0RBBww +GoIYYy5lbmNyeXB0aW9uLWV4YW1wbGUuY29tMA0GCSqGSIb3DQEBCwUAA4IBAQBn +2D8loC7pfk28JYpFLr5lmFKJWWmtLGlpsWDj61fVjtTfGKLziJz+MM6il4Y3hIz5 +58qiFK0ue0M63dIBJ33N+XxSEXon4Q0gy/zRWfH9jtPJ3FwfjkU/RT9PAUClYi0G +ptNWnTmgQkNzousbcAtRNXuuShH3856vhUnwkX+xM+cbIDi1JVmFjcGrEEQJ0rUF +mv2ZTyfbWbUs3v4rReETi2NVzr1Ql6J+ByNcMvHODzFy3t0L6yelAw2ca1I+c9HU ++Z0tnp/ykR7eXNuVLivok8UBf5OC413lh8ZO5g+Bgzh/LdtkUuavg1MYtEX0H6mX +9U7y3nVI8WEbPGf+HDeu +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDUDCCAjigAwIBAgIIbi787yVrcMAwDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE +AxMVUGViYmxlIFJvb3QgQ0EgMGM1MjI1MCAXDTIwMTAxMjIwMjI0NloYDzIwNTAx +MDEyMjEyMjQ2WjAoMSYwJAYDVQQDEx1QZWJibGUgSW50ZXJtZWRpYXRlIENBIDEy +NmM0YjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALGeVk1BMJraeqRq +mJ2+hgso8VOAv2s2CVxUJjIVcn7f2adE8NyTsSQ1brlsnKCUYUw7yLTQH0izLQRB +qKVIDFkUqo5/FuTJ2QlfA2EwBL8J7s/7L7vj3L0DiVpwgxPSyFEwdl/Y5y7ofsX5 +CIhCFcaMAmTIuKLiSfCJjGwkbEMuolm+lO8Mikxxc/JtDVUC479ugU7PU9O09bMH +nm+sD6Bgd+KMoPkCCCoeShJS9X3Ziq9HGc7Z6nhM/zirFARt2XkonEdAZ8br01zY +MRiY9txhlWQ7mUkOtzOSoEuYJNoUbvMUf0+tNzto26WRyF7dJmh7lTBsYrvAwUTx +PzNyst0CAwEAAaOBgzCBgDAOBgNVHQ8BAf8EBAMCAoQwHQYDVR0lBBYwFAYIKwYB +BQUHAwEGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFBIhsZZE +bIvlb3KIB+XVQ94dZWgHMB8GA1UdIwQYMBaAFOaKTaXg37vKgRt7d79YOjAoAtJT +MA0GCSqGSIb3DQEBCwUAA4IBAQAU2mZii7PH2pkw2lNM0QqPbcW/UYyvFoUeM8Aq +uCtsI2s+oxCJTqzfLsA0N8NY4nHLQ5wAlNJfJekngni8hbmJTKU4JFTMe7kLQO8P +fJbk0pTzhhHVQw7CVwB6Pwq3u2m/JV+d6xDIDc+AVkuEl19ZJU0rTWyooClfFLZV +EdZmEiUtA3PGlxoYwYhoGHYlhFxsoFONhCsBEdN7k7FKtFGVxN7oc5SKmKp0YZTW +fcrEtrdNThATO4ymhCC2zh33NI/MT1O74fpaAc2k6LcTl57MKiLfTYX4LTL6v9JG +9tlNqjFVRRmzEbtXTPcCb+w9g1VqoOGok7mGXYLTYtShCuvE +-----END CERTIFICATE----- diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/privkey.pem b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/privkey.pem new file mode 100644 index 000000000..6843b83d6 --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/archive/c.encryption-example.com/privkey.pem @@ -0,0 +1,5 @@ +-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgNgefv2dad4U1VYEi +0WkdHuqywi5QXAe30OwNTTGjhbihRANCAARHEzR8JPWrEmpmgM+F2bk59mT0u6Cj +zmJG0QpbaqprLiG5NGpW84VQ5TFCrmC4KxYfigCfMhfHRNfFYvNUK3V/ +-----END PRIVATE KEY----- diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/README b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/README new file mode 100644 index 000000000..5050078ff --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/README @@ -0,0 +1,14 @@ +This directory contains your keys and certificates. + +`privkey.pem` : the private key for your certificate. +`fullchain.pem`: the certificate file used in most server software. +`chain.pem` : used for OCSP stapling in Nginx >=1.3.7. +`cert.pem` : will break many server configurations, and should not be used + without reading further documentation (see link below). + +WARNING: DO NOT MOVE OR RENAME THESE FILES! + Certbot expects these files to remain in this location in order + to function properly! + +We recommend not moving these files. For more information, see the Certbot +User Guide at https://certbot.eff.org/docs/using.html#where-are-my-certificates. diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/cert.pem b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/cert.pem new file mode 120000 index 000000000..23e9f36ef --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/cert.pem @@ -0,0 +1 @@ +../../archive/c.encryption-example.com/cert.pem \ No newline at end of file diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/chain.pem b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/chain.pem new file mode 120000 index 000000000..3ce63c220 --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/chain.pem @@ -0,0 +1 @@ +../../archive/c.encryption-example.com/chain.pem \ No newline at end of file diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/fullchain.pem b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/fullchain.pem new file mode 120000 index 000000000..5f86022af --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/fullchain.pem @@ -0,0 +1 @@ +../../archive/c.encryption-example.com/fullchain.pem \ No newline at end of file diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/privkey.pem b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/privkey.pem new file mode 120000 index 000000000..4a8866fdc --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/live/c.encryption-example.com/privkey.pem @@ -0,0 +1 @@ +../../archive/c.encryption-example.com/privkey.pem \ No newline at end of file diff --git a/certbot-ci/certbot_integration_tests/assets/sample-config/renewal/c.encryption-example.com.conf b/certbot-ci/certbot_integration_tests/assets/sample-config/renewal/c.encryption-example.com.conf new file mode 100644 index 000000000..0c65891e4 --- /dev/null +++ b/certbot-ci/certbot_integration_tests/assets/sample-config/renewal/c.encryption-example.com.conf @@ -0,0 +1,17 @@ +# renew_before_expiry = 30 days +version = 1.10.0.dev0 +archive_dir = sample-config/archive/c.encryption-example.com +cert = sample-config/live/c.encryption-example.com/cert.pem +privkey = sample-config/live/c.encryption-example.com/privkey.pem +chain = sample-config/live/c.encryption-example.com/chain.pem +fullchain = sample-config/live/c.encryption-example.com/fullchain.pem + +# Options used in the renewal process +[renewalparams] +authenticator = apache +installer = apache +account = 48d6b9e8d767eccf7e4d877d6ffa81e3 +key_type = ecdsa +config_dir = sample-config-ec +elliptic_curve = secp256r1 +manual_public_ip_logging_ok = True diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py b/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py index 53df6b890..c19c0762e 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py @@ -2,6 +2,10 @@ import io import os +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.asymmetric.ec import EllipticCurvePrivateKey +from cryptography.hazmat.primitives.serialization import load_pem_private_key + try: import grp POSIX_MODE = True @@ -16,6 +20,16 @@ SYSTEM_SID = 'S-1-5-18' ADMINS_SID = 'S-1-5-32-544' +def assert_elliptic_key(key, curve): + with open(key, 'rb') as file: + privkey1 = file.read() + + key = load_pem_private_key(data=privkey1, password=None, backend=default_backend()) + + assert isinstance(key, EllipticCurvePrivateKey) + assert isinstance(key.curve, curve) + + def assert_hook_execution(probe_path, probe_content): """ Assert that a certbot hook has been executed diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py index caef80af3..02c537733 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py @@ -9,12 +9,17 @@ import shutil import subprocess import time +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.asymmetric.ec import SECP256R1, SECP384R1 +from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey +from cryptography.hazmat.primitives.serialization import load_pem_private_key from cryptography.x509 import NameOID import pytest from certbot_integration_tests.certbot_tests import context as certbot_context from certbot_integration_tests.certbot_tests.assertions import assert_cert_count_for_lineage +from certbot_integration_tests.certbot_tests.assertions import assert_elliptic_key from certbot_integration_tests.certbot_tests.assertions import assert_equals_group_owner from certbot_integration_tests.certbot_tests.assertions import assert_equals_group_permissions from certbot_integration_tests.certbot_tests.assertions import assert_equals_world_read_permissions @@ -289,7 +294,7 @@ def test_renew_with_changed_private_key_complexity(context): assert_cert_count_for_lineage(context.config_dir, certname, 1) context.certbot(['renew']) - + assert_cert_count_for_lineage(context.config_dir, certname, 2) key2 = join(context.config_dir, 'archive', certname, 'privkey2.pem') assert os.stat(key2).st_size > 3000 @@ -421,20 +426,80 @@ def test_reuse_key(context): assert len({cert1, cert2, cert3}) == 3 +def test_incorrect_key_type(context): + with pytest.raises(subprocess.CalledProcessError): + context.certbot(['--key-type="failwhale"']) + + def test_ecdsa(context): - """Test certificate issuance with ECDSA key.""" + """Test issuance for ECDSA CSR based request (legacy supported mode).""" key_path = join(context.workspace, 'privkey-p384.pem') csr_path = join(context.workspace, 'csr-p384.der') cert_path = join(context.workspace, 'cert-p384.pem') chain_path = join(context.workspace, 'chain-p384.pem') - misc.generate_csr([context.get_domain('ecdsa')], key_path, csr_path, key_type=misc.ECDSA_KEY_TYPE) - context.certbot(['auth', '--csr', csr_path, '--cert-path', cert_path, '--chain-path', chain_path]) + misc.generate_csr( + [context.get_domain('ecdsa')], + key_path, csr_path, + key_type=misc.ECDSA_KEY_TYPE + ) + context.certbot([ + 'auth', '--csr', csr_path, '--cert-path', cert_path, + '--chain-path', chain_path, + ]) certificate = misc.read_certificate(cert_path) assert 'ASN1 OID: secp384r1' in certificate +def test_default_key_type(context): + """Test default key type is RSA""" + certname = context.get_domain('renew') + context.certbot([ + 'certonly', + '--cert-name', certname, '-d', certname + ]) + filename = join(context.config_dir, 'archive/{0}/privkey1.pem').format(certname) + with open(filename, 'rb') as file: + privkey1 = file.read() + + key = load_pem_private_key(data=privkey1, password=None, backend=default_backend()) + assert isinstance(key, RSAPrivateKey) + + +def test_default_curve_type(context): + """test that the curve used when not specifying any is secp256r1""" + certname = context.get_domain('renew') + context.certbot([ + '--key-type', 'ecdsa', '--cert-name', certname, '-d', certname + ]) + key1 = join(context.config_dir, 'archive/{0}/privkey1.pem'.format(certname)) + assert_elliptic_key(key1, SECP256R1) + + +def test_renew_with_ec_keys(context): + """Test proper renew with updated private key complexity.""" + certname = context.get_domain('renew') + context.certbot([ + 'certonly', + '--cert-name', certname, + '--key-type', 'ecdsa', '--elliptic-curve', 'secp256r1', + '--force-renewal', '-d', certname, + ]) + + key1 = join(context.config_dir, "archive", certname, 'privkey1.pem') + assert 200 < os.stat(key1).st_size < 250 # ec keys of 256 bits are ~225 bytes + assert_elliptic_key(key1, SECP256R1) + assert_cert_count_for_lineage(context.config_dir, certname, 1) + + context.certbot(['renew', '--elliptic-curve', 'secp384r1']) + + assert_cert_count_for_lineage(context.config_dir, certname, 2) + key2 = join(context.config_dir, 'archive', certname, 'privkey2.pem') + assert_elliptic_key(key2, SECP384R1) + assert 280 < os.stat(key2).st_size < 320 # ec keys of 384 bits are ~310 bytes + + def test_ocsp_must_staple(context): """Test that OCSP Must-Staple is correctly set in the generated certificate.""" if context.acme_server == 'pebble': @@ -657,4 +722,4 @@ def test_preferred_chain(context): with open(conf_path, 'r') as f: assert 'preferred_chain = {}'.format(requested) in f.read(), \ - 'Expected preferred_chain to be set in renewal config' \ No newline at end of file + 'Expected preferred_chain to be set in renewal config' diff --git a/certbot-ci/certbot_integration_tests/utils/misc.py b/certbot-ci/certbot_integration_tests/utils/misc.py index 38c2e60a8..d83f276ef 100644 --- a/certbot-ci/certbot_integration_tests/utils/misc.py +++ b/certbot-ci/certbot_integration_tests/utils/misc.py @@ -280,7 +280,11 @@ def load_sample_data_path(workspace): if os.name == 'nt': # Fix the symlinks on Windows if GIT is not configured to create them upon checkout - for lineage in ['a.encryption-example.com', 'b.encryption-example.com']: + for lineage in [ + 'a.encryption-example.com', + 'b.encryption-example.com', + 'c.encryption-example.com', + ]: current_live = os.path.join(copied, 'live', lineage) for name in os.listdir(current_live): if name != 'README': diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index fc83d3b46..e32bd1072 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -8,7 +8,12 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * Added timeout to DNS query function calls for dns-rfc2136 plugin. * Confirmation when deleting certificates -* +* CLI flag `--key-type` has been added to specify 'rsa' or 'ecdsa' (default 'rsa'). + Only accepts a single value at this time. +* CLI flag `--elliptic-curve` has been added which takes an NIST/SECG elliptic curve. Either of + `secp256r1`, `secp284r1` and `secp521r1` are accepted values. +* The command `certbot certficates` lists the which type of the private key that was used + for the private key. ### Changed @@ -55,7 +60,7 @@ More details about these changes can be found on our GitHub repo. ### Added -* Added the ability to remove email and phone contact information from an account +* Added the ability to remove email and phone contact information from an account using `update_account --register-unsafely-without-email` ### Changed @@ -67,7 +72,7 @@ More details about these changes can be found on our GitHub repo. * The problem causing the Apache plugin in the Certbot snap on ARM systems to fail to load the Augeas library it depends on has been fixed. * The `acme` library can now tell the ACME server to clear contact information by passing an empty - `tuple` to the `contact` field of a `Registration` message. + `tuple` to the `contact` field of a `Registration` message. * Fixed the `*** stack smashing detected ***` error in the Certbot snap on some systems. More details about these changes can be found on our GitHub repo. diff --git a/certbot/README.rst b/certbot/README.rst index 6d04e6fa3..3bd5e4cd7 100644 --- a/certbot/README.rst +++ b/certbot/README.rst @@ -106,6 +106,8 @@ Current Features * Can get domain-validated (DV) certificates. * Can revoke certificates. * Adjustable RSA key bit-length (2048 (default), 4096, ...). +* Adjustable [EC](https://en.wikipedia.org/wiki/Elliptic-curve_cryptography) + key (`secp256r1` (default), `secp384r1`, `secp521r1`). * Can optionally install a http -> https redirect, so your site effectively runs https only (Apache only) * Fully automated. diff --git a/certbot/certbot/_internal/cert_manager.py b/certbot/certbot/_internal/cert_manager.py index 5dc909330..5e1b05e9e 100644 --- a/certbot/certbot/_internal/cert_manager.py +++ b/certbot/certbot/_internal/cert_manager.py @@ -65,6 +65,7 @@ def rename_lineage(config): disp.notification("Successfully renamed {0} to {1}." .format(certname, new_certname), pause=False) + def certificates(config): """Display information about certs configured with Certbot @@ -87,6 +88,7 @@ def certificates(config): # Describe all the certs _describe_certs(config, parsed_certs, parse_failures) + def delete(config): """Delete Certbot files associated with a certificate lineage.""" certnames = get_certnames(config, "delete", allow_multiple=True) @@ -123,11 +125,13 @@ def lineage_for_certname(cli_config, certname): logger.debug("Traceback was:\n%s", traceback.format_exc()) return None + def domains_for_certname(config, certname): """Find the domains in the cert with name certname.""" lineage = lineage_for_certname(config, certname) return lineage.names() if lineage else None + def find_duplicative_certs(config, domains): """Find existing certs that match the given domain names. @@ -172,6 +176,7 @@ def find_duplicative_certs(config, domains): return _search_lineages(config, update_certs_for_domain_matches, (None, None)) + def _archive_files(candidate_lineage, filetype): """ In order to match things like: /etc/letsencrypt/archive/example.com/chain1.pem. @@ -193,6 +198,7 @@ def _archive_files(candidate_lineage, filetype): return pattern return None + def _acceptable_matches(): """ Generates the list that's passed to match_and_check_overlaps. Is its own function to make unit testing easier. @@ -203,6 +209,7 @@ def _acceptable_matches(): return [lambda x: x.fullchain_path, lambda x: x.cert_path, lambda x: _archive_files(x, "cert"), lambda x: _archive_files(x, "fullchain")] + def cert_path_to_lineage(cli_config): """ If config.cert_path is defined, try to find an appropriate value for config.certname. @@ -219,6 +226,7 @@ def cert_path_to_lineage(cli_config): lambda x: cli_config.cert_path[0], lambda x: x.lineagename) return match[0] + def match_and_check_overlaps(cli_config, acceptable_matches, match_func, rv_func): """ Searches through all lineages for a match, and checks for duplicates. If a duplicate is found, an error is raised, as performing operations on lineages @@ -284,20 +292,23 @@ def human_readable_cert_info(config, cert, skip_filter_checks=False): valid_string = "{0} ({1})".format(cert.target_expiry, status) serial = format(crypto_util.get_serial_from_cert(cert.cert_path), 'x') - certinfo.append(" Certificate Name: {0}\n" - " Serial Number: {1}\n" - " Domains: {2}\n" - " Expiry Date: {3}\n" - " Certificate Path: {4}\n" - " Private Key Path: {5}".format( + certinfo.append(" Certificate Name: {}\n" + " Serial Number: {}\n" + " Key Type: {}\n" + " Domains: {}\n" + " Expiry Date: {}\n" + " Certificate Path: {}\n" + " Private Key Path: {}".format( cert.lineagename, serial, + cert.private_key_type, " ".join(cert.names()), valid_string, cert.fullchain, cert.privkey)) return "".join(certinfo) + def get_certnames(config, verb, allow_multiple=False, custom_prompt=None): """Get certname from flag, interactively, or error out. """ @@ -337,10 +348,12 @@ def get_certnames(config, verb, allow_multiple=False, custom_prompt=None): # Private Helpers ################### + def _report_lines(msgs): """Format a results report for a category of single-line renewal outcomes""" return " " + "\n ".join(str(msg) for msg in msgs) + def _report_human_readable(config, parsed_certs): """Format a results report for a parsed cert""" certinfo = [] @@ -348,6 +361,7 @@ def _report_human_readable(config, parsed_certs): certinfo.append(human_readable_cert_info(config, cert)) return "\n".join(certinfo) + def _describe_certs(config, parsed_certs, parse_failures): """Print information about the certs we know about""" out = [] # type: List[str] @@ -369,6 +383,7 @@ def _describe_certs(config, parsed_certs, parse_failures): disp = zope.component.getUtility(interfaces.IDisplay) disp.notification("\n".join(out), pause=False, wrap=False) + def _search_lineages(cli_config, func, initial_rv, *args): """Iterate func over unbroken lineages, allowing custom return conditions. diff --git a/certbot/certbot/_internal/cli/__init__.py b/certbot/certbot/_internal/cli/__init__.py index f6f648aa9..e50cb338a 100644 --- a/certbot/certbot/_internal/cli/__init__.py +++ b/certbot/certbot/_internal/cli/__init__.py @@ -313,6 +313,16 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): 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", "--key-type", choices=['rsa', 'ecdsa'], type=str, + default=flag_default("key_type"), help=config_help("key_type")) + helpful.add( + "security", "--elliptic-curve", type=str, choices=[ + 'secp256r1', + 'secp384r1', + 'secp521r1', + ], metavar="N", + default=flag_default("elliptic_curve"), help=config_help("elliptic_curve")) helpful.add( "security", "--must-staple", action="store_true", dest="must_staple", default=flag_default("must_staple"), diff --git a/certbot/certbot/_internal/cli/helpful.py b/certbot/certbot/_internal/cli/helpful.py index a86ff3743..141dd41a4 100644 --- a/certbot/certbot/_internal/cli/helpful.py +++ b/certbot/certbot/_internal/cli/helpful.py @@ -230,6 +230,10 @@ class HelpfulArgumentParser(object): raise errors.Error( "Parameters --hsts and --auto-hsts cannot be used simultaneously.") + if isinstance(parsed_args.key_type, list) and len(parsed_args.key_type) > 1: + raise errors.Error( + "Only *one* --key-type type may be provided at this time.") + return parsed_args def set_test_server(self, parsed_args): diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index b198a8c27..5dc62580e 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -312,7 +312,6 @@ class Client(object): :rtype: tuple """ - # We need to determine the key path, key PEM data, CSR path, # and CSR PEM data. For a dry run, the paths are None because # they aren't permanently saved to disk. For a lineage with @@ -335,16 +334,41 @@ class Client(object): # The key is set to None here but will be created below. key = None + key_size = self.config.rsa_key_size + elliptic_curve = None + + # key-type defaults to a list, but we are only handling 1 currently + if isinstance(self.config.key_type, list): + self.config.key_type = self.config.key_type[0] + if self.config.elliptic_curve and self.config.key_type == 'ecdsa': + elliptic_curve = self.config.elliptic_curve + self.config.auth_chain_path = "./chain-ecdsa.pem" + self.config.auth_cert_path = "./cert-ecdsa.pem" + self.config.key_path = "./key-ecdsa.pem" + elif self.config.rsa_key_size and self.config.key_type.lower() == 'rsa': + key_size = self.config.rsa_key_size + # Create CSR from names if self.config.dry_run: - key = key or util.Key(file=None, - pem=crypto_util.make_key(self.config.rsa_key_size)) + key = key or util.Key( + file=None, + pem=crypto_util.make_key( + bits=key_size, + elliptic_curve=elliptic_curve, + key_type=self.config.key_type, + + ), + ) csr = util.CSR(file=None, form="pem", data=acme_crypto_util.make_csr( key.pem, domains, self.config.must_staple)) else: - key = key or crypto_util.init_save_key(self.config.rsa_key_size, - self.config.key_dir) + key = key or crypto_util.init_save_key( + key_size=key_size, + key_dir=self.config.key_dir, + key_type=self.config.key_type, + elliptic_curve=elliptic_curve, + ) csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir) orderr = self._get_order_and_authorizations(csr.data, self.config.allow_subset_of_names) diff --git a/certbot/certbot/_internal/constants.py b/certbot/certbot/_internal/constants.py index 9e9373f13..f79d6b9db 100644 --- a/certbot/certbot/_internal/constants.py +++ b/certbot/certbot/_internal/constants.py @@ -57,6 +57,8 @@ CLI_DEFAULTS = dict( https_port=443, break_my_certs=False, rsa_key_size=2048, + elliptic_curve="secp256r1", + key_type="rsa", must_staple=False, redirect=None, auto_hsts=False, diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 6c1482b65..fd7df8d83 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -1009,6 +1009,7 @@ def delete(config, unused_plugins): """ cert_manager.delete(config) + def certificates(config, unused_plugins): """Display information about certs configured with Certbot @@ -1024,6 +1025,7 @@ def certificates(config, unused_plugins): """ cert_manager.certificates(config) + # TODO: coop with renewal config def revoke(config, unused_plugins): """Revoke a previously obtained certificate. @@ -1156,6 +1158,7 @@ def _csr_get_and_save_cert(config, le_client): os.path.normpath(config.chain_path), os.path.normpath(config.fullchain_path)) return cert_path, fullchain_path + def renew_cert(config, plugins, lineage): """Renew & save an existing cert. Do not install it. diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py index 50e4d3a4c..1d008193d 100644 --- a/certbot/certbot/_internal/renewal.py +++ b/certbot/certbot/_internal/renewal.py @@ -10,7 +10,7 @@ import time import traceback from cryptography.hazmat.backends import default_backend -from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.hazmat.primitives.asymmetric import ec, rsa from cryptography.hazmat.primitives.serialization import load_pem_private_key import OpenSSL import six @@ -40,7 +40,7 @@ logger = logging.getLogger(__name__) STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent", "server", "account", "authenticator", "installer", "renew_hook", "pre_hook", "post_hook", "http01_address", - "preferred_chain"] + "preferred_chain", "key_type", "elliptic_curve"] INT_CONFIG_ITEMS = ["rsa_key_size", "http01_port"] BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names", "reuse_key", "autorenew"] @@ -506,6 +506,10 @@ def _update_renewal_params_from_key(key_path, config): with open(key_path, 'rb') as file_h: key = load_pem_private_key(file_h.read(), password=None, backend=default_backend()) if isinstance(key, rsa.RSAPrivateKey): + config.key_type = 'rsa' config.rsa_key_size = key.key_size + elif isinstance(key, ec.EllipticCurvePrivateKey): + config.key_type = 'ecdsa' + config.elliptic_curve = key.curve.name else: raise errors.Error('Key at {0} is of an unsupported type: {1}.'.format(key_path, type(key))) diff --git a/certbot/certbot/_internal/storage.py b/certbot/certbot/_internal/storage.py index 84bd33143..b6c37a5ba 100644 --- a/certbot/certbot/_internal/storage.py +++ b/certbot/certbot/_internal/storage.py @@ -10,6 +10,9 @@ import configobj import parsedatetime import pytz import six +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey +from cryptography.hazmat.primitives.serialization import load_pem_private_key import certbot from certbot import crypto_util @@ -46,6 +49,7 @@ def renewal_conf_files(config): result.sort() return result + def renewal_file_for_certname(config, certname): """Return /path/to/certname.conf in the renewal conf directory""" path = os.path.join(config.renewal_configs_dir, "{0}.conf".format(certname)) @@ -1055,6 +1059,23 @@ class RenewableCert(interfaces.RenewableCert): target, values) return cls(new_config.filename, cli_config) + @property + def private_key_type(self): + """ + :returns: The type of algorithm for the private, RSA or ECDSA + :rtype: str + """ + with open(self.configuration["privkey"], "rb") as priv_key_file: + key = load_pem_private_key( + data=priv_key_file.read(), + password=None, + backend=default_backend() + ) + if isinstance(key, RSAPrivateKey): + return "RSA" + else: + return "ECDSA" + def save_successor(self, prior_version, new_cert, new_privkey, new_chain, cli_config): """Save new cert and chain as a successor of a prior version. diff --git a/certbot/certbot/crypto_util.py b/certbot/certbot/crypto_util.py index d1a667548..b02e06ed7 100644 --- a/certbot/certbot/crypto_util.py +++ b/certbot/certbot/crypto_util.py @@ -11,14 +11,16 @@ import warnings import re # See https://github.com/pyca/cryptography/issues/4275 from cryptography import x509 # type: ignore -from cryptography.exceptions import InvalidSignature +from cryptography.exceptions import InvalidSignature, UnsupportedAlgorithm from cryptography.hazmat.backends import default_backend -from cryptography.hazmat.primitives.asymmetric.ec import ECDSA -from cryptography.hazmat.primitives.asymmetric.ec import EllipticCurvePublicKey +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.hazmat.primitives.asymmetric.ec import ECDSA, EllipticCurvePublicKey from cryptography.hazmat.primitives.asymmetric.padding import PKCS1v15 from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey +from cryptography.hazmat.primitives.serialization import Encoding, NoEncryption, PrivateFormat from OpenSSL import crypto from OpenSSL import SSL # type: ignore + import pyrfc3339 import six import zope.component @@ -34,7 +36,9 @@ logger = logging.getLogger(__name__) # High level functions -def init_save_key(key_size, key_dir, keyname="key-certbot.pem"): +def init_save_key( + key_size, key_dir, key_type="rsa", elliptic_curve="secp256r1", keyname="key-certbot.pem" +): """Initializes and saves a privkey. Inits key and saves it in PEM format on the filesystem. @@ -42,8 +46,10 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"): .. 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 int key_size: key size in bits if key size is rsa. :param str key_dir: Key save directory. + :param str key_type: Key Type [rsa, ecdsa] + :param str elliptic_curve: Name of the elliptic curve if key type is ecdsa. :param str keyname: Filename of key :returns: Key @@ -53,7 +59,9 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"): """ try: - key_pem = make_key(key_size) + key_pem = make_key( + bits=key_size, elliptic_curve=elliptic_curve or "secp256r1", key_type=key_type, + ) except ValueError as err: logger.error("", exc_info=True) raise err @@ -65,7 +73,10 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"): 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) + if key_type == 'rsa': + logger.debug("Generating RSA key (%d bits): %s", key_size, key_path) + else: + logger.debug("Generating ECDSA key (%d bits): %s", key_size, key_path) return util.Key(key_path, key_pem) @@ -174,18 +185,45 @@ def import_csr_file(csrfile, data): return PEM, util.CSR(file=csrfile, data=data_pem, form="pem"), domains -def make_key(bits): - """Generate PEM encoded RSA key. +def make_key(bits=1024, key_type="rsa", elliptic_curve=None): + """Generate PEM encoded RSA|EC key. - :param int bits: Number of bits, at least 1024. + :param int bits: Number of bits if key_type=rsa. At least 1024 for RSA. - :returns: new RSA key in PEM form with specified number of bits + :param str ec_curve: The elliptic curve to use. + + :returns: new RSA or ECDSA key in PEM form with specified number of bits + or of type ec_curve when key_type ecdsa is used. :rtype: str - """ - assert bits >= 1024 # XXX - key = crypto.PKey() - key.generate_key(crypto.TYPE_RSA, bits) + if key_type == 'rsa': + if bits < 1024: + raise errors.Error("Unsupported RSA key length: {}".format(bits)) + + key = crypto.PKey() + key.generate_key(crypto.TYPE_RSA, bits) + elif key_type == 'ecdsa': + try: + name = elliptic_curve.upper() + if name in ('SECP256R1', 'SECP384R1', 'SECP512R1'): + _key = ec.generate_private_key( + curve=getattr(ec, elliptic_curve.upper(), None)(), + backend=default_backend() + ) + else: + raise errors.Error("Unsupported elliptic curve: {}".format(elliptic_curve)) + except TypeError: + raise errors.Error("Unsupported elliptic curve: {}".format(elliptic_curve)) + except UnsupportedAlgorithm as e: + raise six.raise_from(e, errors.Error(str(e))) + _key_pem = _key.private_bytes( + encoding=Encoding.PEM, + format=PrivateFormat.TraditionalOpenSSL, + encryption_algorithm=NoEncryption() + ) + key = crypto.load_privatekey(crypto.FILETYPE_PEM, _key_pem) + else: + raise errors.Error("Invalid key_type specified: {}. Use [rsa|ecdsa]".format(key_type)) return crypto.dump_privatekey(crypto.FILETYPE_PEM, key) diff --git a/certbot/certbot/interfaces.py b/certbot/certbot/interfaces.py index 43f081b95..6ba28bd56 100644 --- a/certbot/certbot/interfaces.py +++ b/certbot/certbot/interfaces.py @@ -198,6 +198,13 @@ class IConfig(zope.interface.Interface): "register multiple emails, ex: u1@example.com,u2@example.com. " "(default: Ask).") rsa_key_size = zope.interface.Attribute("Size of the RSA key.") + elliptic_curve = zope.interface.Attribute( + "The SECG elliptic curve name to use. Please see RFC 8446 " + "for supported values." + ) + key_type = zope.interface.Attribute( + "Type of generated private key" + "(Only *ONE* per invocation can be provided at this time)") must_staple = zope.interface.Attribute( "Adds the OCSP Must Staple extension to the certificate. " "Autoconfigures OCSP Stapling for supported setups " @@ -260,6 +267,7 @@ class IConfig(zope.interface.Interface): "offered chain will be used." ) + class IInstaller(IPlugin): """Generic Certbot Installer Interface. diff --git a/certbot/certbot/tests/testdata/README b/certbot/certbot/tests/testdata/README index 867215916..ade2fae2b 100644 --- a/certbot/certbot/tests/testdata/README +++ b/certbot/certbot/tests/testdata/README @@ -2,10 +2,16 @@ The following command has been used to generate test keys: for x in 256 512 2048; do openssl genrsa -out rsa${k}_key.pem $k; done +For the elliptic curve private keys, this command was used: + + for k in "prime256v1" "secp384r1" "secp521r1" do + openssl genpkey -algorithm ${k} -out ec_${k}_key.pem + done + and for the CSR PEM (Certificate Signing Request): openssl req -new -out csr-Xsans_X.pem -key rsa512_key.pem [-config csr-Xsans_X.conf | -subj '/CN=example.com'] [-outform DER > csr_X.der] and for the certificate: - openssl req -new -out cert_X.pem -key rsaX_key.pem -subj '/CN=example.com' -x509 [-outform DER > cert_X.der] \ No newline at end of file + openssl req -new -out cert_X.pem -key rsaX_key.pem -subj '/CN=example.com' -x509 [-outform DER > cert_X.der] diff --git a/certbot/certbot/tests/testdata/ec_prime256v1_key.pem b/certbot/certbot/tests/testdata/ec_prime256v1_key.pem new file mode 100644 index 000000000..77552a656 --- /dev/null +++ b/certbot/certbot/tests/testdata/ec_prime256v1_key.pem @@ -0,0 +1,8 @@ +-----BEGIN EC PARAMETERS----- +BggqhkjOPQMBBw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIDqQPQl69kuh+DrecC8SFPt21f0F/HHDP3T4/Lf0zIVFoAoGCCqGSM49 +AwEHoUQDQgAEHou50Ee9u+8Vial6VbUHExlzsiCHtORlW0X0pKo5RspIKB0QyKwo +dUXvBbv95I9yCO5+MlGkKjwLHtIEze0Hww== +-----END EC PRIVATE KEY----- diff --git a/certbot/certbot/tests/testdata/ec_secp384r1_key.pem b/certbot/certbot/tests/testdata/ec_secp384r1_key.pem new file mode 100644 index 000000000..19a846a46 --- /dev/null +++ b/certbot/certbot/tests/testdata/ec_secp384r1_key.pem @@ -0,0 +1,9 @@ +-----BEGIN EC PARAMETERS----- +BgUrgQQAIg== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MIGkAgEBBDAgvZGw5C7Mp26N0cXA+vIg5K/J5MJw+MVGYfGF4ZutuCLeYMrWT68R +A0h6hJvDtMSgBwYFK4EEACKhZANiAAR1uQYZeU5Kml5o53Q8/PCdwUbqdgCSkV0C +J5a6bhDRMp20fdp2T/mbkdxuVEl81lqfKPZhsd4CZsLaVIU3RUoGgIT1R3QKawpJ +SuXq37yWFX2hqlgt+lsBufZ8RD5QnZc= +-----END EC PRIVATE KEY----- diff --git a/certbot/certbot/tests/testdata/ec_secp521r1_key.pem b/certbot/certbot/tests/testdata/ec_secp521r1_key.pem new file mode 100644 index 000000000..78bb9a0ea --- /dev/null +++ b/certbot/certbot/tests/testdata/ec_secp521r1_key.pem @@ -0,0 +1,10 @@ +-----BEGIN EC PARAMETERS----- +BgUrgQQAIw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MIHcAgEBBEIACWWVKm1qAIejZ6qmqk9D69wQW5FAe3Er0IxWAMkonTEhu8EH5Q2i +2vT2bESm730zhGTe2Pn11b85H6UI9hxhCHygBwYFK4EEACOhgYkDgYYABAEQi1WF +m3suHjPyWACyOJYGUn1Kx6rfBo0PjC7X2TU9jr8umLkIpaaF5UsBuMBmdz1IHL0U +k0gQtoOQ0Qu8N74GuAGzGR0S3RYIv6gfYVz3dS1K4n4b307Lx62bnvtlNxcIvt3w +hmS5OdvQ1Kdxh6oqbSVhhbQmJcgab78Txx3R2QeCxw== +-----END EC PRIVATE KEY----- diff --git a/certbot/certbot/tests/testdata/sample-archive-ec/cert1.pem b/certbot/certbot/tests/testdata/sample-archive-ec/cert1.pem new file mode 100644 index 000000000..b07af2bf7 --- /dev/null +++ b/certbot/certbot/tests/testdata/sample-archive-ec/cert1.pem @@ -0,0 +1,18 @@ +-----BEGIN CERTIFICATE----- +MIIC2zCCAcOgAwIBAgIIBvrEnbPRYu8wDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UE +AxMdUGViYmxlIEludGVybWVkaWF0ZSBDQSAxMjZjNGIwHhcNMjAxMDEyMjEwNzQw +WhcNMjUxMDEyMjEwNzQwWjAjMSEwHwYDVQQDExhjLmVuY3J5cHRpb24tZXhhbXBs +ZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARjMhuW0ENPPC33PjB5XsYU +CRw640kPQENIDatcTJaENZIZdqKd6rI6jc+lpbmXot7Zi52clJlSJS+V6oDAt2Lh +o4HYMIHVMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB +BQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUj7Kd3ENqxlPf8B2bIGhsjydX +mPswHwYDVR0jBBgwFoAUEiGxlkRsi+VvcogH5dVD3h1laAcwMQYIKwYBBQUHAQEE +JTAjMCEGCCsGAQUFBzABhhVodHRwOi8vMTI3LjAuMC4xOjQwMDIwIwYDVR0RBBww +GoIYYy5lbmNyeXB0aW9uLWV4YW1wbGUuY29tMA0GCSqGSIb3DQEBCwUAA4IBAQCl +k0JXsa8y7fg41WWMDhw60bPW77O0FtOmTcnhdI5daYNemQVk+Q5EMaBLQ/oGjgXd +9QXFzXH1PL904YEnSLt+iTpXn++7rQSNzQsdYqw0neWk4f5pEBiN+WORpb6mwobV +ifMtBOkNEHvrJ2Pkci9U1lLwtKD/DSew6QtJU5DSkmH1XdGuMJiubygEIvELtvgq +cP9S368ZvPmPGmKaJQXBiuaR8MTjY/Bkr79aXQMjKbf+mpn7h0POCcePk1DY/rm6 +Da+X16lf0hHyQhSUa7Vgyim6rK1/hlw+Z00i+sQCKD9Ih7kXuuGqfSDC33cfO8Tj +o/MXO8lcxkrem5zU5QWP +-----END CERTIFICATE----- diff --git a/certbot/certbot/tests/testdata/sample-archive-ec/chain1.pem b/certbot/certbot/tests/testdata/sample-archive-ec/chain1.pem new file mode 100644 index 000000000..64a805c0f --- /dev/null +++ b/certbot/certbot/tests/testdata/sample-archive-ec/chain1.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDUDCCAjigAwIBAgIIbi787yVrcMAwDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE +AxMVUGViYmxlIFJvb3QgQ0EgMGM1MjI1MCAXDTIwMTAxMjIwMjI0NloYDzIwNTAx +MDEyMjEyMjQ2WjAoMSYwJAYDVQQDEx1QZWJibGUgSW50ZXJtZWRpYXRlIENBIDEy +NmM0YjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALGeVk1BMJraeqRq +mJ2+hgso8VOAv2s2CVxUJjIVcn7f2adE8NyTsSQ1brlsnKCUYUw7yLTQH0izLQRB +qKVIDFkUqo5/FuTJ2QlfA2EwBL8J7s/7L7vj3L0DiVpwgxPSyFEwdl/Y5y7ofsX5 +CIhCFcaMAmTIuKLiSfCJjGwkbEMuolm+lO8Mikxxc/JtDVUC479ugU7PU9O09bMH +nm+sD6Bgd+KMoPkCCCoeShJS9X3Ziq9HGc7Z6nhM/zirFARt2XkonEdAZ8br01zY +MRiY9txhlWQ7mUkOtzOSoEuYJNoUbvMUf0+tNzto26WRyF7dJmh7lTBsYrvAwUTx +PzNyst0CAwEAAaOBgzCBgDAOBgNVHQ8BAf8EBAMCAoQwHQYDVR0lBBYwFAYIKwYB +BQUHAwEGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFBIhsZZE +bIvlb3KIB+XVQ94dZWgHMB8GA1UdIwQYMBaAFOaKTaXg37vKgRt7d79YOjAoAtJT +MA0GCSqGSIb3DQEBCwUAA4IBAQAU2mZii7PH2pkw2lNM0QqPbcW/UYyvFoUeM8Aq +uCtsI2s+oxCJTqzfLsA0N8NY4nHLQ5wAlNJfJekngni8hbmJTKU4JFTMe7kLQO8P +fJbk0pTzhhHVQw7CVwB6Pwq3u2m/JV+d6xDIDc+AVkuEl19ZJU0rTWyooClfFLZV +EdZmEiUtA3PGlxoYwYhoGHYlhFxsoFONhCsBEdN7k7FKtFGVxN7oc5SKmKp0YZTW +fcrEtrdNThATO4ymhCC2zh33NI/MT1O74fpaAc2k6LcTl57MKiLfTYX4LTL6v9JG +9tlNqjFVRRmzEbtXTPcCb+w9g1VqoOGok7mGXYLTYtShCuvE +-----END CERTIFICATE----- diff --git a/certbot/certbot/tests/testdata/sample-archive-ec/fullchain1.pem b/certbot/certbot/tests/testdata/sample-archive-ec/fullchain1.pem new file mode 100644 index 000000000..1ba80ba4e --- /dev/null +++ b/certbot/certbot/tests/testdata/sample-archive-ec/fullchain1.pem @@ -0,0 +1,38 @@ +-----BEGIN CERTIFICATE----- +MIIC2zCCAcOgAwIBAgIILlmGtZhUFEwwDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UE +AxMdUGViYmxlIEludGVybWVkaWF0ZSBDQSAxMjZjNGIwHhcNMjAxMDEyMjA1MDM0 +WhcNMjUxMDEyMjA1MDM0WjAjMSEwHwYDVQQDExhjLmVuY3J5cHRpb24tZXhhbXBs +ZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARHEzR8JPWrEmpmgM+F2bk5 +9mT0u6CjzmJG0QpbaqprLiG5NGpW84VQ5TFCrmC4KxYfigCfMhfHRNfFYvNUK3V/ +o4HYMIHVMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB +BQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU1CsVL+bPnzaxxQ5jUENmQJIO +lKwwHwYDVR0jBBgwFoAUEiGxlkRsi+VvcogH5dVD3h1laAcwMQYIKwYBBQUHAQEE +JTAjMCEGCCsGAQUFBzABhhVodHRwOi8vMTI3LjAuMC4xOjQwMDIwIwYDVR0RBBww +GoIYYy5lbmNyeXB0aW9uLWV4YW1wbGUuY29tMA0GCSqGSIb3DQEBCwUAA4IBAQBn +2D8loC7pfk28JYpFLr5lmFKJWWmtLGlpsWDj61fVjtTfGKLziJz+MM6il4Y3hIz5 +58qiFK0ue0M63dIBJ33N+XxSEXon4Q0gy/zRWfH9jtPJ3FwfjkU/RT9PAUClYi0G +ptNWnTmgQkNzousbcAtRNXuuShH3856vhUnwkX+xM+cbIDi1JVmFjcGrEEQJ0rUF +mv2ZTyfbWbUs3v4rReETi2NVzr1Ql6J+ByNcMvHODzFy3t0L6yelAw2ca1I+c9HU ++Z0tnp/ykR7eXNuVLivok8UBf5OC413lh8ZO5g+Bgzh/LdtkUuavg1MYtEX0H6mX +9U7y3nVI8WEbPGf+HDeu +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDUDCCAjigAwIBAgIIbi787yVrcMAwDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE +AxMVUGViYmxlIFJvb3QgQ0EgMGM1MjI1MCAXDTIwMTAxMjIwMjI0NloYDzIwNTAx +MDEyMjEyMjQ2WjAoMSYwJAYDVQQDEx1QZWJibGUgSW50ZXJtZWRpYXRlIENBIDEy +NmM0YjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALGeVk1BMJraeqRq +mJ2+hgso8VOAv2s2CVxUJjIVcn7f2adE8NyTsSQ1brlsnKCUYUw7yLTQH0izLQRB +qKVIDFkUqo5/FuTJ2QlfA2EwBL8J7s/7L7vj3L0DiVpwgxPSyFEwdl/Y5y7ofsX5 +CIhCFcaMAmTIuKLiSfCJjGwkbEMuolm+lO8Mikxxc/JtDVUC479ugU7PU9O09bMH +nm+sD6Bgd+KMoPkCCCoeShJS9X3Ziq9HGc7Z6nhM/zirFARt2XkonEdAZ8br01zY +MRiY9txhlWQ7mUkOtzOSoEuYJNoUbvMUf0+tNzto26WRyF7dJmh7lTBsYrvAwUTx +PzNyst0CAwEAAaOBgzCBgDAOBgNVHQ8BAf8EBAMCAoQwHQYDVR0lBBYwFAYIKwYB +BQUHAwEGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFBIhsZZE +bIvlb3KIB+XVQ94dZWgHMB8GA1UdIwQYMBaAFOaKTaXg37vKgRt7d79YOjAoAtJT +MA0GCSqGSIb3DQEBCwUAA4IBAQAU2mZii7PH2pkw2lNM0QqPbcW/UYyvFoUeM8Aq +uCtsI2s+oxCJTqzfLsA0N8NY4nHLQ5wAlNJfJekngni8hbmJTKU4JFTMe7kLQO8P +fJbk0pTzhhHVQw7CVwB6Pwq3u2m/JV+d6xDIDc+AVkuEl19ZJU0rTWyooClfFLZV +EdZmEiUtA3PGlxoYwYhoGHYlhFxsoFONhCsBEdN7k7FKtFGVxN7oc5SKmKp0YZTW +fcrEtrdNThATO4ymhCC2zh33NI/MT1O74fpaAc2k6LcTl57MKiLfTYX4LTL6v9JG +9tlNqjFVRRmzEbtXTPcCb+w9g1VqoOGok7mGXYLTYtShCuvE +-----END CERTIFICATE----- diff --git a/certbot/certbot/tests/testdata/sample-archive-ec/privkey1.pem b/certbot/certbot/tests/testdata/sample-archive-ec/privkey1.pem new file mode 100644 index 000000000..6843b83d6 --- /dev/null +++ b/certbot/certbot/tests/testdata/sample-archive-ec/privkey1.pem @@ -0,0 +1,5 @@ +-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgNgefv2dad4U1VYEi +0WkdHuqywi5QXAe30OwNTTGjhbihRANCAARHEzR8JPWrEmpmgM+F2bk59mT0u6Cj +zmJG0QpbaqprLiG5NGpW84VQ5TFCrmC4KxYfigCfMhfHRNfFYvNUK3V/ +-----END PRIVATE KEY----- diff --git a/certbot/certbot/tests/testdata/sample-renewal-ec.conf b/certbot/certbot/tests/testdata/sample-renewal-ec.conf new file mode 100644 index 000000000..d8a4b32bf --- /dev/null +++ b/certbot/certbot/tests/testdata/sample-renewal-ec.conf @@ -0,0 +1,79 @@ +# add some stuff here +# assets/integration_tests + +cert = MAGICDIR/live/sample-renewal-ec/cert.pem +privkey = MAGICDIR/live/sample-renewal-ec/privkey.pem +chain = MAGICDIR/live/sample-renewal-ec/chain.pem +fullchain = MAGICDIR/live/sample-renewal-ec/fullchain.pem +renew_before_expiry = 4 years + +# Options and defaults used in the renewal process +[renewalparams] +no_self_upgrade = False +apache_enmod = a2enmod +no_verify_ssl = False +ifaces = None +apache_dismod = a2dismod +register_unsafely_without_email = False +apache_handle_modules = True +uir = None +installer = None +nginx_ctl = nginx +config_dir = MAGICDIR +text_mode = False +func = +staging = True +prepare = False +work_dir = /var/lib/letsencrypt +tos = False +init = False +http01_port = 80 +duplicate = False +noninteractive_mode = True +key_path = None +nginx = False +nginx_server_root = /etc/nginx +fullchain_path = /home/ubuntu/letsencrypt/chain.pem +email = None +csr = None +agree_dev_preview = None +redirect = None +verb = certonly +verbose_count = -3 +config_file = None +renew_by_default = False +hsts = False +apache_handle_sites = True +authenticator = standalone +domains = isnot.org, +key_type = ecdsa +elliptic_curve = secp256r1 +apache_challenge_location = /etc/apache2 +checkpoints = 1 +manual_test_mode = False +apache = False +cert_path = /home/ubuntu/letsencrypt/cert.pem +webroot_path = None +reinstall = False +expand = False +strict_permissions = False +apache_server_root = /etc/apache2 +account = None +dry_run = False +manual_public_ip_logging_ok = False +chain_path = /home/ubuntu/letsencrypt/chain.pem +break_my_certs = False +standalone = True +manual = False +server = https://acme-staging-v02.api.letsencrypt.org/directory +webroot = False +os_packages_only = False +apache_init_script = None +user_agent = None +apache_le_vhost_ext = -le-ssl.conf +debug = False +logs_dir = /var/log/letsencrypt +apache_vhost_root = /etc/apache2/sites-available +configurator = None +must_staple = True +[[webroot_map]] diff --git a/certbot/certbot/tests/testdata/sample-renewal.conf b/certbot/certbot/tests/testdata/sample-renewal.conf index 936c5c0e0..0c56781b3 100644 --- a/certbot/certbot/tests/testdata/sample-renewal.conf +++ b/certbot/certbot/tests/testdata/sample-renewal.conf @@ -44,6 +44,7 @@ apache_handle_sites = True authenticator = standalone domains = isnot.org, rsa_key_size = 2048 +elliptic_curve = secp256r1 apache_challenge_location = /etc/apache2 checkpoints = 1 manual_test_mode = False diff --git a/certbot/certbot/tests/util.py b/certbot/certbot/tests/util.py index 92f52a852..b9d5caa08 100644 --- a/certbot/certbot/tests/util.py +++ b/certbot/certbot/tests/util.py @@ -93,7 +93,7 @@ def load_pyopenssl_private_key(*names): return OpenSSL.crypto.load_privatekey(loader, load_vector(*names)) -def make_lineage(config_dir, testfile): +def make_lineage(config_dir, testfile, ec=False): """Creates a lineage defined by testfile. This creates the archive, live, and renewal directories if @@ -119,7 +119,7 @@ def make_lineage(config_dir, testfile): if not os.path.exists(directory): filesystem.makedirs(directory) - sample_archive = vector_path('sample-archive') + sample_archive = vector_path('sample-archive{}'.format('-ec' if ec else '')) for kind in os.listdir(sample_archive): shutil.copyfile(os.path.join(sample_archive, kind), os.path.join(archive_dir, kind)) diff --git a/certbot/docs/ciphers.rst b/certbot/docs/ciphers.rst index 294e6a7fa..43f648898 100644 --- a/certbot/docs/ciphers.rst +++ b/certbot/docs/ciphers.rst @@ -80,8 +80,8 @@ its preferences in accordance with its own policy or its administrators' preferences, and use different cryptographic mechanisms or parameters, or a different priority order, than the defaults provided by Certbot. -If you don't use Certbot to configure your server directly, because the -client doesn't integrate with your server software or because you chose +If you don't use Certbot to configure your server directly, because the +client doesn't integrate with your server software or because you chose not to use this integration, then the cryptographic defaults haven't been modified, and the cryptography chosen by the server will still be whatever the default for your software was. For example, if you obtain a @@ -254,7 +254,7 @@ https://docs.aws.amazon.com/ElasticLoadBalancing/latest/DeveloperGuide/elb-secur U.S. Government 18F ~~~~~~~~~~~~~~~~~~~ -The 18F site (https://18f.gsa.gov/) is using +The 18F site (https://18f.gsa.gov/) is using :: diff --git a/certbot/docs/cli-help.txt b/certbot/docs/cli-help.txt index 3c3497282..5f8e2e00b 100644 --- a/certbot/docs/cli-help.txt +++ b/certbot/docs/cli-help.txt @@ -1,4 +1,4 @@ -usage: +usage: certbot [SUBCOMMAND] [options] [-d DOMAIN] [-d DOMAIN] ... Certbot can obtain and install HTTPS/TLS/SSL certificates. By default, @@ -188,6 +188,12 @@ security: Security parameters & server settings --rsa-key-size N Size of the RSA key. (default: 2048) + --key-type type The type of algorithm to use for the the private key. + Either ``rsa`` or ``ecdsa``. (default: ``rsa``). + --elliptic-curve The elliptic curve to use when choosing ``ecdsa`` as the key + type. Accepted values are SECG curve names as defined by + the cryptography library. ``secp256r1``, ``secp384r1``, + ``secp521r1``. (default: secp256r1). --must-staple Adds the OCSP Must Staple extension to the certificate. Autoconfigures OCSP Stapling for supported setups (Apache version >= 2.3.3 ). (default: diff --git a/certbot/docs/using.rst b/certbot/docs/using.rst index 290ac817a..353029822 100644 --- a/certbot/docs/using.rst +++ b/certbot/docs/using.rst @@ -319,6 +319,7 @@ This returns information in the following format:: Domains: example.com, www.example.com Expiry Date: 2017-02-19 19:53:00+00:00 (VALID: 30 days) Certificate Path: /etc/letsencrypt/live/example.com/fullchain.pem + Key Type: RSA Private Key Path: /etc/letsencrypt/live/example.com/privkey.pem ``Certificate Name`` shows the name of the certificate. Pass this name @@ -346,7 +347,6 @@ control Certbot's behavior when re-creating a certificate with the same name as an existing certificate. If you don't specify a requested behavior, Certbot may ask you what you intended. - ``--force-renewal`` tells Certbot to request a new certificate with the same domains as an existing certificate. Each domain must be explicitly specified via ``-d``. If successful, this certificate @@ -380,7 +380,6 @@ If you prefer, you can specify the domains individually like this: Consider using ``--cert-name`` instead of ``--expand``, as it gives more control over which certificate is modified and it lets you remove domains as well as adding them. - ``--allow-subset-of-names`` tells Certbot to continue with certificate generation if only some of the specified domain authorizations can be obtained. This may be useful if some domains specified in a certificate no longer point at this @@ -411,6 +410,19 @@ replace that set entirely:: certbot certonly --cert-name example.com -d example.org,www.example.org +Migrating to certificates based on ECDSA keys +--------------------------------------------- + +As of version 1.10, Certbot supports two types of private key algorithms: +``rsa`` and ``ecdsa``. You may freely upgrade an existing certificate with a +new private key. This requires issuing a new command, or changing the renewal +file for the certificates so it will happen on the next renewal. The two +options that you need for the renewal command are ``--key-type`` and +``--elliptic-curve `` in case you either want to be explicit or want to +use something else than the default curve ``secp256r1``:: + + certbot renew --key-type ecdsa --cert-name example.com -d example.org,www.example.org + Revoking certificates --------------------- diff --git a/certbot/examples/cli.ini b/certbot/examples/cli.ini index 4215fda5b..dfb1d6fff 100644 --- a/certbot/examples/cli.ini +++ b/certbot/examples/cli.ini @@ -7,6 +7,10 @@ # certificate on a system with several certificates should not be placed # here. +# Use ECC for the private key +key-type = ecdsa +elliptic-curve = secp384r1 + # Use a 4096 bit RSA key instead of 2048 rsa-key-size = 4096 diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 7475e99ea..e65ec32ec 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -359,6 +359,21 @@ class ParseTest(unittest.TestCase): self.assertFalse(cli.option_was_set( 'authenticator', cli.flag_default('authenticator'))) + def test_ecdsa_key_option(self): + elliptic_curve_option = 'elliptic_curve' + elliptic_curve_option_value = cli.flag_default(elliptic_curve_option) + self.parse('--elliptic-curve {0}'.format(elliptic_curve_option_value).split()) + self.assertIs(cli.option_was_set(elliptic_curve_option, elliptic_curve_option_value), True) + + def test_invalid_key_type(self): + key_type_option = 'key_type' + key_type_value = cli.flag_default(key_type_option) + self.parse('--key-type {0}'.format(key_type_value).split()) + self.assertIs(cli.option_was_set(key_type_option, key_type_value), True) + + with self.assertRaises(SystemExit): + self.parse("--key-type foo") + def test_encode_revocation_reason(self): for reason, code in constants.REVOCATION_REASONS.items(): namespace = self.parse(['--reason', reason]) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 1b2a7413e..f40689e57 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -329,7 +329,11 @@ class ClientTest(ClientTestCommon): self._test_obtain_certificate_common(mock.sentinel.key, csr) mock_crypto_util.init_save_key.assert_called_once_with( - self.config.rsa_key_size, self.config.key_dir) + key_size=self.config.rsa_key_size, + key_dir=self.config.key_dir, + key_type=self.config.key_type, + elliptic_curve=None, # elliptic curve is not set + ) mock_crypto_util.init_save_csr.assert_called_once_with( mock.sentinel.key, self.eg_domains, self.config.csr_dir) mock_crypto_util.cert_and_chain_from_fullchain.assert_called_once_with( @@ -365,7 +369,11 @@ class ClientTest(ClientTestCommon): self.client.config.dry_run = True self._test_obtain_certificate_common(key, csr) - mock_crypto.make_key.assert_called_once_with(self.config.rsa_key_size) + mock_crypto.make_key.assert_called_once_with( + bits=self.config.rsa_key_size, + elliptic_curve=None, # not making an elliptic private key + key_type=self.config.key_type, + ) mock_acme_crypto.make_csr.assert_called_once_with( mock.sentinel.key_pem, self.eg_domains, self.config.must_staple) mock_crypto.init_save_key.assert_not_called() diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index bbd484c91..37673db99 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -4,7 +4,7 @@ import unittest try: import mock -except ImportError: # pragma: no cover +except ImportError: # pragma: no cover from unittest import mock import OpenSSL import zope.component @@ -32,6 +32,7 @@ CERT_LEAF = test_util.load_vector('cert_leaf.pem') CERT_ISSUER = test_util.load_vector('cert_intermediate_1.pem') CERT_ALT_ISSUER = test_util.load_vector('cert_intermediate_2.pem') + class InitSaveKeyTest(test_util.TempDirTestCase): """Tests for certbot.crypto_util.init_save_key.""" def setUp(self): @@ -174,11 +175,54 @@ class ImportCSRFileTest(unittest.TestCase): class MakeKeyTest(unittest.TestCase): """Tests for certbot.crypto_util.make_key.""" - def test_it(self): # pylint: disable=no-self-use + def test_rsa(self): # pylint: disable=no-self-use + # RSA Key Type Test from certbot.crypto_util import make_key # Do not test larger keys as it takes too long. + OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, make_key(1024)) + + def test_ec(self): # pylint: disable=no-self-use + # ECDSA Key Type Tests + from certbot.crypto_util import make_key + # Do not test larger keys as it takes too long. + + # Try a good key size for ECDSA OpenSSL.crypto.load_privatekey( - OpenSSL.crypto.FILETYPE_PEM, make_key(1024)) + OpenSSL.crypto.FILETYPE_PEM, make_key(elliptic_curve="secp256r1", key_type='ecdsa')) + + def test_bad_key_sizes(self): + from certbot.crypto_util import make_key + # Try a bad key size for RSA and ECDSA + with self.assertRaises(errors.Error) as e: + make_key(bits=512, key_type='rsa') + self.assertEqual( + "Unsupported RSA key length: 512", + str(e.exception), + "Unsupported RSA key length: 512" + ) + + def test_bad_elliptic_curve_name(self): + from certbot.crypto_util import make_key + with self.assertRaises(errors.Error) as e: + make_key(elliptic_curve="nothere", key_type='ecdsa') + self.assertEqual( + "Unsupported elliptic curve: nothere", + str(e.exception), + "Unsupported elliptic curve: nothere" + ) + + def test_bad_key_type(self): + from certbot.crypto_util import make_key + + # Try a bad --key-type + with self.assertRaises(errors.Error) as e: + OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, make_key(1024, key_type='unf')) + self.assertEqual( + "Invalid key_type specified: unf. Use [rsa|ecdsa]", + str(e.exception), + "Invalid key_type specified: unf. Use [rsa|ecdsa]", + ) class VerifyCertSetup(unittest.TestCase): diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py index 0957b5c31..e292d24fb 100644 --- a/certbot/tests/renewal_test.py +++ b/certbot/tests/renewal_test.py @@ -74,6 +74,30 @@ class RenewalTest(test_util.ConfigTestCase): assert self.config.rsa_key_size == 2048 + def test_reuse_ec_key_renewal_params(self): + self.config.elliptic_curve = 'INVALID_CURVE' + self.config.reuse_key = True + self.config.dry_run = True + self.config.key_type = 'ecdsa' + config = configuration.NamespaceConfig(self.config) + + rc_path = test_util.make_lineage( + self.config.config_dir, + 'sample-renewal-ec.conf', + ec=True, + ) + lineage = storage.RenewableCert(rc_path, config) + + le_client = mock.MagicMock() + le_client.obtain_certificate.return_value = (None, None, None, None) + + from certbot._internal import renewal + + with mock.patch('certbot._internal.renewal.hooks.renew_hook'): + renewal.renew_cert(self.config, None, le_client, lineage) + + assert self.config.elliptic_curve == 'secp256r1' + class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase): """Tests for certbot._internal.renewal.restore_required_config_elements.""" From 8f5787008de288c2cec6e5d3763854703d02f298 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Wed, 11 Nov 2020 21:36:16 +0100 Subject: [PATCH 2/5] Handle unexpected key type migration. (#8435) Fixes #8365 This PR adds a control when `certbot certonly` or `certbot run` are called for a certificate that already exists and would eventually be replaced. As described in #8365, this control is here to ensure that the user will not modify the key type of their certificate (eg. ECDSA to RSA) without an explicit approval (set explicitly `--cert-name` and `--key-type`), since RSA is the default if not specified. * Handle unexpected key type migration. * Update certbot-ci/certbot_integration_tests/certbot_tests/test_main.py Co-authored-by: Brad Warren --- .../certbot_tests/test_main.py | 10 ++++ certbot/certbot/_internal/main.py | 51 +++++++++++++++++-- certbot/tests/main_test.py | 51 +++++++++++++++++-- 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py index 02c537733..58d5a852d 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py @@ -499,6 +499,16 @@ def test_renew_with_ec_keys(context): assert_elliptic_key(key2, SECP384R1) assert 280 < os.stat(key2).st_size < 320 # ec keys of 384 bits are ~310 bytes + # We expect here that the command will fail because without --key-type specified, + # Certbot must error out to prevent changing an existing certificate key type, + # without explicit user consent (by specifying both --cert-name and --key-type). + with pytest.raises(subprocess.CalledProcessError): + context.certbot([ + 'certonly', + '--force-renewal', + '-d', certname + ]) + def test_ocsp_must_staple(context): """Test that OCSP Must-Staple is correctly set in the generated certificate.""" diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index fd7df8d83..8d62210e0 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -11,7 +11,7 @@ import josepy as jose import zope.component from acme import errors as acme_errors -from acme.magic_typing import Union, Iterable, Optional # pylint: disable=unused-import +from acme.magic_typing import Union, Iterable, Optional, List, Tuple # pylint: disable=unused-import import certbot from certbot import crypto_util from certbot import errors @@ -130,7 +130,33 @@ def _get_and_save_cert(le_client, config, domains=None, certname=None, lineage=N return lineage -def _handle_subset_cert_request(config, domains, cert): +def _handle_unexpected_key_type_migration(config, cert): + # type: (configuration.NamespaceConfig, storage.RenewableCert) -> None + """ + This function ensures that the user will not implicitly migrate an existing key + from one type to another in the situation where a certificate for that lineage + already exist and they have not provided explicitly --key-type and --cert-name. + :param config: Current configuration provided by the client + :param cert: Matching certificate that could be renewed + """ + if not cli.set_by_cli("key_type") or not cli.set_by_cli("certname"): + + new_key_type = config.key_type.upper() + cur_key_type = cert.private_key_type.upper() + + if new_key_type != cur_key_type: + msg = ('Are you trying to change the key type of the certificate named {0} ' + 'from {1} to {2}? Please provide both --cert-name and --key-type on ' + 'the command line confirm the change you are trying to make.') + msg = msg.format(cert.lineagename, cur_key_type, new_key_type) + raise errors.Error(msg) + + +def _handle_subset_cert_request(config, # type: configuration.NamespaceConfig + domains, # type: List[str] + cert # type: storage.RenewableCert + ): + # type: (...) -> Tuple[str, Optional[storage.RenewableCert]] """Figure out what to do if a previous cert had a subset of the names now requested :param config: Configuration object @@ -147,6 +173,8 @@ def _handle_subset_cert_request(config, domains, cert): :rtype: `tuple` of `str` """ + _handle_unexpected_key_type_migration(config, cert) + existing = ", ".join(cert.names()) question = ( "You have an existing certificate that contains a portion of " @@ -177,7 +205,10 @@ def _handle_subset_cert_request(config, domains, cert): raise errors.Error(USER_CANCELLED) -def _handle_identical_cert_request(config, lineage): +def _handle_identical_cert_request(config, # type: configuration.NamespaceConfig + lineage, # type: storage.RenewableCert + ): + # type: (...) -> Tuple[str, Optional[storage.RenewableCert]] """Figure out what to do if a lineage has the same names as a previously obtained one :param config: Configuration object @@ -191,6 +222,8 @@ def _handle_identical_cert_request(config, lineage): :rtype: `tuple` of `str` """ + _handle_unexpected_key_type_migration(config, lineage) + if not lineage.ensure_deployed(): return "reinstall", lineage if renewal.should_renew(config, lineage): @@ -266,6 +299,7 @@ def _find_lineage_for_domains(config, domains): return _handle_subset_cert_request(config, domains, subset_names_cert) return None, None + def _find_cert(config, domains, certname): """Finds an existing certificate object given domains and/or a certificate name. @@ -289,7 +323,12 @@ def _find_cert(config, domains, certname): logger.info("Keeping the existing certificate") return (action != "reinstall"), lineage -def _find_lineage_for_domains_and_certname(config, domains, certname): + +def _find_lineage_for_domains_and_certname(config, # type: configuration.NamespaceConfig + domains, # type: List[str] + certname # type: str + ): + # type: (...) -> Tuple[str, Optional[storage.RenewableCert]] """Find appropriate lineage based on given domains and/or certname. :param config: Configuration object @@ -316,8 +355,9 @@ def _find_lineage_for_domains_and_certname(config, domains, certname): if lineage: if domains: if set(cert_manager.domains_for_certname(config, certname)) != set(domains): + _handle_unexpected_key_type_migration(config, lineage) _ask_user_to_confirm_new_names(config, domains, certname, - lineage.names()) # raises if no + lineage.names()) # raises if no return "renew", lineage # unnecessarily specified domains or no domains specified return _handle_identical_cert_request(config, lineage) @@ -386,6 +426,7 @@ def _ask_user_to_confirm_new_names(config, new_domains, certname, old_domains): if not obj.yesno(msg, "Update cert", "Cancel", default=True): raise errors.ConfigurationError("Specified mismatched cert name and domains.") + def _find_domains_or_certname(config, installer, question=None): """Retrieve domains and certname from config or user input. diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 691ced439..26154ae16 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -49,14 +49,51 @@ RSA2048_KEY_PATH = test_util.vector_path('rsa2048_key.pem') SS_CERT_PATH = test_util.vector_path('cert_2048.pem') -class TestHandleIdenticalCerts(unittest.TestCase): - """Test for certbot._internal.main._handle_identical_cert_request""" - def test_handle_identical_cert_request_pending(self): +class TestHandleCerts(unittest.TestCase): + """Test for certbot._internal.main._handle_* methods""" + @mock.patch("certbot._internal.main._handle_unexpected_key_type_migration") + def test_handle_identical_cert_request_pending(self, mock_handle_migration): mock_lineage = mock.Mock() mock_lineage.ensure_deployed.return_value = False # pylint: disable=protected-access ret = main._handle_identical_cert_request(mock.Mock(), mock_lineage) self.assertEqual(ret, ("reinstall", mock_lineage)) + self.assertTrue(mock_handle_migration.called) + + @mock.patch("certbot._internal.main._handle_unexpected_key_type_migration") + def test_handle_subset_cert_request(self, mock_handle_migration): + mock_config = mock.Mock() + mock_config.expand = True + mock_lineage = mock.Mock() + mock_lineage.names.return_value = ["dummy1", "dummy2"] + ret = main._handle_subset_cert_request(mock_config, ["dummy1"], mock_lineage) + self.assertEqual(ret, ("renew", mock_lineage)) + self.assertTrue(mock_handle_migration.called) + + @mock.patch("certbot._internal.main.cli.set_by_cli") + def test_handle_unexpected_key_type_migration(self, mock_set): + config = mock.Mock() + config.key_type = "rsa" + cert = mock.Mock() + cert.private_key_type = "ecdsa" + + mock_set.return_value = True + main._handle_unexpected_key_type_migration(config, cert) + + mock_set.return_value = False + with self.assertRaises(errors.Error) as raised: + main._handle_unexpected_key_type_migration(config, cert) + self.assertTrue("Please provide both --cert-name and --key-type" in str(raised.exception)) + + mock_set.side_effect = lambda var: var != "certname" + with self.assertRaises(errors.Error) as raised: + main._handle_unexpected_key_type_migration(config, cert) + self.assertTrue("Please provide both --cert-name and --key-type" in str(raised.exception)) + + mock_set.side_effect = lambda var: var != "key_type" + with self.assertRaises(errors.Error) as raised: + main._handle_unexpected_key_type_migration(config, cert) + self.assertTrue("Please provide both --cert-name and --key-type" in str(raised.exception)) class RunTest(test_util.ConfigTestCase): @@ -163,9 +200,10 @@ class CertonlyTest(unittest.TestCase): @mock.patch('certbot._internal.cert_manager.lineage_for_certname') @mock.patch('certbot._internal.cert_manager.domains_for_certname') @mock.patch('certbot._internal.renewal.renew_cert') + @mock.patch('certbot._internal.main._handle_unexpected_key_type_migration') @mock.patch('certbot._internal.main._report_new_cert') def test_find_lineage_for_domains_and_certname(self, mock_report_cert, - mock_renew_cert, mock_domains, mock_lineage): + mock_handle_type, mock_renew_cert, mock_domains, mock_lineage): domains = ['example.com', 'test.org'] mock_domains.return_value = domains mock_lineage.names.return_value = domains @@ -175,6 +213,7 @@ class CertonlyTest(unittest.TestCase): self.assertTrue(mock_domains.call_count == 1) self.assertTrue(mock_renew_cert.call_count == 1) self.assertTrue(mock_report_cert.call_count == 1) + self.assertTrue(mock_handle_type.call_count == 1) # user confirms updating lineage with new domains self._call(('certonly --webroot -d example.com -d test.com ' @@ -183,11 +222,12 @@ class CertonlyTest(unittest.TestCase): self.assertTrue(mock_domains.call_count == 2) self.assertTrue(mock_renew_cert.call_count == 2) self.assertTrue(mock_report_cert.call_count == 2) + self.assertTrue(mock_handle_type.call_count == 2) # error in _ask_user_to_confirm_new_names self.mock_get_utility().yesno.return_value = False self.assertRaises(errors.ConfigurationError, self._call, - ('certonly --webroot -d example.com -d test.com --cert-name example.com').split()) + 'certonly --webroot -d example.com -d test.com --cert-name example.com'.split()) @mock.patch('certbot._internal.cert_manager.domains_for_certname') @mock.patch('certbot.display.ops.choose_names') @@ -982,6 +1022,7 @@ class MainTest(test_util.ConfigTestCase): mock_lineage.should_autorenew.return_value = due_for_renewal mock_lineage.has_pending_deployment.return_value = False mock_lineage.names.return_value = ['isnot.org'] + mock_lineage.private_key_type = 'RSA' mock_certr = mock.MagicMock() mock_key = mock.MagicMock(pem='pem_key') mock_client = mock.MagicMock() From f15f4f9838c46e015fb4a183aeb415c794418ea9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 11 Nov 2020 15:06:50 -0800 Subject: [PATCH 3/5] Add certbot renew --key-type test (#8447) * Test certbot renew --key-type * Fix typo --- .../certbot_tests/assertions.py | 18 ++++++++++++++++++ .../certbot_tests/test_main.py | 17 +++++++++-------- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py b/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py index c19c0762e..c223d524c 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py @@ -4,6 +4,7 @@ import os from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives.asymmetric.ec import EllipticCurvePrivateKey +from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey from cryptography.hazmat.primitives.serialization import load_pem_private_key try: @@ -21,6 +22,11 @@ ADMINS_SID = 'S-1-5-32-544' def assert_elliptic_key(key, curve): + """ + Asserts that the key at the given path is an EC key using the given curve. + :param key: path to key + :param curve: name of the expected elliptic curve + """ with open(key, 'rb') as file: privkey1 = file.read() @@ -30,6 +36,18 @@ def assert_elliptic_key(key, curve): assert isinstance(key.curve, curve) +def assert_rsa_key(key): + """ + Asserts that the key at the given path is an RSA key. + :param key: path to key + """ + with open(key, 'rb') as file: + privkey1 = file.read() + + key = load_pem_private_key(data=privkey1, password=None, backend=default_backend()) + assert isinstance(key, RSAPrivateKey) + + def assert_hook_execution(probe_path, probe_content): """ Assert that a certbot hook has been executed diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py index 58d5a852d..d77a71064 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py @@ -9,10 +9,7 @@ import shutil import subprocess import time -from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives.asymmetric.ec import SECP256R1, SECP384R1 -from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey -from cryptography.hazmat.primitives.serialization import load_pem_private_key from cryptography.x509 import NameOID import pytest @@ -20,6 +17,7 @@ import pytest from certbot_integration_tests.certbot_tests import context as certbot_context from certbot_integration_tests.certbot_tests.assertions import assert_cert_count_for_lineage from certbot_integration_tests.certbot_tests.assertions import assert_elliptic_key +from certbot_integration_tests.certbot_tests.assertions import assert_rsa_key from certbot_integration_tests.certbot_tests.assertions import assert_equals_group_owner from certbot_integration_tests.certbot_tests.assertions import assert_equals_group_permissions from certbot_integration_tests.certbot_tests.assertions import assert_equals_world_read_permissions @@ -460,11 +458,7 @@ def test_default_key_type(context): '--cert-name', certname, '-d', certname ]) filename = join(context.config_dir, 'archive/{0}/privkey1.pem').format(certname) - with open(filename, 'rb') as file: - privkey1 = file.read() - - key = load_pem_private_key(data=privkey1, password=None, backend=default_backend()) - assert isinstance(key, RSAPrivateKey) + assert_rsa_key(filename) def test_default_curve_type(context): @@ -509,6 +503,13 @@ def test_renew_with_ec_keys(context): '-d', certname ]) + # We expect that the previous behavior of requiring both --cert-name and + # --key-type to be set to not apply to the renew subcommand. + context.certbot(['renew', '--force-renewal', '--key-type', 'rsa']) + assert_cert_count_for_lineage(context.config_dir, certname, 3) + key3 = join(context.config_dir, 'archive', certname, 'privkey3.pem') + assert_rsa_key(key3) + def test_ocsp_must_staple(context): """Test that OCSP Must-Staple is correctly set in the generated certificate.""" From b742b60c4d9422e00871555a6db29e9ba27dbf78 Mon Sep 17 00:00:00 2001 From: Mads Jensen Date: Thu, 12 Nov 2020 23:33:02 +0100 Subject: [PATCH 4/5] Use better asserts. Added notes to style guide. (#8451) --- acme/tests/messages_test.py | 8 ++--- certbot-apache/tests/configurator_test.py | 8 ++--- certbot-apache/tests/dualnode_test.py | 6 ++-- certbot-apache/tests/obj_test.py | 20 ++++++------- certbot-dns-rfc2136/tests/dns_rfc2136_test.py | 2 +- certbot-nginx/tests/configurator_test.py | 2 +- certbot-nginx/tests/obj_test.py | 4 +-- certbot/docs/contributing.rst | 9 +++++- certbot/tests/auth_handler_test.py | 4 +-- certbot/tests/display/ops_test.py | 4 +-- certbot/tests/main_test.py | 30 ++++++++++--------- certbot/tests/plugins/common_test.py | 4 +-- certbot/tests/util_test.py | 2 +- 13 files changed, 56 insertions(+), 47 deletions(-) diff --git a/acme/tests/messages_test.py b/acme/tests/messages_test.py index 3458105b2..70b05b419 100644 --- a/acme/tests/messages_test.py +++ b/acme/tests/messages_test.py @@ -108,11 +108,11 @@ class ConstantTest(unittest.TestCase): def test_equality(self): const_a_prime = self.MockConstant('a') - self.assertFalse(self.const_a == self.const_b) - self.assertTrue(self.const_a == const_a_prime) + self.assertNotEqual(self.const_a, self.const_b) + self.assertEqual(self.const_a, const_a_prime) - self.assertTrue(self.const_a != self.const_b) - self.assertFalse(self.const_a != const_a_prime) + self.assertNotEqual(self.const_a, self.const_b) + self.assertEqual(self.const_a, const_a_prime) class DirectoryTest(unittest.TestCase): diff --git a/certbot-apache/tests/configurator_test.py b/certbot-apache/tests/configurator_test.py index 091a6a828..433229727 100644 --- a/certbot-apache/tests/configurator_test.py +++ b/certbot-apache/tests/configurator_test.py @@ -1357,10 +1357,10 @@ class MultipleVhostsTest(util.ApacheTest): # And the actual returned values self.assertEqual(len(vhs), 1) - self.assertTrue(vhs[0].name == "certbot.demo") + self.assertEqual(vhs[0].name, "certbot.demo") self.assertTrue(vhs[0].ssl) - self.assertFalse(vhs[0] == self.vh_truth[3]) + self.assertNotEqual(vhs[0], self.vh_truth[3]) @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.make_vhost_ssl") def test_choose_vhosts_wildcard_no_ssl(self, mock_makessl): @@ -1471,10 +1471,10 @@ class MultipleVhostsTest(util.ApacheTest): self.config.parser.aug.match = mock_match vhs = self.config.get_virtual_hosts() self.assertEqual(len(vhs), 2) - self.assertTrue(vhs[0] == self.vh_truth[1]) + self.assertEqual(vhs[0], self.vh_truth[1]) # mock_vhost should have replaced the vh_truth[0], because its filepath # isn't a symlink - self.assertTrue(vhs[1] == mock_vhost) + self.assertEqual(vhs[1], mock_vhost) class AugeasVhostsTest(util.ApacheTest): diff --git a/certbot-apache/tests/dualnode_test.py b/certbot-apache/tests/dualnode_test.py index 44cc69ff4..ef7f8b2d8 100644 --- a/certbot-apache/tests/dualnode_test.py +++ b/certbot-apache/tests/dualnode_test.py @@ -412,9 +412,9 @@ class DualParserNodeTest(unittest.TestCase): # pylint: disable=too-many-public- ancestor=self.block, filepath="/path/to/whatever", metadata=self.metadata) - self.assertFalse(self.block == ne_block) - self.assertFalse(self.directive == ne_directive) - self.assertFalse(self.comment == ne_comment) + self.assertNotEqual(self.block, ne_block) + self.assertNotEqual(self.directive, ne_directive) + self.assertNotEqual(self.comment, ne_comment) def test_parsed_paths(self): mock_p = mock.MagicMock(return_value=['/path/file.conf', diff --git a/certbot-apache/tests/obj_test.py b/certbot-apache/tests/obj_test.py index eac6a64ef..8aae3ad45 100644 --- a/certbot-apache/tests/obj_test.py +++ b/certbot-apache/tests/obj_test.py @@ -27,14 +27,14 @@ class VirtualHostTest(unittest.TestCase): "certbot_apache._internal.obj.Addr(('127.0.0.1', '443'))") def test_eq(self): - self.assertTrue(self.vhost1b == self.vhost1) - self.assertFalse(self.vhost1 == self.vhost2) + self.assertEqual(self.vhost1b, self.vhost1) + self.assertNotEqual(self.vhost1, self.vhost2) self.assertEqual(str(self.vhost1b), str(self.vhost1)) - self.assertFalse(self.vhost1b == 1234) + self.assertNotEqual(self.vhost1b, 1234) def test_ne(self): - self.assertTrue(self.vhost1 != self.vhost2) - self.assertFalse(self.vhost1 != self.vhost1b) + self.assertNotEqual(self.vhost1, self.vhost2) + self.assertEqual(self.vhost1, self.vhost1b) def test_conflicts(self): from certbot_apache._internal.obj import Addr @@ -128,13 +128,13 @@ class AddrTest(unittest.TestCase): self.assertTrue(self.addr1.conflicts(self.addr2)) def test_equal(self): - self.assertTrue(self.addr1 == self.addr2) - self.assertFalse(self.addr == self.addr1) - self.assertFalse(self.addr == 123) + self.assertEqual(self.addr1, self.addr2) + self.assertNotEqual(self.addr, self.addr1) + self.assertNotEqual(self.addr, 123) def test_not_equal(self): - self.assertFalse(self.addr1 != self.addr2) - self.assertTrue(self.addr != self.addr1) + self.assertEqual(self.addr1, self.addr2) + self.assertNotEqual(self.addr, self.addr1) if __name__ == "__main__": diff --git a/certbot-dns-rfc2136/tests/dns_rfc2136_test.py b/certbot-dns-rfc2136/tests/dns_rfc2136_test.py index de5e2912b..dc4a73a04 100644 --- a/certbot-dns-rfc2136/tests/dns_rfc2136_test.py +++ b/certbot-dns-rfc2136/tests/dns_rfc2136_test.py @@ -154,7 +154,7 @@ class RFC2136ClientTest(unittest.TestCase): # _find_domain | pylint: disable=protected-access domain = self.rfc2136_client._find_domain('foo.bar.'+DOMAIN) - self.assertTrue(domain == DOMAIN) + self.assertEqual(domain, DOMAIN) def test_find_domain_wraps_errors(self): # _query_soa | pylint: disable=protected-access diff --git a/certbot-nginx/tests/configurator_test.py b/certbot-nginx/tests/configurator_test.py index e677ad471..5e788e394 100644 --- a/certbot-nginx/tests/configurator_test.py +++ b/certbot-nginx/tests/configurator_test.py @@ -842,7 +842,7 @@ class NginxConfiguratorTest(util.NginxTest): self.config.recovery_routine() self.config.revert_challenge_config() self.config.rollback_checkpoints() - self.assertTrue(mock_parser_load.call_count == 3) + self.assertEqual(mock_parser_load.call_count, 3) def test_choose_vhosts_wildcard(self): # pylint: disable=protected-access diff --git a/certbot-nginx/tests/obj_test.py b/certbot-nginx/tests/obj_test.py index 93b9493eb..f385649ed 100644 --- a/certbot-nginx/tests/obj_test.py +++ b/certbot-nginx/tests/obj_test.py @@ -75,7 +75,7 @@ class AddrTest(unittest.TestCase): new_addr1 = Addr.fromstring("192.168.1.1 spdy") self.assertEqual(self.addr1, new_addr1) self.assertNotEqual(self.addr1, self.addr2) - self.assertFalse(self.addr1 == 3333) + self.assertNotEqual(self.addr1, 3333) def test_equivalent_any_addresses(self): from certbot_nginx._internal.obj import Addr @@ -168,7 +168,7 @@ class VirtualHostTest(unittest.TestCase): self.assertEqual(vhost1b, self.vhost1) self.assertEqual(str(vhost1b), str(self.vhost1)) - self.assertFalse(vhost1b == 1234) + self.assertNotEqual(vhost1b, 1234) def test_str(self): stringified = '\n'.join(['file: filep', 'addrs: localhost', diff --git a/certbot/docs/contributing.rst b/certbot/docs/contributing.rst index 32a1ee72b..908f75459 100644 --- a/certbot/docs/contributing.rst +++ b/certbot/docs/contributing.rst @@ -431,16 +431,23 @@ Please: 4. Remember to use ``pylint``. +5. You may consider installing a plugin for `editorconfig`_ in + your editor to prevent some linting warnings. + +6. Please avoid `unittest.assertTrue` or `unittest.assertFalse` when + possible, and use `assertEqual` or more specific assert. They give + better messages when it's failing, and are generally more correct. + .. _Google Python Style Guide: https://google.github.io/styleguide/pyguide.html .. _Sphinx-style: https://www.sphinx-doc.org/ .. _PEP 8 - Style Guide for Python Code: https://www.python.org/dev/peps/pep-0008 +.. _editorconfig: https://editorconfig.org/ Use ``certbot.compat.os`` instead of ``os`` =========================================== - Python's standard library ``os`` module lacks full support for several Windows security features about file permissions (eg. DACLs). However several files handled by Certbot (eg. private keys) need strongly restricted access diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index 6cd207b4b..8eb5d7702 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -510,7 +510,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase): auth_handler._report_failed_authzrs([self.authzr1], 'key') call_list = mock_zope().add_message.call_args_list - self.assertTrue(len(call_list) == 1) + self.assertEqual(len(call_list), 1) self.assertTrue("Domain: example.com\nType: tls\nDetail: detail" in call_list[0][0][0]) @test_util.patch_get_utility() @@ -518,7 +518,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase): from certbot._internal import auth_handler auth_handler._report_failed_authzrs([self.authzr1, self.authzr2], 'key') - self.assertTrue(mock_zope().add_message.call_count == 2) + self.assertEqual(mock_zope().add_message.call_count, 2) def gen_auth_resp(chall_list): diff --git a/certbot/tests/display/ops_test.py b/certbot/tests/display/ops_test.py index bdc6472bf..417c14a76 100644 --- a/certbot/tests/display/ops_test.py +++ b/certbot/tests/display/ops_test.py @@ -43,7 +43,7 @@ class GetEmailTest(unittest.TestCase): mock_input.return_value = (display_util.OK, "foo@bar.baz") with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email: mock_safe_email.return_value = True - self.assertTrue(self._call() == "foo@bar.baz") + self.assertEqual(self._call(), "foo@bar.baz") @test_util.patch_get_utility("certbot.display.ops.z_util") def test_ok_not_safe(self, mock_get_utility): @@ -51,7 +51,7 @@ class GetEmailTest(unittest.TestCase): mock_input.return_value = (display_util.OK, "foo@bar.baz") with mock.patch("certbot.display.ops.util.safe_email") as mock_safe_email: mock_safe_email.side_effect = [False, True] - self.assertTrue(self._call() == "foo@bar.baz") + self.assertEqual(self._call(), "foo@bar.baz") @test_util.patch_get_utility("certbot.display.ops.z_util") def test_invalid_flag(self, mock_get_utility): diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 26154ae16..3e712a039 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -209,20 +209,21 @@ class CertonlyTest(unittest.TestCase): mock_lineage.names.return_value = domains self._call(('certonly --webroot -d example.com -d test.org ' '--cert-name example.com').split()) - self.assertTrue(mock_lineage.call_count == 1) - self.assertTrue(mock_domains.call_count == 1) - self.assertTrue(mock_renew_cert.call_count == 1) - self.assertTrue(mock_report_cert.call_count == 1) - self.assertTrue(mock_handle_type.call_count == 1) + + self.assertEqual(mock_lineage.call_count, 1) + self.assertEqual(mock_domains.call_count, 1) + self.assertEqual(mock_renew_cert.call_count, 1) + self.assertEqual(mock_report_cert.call_count, 1) + self.assertEqual(mock_handle_type.call_count, 1) # user confirms updating lineage with new domains self._call(('certonly --webroot -d example.com -d test.com ' '--cert-name example.com').split()) - self.assertTrue(mock_lineage.call_count == 2) - self.assertTrue(mock_domains.call_count == 2) - self.assertTrue(mock_renew_cert.call_count == 2) - self.assertTrue(mock_report_cert.call_count == 2) - self.assertTrue(mock_handle_type.call_count == 2) + self.assertEqual(mock_lineage.call_count, 2) + self.assertEqual(mock_domains.call_count, 2) + self.assertEqual(mock_renew_cert.call_count, 2) + self.assertEqual(mock_report_cert.call_count, 2) + self.assertEqual(mock_handle_type.call_count, 2) # error in _ask_user_to_confirm_new_names self.mock_get_utility().yesno.return_value = False @@ -240,14 +241,15 @@ class CertonlyTest(unittest.TestCase): # no lineage with this name but we specified domains so create a new cert self._call(('certonly --webroot -d example.com -d test.com ' '--cert-name example.com').split()) - self.assertTrue(mock_lineage.call_count == 1) - self.assertTrue(mock_report_cert.call_count == 1) + self.assertEqual(mock_lineage.call_count, 1) + self.assertEqual(mock_report_cert.call_count, 1) # no lineage with this name and we didn't give domains mock_choose_names.return_value = ["somename"] mock_domains_for_certname.return_value = None self._call(('certonly --webroot --cert-name example.com').split()) - self.assertTrue(mock_choose_names.called) + self.assertIs(mock_choose_names.called, True) + class FindDomainsOrCertnameTest(unittest.TestCase): """Tests for certbot._internal.main._find_domains_or_certname.""" @@ -755,7 +757,7 @@ class MainTest(test_util.ConfigTestCase): # This needed two calls to find_all(), which we're avoiding for now # because of possible side effects: # https://github.com/letsencrypt/letsencrypt/commit/51ed2b681f87b1eb29088dd48718a54f401e4855 - #with mock.patch('certbot._internal.cli.plugins_testable') as plugins: + # with mock.patch('certbot._internal.cli.plugins_testable') as plugins: # plugins.return_value = {"apache": True, "nginx": True} # ret, _, _, _ = self._call(args) # self.assertTrue("Too many flags setting" in ret) diff --git a/certbot/tests/plugins/common_test.py b/certbot/tests/plugins/common_test.py index 344e6312f..bcd190e9b 100644 --- a/certbot/tests/plugins/common_test.py +++ b/certbot/tests/plugins/common_test.py @@ -233,11 +233,11 @@ class AddrTest(unittest.TestCase): def test_eq(self): self.assertEqual(self.addr1, self.addr2.get_addr_obj("")) self.assertNotEqual(self.addr1, self.addr2) - self.assertFalse(self.addr1 == 3333) + self.assertNotEqual(self.addr1, 3333) self.assertEqual(self.addr4, self.addr4.get_addr_obj("")) self.assertNotEqual(self.addr4, self.addr5) - self.assertFalse(self.addr4 == 3333) + self.assertNotEqual(self.addr4, 3333) from certbot.plugins.common import Addr self.assertEqual(self.addr4, Addr.fromstring("[fe00:0:0::1]")) self.assertEqual(self.addr4, Addr.fromstring("[fe00:0::0:0:1]")) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index d28d7df6f..4b93d2c38 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -127,7 +127,7 @@ class LockDirUntilExit(test_util.TempDirTestCase): from certbot import util # Despite lock_dir_until_exit has been called twice to subdir, its lock should have been # added only once. So we expect to have two lock references: for self.tempdir and subdir - self.assertTrue(len(util._LOCKS) == 2) # pylint: disable=protected-access + self.assertEqual(len(util._LOCKS), 2) # pylint: disable=protected-access registered_func() # Exception should not be raised # Logically, logger.debug, that would be invoked in case of unlock failure, # should never been called. From a8cede6ae1f85c32227ccb976b145d971a913f66 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 19 Nov 2020 00:10:56 -0800 Subject: [PATCH 5/5] Flesh out ECDSA documentation (#8464) * Changelog tweaks. * Add ECDSA documentation * Fix typo --- certbot/CHANGELOG.md | 3 +- certbot/docs/using.rst | 67 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index e32bd1072..b7d57a62d 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -9,8 +9,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * Added timeout to DNS query function calls for dns-rfc2136 plugin. * Confirmation when deleting certificates * CLI flag `--key-type` has been added to specify 'rsa' or 'ecdsa' (default 'rsa'). - Only accepts a single value at this time. -* CLI flag `--elliptic-curve` has been added which takes an NIST/SECG elliptic curve. Either of +* CLI flag `--elliptic-curve` has been added which takes an NIST/SECG elliptic curve. Any of `secp256r1`, `secp284r1` and `secp521r1` are accepted values. * The command `certbot certficates` lists the which type of the private key that was used for the private key. diff --git a/certbot/docs/using.rst b/certbot/docs/using.rst index 353029822..1912dafa4 100644 --- a/certbot/docs/using.rst +++ b/certbot/docs/using.rst @@ -410,18 +410,67 @@ replace that set entirely:: certbot certonly --cert-name example.com -d example.org,www.example.org -Migrating to certificates based on ECDSA keys ---------------------------------------------- +Using ECDSA keys +---------------- As of version 1.10, Certbot supports two types of private key algorithms: -``rsa`` and ``ecdsa``. You may freely upgrade an existing certificate with a -new private key. This requires issuing a new command, or changing the renewal -file for the certificates so it will happen on the next renewal. The two -options that you need for the renewal command are ``--key-type`` and -``--elliptic-curve `` in case you either want to be explicit or want to -use something else than the default curve ``secp256r1``:: +``rsa`` and ``ecdsa``. The type of key used by Certbot can be controlled +through the ``--key-type`` option. You can also use the ``--elliptic-curve`` +option to control the curve used in ECDSA certificates. - certbot renew --key-type ecdsa --cert-name example.com -d example.org,www.example.org +.. warning:: If you obtain certificates using ECDSA keys, you should be careful + not to downgrade your Certbot installation since ECDSA keys are not + supported by older versions of Certbot. Downgrades like this are possible if + you switch from something like the snaps or certbot-auto to packages + provided by your operating system which often lag behind. + +Changing existing certificates from RSA to ECDSA +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Unless you are aware that you need to support very old HTTPS clients that are +not supported by most sites, you can safely just transition your site to use +ECDSA keys instead of RSA keys. To accomplish this if you have existing +certificates managed by Certbot, you may freely change the certificate to a new +private key. + +If you want to use ECDSA keys for all certificates in the future, you can +simply add the following line to Certbot's :ref:`configuration file ` + +.. code-block:: ini + + key-type = ecdsa + +After this option is set, newly obtained certificates will use ECDSA keys. This +includes certificates managed by Certbot that previously used RSA keys. + +If you want to change a single certificate to use ECDSA keys, you'll need to +issue a new Certbot command setting ``--key-type ecdsa`` on the command line +like + +.. code-block:: shell + + certbot renew --key-type ecdsa --cert-name example.com --force-renewal + +Obtaining ECDSA certificates in addition to RSA certificates +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When Certbot configures the certificates it obtains with Apache or Nginx, all +HTTPS clients that we try to support can use certificates with ECDSA keys. If, +however, you are aware of having a specific need to support very old TLS +clients, you may want to obtain both ECDSA and RSA certificates for the same +domains. Certbot can only configure Apache or Nginx to use a single +certificate, however, you could manually configure your software to use the +different certificates depending on your needs. + +When obtaining both ECDSA and RSA certificates for the same domains with +Certbot, we recommend using the ``--cert-name`` option to give your +certificates names so that you can easily identify them. For instance, you may +want to append "ecdsa" to the name of your ECDSA certificate by using a command +like + +.. code-block:: shell + + certbot certonly --key-type ecdsa --cert-name example.com-ecdsa Revoking certificates ---------------------