debops / ansible-apache

Manage and configure the Apache HTTP Server
GNU General Public License v3.0
2 stars 6 forks source link

Added apache__redirect_to_https variable #9

Closed muelli closed 7 years ago

muelli commented 7 years ago

Currently, each vhost can define "redirect_to_https". If True, which is the current default, then the Apache configuration will include a Redirect line.

This is good, except that VirtualHost rules seem to have precedence over other rules. That is annoying when you want to define a global rule to serve, say, ACME requests, because Apache would respect the VirtualHost rules and redirect the plain HTTP request instead of serving the token. To help debops users to not having to touch each and every defined vhost, the new variable defines the default value for redirect_to_https, which used to be "True". The default is still "True", but now you can override that easily.

muelli commented 7 years ago

FWIW: A group snippet like the following could be employed to enable global redirection to HTTPS:

    httpsredirect:
        enabled: True
        type: raw
        raw: |
            # From http://serverfault.com/a/739128/193114
            <ifmodule mod_rewrite.c>
                RewriteEngine On
                RewriteOptions InheritDown
                RewriteCond %{HTTPS} off
                RewriteCond %{REQUEST_URI} !^/\.well\-known/acme\-challenge/
                RewriteRule (.*) https://%{HTTP_HOST}%{REQUEST_URI} [R=301,L]
            </ifmodule>
ypid commented 7 years ago

Currently, each vhost can define "redirect_to_https". If True, which is the current default, then the Apache configuration will include a Redirect line.

Redirect is not necessarily being used for this anymore. While working on support for Apache status (#8) I had basically the same problem because the Apache status is one of a very limited set of valid cases where a redirect to HTTPS should not be the default (localhost). So I made the role smart enough to use the rewrite module to redirect all requests except for Apache status.

This is what I would prefer for this valid use case as well, dedicated support for ACME (ref: #1). I don’t see much valid use cases except those two and would prefer to avoid a global default like apache__redirect_to_https.

Does that work for you?

muelli commented 7 years ago

hm. It doesn't seem to work. I also don't see why it would. As I've written in the commit message, at least with Apache 2.4, the VirtualHost rules have higher priority. Thus the vhosts' Redirect statements have higher priority than the global rewrite I added. It's Apache being a bit silly there, especially given the existence of the InheritDown option which forces rules to be injected in vhosts' space. But now we have to live with it somehow. One option for me would be to add the same configuration snippet to each and every vhost. But that feels more silly than to just disable redirection by default. Especially, because HSTS seems to be the more appropriate tool to perform the job of upgrading clients.

Note that this is also not necessarily about not redirecting the default host, only. I guess that I don't want any vhost to be redirected to a https version for ACME URLs.

The test is relatively simple. I execute wget -O- http://localhost/.well-known/acme-challenge/G6P3G9djqx8Lx0QkijKvT6Rfz7CB0DoQHZqu_OtOwpY and see whether redirects. I don't want it to redirect by itself for the ACME URLs.

ypid commented 7 years ago

Configuring ACME in the vhosts using an include snippet should do, similar as it is done on nginx.

About using HSTS for this that can be discussed if the coverage on client support is high enough but I would still like a HTTP redirect which all clients understand.

Have you checked my workaround and template code which I used to implement Apache status? The same can be done for ACME

muelli commented 7 years ago

Which clients do you think still need to understand HSTS? It seems that HSTS works with all browsers except Opera mini: http://caniuse.com/#search=hsts. I even had to delete my ~/.wget-hsts to test properly.

The server-info is a bit of a different usecase, I think. In that case you only want to serve server-status from localhost. In the ACME case, you want to serve the ACME files unconditionally. Unfortunately, Apache does not allow overwriting a vhost's decision to Rewrite. As far as I am aware at least. So whenever the vhost does a Rewrite of ACME URLs, we're in trouble. That said, I tried to comprehend how the default for apache__status_vhost sets https_enabled: False and how the macro then checks for a vhost's redirect_https in order to not Rewrite the status location. But I still don't think I've fully grasped how it works let alone how to make it work for the ACME case.

If you are alluding to implementing another conditional rewrite like the one for the server-status but for ACME for every defined vhost, then yes, that should be possible. But that seems to require making the role too complex for me to patch. Having a global rewrite for ACME URLs seems to be a bit cleaner and more comprehensible than to inject something into every vhost. For both admins to understand and the debops role to implement.

Another option might be to use something like RewriteMatch !^.well-known/acme... instead of the current Rewrite in every rewriting vhost. But whatever works best with the role is great.

ypid commented 7 years ago

HSTS

The 301 redirect is an important part of deploying an HTTPS website. As part of the HTTP protocol, it is supported by more browsers than HSTS. It serves as the primary means for upgrading a plaintext connection to HTTPS, updating search indexes, and avoiding link rot.

Also, I don’t think we should make assumptions that all clients support HSTS. Ref: List of podcatchers. I think there is no reason for removing the HTTP redirect default.

@drybjed What do you think about HSTS and HTTP redirecting?

Apache does not allow overwriting a vhost's decision to Rewrite. As far as I am aware at least.

I also don’t know a way and assume it is not supported, mod_alias takes precedence. That is why get_vhost_content_directives uses the rewrite module in such cases. You could check in this macro if either Apache status or ACME require mod_rewrite and then use this. Maybe introduce a array of booleans where True means mod_rewrite is needed, then use that array to generate the directives. Complexity of this should be manageable. In the end, debops.nginx already does exactly that. Ref: /etc/nginx/snippets/acme-challenge.conf.j2

drybjed commented 7 years ago

I don't believe that HTTP -> HTTPS redirect will ever go away, in this field that rarely happens. Even in 100+ years servers will have it. Only reasonable way to make it less needed would be if HTTP clients were changed to try and connect to https port first and then fallback to http, but that's unlikely and breaks conventions. An alternative would be something like STARTTLS for websites but that still needs to be made stronger and requires client support. So the redirect should be kept.

Maybe an alternative would be to use nginx beside apache2 to redirect HTTP connections to HTTPS. With current debops.nginx it should be farily easy, just disable all HTTPS support and listening on 443/tcp port. Although the whole setup will be a bit more complicated in terms of maintainability. Still, there's an idea.

ypid commented 7 years ago

Only reasonable way to make it less needed would be if HTTP clients were changed to try and connect to https port first and then fallback to http, but that's unlikely and breaks conventions.

Nice and consequent idea :+1: Don’t say that it is unlikely. I think with the trend in the last few years, this could happen in the foreseeable future.

Maybe an alternative would be to use nginx beside apache2 to redirect HTTP connections to HTTPS.

Is an idea at least, but I assume not one that we would like/recommend. For clarification, nginx would listen on 80/tcp and Apache would listen on 443/tcp (not supported/tested by debops.apache currently).

muelli commented 7 years ago

I think there is no reason for removing the HTTP redirect default.

note that I have not proposed that. My proposal was to make it easy to disable it if it gets in the way. As far as I understand, #8 does not provide an easy way to disable it. It would be easy to add, though.

That is why get_vhost_content_directives uses the rewrite module in such cases. You could check in this macro if either Apache status or ACME require mod_rewrite and then use this. Maybe introduce a array of booleans where True means mod_rewrite is needed, then use that array to generate the directives. Complexity of this should be manageable.

If I understand correctly then you think of introducing more template code to check for ACME directives in order to make it work. Do you want me to try to code something up? Or do you think it should be possible to configure ACME compatibility with the currently proposed changes?

I predict that there will be cases where the redirect by default will get in the way and in which it will be beneficial to have an option to disable redirect by default. The cost of creating and maintaining such a switch should be low. So my feeling is that the benefits of such a mechanism outweigh the costs.

ypid commented 7 years ago

As far as I understand, #8 does not provide an easy way to disable it. It would be easy to add, though.

The cost of creating and maintaining such a switch should be low. So my feeling is that the benefits of such a mechanism outweigh the costs.

Sure, but your use case is not a good example because this use case is supposed to have native support by the role. Any other good examples?

Do you want me to try to code something up?

That would be great! I have not had time to look into this yet. But note that this is a bit of work to do it properly: Checking debops.nginx, implementing it for this role, writing documentation and updating debops.pki.

muelli commented 7 years ago

Sure, but your use case is not a good example because this use case is supposed to have native support by the role. Any other good examples?

As I've said: I believe that there will be an instance in the future where one wishes for the automatic redirection to be disabled. If only because the configuration is unexpectedly incompatible with something else.

So I see value in being able to disable the role's redirection facilities with a simple switch even if it's not about this very ACME use case. Especially because I think that the value comes at a surprisingly low cost.

A more concrete issue I'm seeing right now is that one might simply don't want to upgrade using HTTP redirects.

Do you want me to try to code something up?

That would be great!

As I've indicated: It's probably too complex for me. I might give it a shot, but don't hold your breath. It's likely that I will go for disabling the role's http redirect in its current form because I think it's too expensive in terms of complexity for what it achieves.

ypid commented 7 years ago

A more concrete issue I'm seeing right now is that one might simply don't want to upgrade using HTTP redirects.

How else would you do it then while being in compliance with the RFC?

An HSTS Host MUST NOT include the STS header field in HTTP responses conveyed over non-secure transport.

muelli commented 7 years ago

You seem to be assuming that one wants to use HSTS. That's not necessarily the case with someone who just doesn't want the requests to be redirected.

ypid commented 7 years ago

Merged, thanks!