Use ffdhe2048 Nginx DH params to fix Weak-DH bug (#4973)

* Rename plugins.common.install_ssl_options_conf to plugins.common.install_version_controlled_file

* Install ssl_dhparams file

* Add installation test

* Add ssl_dhparam option when making a server block ssl

* add install_ssl_dhparams to Installer common plugin class

* Remove redundant code and tests

* update MANIFEST.in

9 files changed, 118 insertions, 53 deletions

diff --git a/MANIFEST.in b/MANIFEST.in
index 18393e3e1..434a156b7 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -6,3 +6,4 @@ include linter_plugin.py
 recursive-include docs *
 recursive-include examples *
 recursive-include certbot/tests/testdata *
+include certbot/ssl-dhparams.pem
diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 9e3eb4139..4960d08d7 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -1989,5 +1989,5 @@ def install_ssl_options_conf(options_ssl, options_ssl_digest):
     # XXX if we ever try to enforce a local privilege boundary (eg, running
     # certbot for unprivileged users via setuid), this function will need
     # to be modified.
-    return common.install_ssl_options_conf(options_ssl, options_ssl_digest,
+    return common.install_version_controlled_file(options_ssl, options_ssl_digest,
         constants.os_constant("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index 23242d091..5805a8b31 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -1497,7 +1497,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
         with mock.patch("certbot.plugins.common.logger") as mock_logger:
             self._call()
             self.assertEqual(mock_logger.warning.call_args[0][0],
-                "%s has been manually modified; updated ssl configuration options "
+                "%s has been manually modified; updated file "
                 "saved to %s. We recommend updating %s for security purposes.")
         self.assertEqual(crypto_util.sha256sum(constants.os_constant("MOD_SSL_CONF_SRC")),
             self._current_ssl_options_hash())
diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py
index aac8c1237..7e55ff0ea 100644
--- a/certbot-nginx/certbot_nginx/configurator.py
+++ b/certbot-nginx/certbot_nginx/configurator.py
@@ -157,6 +157,8 @@ class NginxConfigurator(common.Installer):
 
         install_ssl_options_conf(self.mod_ssl_conf, self.updated_mod_ssl_conf_digest)
 
+        self.install_ssl_dhparams()
+
         # Set Version
         if self.version is None:
             self.version = self.get_version()
@@ -449,11 +451,13 @@ class NginxConfigurator(common.Installer):
 
         snakeoil_cert, snakeoil_key = self._get_snakeoil_paths()
 
-        ssl_block = (
-            [['\n    ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)],
-             ['\n    ', 'ssl_certificate', ' ', snakeoil_cert],
-             ['\n    ', 'ssl_certificate_key', ' ', snakeoil_key],
-             ['\n    ', 'include', ' ', self.mod_ssl_conf]])
+        ssl_block = ([
+            ['\n    ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)],
+            ['\n    ', 'ssl_certificate', ' ', snakeoil_cert],
+            ['\n    ', 'ssl_certificate_key', ' ', snakeoil_key],
+            ['\n    ', 'include', ' ', self.mod_ssl_conf],
+            ['\n    ', 'ssl_dhparam', ' ', self.ssl_dhparams],
+        ])
 
         self.parser.add_server_directives(
             vhost, ssl_block, replace=False)
@@ -822,5 +826,5 @@ def nginx_restart(nginx_ctl, nginx_conf):
 
 def install_ssl_options_conf(options_ssl, options_ssl_digest):
     """Copy Certbot's SSL options file into the system's config dir if required."""
-    return common.install_ssl_options_conf(options_ssl, options_ssl_digest,
+    return common.install_version_controlled_file(options_ssl, options_ssl_digest,
         constants.MOD_SSL_CONF_SRC, constants.ALL_SSL_OPTIONS_HASHES)
diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py
index 1f9d3e253..6a917204c 100644
--- a/certbot-nginx/certbot_nginx/tests/configurator_test.py
+++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py
@@ -226,8 +226,9 @@ class NginxConfiguratorTest(util.NginxTest):
                             ['listen', '5001', 'ssl'],
                             ['ssl_certificate', 'example/fullchain.pem'],
                             ['ssl_certificate_key', 'example/key.pem'],
-                            ['include', self.config.mod_ssl_conf]]
-                            ]],
+                            ['include', self.config.mod_ssl_conf],
+                            ['ssl_dhparam', self.config.ssl_dhparams],
+                            ]]],
                          parsed_example_conf)
         self.assertEqual([['server_name', 'somename', 'alias', 'another.alias']],
                          parsed_server_conf)
@@ -244,8 +245,9 @@ class NginxConfiguratorTest(util.NginxTest):
               ['listen', '5001', 'ssl'],
               ['ssl_certificate', '/etc/nginx/fullchain.pem'],
               ['ssl_certificate_key', '/etc/nginx/key.pem'],
