Repository: caddyserver/caddy (language: go)

Issue:
Weird bug with default server directive that has both HTTP and HTTPS?
With the following Caddyfile:
```
http:// {
        respond "DEFAULT"
}
```
Caddy will respond to a HTTP request on port 80 over direct IP as expected with "DEFAULT".
```
> GET / HTTP/1.1
> Host: 192.168.1.100
> User-Agent: curl/7.79.1
> Accept: */*
>
< HTTP/1.1 200 OK
< Server: Caddy
< Date: Tue, 07 Jun 2022 23:47:19 GMT
< Content-Length: 7
<
DEFAULT
```

However, if we try adding a https:// there like so:
```
http:// https:// {
        respond "DEFAULT"
}
```
suddenly the HTTP port starts redirecting to HTTPS.
```
> GET / HTTP/1.1
> Host: 192.168.1.100
> User-Agent: curl/7.79.1
> Accept: */*
>
< HTTP/1.1 308 Permanent Redirect
< Connection: close
< Location: https://192.168.1.100/
< Server: Caddy
< Date: Tue, 07 Jun 2022 23:46:06 GMT
< Content-Length: 0
<
```
This is most unexpected, given Caddy is normally supposed to support mixed HTTP and HTTPS site blocks, like this:
```
http://test.example.com test.example.com {
        respond "DOMAIN"
}
```
In the above example, you can normally access test.example.com over HTTP as expected (over HTTPS too ofc).

What's even more surprising, is that when we modify the second Caddyfile snippet to be this:
```
test.example.com {
        # must be https
        respond "TEST"
}

http:// https:// {
        respond "DEFAULT"
}
```
**Suddenly HTTP port responds with "DEFAULT" as expected again!**

Looking further at the default server example though, it seems that its HTTPS port does not work (no matter if tried by IP or dummy domain):
```
>curl -k -v https://192.168.1.100
*   Trying 192.168.1.100:443...
* Connected to 192.168.1.100 (192.168.1.100) port 443 (#0)
* schannel: disabled automatic use of client certificate
* schannel: using IP address, SNI is not supported by OS.
* schannel: ALPN, offering http/1.1
* schannel: next InitializeSecurityContext failed: SEC_E_ILLEGAL_MESSAGE (0x80090326) - This error usually occurs when a fatal SSL/TLS alert is received (e.g. handshake failed). More detail may be available in the Windows System event log.
* Closing connection 0
curl: (35) schannel: next InitializeSecurityContext failed: SEC_E_ILLEGAL_MESSAGE (0x80090326) - This error usually occurs when a fatal SSL/TLS alert is received (e.g. handshake failed). More detail may be available in the Windows System event log.
```
If I add `tls internal` into the "DEFAULT" block, nothing changes. If I add `tls <path>.pem <path>.pem` manually with a path to the certificate into the section, Caddy fails to start:
```
run: adapting config using caddyfile: server listening on [:80] is HTTP, but attempts to configure TLS connection policies
```

---

# **So to summarize, it seems there are two issues**:
1. Inconsistent behavior of the "default" (no-hostname) site block for both HTTP and HTTPS based on existence of **another** unrelated HTTPS site block (if "default" block is the only one, HTTP is redirected to HTTPS for no reason?).
2. You are supposed to be able to create a site block that can be served over HTTP and HTTPS both, but you seemingly cannot then use `tls` directive inside to set the "internal" or custom certificate paths, it only works with default encryption that specifies no custom options. Expected behavior would be that whatever is set in `tls` option would simply be ignored for HTTP connections...

It is worth to note that I was previously successfully using both Apache2 and nginx with such configuration that is basically the default if you enable SSL there, that is the same default server block is also served over HTTPS with a self-signed certificate, which works both over IP or any unrecognized domain, thus titled the "default".

### Workaround attempt

I tried this:
```
(default) {
        respond "DEFAULT"
}

http:// {
        import default
}

https:// {
        # I could not access this over HTTPS with neither TLS options or lack thereof
        #tls internal
        # adding the line below (instead of the above ofc) makes the Caddy server start with no error, but stop responding to ANY requests, both HTTP or HTTPS (???)
        #tls /etc/ssl/certs/ssl-cert-snakeoil.pem /etc/ssl/private/ssl-cert-snakeoil.key
        import default
}
```
Result: I still cannot load the server via HTTP, I still need to add a random HTTPS site block in order to be able to reach the default HTTP site.

---

Now of course I could simply accept that I will not get an unsecure default unnamed HTTPS endpoint with Caddy that I could by default with Apache2 and nginx. But I think my attempts still bring up some logic errors in Caddy config processing that shouldn't be behaving the way they behave regardless.

---

Caddy version: v2.5.1 h1:bAWwslD1jNeCzDa+jDCNwb8M3UJ2tPa8UZFFzPVmGKs=


Discussion / hints:
It seems that even an example of
```
:80 http://domain.example.com domain.example.com
```
confuses Caddy.

With a site block like above you can access the server via IP and port 80 as expected, but trying to access it via `http://domain.example.com` causes the unexpected redirect to HTTPS version to occur
This is working as intended, actually. The HTTP->HTTPS redirect routes get inserted _just before_ any HTTP catch-all routes, so that it's possible to keep the redirect and force certain domains to be served over HTTP.

See https://github.com/caddyserver/caddy/pull/4033 and https://github.com/caddyserver/caddy/issues/3212 which explain the problem.

To work around it in your situation, you'll want to use the global option `auto_https disable_redirects` which will remove the redirect routes and let HTTP requests actually reach your catch-all block.

As for the `tls` directive thing, I disagree. It's a configuration error to use `tls` inside of a site block configuring an HTTP server. Split them up.
> To work around it in your situation, you'll want to use the global option auto_https disable_redirects which will remove the redirect routes and let HTTP requests actually reach your catch-all block.

Honestly I think this might be the best solution in my case, thanks.

> As for the `tls` directive thing, I disagree. It's a configuration error to use tls inside of a site block configuring an HTTP server. Split them up.

I think this is inconsistent though -- because a single site block for HTTP and HTTPS is otherwise allowed and perfectly functional. It works fine with automatic HTTPS, but if you want to define custom TLS options, then you are required to split them up just to insert the `tls` option. I think that's just pretty inconvenient.

---

I still cannot wrap my head around on how to get catch-all HTTPS working, is it even supported? And speaking of which, I found another weird behavior. Suppose this config file:
```
{
    auto_https disable_redirects
}
(default) {
    respond "DEFAULT"
}
http:// {
    import default
}
https:// {
    tls internal
    import default
}
# ...
domain1.example.com:8443 {
    respond "UNRELATED BLOCK"
}
```
Let's ignore the last block that works independently and let's assume *.example.com resolves to our server's IP. The following happens:
1. I can access catch-all http://<serverIP> and http://domain1.example.com and http://random.example.com, all lead to "DEFAULT"
2. I cannot access catch-all https://<serverIP> or https://random.example.com
3. I can access only https://domain.example.com

^ Note that in point 3 I can access the catch-all only with a certificate that's in Caddy's memory, despite being there (generated by Let's Encrypt) for a completely unrelated block

Now if I do this:
```
{
    auto_https disable_redirects
}
(default) {
    respond "DEFAULT"
}
http:// {
    import default
}
https:// {
    # self-signed certs
    tls /var/lib/caddy/ssl-cert-snakeoil.pem /var/lib/caddy/ssl-cert-snakeoil.key
    import default
}
```

It works, I can curl the server on HTTPS with any domain or IP! But!!! Apparently now I cannot add any normal HTTPS site block on a normal. Let's add this:
```
domain1.example.com {
    respond "NON DEFAULT"
}
```
**If I open https://domain1.example.com, I get "NON DEFAULT" served, which means the block works. But I get certificate specified in https:// catch-all instead!!!** This is clearly a bug in certificate selection routines, since otherwise the block wouldn't be used at all if catch-all took precedence.

The automatic certificate in my case of `domain1.example.com` exists, and I can add a site block on this domain on another port and it works. It also works on the default port if I comment out the catch-all https block...

> I think this is inconsistent though -- because a single site block for HTTP and HTTPS is otherwise allowed and perfectly functional. It works fine with automatic HTTPS, but if you want to define custom TLS options, then you are required to split them up just to insert the tls option. I think that's just pretty inconvenient.

I don't think it's inconsistent. It's the difference between implicit and explicit config for HTTPS. If you let Caddy enable automatic HTTPS (implicitly) then a single block is fine. Obviously you need to disable redirects if you want to do that though. But if you need to configure `tls` explicitly, then yes you need to split it up, because otherwise it's a logic error; you can't configure TLS for HTTP.

> I still cannot wrap my head around on how to get catch-all HTTPS working, is it even supported?

Yes it is supported, but you need to have a certificate to serve for the requested domain. If you don't put a domain in your config, Caddy won't be able to issue a certificate ahead of time, so it won't be able to complete a handshake.

If you use `tls internal`, then you are telling Caddy to use its internal CA, but you still didn't tell it which domains to manage a cert for. If you don't know the domains ahead of time, then you need to use On-Demand TLS, which enables a mode where Caddy will attempt to issue a certificate for the domain that was requested in the TLS handshake (the SNI field).

But keep in mind, enabling `on_demand` has risk, because it's unbounded by default; some attacker could force your server to issue an infinite amount of TLS certificates and fill up your disk storage with garbage, if your server is publicly accessible.

> Note that in point 3 I can access the catch-all only with a certificate that's in Caddy's memory, despite being there (generated by Let's Encrypt) for a completely unrelated block

Yeah, that's normal. Under the hood, Caddy's actual config language is JSON. If you run `caddy adapt --pretty` on your Caddyfile, you'll see what your config looks like. It'll probably be a bit clearer what's going on.

Keep in mind that this won't show the routes that automatic HTTPS adds in, because that happens at runtime just before starting the server, it's not included in the config.

How it works is Caddy has a `tls` app, which is what does all the certificate maintenance and caching, and there's the `http` app, which is what actually runs the server. The HTTP app asks the TLS app "hey do you have a cert for _this domain_?" and if the TLS app has one in cache, it'll use it to serve.

In JSON config, there's no concept of "sites" at all, that's just syntax sugar in the Caddyfile to try to make it easier to write configs. But you're kinda trying to abuse all the edgecases here, so of course you'll run up against all pitfalls/caveats of that abstraction.

> If I open [domain1.example.com](https://domain1.example.com/), I get "NON DEFAULT" served, which means the block works. But I get certificate specified in https:// catch-all instead!!!

In that case, you would need to use `auto_https ignore_loaded_certs` to tell Caddy "don't use certs from `tls` loaded into cache, and try to manage a cert anyways if there's an explicit domain listed in the config". But we didn't make it possible to specify both `disable_redirects` and `ignore_loaded_certs` at the same time... because that's _insanely_ edgecase-y. Do you _really_ need to do that? It seems absolutely crazy-bonkers to me. If you really, really need that, honestly you should probably just use JSON config directly, so you have full control over what's going on.
I think there might be improvements we could make here, either in docs or in implementation, but I need to reread this issue with fresher eyes.
> I don't think it's inconsistent. It's the difference between implicit and explicit config for HTTPS.

I think there is inconsistency. To bring this down to, ultimately, the simplest expression I can demonstrate with:

```
https:// {
  tls internal {
    on_demand
  }
  respond "HTTPS"
}

http:// {
  respond "HTTP"
}
```

And I make a request to `http://localhost`, I expect a 200 OK with "HTTP", period. Currently, I get a redirect to `https://localhost` and receive a 200 OK with "HTTPS". I'm not sure I can get more explicit than this, but the HTTP listener is just getting completely trodden over right now.

This changes as soon as I change the HTTPS site to `https://localhost`, in which case its overriding redirect becomes limited in scope to `http://localhost` requests and not e.g. `http://local.test`.

I haven't investigated, but I have a hunch this is part of the interaction of your logic, @francislavoie (which allows for another site to override a HTTP catch-all with its listener), and the fact that Caddy seems to interpret an explicit HTTPS listener as "HTTPS with a HTTP redirect", meaning that this HTTPS catch-all is having its catch-all redirect listener override the _actually configured_ HTTP listener.

Does that sound like it makes sense?

That said, from an ergonomics perspective, I think that:
- A manually configured catch-all HTTP site should take precedence over an auto-configured HTTP->S catch-all redirect
- I shouldn't have to use `auto_https disable_redirects` in the long run for this to work

And further (possibly strictly beyond the scope of this issue, but related because I believe it's interacting here):
- An explicit HTTPS listener shouldn't configure a HTTP redirect (only scheme-agnostic sites should)
> I shouldn't have to use `auto_https disable_redirects` in the long run for this to work

I disagree, this is exactly the situation where you should use it. I don't see how a redirect would ever be correct with your intent as described here, so you should turn off redirects.

I don't think more magic is warranted here. The logic is complex enough as it is. I don't think there's any particular situation that is "impossible" to configure anymore, which really is the goal. You have two knobs, the HTTP sites and `disable_redirects`, and armed with those, you can make it do whatever you need.
Yeah, sure, I agree that it functions as a workaround, but there's _no way_ that a HTTPS and a HTTP block, explicitly configured in the Caddyfile, should be interpreted as "please ignore my HTTP block entirely and redirect it to HTTPS" in the long run.

Whether there's an additional check that does this automatically or not, the intent behind this configuration is abundantly clear, and any logic that produces a result counter to that intent with that incredibly simple configuration is, in my opinion, not ideal.
(FYI, I'll chime in too when I'm back at the office tomorrow)

Fix patch:
diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go
index 3d476cb1259..be229ea8b5c 100644
--- a/modules/caddyhttp/autohttps.go
+++ b/modules/caddyhttp/autohttps.go
@@ -378,19 +378,29 @@ redirServersLoop:
 		// we'll create a new server for all the listener addresses
 		// that are unused and serve the remaining redirects from it
 		for _, srv := range app.Servers {
-			if srv.hasListenerAddress(redirServerAddr) {
-				// find the index of the route after the last route with a host
-				// matcher, then insert the redirects there, but before any
-				// user-defined catch-all routes
-				// see https://github.com/caddyserver/caddy/issues/3212
-				insertIndex := srv.findLastRouteWithHostMatcher()
-				srv.Routes = append(srv.Routes[:insertIndex], append(routes, srv.Routes[insertIndex:]...)...)
+			// only look at servers which listen on an address which
+			// we want to add redirects to
+			if !srv.hasListenerAddress(redirServerAddr) {
+				continue
+			}
 
-				// append our catch-all route in case the user didn't define their own
-				srv.Routes = appendCatchAll(srv.Routes)
+			// find the index of the route after the last route with a host
+			// matcher, then insert the redirects there, but before any
+			// user-defined catch-all routes
+			// see https://github.com/caddyserver/caddy/issues/3212
+			insertIndex := srv.findLastRouteWithHostMatcher()
 
-				continue redirServersLoop
+			// add the redirects at the insert index, except for when
+			// we have a catch-all for HTTPS, in which case the user's
+			// defined catch-all should take precedence. See #4829
+			if len(uniqueDomainsForCerts) != 0 {
+				srv.Routes = append(srv.Routes[:insertIndex], append(routes, srv.Routes[insertIndex:]...)...)
 			}
+
+			// append our catch-all route in case the user didn't define their own
+			srv.Routes = appendCatchAll(srv.Routes)
+
+			continue redirServersLoop
 		}
 
 		// no server with this listener address exists;


Test patch: