From b3aeeefe20aae784fb6959e935dde87a3c761feb Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 May 2016 20:03:45 +0000 Subject: [PATCH 1/6] Autoconfigure OCSP Stapling with --must-staple --- certbot/client.py | 5 +++-- certbot/interfaces.py | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 0159d3946..a81b7cd70 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -398,6 +398,7 @@ class Client(object): hsts = config.hsts 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 + must_staple = config.must_staple if redirect is None: redirect = enhancements.ask("redirect") @@ -411,11 +412,11 @@ class Client(object): if uir: self.apply_enhancement(domains, "ensure-http-header", "Upgrade-Insecure-Requests") - if staple: + if staple or must_staple: self.apply_enhancement(domains, "staple-ocsp") msg = ("We were unable to restart web server") - if redirect or hsts or uir or staple: + if redirect or hsts or uir or staple or must_staple: with error_handler.ErrorHandler(self._rollback_and_restart, msg): self.installer.restart() diff --git a/certbot/interfaces.py b/certbot/interfaces.py index 8e8666e70..19d9f0c07 100644 --- a/certbot/interfaces.py +++ b/certbot/interfaces.py @@ -201,9 +201,9 @@ class IConfig(zope.interface.Interface): "Email used for registration and recovery contact.") rsa_key_size = zope.interface.Attribute("Size of the RSA key.") must_staple = zope.interface.Attribute( - "Whether to request the OCSP Must Staple certificate extension. " - "Additional setup may be required after issuance. This does not " - "currently autoconfigure web servers for OCSP stapling. ") + "Adds the OCSP Must Staple extension to the certificate." + "Autoconfigures OCSP Stapling for supported setups " + "(Apache version >= 2.3.3 ).") config_dir = zope.interface.Attribute("Configuration directory.") work_dir = zope.interface.Attribute("Working directory.") From 5a3397cf63ac0a4d0d5196e6bf20c50ec3356f96 Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 May 2016 21:07:47 +0000 Subject: [PATCH 2/6] Fix tests --- certbot/tests/client_test.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 8490efd9f..e1aea1b64 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -306,7 +306,7 @@ class ClientTest(unittest.TestCase): @mock.patch("certbot.client.enhancements") def test_enhance_config(self, mock_enhancements): - config = ConfigHelper(redirect=True, hsts=False, uir=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) self.assertRaises(errors.Error, self.client.enhance_config, ["foo.bar"], config) @@ -322,7 +322,7 @@ class ClientTest(unittest.TestCase): @mock.patch("certbot.client.enhancements") def test_enhance_config_no_ask(self, mock_enhancements): - config = ConfigHelper(redirect=True, hsts=False, uir=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) self.assertRaises(errors.Error, self.client.enhance_config, ["foo.bar"], config) @@ -331,16 +331,16 @@ class ClientTest(unittest.TestCase): self.client.installer = installer installer.supported_enhancements.return_value = ["redirect", "ensure-http-header"] - config = ConfigHelper(redirect=True, hsts=False, uir=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_called_with("foo.bar", "redirect", None) - config = ConfigHelper(redirect=False, hsts=True, uir=False) + config = ConfigHelper(redirect=False, hsts=True, uir=False, must_staple=False) self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_called_with("foo.bar", "ensure-http-header", "Strict-Transport-Security") - config = ConfigHelper(redirect=False, hsts=False, uir=True) + config = ConfigHelper(redirect=False, hsts=False, uir=True, must_staple=False) self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_called_with("foo.bar", "ensure-http-header", "Upgrade-Insecure-Requests") @@ -354,13 +354,13 @@ class ClientTest(unittest.TestCase): self.client.installer = installer installer.supported_enhancements.return_value = [] - config = ConfigHelper(redirect=None, hsts=True, uir=True) + config = ConfigHelper(redirect=None, hsts=True, uir=True, must_staple=False) self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_not_called() mock_enhancements.ask.assert_not_called() def test_enhance_config_no_installer(self): - config = ConfigHelper(redirect=True, hsts=False, uir=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) self.assertRaises(errors.Error, self.client.enhance_config, ["foo.bar"], config) @@ -374,7 +374,7 @@ class ClientTest(unittest.TestCase): installer.supported_enhancements.return_value = ["redirect"] installer.enhance.side_effect = errors.PluginError - config = ConfigHelper(redirect=True, hsts=False, uir=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], config) @@ -391,7 +391,7 @@ class ClientTest(unittest.TestCase): installer.supported_enhancements.return_value = ["redirect"] installer.save.side_effect = errors.PluginError - config = ConfigHelper(redirect=True, hsts=False, uir=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], config) @@ -408,7 +408,7 @@ class ClientTest(unittest.TestCase): installer.supported_enhancements.return_value = ["redirect"] installer.restart.side_effect = [errors.PluginError, None] - config = ConfigHelper(redirect=True, hsts=False, uir=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], config) @@ -428,7 +428,7 @@ class ClientTest(unittest.TestCase): installer.restart.side_effect = errors.PluginError installer.rollback_checkpoints.side_effect = errors.ReverterError - config = ConfigHelper(redirect=True, hsts=False, uir=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], config) From efcd0090da49c9649b835d1bd5e3350f17b19fcc Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 May 2016 21:20:13 +0000 Subject: [PATCH 3/6] Add a specific must-staple test --- certbot/tests/client_test.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index e1aea1b64..ae66ab5d0 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -304,6 +304,25 @@ class ClientTest(unittest.TestCase): installer.rollback_checkpoints.assert_called_once_with() self.assertEqual(installer.restart.call_count, 1) + @mock.patch("certbot.client.enhancements") + def test_must_staple(self, mock_enhancements): + # Testing our wanted behaviour: Enable OCSP Stapling when the + # --must-staple flag is on. [Without having the --staple-ocsp flag on] + config = ConfigHelper(must_staple=True, staple=False) + self.assertRaises(errors.Error, + self.client.enhance_config, ["foo.bar"], config) + + mock_enhancements.ask.return_value = True + installer = mock.MagicMock() + self.client.installer = installer + installer.supported_enhancements.return_value = ["staple-ocsp"] + + self.client.enhance_config(["foo.bar"], config) + installer.enhance.assert_called_once_with("foo.bar", "staple-ocsp", None) + self.assertEqual(installer.save.call_count, 1) + installer.restart.assert_called_once_with() + + @mock.patch("certbot.client.enhancements") def test_enhance_config(self, mock_enhancements): config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) From b77d288adb0f78db6032a1d066a546d3c99ca9b0 Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 May 2016 21:49:53 +0000 Subject: [PATCH 4/6] Use cli.py to set .staple given .must_staple --- certbot/cli.py | 3 +++ certbot/tests/cli_test.py | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/certbot/cli.py b/certbot/cli.py index 3bd565496..05a189712 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -343,6 +343,9 @@ class HelpfulArgumentParser(object): if parsed_args.csr: self.handle_csr(parsed_args) + if parsed_args.must_staple: + parsed_args.staple = True + hooks.validate_hooks(parsed_args) return parsed_args diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 4ae69e69d..00c9a0a26 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -422,6 +422,13 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods for arg in conflicting_args: self.assertTrue(arg in error.message) + def test_must_staple_flag(self): + parse = self._get_argument_parser() + short_args = ['--must-staple'] + namespace = parse(short_args) + self.assertTrue(namespace.must_staple) + self.assertTrue(namespace.staple) + def test_staging_flag(self): parse = self._get_argument_parser() short_args = ['--staging'] From 20be730a924e3a32c005556ce5a62ac1e279f1d0 Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 May 2016 21:56:15 +0000 Subject: [PATCH 5/6] Revert client, client_test back --- certbot/client.py | 5 ++--- certbot/tests/client_test.py | 41 ++++++++++-------------------------- 2 files changed, 13 insertions(+), 33 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 5d1338baf..ba31f8760 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -406,7 +406,6 @@ class Client(object): hsts = config.hsts 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 - must_staple = config.must_staple if redirect is None: redirect = enhancements.ask("redirect") @@ -420,11 +419,11 @@ class Client(object): if uir: self.apply_enhancement(domains, "ensure-http-header", "Upgrade-Insecure-Requests") - if staple or must_staple: + if staple: self.apply_enhancement(domains, "staple-ocsp") msg = ("We were unable to restart web server") - if redirect or hsts or uir or staple or must_staple: + if redirect or hsts or uir or staple: with error_handler.ErrorHandler(self._rollback_and_restart, msg): self.installer.restart() diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index ae66ab5d0..8490efd9f 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -304,28 +304,9 @@ class ClientTest(unittest.TestCase): installer.rollback_checkpoints.assert_called_once_with() self.assertEqual(installer.restart.call_count, 1) - @mock.patch("certbot.client.enhancements") - def test_must_staple(self, mock_enhancements): - # Testing our wanted behaviour: Enable OCSP Stapling when the - # --must-staple flag is on. [Without having the --staple-ocsp flag on] - config = ConfigHelper(must_staple=True, staple=False) - self.assertRaises(errors.Error, - self.client.enhance_config, ["foo.bar"], config) - - mock_enhancements.ask.return_value = True - installer = mock.MagicMock() - self.client.installer = installer - installer.supported_enhancements.return_value = ["staple-ocsp"] - - self.client.enhance_config(["foo.bar"], config) - installer.enhance.assert_called_once_with("foo.bar", "staple-ocsp", None) - self.assertEqual(installer.save.call_count, 1) - installer.restart.assert_called_once_with() - - @mock.patch("certbot.client.enhancements") def test_enhance_config(self, mock_enhancements): - config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.Error, self.client.enhance_config, ["foo.bar"], config) @@ -341,7 +322,7 @@ class ClientTest(unittest.TestCase): @mock.patch("certbot.client.enhancements") def test_enhance_config_no_ask(self, mock_enhancements): - config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.Error, self.client.enhance_config, ["foo.bar"], config) @@ -350,16 +331,16 @@ class ClientTest(unittest.TestCase): self.client.installer = installer installer.supported_enhancements.return_value = ["redirect", "ensure-http-header"] - config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_called_with("foo.bar", "redirect", None) - config = ConfigHelper(redirect=False, hsts=True, uir=False, must_staple=False) + config = ConfigHelper(redirect=False, hsts=True, uir=False) self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_called_with("foo.bar", "ensure-http-header", "Strict-Transport-Security") - config = ConfigHelper(redirect=False, hsts=False, uir=True, must_staple=False) + config = ConfigHelper(redirect=False, hsts=False, uir=True) self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_called_with("foo.bar", "ensure-http-header", "Upgrade-Insecure-Requests") @@ -373,13 +354,13 @@ class ClientTest(unittest.TestCase): self.client.installer = installer installer.supported_enhancements.return_value = [] - config = ConfigHelper(redirect=None, hsts=True, uir=True, must_staple=False) + config = ConfigHelper(redirect=None, hsts=True, uir=True) self.client.enhance_config(["foo.bar"], config) installer.enhance.assert_not_called() mock_enhancements.ask.assert_not_called() def test_enhance_config_no_installer(self): - config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.Error, self.client.enhance_config, ["foo.bar"], config) @@ -393,7 +374,7 @@ class ClientTest(unittest.TestCase): installer.supported_enhancements.return_value = ["redirect"] installer.enhance.side_effect = errors.PluginError - config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], config) @@ -410,7 +391,7 @@ class ClientTest(unittest.TestCase): installer.supported_enhancements.return_value = ["redirect"] installer.save.side_effect = errors.PluginError - config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], config) @@ -427,7 +408,7 @@ class ClientTest(unittest.TestCase): installer.supported_enhancements.return_value = ["redirect"] installer.restart.side_effect = [errors.PluginError, None] - config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], config) @@ -447,7 +428,7 @@ class ClientTest(unittest.TestCase): installer.restart.side_effect = errors.PluginError installer.rollback_checkpoints.side_effect = errors.ReverterError - config = ConfigHelper(redirect=True, hsts=False, uir=False, must_staple=False) + config = ConfigHelper(redirect=True, hsts=False, uir=False) self.assertRaises(errors.PluginError, self.client.enhance_config, ["foo.bar"], config) From d57353a6fea562b00cc3e37e71b76e12ff380fef Mon Sep 17 00:00:00 2001 From: sagi Date: Wed, 25 May 2016 22:01:43 +0000 Subject: [PATCH 6/6] Add missing space. --- certbot/interfaces.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/interfaces.py b/certbot/interfaces.py index 19d9f0c07..37835462e 100644 --- a/certbot/interfaces.py +++ b/certbot/interfaces.py @@ -201,7 +201,7 @@ class IConfig(zope.interface.Interface): "Email used for registration and recovery contact.") rsa_key_size = zope.interface.Attribute("Size of the RSA key.") must_staple = zope.interface.Attribute( - "Adds the OCSP Must Staple extension to the certificate." + "Adds the OCSP Must Staple extension to the certificate. " "Autoconfigures OCSP Stapling for supported setups " "(Apache version >= 2.3.3 ).")