fix preauth key logging in as previous user

[kradalby]

Fixes #1885

Signed-off-by: Kristoffer Dalby [email protected]

[ItsShadowCone]

Could you confirm #1885 was a dupe of #1310 I.e. Both are fixed now?

[kradalby]

@ItsShadowCone based on the description it looks like it is, but I find it a bit hard to understand the description, it would be helpful if you test and help me confirm.

[ItsShadowCone]

My observation in #1310 was based on fast user switching and logging in via preauth keys, I.e. The purported use case was a "personal" account for admins via OIDC as well as an "admin" account with network access to the entire network via preauth key and fast user switching to limit attack surface when not performing administrative tasks.

Looking through the code, the old version seemed to see that a node (I.e. "instance of tailscaled") was already registered by matching the old node key and refreshing only expiry date and node key, not user key, thereby leading to issues with any user switching, fast or not.

Now I don't know specifically what the tailscale client does when fast user switching, but I would naively assume the node key stays the same (if there was a new node key, we would never have noticed this issue).

Reading naively into the code this fix would then not fully allow fast user switching since the old node-user key combo is swapped out for the new one, effectively logging out the previous user in the process.

I can test it when I get to it, but what was the logic behind reusing the same node entry instead of registering a new one on every login? In my naive understanding, login should add a new node-user pair, logout should expire it immediately as administrative "revoke this login" should (this is iirc only implemented via cli, If at all).

The last point of consideration is, what are the differences between preauth login and OIDC? From a naive understanding both should only assert user identity while the rest of the "login" follows a common code path, but this is apparently not the case. Would this be a possible goal for one of the next alphas?

Most of what I wrote are assumptions or vague observations, please correct me where I'm wrong 😉