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

Allow tagged nodes to authenticate #2

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

Xe
Copy link
Contributor

@Xe Xe commented Sep 23, 2022

The basic logic here is that we want tagged machines to be able to authenticate with Tailscale auth. The main problem here is that our identity model is usually one to one (one machine has one owner) but tags make this complicated (one machine can have n tags). As a stick in the mud, I propose that we allow tagged machines to connect with Tailscale auth but the first tag is the one that has "identity power".

This decision was made arbitrarily. It should probably be brought up in an eng meeting, but this works enough to make it work on my Steam Deck:

FdWjuNcWAAEiNIJ

(pictured: a Valve Steam Deck running Firefox connected to Jenkins over Tailscale, proving that both Jenkins is configured to use Tailscale for auth and that a tagged node can authenticate to Jenkins)

Points of contention

  1. I transformed tag:name to tag___name so that it's less likely to collide with an actual human user
  2. It only picks the first tag (which may not pan out in the real world)

Xe Iaso added 2 commits September 23, 2022 14:22
@Xe Xe requested a review from willnorris September 23, 2022 17:05
@willnorris
Copy link
Member

Several questions...

  • do we know if the order of tags is stable and/or predictable?
  • are tags the right thing to use as the identify of the machine? Given that multiple machines can have the same tags, is it the right behavior for all of them to have the same identity when connecting to an application? Or should it be something like the node name? I don't think there's a correct answer to that, since it's highly dependent on how someone uses tags in their tailnet and what behavior they want on the application.

Since our expectation is that a user of the plugin is always going to have their own caddy config which maps user attributes to HTTP headers, I wonder if we could provide the building blocks to allow people to map things however is appropriate for their use case. What if we just populated a tailscale_tags field on the user metadata as well as a tailscale_node and leave the username empty? I wonder what the caddy config would look like (or if it's even really possible) to map that to X-WebAuth-User if and only if user.id isn't present?

Or if that's too gnarly, maybe it's configuration for the tailscale_auth directive? So you'd have something like:

:80 {
    tailscale_auth {
        tagged_node_user first_tag
    }
}

Or whatever the right configuration there is

@Xe
Copy link
Contributor Author

Xe commented Sep 26, 2022

stick in the mud: what if we only allow nodes with a single tag to be used with this? then it's not ambiguous

@willnorris
Copy link
Member

what if we only allow nodes with a single tag to be used with this?

Well that definitely solves the tag sorting/ordering question.

This still feels round-peg-square-hole enough that worry about this being enabled with exactly these settings by default. How do you feel about tailscale_auth having some minimal additional configuration to enable tagged-node authentication? Maybe not expose any additional options right now... just a boolean to turn it on with the settings you have here?

@Xe
Copy link
Contributor Author

Xe commented Sep 27, 2022

I'd be okay with that yeah

@willnorris
Copy link
Member

The current version doesn't have any configuration, but you can see how it worked in a previous version:

caddy-tailscale/module.go

Lines 102 to 131 in 68b91cc

func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
var ta TailscaleAuth
repl := caddy.NewReplacer()
for h.Next() {
for nesting := h.Nesting(); h.NextBlock(nesting); {
switch h.Val() {
case "auth_key":
if !h.NextArg() {
return nil, h.ArgErr()
}
ta.AuthKey = h.Val()
ta.AuthKey = repl.ReplaceAll(ta.AuthKey, "")
case "hostname":
if !h.NextArg() {
return nil, h.ArgErr()
}
ta.Hostname = h.Val()
case "verbose":
ta.Verbose = true
}
}
}
return caddyauth.Authentication{
ProvidersRaw: caddy.ModuleMap{
"tailscale": caddyconfig.JSON(ta, nil),
},
}, nil
}

Another thought I had in the direction of letting users configure this how they want on their own would be to use the map directive, particularly now that caddyserver/caddy#5081 has merged. So, if we set the tag on the caddy user object, users could map it in if they want to:

tailscale_auth

// use node tag to construct webauth.user if present, otherwise use tailscale_login
map {http.auth.user.tailscale_nodetag} {webauth.user} {
    ~.+ tag___{http.auth.user.tailscale_nodetag}
    default {http.auth.user.tailscale_login}
}

reverse_proxy http://server {
    header_up X-Webauth-User {webauth.user}
}

That's not near as elegant as a simple flag like:

tailscale_auth {
    allow_tagged_nodes
}

But does provide a different way for users to customize how tags map to usernames, using caddy's existing directives. I'm not sure how common this would be likely to be, so don't know how easy we need to make it, versus just making it possible, even if it takes a little extra config.

@mholt
Copy link
Contributor

mholt commented Sep 29, 2022

(In my experience, start minimal and see what users request, then add it for valid use cases.)

Something we sometimes do is to have some directives or sub-directives be shortcuts for expanded Caddyfile config that yes, the user could write themselves, but is much simpler with the shortcut because it is so common. For example, try_files and php_fastcgi just expand to longer Caddyfile config. If you want to do this I can point you to some source code you can use as a starting point.

But you could also just wait and see what users request first 🤷‍♂️

@willnorris
Copy link
Member

(In my experience, start minimal and see what users request, then add it for valid use cases.)

Which are you suggesting would be the minimal option here? Not supporting tagged nodes at all? That's the case today, and we already know it's a model we want to support somehow. Or are you saying suggesting letting users use the map directive like I mentioned above, and add a new option when needed? Or something else?

@mholt
Copy link
Contributor

mholt commented Sep 29, 2022

Or are you saying suggesting letting users use the map directive like I mentioned above, and add a new option when needed?

^ that one

For now. Just to save yourselves some time and see what the real uses cases are to help form the final solution.

I mean, if you can foresee it or want it yourself go ahead -- it sounds really convenient -- I'm just telling you my usual approach to adding config surface area. 👍

@mholt
Copy link
Contributor

mholt commented Feb 24, 2023

Hey @Xe and @willnorris -- just wanted to follow up. How can I help with this?

We'd love to be able to use Caddy to have multiple hosts per TS node "just work" -- I think we're close, yeah?

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.

4 participants