sendnrw/netbird
2
0
Fork 0
mirror of https://github.com/netbirdio/netbird.git synced 2026-09-12 17:59:06 +02:00
Code Issues Packages Projects Releases Wiki Activity

[client] Let an unprivileged caller log out a profile with no identity

Browse Source 
The empty-key check sat behind requirePrivilegeForDeregistration, so an
unprivileged logout of an identity-less profile was refused with
PermissionDenied instead of completing as the no-op it is. And it was refused
for most profiles, not a corner case: the gate arms whenever the SSH server is
enabled, and sshServerEnabled reads an absent ServerSSHAllowed as enabled, so
every legacy profile qualifies.

The check now runs first. What the gate protects against is handing this
machine's registered key to another management server; with no key there is
nothing to hand over and nothing to protect.

Reported by CodeRabbit and cubic-dev-ai on PR #7398, both on the same defect.
This commit is contained in:
riccardom
2026-09-02 16:24:00 +02:00
parent cbeda854cf
commit 6790c34b08
2 changed files with 21 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/server/server.go
+12 -9
View File
@@ -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)
client/server/update_settings_gate_test.go
+9
View File
@@ -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")
}