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

core: Plugins can register listener networks #5002

Merged
merged 2 commits into from
Sep 1, 2022
Merged

Conversation

mholt
Copy link
Member

@mholt mholt commented Aug 31, 2022

This can be useful for custom listeners. Plugins should simply register their network type during init():

func init() {
	caddy.RegisterNetwork("foo", newFooListener)
}

func newFooListener(network, addr string) (net.Listener, error) {
	// ...
}

Then in the Caddy config, users can listen on that network:

{
	"listen": ["foo/:80"]
}

(It's a little unclear how this would work in the Caddyfile. Maybe something like foo:// or foo+http:// in the site address, I dunno.)

Multiple registrations are allowed if multiple network types need to be supported.

This feature/API is experimental and may change!

/cc @Xe @willnorris

This can be useful for custom listeners.

This feature/API is experimental and may change!
@mholt mholt added this to the v2.6.0-beta.1 milestone Aug 31, 2022
@willnorris
Copy link
Contributor

willnorris commented Aug 31, 2022

alright, this worked! Though I had to do some ugly hacks with package-level variables.

Is there anyway for the getListener func to be a method on a caddy module? So rather than registering the listener method separate from the module, have caddy find all modules that implement the Listener interface? Could that work?

@willnorris
Copy link
Contributor

okay, that might not actually be necessary. However, is there a way (perhaps through caddy.Context?) for a module to know information about the listener it's connected to? For example, an authentication provider running inside the http app to know its listener network and address?

@mholt
Copy link
Member Author

mholt commented Aug 31, 2022

Is there anyway for the getListener func to be a method on a caddy module? So rather than registering the listener method separate from the module, have caddy find all modules that implement the Listener interface? Could that work?

Hmm, I was thinking about this and that'll be tricky because Listen() isn't associated with a particular config. And maybe that's a design flaw, but I'm reminded that we also use it with our admin endpoint which does the config loading. So it's kind of a weird chicken-and-egg problem.

I can think more on that if really necessary but it sounds like you got it to work without being attached to a module... 😅👍

okay, that might not actually be necessary. However, is there a way (perhaps through caddy.Context?) for a module to know information about the listener it's connected to? For example, an authentication provider running inside the http app to know its listener network and address?

You've got all the tricky requests dontcha!

This is probably more feasible than the earlier request though 😉

There might be a way to do this already, and if not, let me see what I can figure out.

@mholt mholt added the in progress 🏃‍♂️ Being actively worked on label Aug 31, 2022
@mholt
Copy link
Member Author

mholt commented Aug 31, 2022

Alrighty, well I'm not sure Go gives us that information from the http.Request.

I can, however, add information about the server's listeners to the request context for you, but I'm not sure I can isolate which listener is being used for that request. In other words, if a server listens on 3 addresses, you'd have all 3 listeners in the context. I don't quite know how to be sure which listener a request came in on.

... unless I wrap all listeners with a function that maps a RemoteAddr to listener at Accept()-time, then refer to that map during ServeHTTP()-time. (Does that make sense?) In other words I can store state by injecting some code into Accept() and access it from ServeHTTP(), then clean it up on Close(). But that's getting a bit more involved. I can do it -- I've done it before in Caddy 1 for MITM detection -- it's just not ideal, and increases memory use for busy servers.

Given the list of all listeners for the server in the request context, and req.RemoteAdddr: how far does that get you?

@mholt
Copy link
Member Author

mholt commented Aug 31, 2022

@willnorris I just pushed a Listeners() method on caddyhttp.Server. You can get the server in the request by: server := req.Context().Value(caddyhttp.ServerCtxKey).(*caddyhttp.Server)

Maybe then you can iterate the listeners and see if their address is compatible with req.RemoteAddr -- if so perhaps you've found your guy!

Let me know if that works.

If not, my other suggestion for the time being would be to map a client's RemoteAddr when your listener does Accept() to the listener itself, then have your ServeHTTP access that map at request-time, using req.RemoteAddr, to get at the listener. (It might not make sense, I can explain more if needed.)

willnorris added a commit to tailscale/tailscale that referenced this pull request Aug 31, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.
@willnorris
Copy link
Contributor

So it looks like this might work: tailscale/caddy-tailscale#1

@mholt
Copy link
Member Author

mholt commented Sep 1, 2022

Woo hoo, that's awesome!

Are you even using servers (the map)? https://github.com/tailscale/caddy-tailscale-auth/pull/1/files#diff-c95a9381cb3d755657cf984eba630f599695529c52a3b407d902d38bcea81f8aR24

It looks like you are able to get what you need from the server's listeners: https://github.com/tailscale/caddy-tailscale-auth/pull/1/files?diff=split&w=0#diff-c95a9381cb3d755657cf984eba630f599695529c52a3b407d902d38bcea81f8aR95-R108

(What if there is more than 1 TS listener per server? Would that ever happen?)

@mholt mholt added the under review 🧐 Review is pending before merging label Sep 1, 2022
@mholt
Copy link
Member Author

mholt commented Sep 1, 2022

@willnorris Just kidding, now I see how you're using the servers map. Makes sense.

So yeah, that's cool. Think I should merge this?

@willnorris
Copy link
Contributor

I'm testing multiple listeners right now on different ports. It should be supported by tsnet I think, but I'm having issues. I'm also trying different tsnet servers connected to different tailnets. More experimenting needed, so maybe hold off on merging for a little longer?

@mholt
Copy link
Member Author

mholt commented Sep 1, 2022

@willnorris Gotcha, keep me posted! I'm interested how your experimenting goes.

willnorris added a commit to tailscale/tailscale that referenced this pull request Sep 1, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.
willnorris added a commit to tailscale/tailscale that referenced this pull request Sep 1, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.
willnorris added a commit to tailscale/tailscale that referenced this pull request Sep 1, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.

Signed-off-by: Will Norris <will@tailscale.com>
willnorris added a commit to tailscale/tailscale that referenced this pull request Sep 1, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.

Signed-off-by: Will Norris <will@tailscale.com>
willnorris added a commit to tailscale/tailscale that referenced this pull request Sep 1, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.

Signed-off-by: Will Norris <will@tailscale.com>
@willnorris
Copy link
Contributor

willnorris commented Sep 1, 2022

(oops, didn't mean to spam this issue with all those pushes 😱 )

Anyway, it was user error on my side. Multiple listeners and multiple tsnet servers works perfectly (with some minor tweaks on my side). Go ahead and merge when ready!

@mholt
Copy link
Member Author

mholt commented Sep 1, 2022

@willnorris Woohoo, you got it!

Did you test with config reloads yet? (Make sure to close listeners that aren't used by the new configuration, etc.)

Let me know if I can help or if anything in Caddy needs to be adjusted to help make that possible.

@mholt mholt removed in progress 🏃‍♂️ Being actively worked on under review 🧐 Review is pending before merging labels Sep 1, 2022
@mholt mholt merged commit 1edc1a4 into master Sep 1, 2022
@mholt mholt deleted the listener-plugins branch September 1, 2022 22:30
willnorris added a commit to tailscale/tailscale that referenced this pull request Sep 2, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.

Signed-off-by: Will Norris <will@tailscale.com>
willnorris added a commit to tailscale/tailscale that referenced this pull request Sep 2, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.

Signed-off-by: Will Norris <will@tailscale.com>
willnorris added a commit to tailscale/tailscale that referenced this pull request Sep 2, 2022
Allow callers to verify that a net.Listener is a tsnet.listener by type
asserting against this Server method, as well as providing access to the
underlying Server.

This is initially being added to support the caddy integration in
caddyserver/caddy#5002.

Signed-off-by: Will Norris <will@tailscale.com>
@mholt mholt modified the milestones: v2.6.0-beta.1, v2.6.0 Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants