Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly handle installing certs/keys into an SSL vhost without cert and key directives #4837

Closed
bmw opened this issue Jun 14, 2017 · 0 comments

Comments

@bmw
Copy link
Member

bmw commented Jun 14, 2017

My operating system is (include version):

Ubuntu 16.04

I installed Certbot with (certbot-auto, OS package manager, pip, etc):

Dev instructions

I ran this command and it produced this output:

Starting with the default Apache config on this system, I removed the SSLCertificateFile and SSLCertificateKeyFile directives from /etc/apache2/sites-available/default-ssl.conf and put them in /etc/apache2/apache2.conf. Then I ran:

sudo rm /etc/apache2/sites-*/000-default.conf
sudo a2enmod ssl
sudo a2ensite default-ssl.conf
sudo certbot -d example.com --apache

Certbot's behavior differed from what I expected because:

This configuration is technically valid so we should ideally support it. The problem is we mark the vhost as SSL and assume cert and key directives exist in the file when they technically could be in the top level configuration.

Here is a Certbot log showing the issue (if available):

Logs are stored in /var/log/letsencrypt by default. Feel free to redact domains, e-mail and IP addresses as you see fit.

The relevant log output is:

Cannot find a cert or key directive in /files/etc/apache2/sites-available/default-ssl.conf/IfModule/VirtualHost. VirtualHost was not modified
Encountered exception:
Traceback (most recent call last):
  File "/home/bmw/src/certbot/certbot/client.py", line 439, in deploy_certificate
    fullchain_path=fullchain_path)
  File "/home/bmw/src/certbot/certbot-apache/certbot_apache/configurator.py", line 269, in deploy_cert
    "Unable to find cert and/or key directives")
PluginError: Unable to find cert and/or key directives

Calling registered functions
Reporting to user: Unable to install the certificate
Exiting abnormally:
Traceback (most recent call last):
  File "/usr/local/bin/certbot", line 11, in <module>
    load_entry_point('certbot', 'console_scripts', 'certbot')()
  File "/home/bmw/src/certbot/certbot/main.py", line 743, in main
    return config.func(config, plugins)
  File "/home/bmw/src/certbot/certbot/main.py", line 604, in run
    _install_cert(config, le_client, domains, new_lineage)
  File "/home/bmw/src/certbot/certbot/main.py", line 469, in _install_cert
    path_provider.cert_path, path_provider.chain_path, path_provider.fullchain_path)
  File "/home/bmw/src/certbot/certbot/client.py", line 439, in deploy_certificate
    fullchain_path=fullchain_path)
  File "/home/bmw/src/certbot/certbot-apache/certbot_apache/configurator.py", line 269, in deploy_cert
    "Unable to find cert and/or key directives")
PluginError: Unable to find cert and/or key directives
@bmw bmw added this to the Wishlist milestone Jun 14, 2017
@bmw bmw closed this as completed in #4104 Sep 25, 2017
bmw pushed a commit that referenced this issue Sep 25, 2017
…on Debian / Ubuntu (#4104)

This changes the apache plugin behaviour to only parse enabled configuration files and respecting the --apache-vhost-root CLI parameter for new SSL vhost creation. If --apache-vhost-root isn't defined, or doesn't exist, the SSL vhost will be created to originating non-SSL vhost directory.

This PR also implements actual check for vhost enabled state, and makes sure parser.parse_file() does not discard changes in Augeas DOM, by doing an autosave.

Also handles enabling the new SSL vhost, if it's on a path that's not parsed by Apache.

Fixes: #1328
Fixes: #3545
Fixes: #3791
Fixes: #4523
Fixes: #4837
Fixes: #4905

* First changes

* Handle rest of the errors

* Test fixes

* Final fixes

* Make parse_files accessible and fix linter problems

* Activate vhost at later time

* Cleanup

* Add a new test case, and fix old

* Enable site later in deploy_cert

* Make apache-conf-test default dummy configuration enabled

* Remove is_sites_available as obsolete

* Cleanup

* Brought back conditional vhost_path parsing

* Parenthesis

* Fix merge leftovers

* Fix to work with the recent changes to new file creation

* Added fix and tests for non-symlink vhost in sites-enabled

* Made vhostroot parameter for ApacheParser optional, and removed extra_path

* Respect vhost-root, and add Include statements to root configuration if needed

* Fixed site enabling order to prevent apache restart error while enabling mod_ssl

* Don't exclude Ubuntu / Debian vhost-root cli argument

* Changed the SSL vhost directory selection priority

* Requested fixes for paths and vhost discovery

* Make sure the Augeas DOM is written to disk before loading new files

* Actual checking for if the file is parsed within existing Apache configuration

* Fix the order of dummy SSL directives addition and enabling modules

* Restructured site_enabled checks

* Enabling vhost correctly for non-debian systems
@bmw bmw modified the milestones: Wishlist, 0.19.0 Sep 25, 2017
sydneyli pushed a commit that referenced this issue Jun 15, 2018
…on Debian / Ubuntu (#4104)

This changes the apache plugin behaviour to only parse enabled configuration files and respecting the --apache-vhost-root CLI parameter for new SSL vhost creation. If --apache-vhost-root isn't defined, or doesn't exist, the SSL vhost will be created to originating non-SSL vhost directory.

This PR also implements actual check for vhost enabled state, and makes sure parser.parse_file() does not discard changes in Augeas DOM, by doing an autosave.

Also handles enabling the new SSL vhost, if it's on a path that's not parsed by Apache.

Fixes: #1328
Fixes: #3545
Fixes: #3791
Fixes: #4523
Fixes: #4837
Fixes: #4905

* First changes

* Handle rest of the errors

* Test fixes

* Final fixes

* Make parse_files accessible and fix linter problems

* Activate vhost at later time

* Cleanup

* Add a new test case, and fix old

* Enable site later in deploy_cert

* Make apache-conf-test default dummy configuration enabled

* Remove is_sites_available as obsolete

* Cleanup

* Brought back conditional vhost_path parsing

* Parenthesis

* Fix merge leftovers

* Fix to work with the recent changes to new file creation

* Added fix and tests for non-symlink vhost in sites-enabled

* Made vhostroot parameter for ApacheParser optional, and removed extra_path

* Respect vhost-root, and add Include statements to root configuration if needed

* Fixed site enabling order to prevent apache restart error while enabling mod_ssl

* Don't exclude Ubuntu / Debian vhost-root cli argument

* Changed the SSL vhost directory selection priority

* Requested fixes for paths and vhost discovery

* Make sure the Augeas DOM is written to disk before loading new files

* Actual checking for if the file is parsed within existing Apache configuration

* Fix the order of dummy SSL directives addition and enabling modules

* Restructured site_enabled checks

* Enabling vhost correctly for non-debian systems
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant