diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 12125d522..1e02ae7b3 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -124,13 +124,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.assoc = dict() # Outstanding challenges self._chall_out = set() + # Maps enhancements to vhosts we've enabled the enhancement for + self._enhanced_vhosts = defaultdict(set) # These will be set in the prepare function self.parser = None self.version = version self.vhosts = None self._enhance_func = {"redirect": self._enable_redirect, - "ensure-http-header": self._set_http_header} + "ensure-http-header": self._set_http_header, + "staple-ocsp": self._enable_ocsp_stapling} @property def mod_ssl_conf(self): @@ -593,8 +596,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :type addr: :class:`~certbot_apache.obj.Addr` """ - loc = parser.get_aug_path(self.parser.loc["name"]) + loc = parser.get_aug_path(self.parser.loc["name"]) if addr.get_port() == "443": path = self.parser.add_dir_to_ifmodssl( loc, "NameVirtualHost", [str(addr)]) @@ -944,7 +947,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ###################################################################### def supported_enhancements(self): # pylint: disable=no-self-use """Returns currently supported enhancements.""" - return ["redirect", "ensure-http-header"] + return ["redirect", "ensure-http-header", "staple-ocsp"] def enhance(self, domain, enhancement, options=None): """Enhance configuration. @@ -971,6 +974,68 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logger.warn("Failed %s for %s", enhancement, domain) raise + def _enable_ocsp_stapling(self, ssl_vhost, unused_options): + """Enables OCSP Stapling + + In OCSP, each client (e.g. browser) would have to query the + OCSP Responder to validate that the site certificate was not revoked. + + Enabling OCSP Stapling, would allow the web-server to query the OCSP + Responder, and staple its response to the offered certificate during + TLS. i.e. clients would not have to query the OCSP responder. + + OCSP Stapling enablement on Apache implicitly depends on + SSLCertificateChainFile being set by other code. + + .. note:: This function saves the configuration + + :param ssl_vhost: Destination of traffic, an ssl enabled vhost + :type ssl_vhost: :class:`~letsencrypt_apache.obj.VirtualHost` + + :param unused_options: Not currently used + :type unused_options: Not Available + + :returns: Success, general_vhost (HTTP vhost) + :rtype: (bool, :class:`~letsencrypt_apache.obj.VirtualHost`) + + """ + min_apache_ver = (2, 3, 3) + if self.get_version() < min_apache_ver: + raise errors.PluginError( + "Unable to set OCSP directives.\n" + "Apache version is below 2.3.3.") + + if "socache_shmcb_module" not in self.parser.modules: + self.enable_mod("socache_shmcb") + + # Check if there's an existing SSLUseStapling directive on. + use_stapling_aug_path = self.parser.find_dir("SSLUseStapling", + "on", start=ssl_vhost.path) + if not use_stapling_aug_path: + self.parser.add_dir(ssl_vhost.path, "SSLUseStapling", "on") + + ssl_vhost_aug_path = parser.get_aug_path(ssl_vhost.filep) + + # Check if there's an existing SSLStaplingCache directive. + stapling_cache_aug_path = self.parser.find_dir('SSLStaplingCache', + None, ssl_vhost_aug_path) + + # We'll simply delete the directive, so that we'll have a + # consistent OCSP cache path. + if stapling_cache_aug_path: + self.aug.remove( + re.sub(r"/\w*$", "", stapling_cache_aug_path[0])) + + self.parser.add_dir_to_ifmodssl(ssl_vhost_aug_path, + "SSLStaplingCache", + ["shmcb:/var/run/apache2/stapling_cache(128000)"]) + + msg = "OCSP Stapling was enabled on SSL Vhost: %s.\n"%( + ssl_vhost.filep) + self.save_notes += msg + self.save() + logger.info(msg) + def _set_http_header(self, ssl_vhost, header_substring): """Enables header that is identified by header_substring on ssl_vhost. @@ -1058,9 +1123,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param unused_options: Not currently used :type unused_options: Not Available - :returns: Success, general_vhost (HTTP vhost) - :rtype: (bool, :class:`~certbot_apache.obj.VirtualHost`) - :raises .errors.PluginError: If no viable HTTP host can be created or used for the redirect. @@ -1083,6 +1145,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "redirection") self._create_redirect_vhost(ssl_vhost) else: + if general_vh in self._enhanced_vhosts["redirect"]: + logger.debug("Already enabled redirect for this vhost") + return + # Check if Certbot redirection already exists self._verify_no_certbot_redirect(general_vh) @@ -1118,6 +1184,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): (general_vh.filep, ssl_vhost.filep)) self.save() + self._enhanced_vhosts["redirect"].add(general_vh) logger.info("Redirecting vhost in %s to ssl vhost in %s", general_vh.filep, ssl_vhost.filep) @@ -1206,6 +1273,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Make a new vhost data structure and add it to the lists new_vhost = self._create_vhost(parser.get_aug_path(redirect_filepath)) self.vhosts.append(new_vhost) + self._enhanced_vhosts["redirect"].add(new_vhost) # Finally create documentation for the change self.save_notes += ("Created a port 80 vhost, %s, for redirection to " diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index f2f78c8f9..d7a5fd75a 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -15,6 +15,7 @@ from certbot import errors from certbot.tests import acme_util from certbot_apache import configurator +from certbot_apache import parser from certbot_apache import obj from certbot_apache.tests import util @@ -85,7 +86,8 @@ class MultipleVhostsTest(util.ApacheTest): mock_getutility.notification = mock.MagicMock(return_value=True) names = self.config.get_all_names() self.assertEqual(names, set( - ["certbot.demo", "encryption-example.demo", "ip-172-30-0-17", "*.blue.purple.com"])) + ["certbot.demo", "ocspvhost.com", "encryption-example.demo", + "ip-172-30-0-17", "*.blue.purple.com"])) @mock.patch("zope.component.getUtility") @mock.patch("certbot_apache.configurator.socket.gethostbyaddr") @@ -100,14 +102,24 @@ class MultipleVhostsTest(util.ApacheTest): obj.Addr(("zombo.com",)), obj.Addr(("192.168.1.2"))]), True, False) + self.config.vhosts.append(vhost) names = self.config.get_all_names() - self.assertEqual(len(names), 6) + self.assertEqual(len(names), 7) self.assertTrue("zombo.com" in names) self.assertTrue("google.com" in names) self.assertTrue("certbot.demo" in names) + def test_bad_servername_alias(self): + ssl_vh1 = obj.VirtualHost( + "fp1", "ap1", set([obj.Addr(("*", "443"))]), + True, False) + # pylint: disable=protected-access + self.config._add_servernames(ssl_vh1) + self.assertTrue( + self.config._add_servername_alias("oy_vey", ssl_vh1) is None) + def test_add_servernames_alias(self): self.config.parser.add_dir( self.vh_truth[2].path, "ServerAlias", ["*.le.co"]) @@ -124,7 +136,7 @@ class MultipleVhostsTest(util.ApacheTest): """ vhs = self.config.get_virtual_hosts() - self.assertEqual(len(vhs), 7) + self.assertEqual(len(vhs), 8) found = 0 for vhost in vhs: @@ -135,7 +147,7 @@ class MultipleVhostsTest(util.ApacheTest): else: raise Exception("Missed: %s" % vhost) # pragma: no cover - self.assertEqual(found, 7) + self.assertEqual(found, 8) # Handle case of non-debian layout get_virtual_hosts with mock.patch( @@ -143,7 +155,7 @@ class MultipleVhostsTest(util.ApacheTest): ) as mock_conf: mock_conf.return_value = False vhs = self.config.get_virtual_hosts() - self.assertEqual(len(vhs), 7) + self.assertEqual(len(vhs), 8) @mock.patch("certbot_apache.display_ops.select_vhost") def test_choose_vhost_none_avail(self, mock_select): @@ -224,16 +236,18 @@ class MultipleVhostsTest(util.ApacheTest): # Assume only the two default vhosts. self.config.vhosts = [ vh for vh in self.config.vhosts - if vh.name not in ["certbot.demo", "encryption-example.demo"] + if vh.name not in ["certbot.demo", + "encryption-example.demo", + "ocspvhost.com"] and "*.blue.purple.com" not in vh.aliases ] - self.assertEqual( - self.config._find_best_vhost("example.demo"), self.vh_truth[2]) + self.config._find_best_vhost("encryption-example.demo"), + self.vh_truth[2]) def test_non_default_vhosts(self): # pylint: disable=protected-access - self.assertEqual(len(self.config._non_default_vhosts()), 5) + self.assertEqual(len(self.config._non_default_vhosts()), 6) def test_is_site_enabled(self): """Test if site is enabled. @@ -539,7 +553,7 @@ class MultipleVhostsTest(util.ApacheTest): self.assertEqual(self.config.is_name_vhost(self.vh_truth[0]), self.config.is_name_vhost(ssl_vhost)) - self.assertEqual(len(self.config.vhosts), 8) + self.assertEqual(len(self.config.vhosts), 9) def test_clean_vhost_ssl(self): # pylint: disable=protected-access @@ -726,16 +740,15 @@ class MultipleVhostsTest(util.ApacheTest): def test_get_all_certs_keys(self): c_k = self.config.get_all_certs_keys() - - self.assertEqual(len(c_k), 2) + self.assertEqual(len(c_k), 3) cert, key, path = next(iter(c_k)) self.assertTrue("cert" in cert) self.assertTrue("key" in key) - self.assertTrue("default-ssl" in path) + self.assertTrue("default-ssl" in path or "ocsp-ssl" in path) def test_get_all_certs_keys_malformed_conf(self): self.config.parser.find_dir = mock.Mock( - side_effect=[["path"], [], ["path"], []]) + side_effect=[["path"], [], ["path"], [], ["path"], []]) c_k = self.config.get_all_certs_keys() self.assertFalse(c_k) @@ -756,15 +769,20 @@ class MultipleVhostsTest(util.ApacheTest): def test_supported_enhancements(self): self.assertTrue(isinstance(self.config.supported_enhancements(), list)) + @mock.patch("certbot_apache.configurator.ApacheConfigurator._get_http_vhost") + @mock.patch("certbot_apache.display_ops.select_vhost") @mock.patch("certbot.le_util.exe_exists") - def test_enhance_unknown_vhost(self, mock_exe): + def test_enhance_unknown_vhost(self, mock_exe, mock_sel_vhost, mock_get): self.config.parser.modules.add("rewrite_module") mock_exe.return_value = True - ssl_vh = obj.VirtualHost( - "fp", "ap", set([obj.Addr(("*", "443"))]), + ssl_vh1 = obj.VirtualHost( + "fp1", "ap1", set([obj.Addr(("*", "443"))]), True, False) - ssl_vh.name = "satoshi.com" - self.config.vhosts.append(ssl_vh) + ssl_vh1.name = "satoshi.com" + self.config.vhosts.append(ssl_vh1) + mock_sel_vhost.return_value = None + mock_get.return_value = None + self.assertRaises( errors.PluginError, self.config.enhance, "satoshi.com", "redirect") @@ -774,6 +792,85 @@ class MultipleVhostsTest(util.ApacheTest): errors.PluginError, self.config.enhance, "certbot.demo", "unknown_enhancement") + @mock.patch("certbot.le_util.run_script") + @mock.patch("certbot.le_util.exe_exists") + def test_ocsp_stapling(self, mock_exe, mock_run_script): + self.config.parser.update_runtime_variables = mock.Mock() + self.config.parser.modules.add("mod_ssl.c") + self.config.get_version = mock.Mock(return_value=(2, 4, 7)) + mock_exe.return_value = True + + # This will create an ssl vhost for certbot.demo + self.config.enhance("certbot.demo", "staple-ocsp") + + self.assertTrue("socache_shmcb_module" in self.config.parser.modules) + self.assertTrue(mock_run_script.called) + + # Get the ssl vhost for certbot.demo + ssl_vhost = self.config.assoc["certbot.demo"] + + ssl_use_stapling_aug_path = self.config.parser.find_dir( + "SSLUseStapling", "on", ssl_vhost.path) + + self.assertEqual(len(ssl_use_stapling_aug_path), 1) + + ssl_vhost_aug_path = parser.get_aug_path(ssl_vhost.filep) + stapling_cache_aug_path = self.config.parser.find_dir('SSLStaplingCache', + "shmcb:/var/run/apache2/stapling_cache(128000)", + ssl_vhost_aug_path) + + self.assertEqual(len(stapling_cache_aug_path), 1) + + @mock.patch("certbot.le_util.exe_exists") + def test_ocsp_stapling_twice(self, mock_exe): + self.config.parser.update_runtime_variables = mock.Mock() + self.config.parser.modules.add("mod_ssl.c") + self.config.parser.modules.add("socache_shmcb_module") + self.config.get_version = mock.Mock(return_value=(2, 4, 7)) + mock_exe.return_value = True + + # Checking the case with already enabled ocsp stapling configuration + self.config.enhance("ocspvhost.com", "staple-ocsp") + + # Get the ssl vhost for letsencrypt.demo + ssl_vhost = self.config.assoc["ocspvhost.com"] + + ssl_use_stapling_aug_path = self.config.parser.find_dir( + "SSLUseStapling", "on", ssl_vhost.path) + + self.assertEqual(len(ssl_use_stapling_aug_path), 1) + + ssl_vhost_aug_path = parser.get_aug_path(ssl_vhost.filep) + stapling_cache_aug_path = self.config.parser.find_dir('SSLStaplingCache', + "shmcb:/var/run/apache2/stapling_cache(128000)", + ssl_vhost_aug_path) + + self.assertEqual(len(stapling_cache_aug_path), 1) + + + @mock.patch("certbot.le_util.exe_exists") + def test_ocsp_unsupported_apache_version(self, mock_exe): + mock_exe.return_value = True + self.config.parser.update_runtime_variables = mock.Mock() + self.config.parser.modules.add("mod_ssl.c") + self.config.parser.modules.add("socache_shmcb_module") + self.config.get_version = mock.Mock(return_value=(2, 2, 0)) + + self.assertRaises(errors.PluginError, + self.config.enhance, "certbot.demo", "staple-ocsp") + + + def test_get_http_vhost_third_filter(self): + ssl_vh = obj.VirtualHost( + "fp", "ap", set([obj.Addr(("*", "443"))]), + True, False) + ssl_vh.name = "satoshi.com" + self.config.vhosts.append(ssl_vh) + + # pylint: disable=protected-access + http_vh = self.config._get_http_vhost(ssl_vh) + self.assertTrue(http_vh.ssl == False) + @mock.patch("certbot.le_util.run_script") @mock.patch("certbot.le_util.exe_exists") def test_http_header_hsts(self, mock_exe, _): @@ -899,7 +996,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_redirect_with_existing_rewrite(self, mock_exe, _): self.config.parser.update_runtime_variables = mock.Mock() mock_exe.return_value = True - self.config.get_version = mock.Mock(return_value=(2, 2)) + self.config.get_version = mock.Mock(return_value=(2, 2, 0)) # Create a preexisting rewrite rule self.config.parser.add_dir( @@ -938,15 +1035,31 @@ class MultipleVhostsTest(util.ApacheTest): self.assertRaises( errors.PluginError, self.config._enable_redirect, ssl_vh, "") - def test_redirect_twice(self): + def test_redirect_two_domains_one_vhost(self): # Skip the enable mod self.config.parser.modules.add("rewrite_module") self.config.get_version = mock.Mock(return_value=(2, 3, 9)) - self.config.enhance("encryption-example.demo", "redirect") + self.config.enhance("red.blue.purple.com", "redirect") + verify_no_redirect = ("certbot_apache.configurator." + "ApacheConfigurator._verify_no_certbot_redirect") + with mock.patch(verify_no_redirect) as mock_verify: + self.config.enhance("green.blue.purple.com", "redirect") + self.assertFalse(mock_verify.called) + + def test_redirect_from_previous_run(self): + # Skip the enable mod + self.config.parser.modules.add("rewrite_module") + self.config.get_version = mock.Mock(return_value=(2, 3, 9)) + + self.config.enhance("red.blue.purple.com", "redirect") + # Clear state about enabling redirect on this run + # pylint: disable=protected-access + self.config._enhanced_vhosts["redirect"].clear() + self.assertRaises( errors.PluginEnhancementAlreadyPresent, - self.config.enhance, "encryption-example.demo", "redirect") + self.config.enhance, "green.blue.purple.com", "redirect") def test_create_own_redirect(self): self.config.parser.modules.add("rewrite_module") @@ -957,7 +1070,7 @@ class MultipleVhostsTest(util.ApacheTest): # pylint: disable=protected-access self.config._enable_redirect(self.vh_truth[1], "") - self.assertEqual(len(self.config.vhosts), 8) + self.assertEqual(len(self.config.vhosts), 9) def test_create_own_redirect_for_old_apache_version(self): self.config.parser.modules.add("rewrite_module") @@ -968,7 +1081,7 @@ class MultipleVhostsTest(util.ApacheTest): # pylint: disable=protected-access self.config._enable_redirect(self.vh_truth[1], "") - self.assertEqual(len(self.config.vhosts), 8) + self.assertEqual(len(self.config.vhosts), 9) def test_sift_line(self): # pylint: disable=protected-access diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/ocsp-ssl.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/ocsp-ssl.conf new file mode 100644 index 000000000..631cf16c8 --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/ocsp-ssl.conf @@ -0,0 +1,36 @@ + +SSLStaplingCache shmcb:/var/run/apache2/stapling_cache(128000) + + # The ServerName directive sets the request scheme, hostname and port that + # the server uses to identify itself. This is used when creating + # redirection URLs. In the context of virtual hosts, the ServerName + # specifies what hostname must appear in the request's Host: header to + # match this virtual host. For the default virtual host (this file) this + # value is not decisive as it is used as a last resort host regardless. + # However, you must set it for any further virtual host explicitly. + ServerName ocspvhost.com + + ServerAdmin webmaster@dumpbits.com + DocumentRoot /var/www/html + + # Available loglevels: trace8, ..., trace1, debug, info, notice, warn, + # error, crit, alert, emerg. + # It is also possible to configure the loglevel for particular + # modules, e.g. + #LogLevel info ssl:warn + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + # For most configuration files from conf-available/, which are + # enabled or disabled at a global level, it is possible to + # include a line for only one particular virtual host. For example the + # following line enables the CGI configuration for this host only + # after it has been globally disabled with "a2disconf". + #Include conf-available/serve-cgi-bin.conf +SSLCertificateFile /etc/apache2/certs/certbot-cert_5.pem +SSLCertificateKeyFile /etc/apache2/ssl/key-certbot_15.pem +SSLUseStapling on + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet + diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/ocsp-ssl.conf b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/ocsp-ssl.conf new file mode 120000 index 000000000..b25ee0482 --- /dev/null +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/ocsp-ssl.conf @@ -0,0 +1 @@ +../sites-available/ocsp-ssl.conf \ No newline at end of file diff --git a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/sites b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/sites index 06bf6a2ae..ab518ee5b 100644 --- a/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/sites +++ b/certbot-apache/certbot_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/sites @@ -1,2 +1,3 @@ sites-available/certbot.conf, certbot.demo sites-available/encryption-example.conf, encryption-example.demo +sites-available/ocsp-ssl.conf, ocspvhost.com diff --git a/certbot-apache/certbot_apache/tests/util.py b/certbot-apache/certbot_apache/tests/util.py index 9fb5dcdfa..8935ee908 100644 --- a/certbot-apache/certbot_apache/tests/util.py +++ b/certbot-apache/certbot_apache/tests/util.py @@ -156,8 +156,12 @@ def get_vh_truth(temp_dir, config_name): os.path.join(prefix, "wildcard.conf"), os.path.join(aug_pre, "wildcard.conf/VirtualHost"), set([obj.Addr.fromstring("*:80")]), False, False, - "ip-172-30-0-17", aliases=["*.blue.purple.com"]) - ] + "ip-172-30-0-17", aliases=["*.blue.purple.com"]), + obj.VirtualHost( + os.path.join(prefix, "ocsp-ssl.conf"), + os.path.join(aug_pre, "ocsp-ssl.conf/IfModule/VirtualHost"), + set([obj.Addr.fromstring("10.2.3.4:443")]), True, True, + "ocspvhost.com")] return vh_truth return None # pragma: no cover diff --git a/certbot/cli.py b/certbot/cli.py index 7a6a11dd5..7898c4c0b 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -739,9 +739,20 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): " https:// for every http:// resource.", dest="uir", default=None) helpful.add( "security", "--no-uir", action="store_false", - help=" Do not automatically set the \"Content-Security-Policy:" + help="Do not automatically set the \"Content-Security-Policy:" " upgrade-insecure-requests\" header to every HTTP response.", dest="uir", default=None) + helpful.add( + "security", "--staple-ocsp", action="store_true", + help="Enables OCSP Stapling. A valid OCSP response is stapled to" + " the certificate that the server offers during TLS.", + dest="staple", default=None) + helpful.add( + "security", "--no-staple-ocsp", action="store_false", + help="Do not automatically enable OCSP Stapling.", + dest="staple", default=None) + + helpful.add( "security", "--strict-permissions", action="store_true", help="Require that all configuration files are owned by the current " diff --git a/certbot/client.py b/certbot/client.py index 6f41a3a0b..0159d3946 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -396,7 +396,8 @@ class Client(object): supported = self.installer.supported_enhancements() redirect = config.redirect if "redirect" in supported else False hsts = config.hsts if "ensure-http-header" in supported else False - uir = config.uir if "ensure-http-header" in supported else False + uir = config.uir if "ensure-http-header" in supported else False + staple = config.staple if "staple-ocsp" in supported else False if redirect is None: redirect = enhancements.ask("redirect") @@ -410,9 +411,11 @@ class Client(object): if uir: self.apply_enhancement(domains, "ensure-http-header", "Upgrade-Insecure-Requests") + if staple: + self.apply_enhancement(domains, "staple-ocsp") msg = ("We were unable to restart web server") - if redirect or hsts or uir: + if redirect or hsts or uir or staple: with error_handler.ErrorHandler(self._rollback_and_restart, msg): self.installer.restart()