diff --git a/client/server/server.go b/client/server/server.go index a9f3a3c6d..61e234fb9 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -1578,20 +1578,23 @@ func (s *Server) sendLogoutRequestWithConfig(ctx context.Context, config *profil // Privilege gate: deregistering frees this machine's key to be registered // against another management server, which is only restricted while the SSH // server makes that a privilege handover. - if err := requirePrivilegeForDeregistration(ctx, config); err != nil { - return err - } - - // A profile with no identity was never registered — a logout clears the - // keys in place, so logging the same profile out twice lands here — and - // there is nothing to deregister. Reads no longer mint a key on the way - // past, so this is where that case surfaces instead of dialing management - // with a key it has never seen. + // Ahead of the privilege gate on purpose. A profile with no identity was + // never registered — a logout clears the keys in place, so logging the same + // profile out twice lands here — so there is nothing to deregister and + // nothing for the gate to protect: what it guards against is handing this + // machine's registered key to another management server. Behind the gate, + // an unprivileged caller would be refused instead, and for a profile whose + // ServerSSHAllowed is unset that is every caller, since an absent value + // counts as SSH enabled. if config.PrivateKey == "" { log.Infof("profile carries no identity, nothing to deregister") return nil } + if err := requirePrivilegeForDeregistration(ctx, config); err != nil { + return err + } + key, err := wgtypes.ParseKey(config.PrivateKey) if err != nil { return fmt.Errorf("parse private key: %w", err) diff --git a/client/server/update_settings_gate_test.go b/client/server/update_settings_gate_test.go index 812ef3554..f167b593b 100644 --- a/client/server/update_settings_gate_test.go +++ b/client/server/update_settings_gate_test.go @@ -377,4 +377,13 @@ func TestLogout_ProfileWithoutAnIdentityIsANoOp(t *testing.T) { require.NoError(t, err) require.NoError(t, s.sendLogoutRequestWithConfig(privilegedTestCtx(), stored), "logging out an identity-less profile must not fail") + + // And for an unprivileged caller too: the deregistration privilege gate + // guards the handover of a registered key, so with no key there is nothing + // to guard. An unset SSH setting is what arms that gate — sshServerEnabled + // reads an absent value as enabled — so this stands in for every legacy + // profile, where behind the gate the caller would be refused. + stored.ServerSSHAllowed = nil + require.NoError(t, s.sendLogoutRequestWithConfig(userCtx(), stored), + "an unprivileged caller could not log out a profile with nothing to deregister") }