Fix auth.go not finding the tsnet.Listener.

[beep-beep-beep-boop]

This fixes #68

auth.go was treating the listener as if it was a tsnet.Listener, but it was actually a tsServerListener which had a member (Listener) that implemented tsnet.Listener. This caused it to fall-through and try to authenticate the request through a locally-running tailscaled, which would fail if caddy was running in a container without one.

I'm not very familiar with go so IDK if there's a less messy-looking way to do this.

[beep-beep-beep-boop]

okay, this doesn't completely fix it. the issue still happens when trying to connect via https to the .ts.net domain. everything else will work, including the http .ts.net domain. (although if you put the https domain itself in the caddy file instead of :443 it seems like it tries to redirect to the https one which will essentially make the http .ts.net stop working as well.)

if you remove the tailscale_auth directive from the https route in the caddyfile it will work fine.

so this won't work:

https://caddytest.abc.ts.net {
  bind tailscale/caddytest
  tailscale_auth # this is causing the issue
  templates
  respond `Hello, {{placeholder "http.auth.user.id"}}`
}

but this will (but it won't authenticate):

https://caddytest.abc.ts.net {
  bind tailscale/caddytest
  templates
  respond `Hello, {{placeholder "http.auth.user.id"}}`
}

[willnorris]

okay, so the problem is that ever since f2562ba, we no longer just have a tsnet.listener. We now have a tsnet.listener wrapped in a [crypto/tls].listener wrapped in a caddyhttp.http2Listener, and neither of the latter two expose methods to unwrap them and get the internal listener. We could send a patch to caddy to add an Unwrap() net.Listener method on caddyhttp.http2Listener, but tls.listener is more challenging.

@mholt, how gross would it be to have something like:

type wrappedListener struct {
	net.Listener
	wrapped net.Listener
}

func (h *wrappedListener) Unwrap() net.Listener {
	return h.wrapped
}

and then app.Start has something like:

	if useTLS {
		// create TLS listener - this enables and terminates TLS
		tlsln := tls.NewListener(ln, tlsCfg)
		ln = &wrappedListener{Listener: tlsln, wrapped: ln}
		// ...
	}

I just tested this, and along with an Unwrap method on caddyhttp.http2Listener, this works to let me recursively unwrap to get to the underlying listener.

So basically this: caddyserver/caddy@master...willnorris:caddy:wrappedListener

[beep-beep-beep-boop]

I was thinking about using reflection to get to it. having a way to do it built into caddy would be better though i think.

[beep-beep-beep-boop]

I updated the PR to use reflection for now but doing it on caddy's end is probably a better permanent solution.

[mholt]

@willnorris We could do that, but like you mentioned, that chain would stop with a listener out of our control (like tls.Listener). Is the unwrapping needed only in this specific case?

[willnorris]

@willnorris We could do that, but like you mentioned, that chain would stop with a listener out of our control (like tls.Listener). Is the unwrapping needed only in this specific case?

Yes, the wrappedListener type is only needed for the case of listeners out of caddy's control. I don't know if tls.Listener is the only example of that... that's just the one at play in this specific instance.

[mholt]

What if we stick with the current solution (reflection -- not as ugly as I'd have thought!) and if other use cases start showing up, we can consider generalizing a solution upstream in Caddy?

[willnorris]

The proposed changes to caddy are quite minimal as well, and far cleaner than having to resort to reflection. Though I do understand that we're likely one of the only ones using caddyhttp.Server.Listeners(), so this would be a change in a highly used code path for a very small number of users.

[mholt]

True -- and to clarify, I'm not opposed to that change in Caddy (someday, if needed) -- because it's so minimal. I'm just thinking that for now, let's see if this can work and if we need it to generalize then we can make the change 👍

[willnorris]

@beep-beep-beep-boop let's add a note on this new func to revisit in the future if and when Caddy support unwrapping listeners. Otherwise, this LGTM

[willnorris]

I pushed a new commit to go ahead and add the wrappedListener interface and use it when present, then use reflection as a fallback.