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

Create blacklist for configuration directives we can't safely copy over #4797

Open
joohoi opened this issue Jun 7, 2017 · 14 comments
Open

Comments

@joohoi
Copy link
Member

joohoi commented Jun 7, 2017

There are some configuration directives that break when duplicated. They are used somewhat rarely, and in very specific configurations. Instead of doing guesswork and trying to modify the directives, we should instead create a blacklisting mechanism that ignores (or comments off) the problematic statements / blocks.

If directives are disabled for the newly created HTTPS configuration, we should display a meaningful message pointing the user to the configuration file(s) that need manual modification.

Examples: #2726 #1820 #6495

@baltasvejas
Copy link

baltasvejas commented Jan 5, 2018

Forget me if I do not get it right, but title suggests that some configuration directives (such as WSGI) should be commented out (in apache .conf file) while performing automated install? Is it no problems with the installation later in such case?

@joohoi
Copy link
Member Author

joohoi commented Jan 6, 2018

It will let Apache to run, and we have a chance to notify the user that there's something that needs manual attention. Currently Apache errors out in restart if such directives are copied over (they should be unique), then we are rolling back our configuration changes and erroring out ourselves.

This change will let us at least configure TLS for the domain, and point the user towards the config that needs attention.

@keros
Copy link

keros commented Jan 7, 2019

Any updates on this?
I got the update to certbot 0.30.0 today and the "FastCgiExternalServer" still hits me.
The report itself (for proxy and wsgi) is open since 2017 and seems to be nearly dead?

@keros
Copy link

keros commented Feb 16, 2019

Cool now 0.31.0 is out and the new version does not longer copy the http config file. Instead it uses the https config as default and copy it (I had seperated configs as workaround for this bug).
The only thing I can now do is to disable the https conf, renew the certs with the http conf and re-enable the https conf.

This becomes more and more a show stopper for me.
Can please anyone take a look at this?

@joohoi
Copy link
Member Author

joohoi commented Feb 18, 2019

Are you hitting the issue when renewing certificates or when installing a new certificate? The HTTP-01 renewal process should not copy any VirtualHosts over to perform the validation, but instead adds an override for validation path /.well-known/acme-challenge/... to existing VirtualHost configuration.

That said, even if we implement the black listing in the future, the behavior would most likely be to error out and tell the user that there's something Certbot cannot handle. This would have to be done as a security measure in order to not expose something in the filesystem (DocumentRoot) path to internet without users knowledge, even temporarily. Just copying a FastCgiExternalServer VirtualHost over without the configuration directive would do just that.

@keros
Copy link

keros commented Feb 18, 2019

I created the certs with:
certbot certonly --apache -d ...
and renew them with:
certbot renew

The problem does not occur on creating an cert (the http.conf is rather small):

<VirtualHost *:80>
    ServerName   mydomain.com:80
    ServerAlias  mydomain.com

    # Enable this after cert creation
    #RewriteEngine  on
    #RewriteCond %{HTTPS} off
    #RewriteRule (.*) https://%{HTTP_HOST}%{REQUEST_URI} [R=301,L]

    ServerAdmin  "mail@mydomain.com"
    DocumentRoot /data/www/mydomain.com/httpdocs
    <Directory /data/www/mydomain.com/httpdocs>
        Options -Indexes
        Require all granted
        AllowOverride All
    </Directory>

    CustomLog /data/www/mydomain.com/logs/access.log combined
    ErrorLog /data/www/mydomain.con/logs/error.log
</VirtualHost>

On the https conf there is the php part:

    # use mod fastcgi -> proxy:fcgi makes some problems
    <IfModule mod_fastcgi.c>
        # Set virtual handler for fpm functions
        <LocationMatch "^/(fpm-ping|fpm-status)$">
            # Change the requre line to your needs if you need the functions
            Require all denied
            SetHandler php-fpm-fcgi-virt
            Action php-fpm-fcgi-virt /php-fpm-fcgi virtual
        </LocationMatch>
        # Set handler for *.php, *.php7, *.phtml, ... files
        <FilesMatch ".+\.ph(p[3457]?|t|tml)$">
            SetHandler php-fpm-fcgi
        </FilesMatch>
        <FilesMatch ".+\.phps$">
            # Deny access to raw php sources by default
            # To re-enable it's recommended to enable access to the files
            # only in specific virtual host or directory
            Require all denied
        </FilesMatch>
        # Deny access to files without filename (e.g. '.php')
        <FilesMatch "^\.ph(p[3457]?|t|tml|ps)$">
            Require all denied
        </FilesMatch>

        Action php-fpm-fcgi /php-fpm-fcgi
        Alias /php-fpm-fcgi /data/www/mydomain.com/cgi-bin/php-fpm-fcgi-ssl-mydomain.com
        FastCgiExternalServer /data/www/mydomain.com/cgi-bin/php-fpm-fcgi-ssl-mydomain.com -idle-timeout 300 -socket /run/php/php7-fpm.mydomain.com.sock -pass-header Authorization

        <Directory "/data/www/mydomain.com/cgi-bin">
            Require all granted
        </Directory>
    </IfModule>

If I renew the certificate now I get:

Cert is due for renewal, auto-renewing...
Plugins selected: Authenticator apache, Installer apache
Renewing an existing certificate
Performing the following challenges:
http-01 challenge for mydomain.com
Error while running apache2ctl configtest.
Action 'configtest' failed.
The Apache error log may have more information.

AH00526: Syntax error on line 50 of /etc/apache2/sites-available/mydomain.com_https.conf:
FastCgiExternalServer: redefinition of previously defined class "/data/www/mydomain.com/cgi-bin/php-fpm-fcgi-ssl-mydomain.com"

Cleaning up challenges
Attempting to renew cert (mydomain.com) from /etc/letsencrypt/renewal/mydomain.com.conf produced an unexpected error: Error while running apache2ctl configtest.
Action 'configtest' failed.
The Apache error log may have more information.

AH00526: Syntax error on line 50 of /etc/apache2/sites-available/mydomain.com_https.conf:
FastCgiExternalServer: redefinition of previously defined class "/data/www/mydomain.com/cgi-bin/php-fpm-fcgi-ssl-mydomain.com"
. Skipping.

Line 50 is the FastCgiExternalServer ... Part in the https.conf.
I got this error the first time with certbot 0.28.0.
My workaround was to create separated http and https config files.
But since certbot 0.31.0 the error is back and it seems certbot is now using the https config for renewing the certs which causes this problem.
The only thing I could now do is to create an /etc/letsencrypt/renewal-hooks/pre/ and an /etc/letsencrypt/renewal-hooks/post script that handles the FastCgiExternalServer line :/

If possible I would prefer if certbot handles this.

@joohoi
Copy link
Member Author

joohoi commented Feb 18, 2019

This seems rather strange as Certbot shouldn't be copying the configuration files over in HTTP-01 challenge. Are you able to run apache2ctl configtest manually (outside of Certbot) without errors?

In order to help you further, I would like to see the contents of /var/log/letsencrypt/letsencrypt.log to be able to pinpoint the error better. If you are not comfortable in sharing that publicly, you can email it to me to address joona(dot)hoikkala(at)eff.org

@keros
Copy link

keros commented Feb 18, 2019

Thank you for the help

Yea configtest runs without problems:

root@myserver:~# apache2ctl configtest 
Syntax OK

I added the log file.
I cut it down to the interesting parts (header and the renew of the domain).
I hopefully obfuscated enough and not to much.

letsencrypt.log

@HgAlexx
Copy link

HgAlexx commented Feb 28, 2019

Hello,
I have the same issue as keros, #6495, same directive, same error.

If I start certbot with no argument, them pick the domain I want to renew, the process works:

Select the appropriate numbers separated by commas and/or spaces, or leave input
blank to select all options shown (Enter 'c' to cancel): 22
Obtaining a new certificate
Performing the following challenges:
http-01 challenge for [domain]
Waiting for verification...
Cleaning up challenges
Deploying Certificate to VirtualHost /etc/apache2/sites-enabled/[domain].conf

The error only occurs with renew options.

@keros
Copy link

keros commented Mar 5, 2019

I have now debugged it and found the problem.
I added an "sys.exit" after the configtest failed and had a look to the modified configs.
Certbot create backups of the config in "/var/lib/letsencrypt/temp_checkpoint" so I made some diffs.

The problem is not in the vhost config file.
In my case certbot modifies 3 configs:
The vhost http config (/etc/apache2/sites-available/mydomain.com.conf) -> added 2 Includes an other minor Changes
The vhost https config (/etc/apache2/sites-available/mydomain.com_https.conf) -> added 2 Includes an other minor Changes
And the apache2.conf (/etc/apache2/apache2.conf):

Include /etc/apache2/sites-available/mydomain.com.conf
Include /etc/apache2/sites-available/mydomain.com_https.conf

And that's the Problem.
The vhost files are already included with:

# Include the virtual host configurations:
IncludeOptional sites-enabled/*.conf

So apache includes the same vhost first with IncludeOptional ... and then with Include ... which brings up the error.
This apache config is the default on all Ubuntu/Debian distros (I think some other distros go the same way).

In my case an easy fix would be to not modify the apache2.conf at all.
But I think this would break certbot for other people.
A not so easy way would be to check if the config is already included (if yes don't add the include).
A maybe easier way could be to comment out the IncludeOptional sites-enabled/*.conf line. But this may also breaks the config in some situations.
There can be multiple IncludeOptional statements.
In my case (should be the same on all Ubuntu/Debian systems):

...
# Include module configuration:
IncludeOptional mods-enabled/*.load
IncludeOptional mods-enabled/*.conf

...

# Include generic snippets of statements
IncludeOptional conf-enabled/*.conf

# Include the virtual host configurations:
IncludeOptional sites-enabled/*.conf

...

@joohoi
Copy link
Member Author

joohoi commented Mar 6, 2019

Thanks for debugging this further! Apparently Certbot fails to detect that these VirtualHosts are already included for some reason. It shouldn't add the custom Include directives otherwise. The part of code that adds the includes is here: https://github.com/certbot/certbot/blob/master/certbot-apache/certbot_apache/http_01.py#L202-L205 it however does this conditionally if VirtualHost is marked as not enabled.

The code that checks this status is called from: https://github.com/certbot/certbot/blob/master/certbot-apache/certbot_apache/parser.py#L648-L667 via https://github.com/certbot/certbot/blob/master/certbot-apache/certbot_apache/configurator.py#L833

So there might be some pathing issues due to symlinks and whatnot. I'll look into this in the near future.

@keros
Copy link

keros commented Mar 7, 2019

Thanks for pointing me to the right place.
I need to get better in python.

I added an print to the function _set_up_include_directives and in theory the function is right.

    def _set_up_include_directives(self, vhost):
        """Includes override configuration to the beginning and to the end of
        VirtualHost. Note that this include isn't added to Augeas search tree"""

        print "vhost:\n%s\n" % vhost

        if vhost not in self.moded_vhosts:
            ...

This returns me:

Performing the following challenges:
http-01 challenge for www.mydomain.com
http-01 challenge for subdomain.mydomain.com
http-01 challenge for mydomain.com
vhost:
File: /etc/apache2/sites-available/mydomain.com_https.conf
Vhost path: /files/etc/apache2/sites-available/mydomain.com_https.conf/VirtualHost
Addresses: *:443
Name: mydomain.com:443
Aliases: subdomain.mydomain.com, www.mydomain.com
TLS Enabled: Yes
Site Enabled: No
mod_macro Vhost: No

vhost:
File: /etc/apache2/sites-available/mydomain.com.conf
Vhost path: /files/etc/apache2/sites-available/mydomain.com.conf/VirtualHost
Addresses: *:80
Name: mydomain.com:80
Aliases: www.mydomain.com
TLS Enabled: No
Site Enabled: No
mod_macro Vhost: No

vhost:
File: /etc/apache2/sites-available/mydomain.com_https.conf
Vhost path: /files/etc/apache2/sites-available/mydomain.com_https.conf/VirtualHost
Addresses: *:443
Name: mydomain.com:443
Aliases: subdomain.mydomain.com, www.mydomain.com
TLS Enabled: Yes
Site Enabled: No
mod_macro Vhost: No

vhost:
File: /etc/apache2/sites-available/mydomain.com_https.conf
Vhost path: /files/etc/apache2/sites-available/mydomain.com_https.conf/VirtualHost
Addresses: *:443
Name: mydomain.com:443
Aliases: subdomain.mydomain.com, www.mydomain.com
TLS Enabled: Yes
Site Enabled: No
mod_macro Vhost: No

vhost:
File: /etc/apache2/sites-available/mydomain.com.conf
Vhost path: /files/etc/apache2/sites-available/mydomain.com.conf/VirtualHost
Addresses: *:80
Name: mydomain.com:80
Aliases: www.mydomain.com
TLS Enabled: No
Site Enabled: No
mod_macro Vhost: No

So certbot writes the files in /etc/apache2/sites-available and checks if they are enabled.
The check returns false because sites-available is not in the enabled list.
It does not check if there are symlinks from sites-enabled to sites-available.
So a quick and dirty hack that works for me is this:

    def _parsed_by_parser_paths(self, filep, paths):
        """Helper function that searches through provided paths and returns
        True if file path is found in the set"""
        #print "Paths:\n%s\n" % paths.keys()
        for directory in paths.keys():
            for filename in paths[directory]:
                #print "directory: %s" % filename
                if fnmatch.fnmatch(filep, os.path.join(directory, filename)):
                    return True
                if fnmatch.fnmatch(os.path.basename(filep), filename):
                    if os.path.islink(os.path.join(directory, os.path.basename(filep))):
                        linkpath = os.readlink(os.path.join(directory, os.path.basename(filep)))
                        realpath = os.path.join(directory, linkpath)
                        if os.path.samefile (realpath, filep):
                            return True
        return False

There are several flaws in my hack.
This only works if the symlink has the same name
This will not work with hardlinks
os.readlink only reads the first level (so link to link to file will not work) -> in python3 os.path.realpath can be used
The if cascade looks awful.

I hope someone has an better idea for this problem.

@dangermouse124
Copy link

I'm still having this problem with certbot 0.36.0
Or is it me?

@keros
Copy link

keros commented Feb 17, 2020

I see it on certbot 1.2.0 as well.
But my apache configs are now different on the new Servers so I don't run into this Problem anymore.
The old servers live with my workaround and will die after some time

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

5 participants