Repository: certbot/certbot
Pull request: Must staple: check for OCSP support
Fixes #3867 as #6014 was closed.

Problem statement:
Error about being unable to staple a must staple cert
If the user requests a cert with `--must-staple`, is asking `certbot` to install it, and the installer doesn't support OCSP-stapling, we should error out before requesting the cert with a helpful error message.

Hints from issue discussion:
This change should be in `certbot.main.run`. If `config.must_staple` is set, after calling `choose_configurator_plugins` you should check if the installer supports OCSP stapling by calling `supported_enchancements` on it which returns a list of strings and seeing if `ocsp-stapling` is included in list. If it's not, we should return a string like "You included --must-staple but the selected installer doesn't support OCSP stapling."

You should also add a unit test in `certbot/tests/main_test.py` which checks for this behavior.
We've made a lot of changes to Certbot since this issue was opened. If you still have this issue with an up-to-date version of Certbot, can you please add a comment letting us know? This helps us to better see what issues are still affecting our users. If there is no further activity, this issue will be automatically closed.

I still think we should do this.

Commit message: Must staple: check for OCSP support

Patch to review:
diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index 29ccd5913c4..76164e6a1ba 100644
--- a/certbot/CHANGELOG.md
+++ b/certbot/CHANGELOG.md
@@ -13,6 +13,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 * Dropped 32 bit support for the Windows beta installer
 * Windows beta installer is now distributed as "certbot-beta-installer-win_amd64.exe".
   Users of the Windows beta should uninstall the old version before running this.
+* Added a check whether OCSP stapling is supported by the installer when requesting a
+  certificate with the `run` subcommand in combination with the `--must-staple` option.
+  If the installer does not support OCSP and the `--must-staple` option is used, Certbot
+  will raise an error and quit.
 
 ### Fixed
 
diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py
index 9d63be23acc..74d33957ce3 100644
--- a/certbot/certbot/_internal/main.py
+++ b/certbot/certbot/_internal/main.py
@@ -1394,6 +1394,10 @@ def run(config: configuration.NamespaceConfig,
     except errors.PluginSelectionError as e:
         return str(e)
 
+    if config.must_staple and installer and "staple-ocsp" not in installer.supported_enhancements():
+        raise errors.NotSupportedError("Must Staple extension requested, but OCSP stapling "
+                                       "is not supported by the selected installer")
+
     # Preflight check for enhancement support by the selected installer
     if not enhancements.are_supported(config, installer):
         raise errors.NotSupportedError("One ore more of the requested enhancements "
diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py
index d75b6ad58f0..c29f4d758e1 100644
--- a/certbot/tests/main_test.py
+++ b/certbot/tests/main_test.py
@@ -197,6 +197,15 @@ def test_cert_success_install_error(self, mock_install_cert):
         # The final success message shouldn't be shown
         self.mock_success_installation.assert_not_called()
 
+    @mock.patch('certbot._internal.main.plug_sel.choose_configurator_plugins')
+    def test_run_must_staple_not_supported(self, mock_choose):
+        mock_choose.return_value = (null.Installer(self.config, "null"), None)
+        plugins = disco.PluginsRegistry.find_all()
+        self.config.must_staple = True
+        self.assertRaises(errors.NotSupportedError,
+                          main.run,
+                          self.config, plugins)
+
 class CertonlyTest(unittest.TestCase):
     """Tests for certbot._internal.main.certonly."""
 


Code review comments:
diff_hunk: @@ -1394,6 +1394,10 @@ def run(config: configuration.NamespaceConfig,
     except errors.PluginSelectionError as e:
         return str(e)
 
+    if config.must_staple and installer and "staple-ocsp" not in installer.supported_enhancements():
+        raise errors.NotSupportedError("Must Staple extension requested, but OCSP stapling "
path: certbot/certbot/_internal/main.py
text: @user1:
```suggestion
        raise errors.NotSupportedError("Must-Staple extension requested, but OCSP stapling "
```

@author:
I'm not sure about the dash. It's called "Must Staple" everywhere else, including the `--help` output.

https://github.com/certbot/certbot/blob/f251a13f322e10c530897be31aa07a1199061f10/certbot/certbot/crypto_util.py#L146

https://github.com/certbot/certbot/blob/f251a13f322e10c530897be31aa07a1199061f10/certbot/certbot/configuration.py#L123

@user1:
Nice catch, those other places should be fixed as well.

diff_hunk: @@ -1394,6 +1394,18 @@ def run(config: configuration.NamespaceConfig,
     except errors.PluginSelectionError as e:
         return str(e)
 
+    if config.must_staple and installer and "staple-ocsp" not in installer.supported_enhancements():
+        raise errors.NotSupportedError("Must-Staple extension requested, but OCSP stapling "
+                                       "is not supported by the selected installer "
+                                       f"({config.installer})\n\nYou can either:\n * remove the "
+                                       "`--must-staple` option from the command line and obtain a "
+                                       "certificate without the Must-Staple extension, or;\n * use "
+                                       "the `certonly` subcommand and manually install the "
+                                       "certificate into your webserver. Note that OCSP stapling "
path: certbot/certbot/_internal/main.py
text: @author:
Not sure about "webserver" as I'm reading the text again.. Other services can use TLS certs too of course, although I'm not sure if there are other services besides webservers/webclients actually implementing Must-Staple..

diff_hunk: @@ -1394,6 +1394,10 @@ def run(config: configuration.NamespaceConfig,
     except errors.PluginSelectionError as e:
         return str(e)
 
+    if config.must_staple and installer and "staple-ocsp" not in installer.supported_enhancements():
+        raise errors.NotSupportedError("Must Staple extension requested, but OCSP stapling "
+                                       "is not supported by the selected installer")
path: certbot/certbot/_internal/main.py
text: @user1:
A couple suggestions here, based on discussions the last time we went to write [a similar error message](https://github.com/certbot/certbot/blob/master/certbot/certbot/_internal/main.py#L539).

1. We tell them what installer they picked somewhere in the error message, so they can copy it into the forums if they need help
2. We should add a helpful message telling them how to fix this -- maybe something like `Remove the --must-staple flag, or use 'certbot certonly' and manually install your certificate.` Probably telling them to change the installer won't help, though I could be wrong.
3. They probably won't know what certbot-internal jargon like "installer" means, so we should try to use as generic language as we can. Maybe `by the {installer} plugin` or `by certbot with {installer}` or `by the selected installation method (installer: {installer})`