-              ['include', self.config.mod_ssl_conf]]
-            ],
+              ['include', self.config.mod_ssl_conf],
+              ['ssl_dhparam', self.config.ssl_dhparams],
+            ]],
             2))
 
     def test_deploy_cert_add_explicit_listen(self):
@@ -268,8 +270,9 @@ class NginxConfiguratorTest(util.NginxTest):
                            ['listen', '5001', 'ssl'],
                            ['ssl_certificate', 'summer/fullchain.pem'],
                            ['ssl_certificate_key', 'summer/key.pem'],
-                           ['include', self.config.mod_ssl_conf]]
-                           ],
+                           ['include', self.config.mod_ssl_conf],
+                           ['ssl_dhparam', self.config.ssl_dhparams],
+                           ]],
                          parsed_migration_conf[0])
 
     @mock.patch("certbot_nginx.configurator.tls_sni_01.NginxTlsSni01.perform")
@@ -601,7 +604,7 @@ class InstallSslOptionsConfTest(util.NginxTest):
         with mock.patch("certbot.plugins.common.logger") as mock_logger:
             self._call()
             self.assertEqual(mock_logger.warning.call_args[0][0],
-                "%s has been manually modified; updated ssl configuration options "
+                "%s has been manually modified; updated file "
                 "saved to %s. We recommend updating %s for security purposes.")
         self.assertEqual(crypto_util.sha256sum(constants.MOD_SSL_CONF_SRC),
             self._current_ssl_options_hash())
diff --git a/certbot/constants.py b/certbot/constants.py
index dfdfcc0e8..557ccd4c6 100644
--- a/certbot/constants.py
+++ b/certbot/constants.py
@@ -1,6 +1,7 @@
 """Certbot constants."""
-import os
 import logging
+import os
+import pkg_resources
 
 from acme import challenges
 
@@ -112,3 +113,18 @@ FORCE_INTERACTIVE_FLAG = "--force-interactive"
 
 EFF_SUBSCRIBE_URI = "https://supporters.eff.org/subscribe/certbot"
 """EFF URI used to submit the e-mail address of users who opt-in."""
+
+SSL_DHPARAMS_DEST = "ssl-dhparams.pem"
+"""Name of the ssl_dhparams file as saved in `IConfig.config_dir`."""
+
+SSL_DHPARAMS_SRC = pkg_resources.resource_filename(
+    "certbot", "ssl-dhparams.pem")
+"""Path to the nginx ssl_dhparams file found in the Certbot distribution."""
+
+UPDATED_SSL_DHPARAMS_DIGEST = ".updated-ssl-dhparams-pem-digest.txt"
+"""Name of the hash of the updated or informed ssl_dhparams as saved in `IConfig.config_dir`."""
+
+ALL_SSL_DHPARAMS_HASHES = [
+    '9ba6429597aeed2d8617a7705b56e96d044f64b07971659382e426675105654b',
+]
+"""SHA256 hashes of the contents of all versions of SSL_DHPARAMS_SRC"""
diff --git a/certbot/plugins/common.py b/certbot/plugins/common.py
index 7bb019163..f605eb751 100644
--- a/certbot/plugins/common.py
+++ b/certbot/plugins/common.py
@@ -103,7 +103,7 @@ class Plugin(object):
 
 
 class Installer(Plugin):
-    """An installer base class with reverter methods defined.
+    """An installer base class with reverter and ssl_dhparam methods defined.
 
     Installer plugins do not have to inherit from this class.
 
@@ -197,6 +197,24 @@ class Installer(Plugin):
         except errors.ReverterError as err:
             raise errors.PluginError(str(err))
 
+    @property
+    def ssl_dhparams(self):
+        """Full absolute path to ssl_dhparams file."""
+        return os.path.join(self.config.config_dir, constants.SSL_DHPARAMS_DEST)
+
+    @property
+    def updated_ssl_dhparams_digest(self):
+        """Full absolute path to digest of updated ssl_dhparams file."""
+        return os.path.join(self.config.config_dir, constants.UPDATED_SSL_DHPARAMS_DIGEST)
+
+    def install_ssl_dhparams(self):
+        """Copy Certbot's ssl_dhparams file into the system's config dir if required."""
+        return install_version_controlled_file(
+            self.ssl_dhparams,
+            self.updated_ssl_dhparams_digest,
+            constants.SSL_DHPARAMS_SRC,
+            constants.ALL_SSL_DHPARAMS_HASHES)
+
 
 class Addr(object):
     r"""Represents an virtual host address.
@@ -368,51 +386,50 @@ class TLSSNI01(object):
         return response
 
 
-def install_ssl_options_conf(options_ssl, options_ssl_digest, mod_ssl_conf_src,
-    all_ssl_options_hashes):
-    """Copy Certbot's SSL options file into the system's config dir if required.
+def install_version_controlled_file(dest_path, digest_path, src_path, all_hashes):
+    """Copy a file into an active location (likely the system's config dir) if required.
 
-       :param str options_ssl: destination path for file containing ssl options
-       :param str options_ssl_digest: path to save a digest of options_ssl in
-       :param str mod_ssl_conf_src: path to file containing ssl options found in distribution
-       :param list all_ssl_options_hashes: hashes of every released version of options_ssl
+       :param str dest_path: destination path for version controlled file
+       :param str digest_path: path to save a digest of the file in
+       :param str src_path: path to version controlled file found in distribution
+       :param list all_hashes: hashes of every released version of the file
     """
-    current_ssl_options_hash = crypto_util.sha256sum(mod_ssl_conf_src)
+    current_hash = crypto_util.sha256sum(src_path)
 
     def _write_current_hash():
-        with open(options_ssl_digest, "w") as f:
-            f.write(current_ssl_options_hash)
+        with open(digest_path, "w") as f:
+            f.write(current_hash)
 
     def _install_current_file():
-        shutil.copyfile(mod_ssl_conf_src, options_ssl)
+        shutil.copyfile(src_path, dest_path)
         _write_current_hash()
 
     # Check to make sure options-ssl.conf is installed
-    if not os.path.isfile(options_ssl):
+    if not os.path.isfile(dest_path):
         _install_current_file()
         return
     # there's already a file there. if it's up to date, do nothing. if it's not but
     # it matches a known file hash, we can update it.
     # otherwise, print a warning once per new version.
-    active_file_digest = crypto_util.sha256sum(options_ssl)
-    if active_file_digest == current_ssl_options_hash: # already up to date
+    active_file_digest = crypto_util.sha256sum(dest_path)
+    if active_file_digest == current_hash: # already up to date
         return
-    elif active_file_digest in all_ssl_options_hashes: # safe to update
+    elif active_file_digest in all_hashes: # safe to update
         _install_current_file()
     else: # has been manually modified, not safe to update
         # did they modify the current version or an old version?
-        if os.path.isfile(options_ssl_digest):
-            with open(options_ssl_digest, "r") as f:
+        if os.path.isfile(digest_path):
+            with open(digest_path, "r") as f:
                 saved_digest = f.read()
             # they modified it after we either installed or told them about this version, so return
-            if saved_digest == current_ssl_options_hash:
+            if saved_digest == current_hash:
                 return
         # there's a new version but we couldn't update the file, or they deleted the digest.
         # save the current digest so we only print this once, and print a warning
         _write_current_hash()
-        logger.warning("%s has been manually modified; updated ssl configuration options "
+        logger.warning("%s has been manually modified; updated file "
             "saved to %s. We recommend updating %s for security purposes.",
-            options_ssl, mod_ssl_conf_src, options_ssl)
+            dest_path, src_path, dest_path)
 
 
 # test utils used by certbot_apache/certbot_nginx (hence
diff --git a/certbot/plugins/common_test.py b/certbot/plugins/common_test.py
index cece2c5f9..8ce68bbb5 100644
--- a/certbot/plugins/common_test.py
+++ b/certbot/plugins/common_test.py
@@ -79,14 +79,16 @@ class PluginTest(unittest.TestCase):
             "--mock-foo-bar", dest="different_to_foo_bar", x=1, y=None)
 
 
-class InstallerTest(unittest.TestCase):
+class InstallerTest(test_util.ConfigTestCase):
     """Tests for certbot.plugins.common.Installer."""
 
     def setUp(self):
+        super(InstallerTest, self).setUp()
+        os.mkdir(self.config.config_dir)
         from certbot.plugins.common import Installer
 
         with mock.patch("certbot.plugins.common.reverter.Reverter"):
-            self.installer = Installer(config=mock.MagicMock(),
+            self.installer = Installer(config=self.config,
                                        name="Installer")
         self.reverter = self.installer.reverter
 
@@ -163,6 +165,20 @@ class InstallerTest(unittest.TestCase):
         self.assertRaises(
             errors.PluginError, installer_func, *passed_args, **passed_kwargs)
 
+    def test_install_ssl_dhparams(self):
+        self.installer.install_ssl_dhparams()
+        self.assertTrue(os.path.isfile(self.installer.ssl_dhparams))
+
+    def _current_ssl_dhparams_hash(self):
+        from certbot.constants import SSL_DHPARAMS_SRC
+        return crypto_util.sha256sum(SSL_DHPARAMS_SRC)
+
+    def test_current_file_hash_in_all_hashes(self):
+        from certbot.constants import ALL_SSL_DHPARAMS_HASHES
+        self.assertTrue(self._current_ssl_dhparams_hash() in ALL_SSL_DHPARAMS_HASHES,
+            "Constants.ALL_SSL_DHPARAMS_HASHES must be appended"
+            " with the sha256 hash of self.config.ssl_dhparams when it is updated.")
+
 
 class AddrTest(unittest.TestCase):
     """Tests for certbot.client.plugins.common.Addr."""
@@ -314,11 +330,11 @@ class TLSSNI01Test(unittest.TestCase):
             achall.response(achall.account_key).z_domain.decode("utf-8"))
 
 
-class InstallSslOptionsConfTest(test_util.TempDirTestCase):
-    """Tests for certbot.plugins.common.install_ssl_options_conf."""
+class InstallVersionControlledFileTest(test_util.TempDirTestCase):
+    """Tests for certbot.plugins.common.install_version_controlled_file."""
 
     def setUp(self):
-        super(InstallSslOptionsConfTest, self).setUp()
+        super(InstallVersionControlledFileTest, self).setUp()
         self.hashes = ["someotherhash"]
         self.dest_path = os.path.join(self.tempdir, "options-ssl-dest.conf")
         self.hash_path = os.path.join(self.tempdir, ".options-ssl-conf.txt")
@@ -330,19 +346,19 @@ class InstallSslOptionsConfTest(test_util.TempDirTestCase):
             self.hashes.append(crypto_util.sha256sum(path))
 
     def _call(self):
-        from certbot.plugins.common import install_ssl_options_conf
-        install_ssl_options_conf(self.dest_path,
-                                 self.hash_path,
-                                 self.source_path,
-                                 self.hashes)
+        from certbot.plugins.common import install_version_controlled_file
+        install_version_controlled_file(self.dest_path,
+                                        self.hash_path,
+                                        self.source_path,
+                                        self.hashes)
 
-    def _current_ssl_options_hash(self):
+    def _current_file_hash(self):
         return crypto_util.sha256sum(self.source_path)
 
     def _assert_current_file(self):
         self.assertTrue(os.path.isfile(self.dest_path))
         self.assertEqual(crypto_util.sha256sum(self.dest_path),
-            self._current_ssl_options_hash())
+            self._current_file_hash())
 
     def test_no_file(self):
         self.assertFalse(os.path.isfile(self.dest_path))
@@ -369,9 +385,9 @@ class InstallSslOptionsConfTest(test_util.TempDirTestCase):
             self.assertFalse(mock_logger.warning.called)
         self.assertTrue(os.path.isfile(self.dest_path))
         self.assertEqual(crypto_util.sha256sum(self.source_path),
-            self._current_ssl_options_hash())
+            self._current_file_hash())
         self.assertNotEqual(crypto_util.sha256sum(self.dest_path),
-            self._current_ssl_options_hash())
+            self._current_file_hash())
 
     def test_manually_modified_past_file_warns(self):
         with open(self.dest_path, "a") as mod_ssl_conf:
@@ -381,10 +397,10 @@ class InstallSslOptionsConfTest(test_util.TempDirTestCase):
         with mock.patch("certbot.plugins.common.logger") as mock_logger:
             self._call()
             self.assertEqual(mock_logger.warning.call_args[0][0],
-                "%s has been manually modified; updated ssl configuration options "
+                "%s has been manually modified; updated file "
                 "saved to %s. We recommend updating %s for security purposes.")
         self.assertEqual(crypto_util.sha256sum(self.source_path),
-            self._current_ssl_options_hash())
+            self._current_file_hash())
         # only print warning once
         with mock.patch("certbot.plugins.common.logger") as mock_logger:
             self._call()
diff --git a/certbot/ssl-dhparams.pem b/certbot/ssl-dhparams.pem
new file mode 100644
index 000000000..9b182b720
--- /dev/null
+++ b/certbot/ssl-dhparams.pem
@@ -0,0 +1,8 @@
+-----BEGIN DH PARAMETERS-----
+MIIBCAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz
++8yTnc4kmz75fS/jY2MMddj2gbICrsRhetPfHtXV/WVhJDP1H18GbtCFY2VVPe0a
+87VXE15/V8k1mE8McODmi3fipona8+/och3xWKE2rec1MKzKT0g6eXq8CrGCsyT7
+YdEIqUuyyOP7uWrat2DX9GgdT0Kj3jlN9K5W7edjcrsZCwenyO4KbXCeAvzhzffi
+7MA0BM0oNC9hkXL+nOmFg/+OTxIy7vKBg8P+OxtMb61zO7X8vC7CIAXFjvGDfRaD
+ssbzSibBsu/6iGtCOGEoXJf//////////wIBAg==
+-----END DH PARAMETERS-----