mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-23 13:59:09 +02:00
Merge remote-tracking branch 'origin/main' into oauth-flow-fallback
# Conflicts: # client/server/server.go
This commit is contained in:
@@ -6,11 +6,21 @@ import (
|
||||
|
||||
"github.com/awnumar/memguard"
|
||||
log "github.com/sirupsen/logrus"
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/ipcauth"
|
||||
)
|
||||
|
||||
type jwtCache struct {
|
||||
mu sync.RWMutex
|
||||
enclave *memguard.Enclave
|
||||
mu sync.RWMutex
|
||||
enclave *memguard.Enclave
|
||||
owner *ipcauth.Identity
|
||||
|
||||
// generation counts the invalidations. A caller that starts an
|
||||
// authentication takes the generation first and hands it back to store, so
|
||||
// a token obtained under a session that ended while the IdP was being
|
||||
// polled cannot land in the cache the new session is using.
|
||||
generation uint64
|
||||
|
||||
expiresAt time.Time
|
||||
timer *time.Timer
|
||||
maxTokenSize int
|
||||
@@ -22,10 +32,23 @@ func newJWTCache() *jwtCache {
|
||||
}
|
||||
}
|
||||
|
||||
func (c *jwtCache) store(token string, maxAge time.Duration) {
|
||||
func (c *jwtCache) currentGeneration() uint64 {
|
||||
c.mu.RLock()
|
||||
defer c.mu.RUnlock()
|
||||
|
||||
return c.generation
|
||||
}
|
||||
|
||||
// store keeps the token only while generation is still the current one, and
|
||||
// reports whether it did. See the generation field.
|
||||
func (c *jwtCache) store(token string, owner ipcauth.Identity, maxAge time.Duration, generation uint64) bool {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
|
||||
if c.generation != generation {
|
||||
return false
|
||||
}
|
||||
|
||||
c.cleanup()
|
||||
|
||||
if c.timer != nil {
|
||||
@@ -35,6 +58,7 @@ func (c *jwtCache) store(token string, maxAge time.Duration) {
|
||||
tokenBytes := []byte(token)
|
||||
c.enclave = memguard.NewEnclave(tokenBytes)
|
||||
|
||||
c.owner = &owner
|
||||
c.expiresAt = time.Now().Add(maxAge)
|
||||
|
||||
var timer *time.Timer
|
||||
@@ -49,9 +73,12 @@ func (c *jwtCache) store(token string, maxAge time.Duration) {
|
||||
log.Debugf("JWT token cache expired after %v, securely wiped from memory", maxAge)
|
||||
})
|
||||
c.timer = timer
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
func (c *jwtCache) get() (string, bool) {
|
||||
// get returns the cached token to the identity that stored it.
|
||||
func (c *jwtCache) get(caller ipcauth.Identity) (string, bool) {
|
||||
c.mu.RLock()
|
||||
defer c.mu.RUnlock()
|
||||
|
||||
@@ -59,6 +86,11 @@ func (c *jwtCache) get() (string, bool) {
|
||||
return "", false
|
||||
}
|
||||
|
||||
if c.owner == nil || !c.owner.SameUser(caller) {
|
||||
log.Warnf("refusing the cached SSH JWT: caller %s is not the identity that obtained it", caller)
|
||||
return "", false
|
||||
}
|
||||
|
||||
buffer, err := c.enclave.Open()
|
||||
if err != nil {
|
||||
log.Debugf("Failed to open JWT token enclave: %v", err)
|
||||
@@ -70,10 +102,23 @@ func (c *jwtCache) get() (string, bool) {
|
||||
return token, true
|
||||
}
|
||||
|
||||
func (c *jwtCache) clear() {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
|
||||
if c.timer != nil {
|
||||
c.timer.Stop()
|
||||
c.timer = nil
|
||||
}
|
||||
c.cleanup()
|
||||
c.generation++
|
||||
}
|
||||
|
||||
// cleanup destroys the secure enclave, must be called with lock held
|
||||
func (c *jwtCache) cleanup() {
|
||||
if c.enclave != nil {
|
||||
c.enclave = nil
|
||||
}
|
||||
c.owner = nil
|
||||
c.expiresAt = time.Time{}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/ipcauth"
|
||||
)
|
||||
|
||||
const testTTL = time.Minute
|
||||
|
||||
func unixCaller(uid uint32) ipcauth.Identity {
|
||||
return ipcauth.Identity{UID: uid, GID: uid}
|
||||
}
|
||||
|
||||
func windowsCaller(sid string) ipcauth.Identity {
|
||||
return ipcauth.Identity{SID: sid}
|
||||
}
|
||||
|
||||
func TestJWTCache_ServesTheOwner(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
owner := unixCaller(1000)
|
||||
c.store("token-for-1000", owner, testTTL, c.currentGeneration())
|
||||
|
||||
got, found := c.get(owner)
|
||||
|
||||
require.True(t, found, "the identity that stored the token must get it back")
|
||||
assert.Equal(t, "token-for-1000", got)
|
||||
}
|
||||
|
||||
// The disclosure this cache guards against: one local account collecting the
|
||||
// SSH JWT another account's authentication put in the daemon-wide cache.
|
||||
func TestJWTCache_RefusesAnotherLocalUser(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
owner ipcauth.Identity
|
||||
caller ipcauth.Identity
|
||||
}{
|
||||
{"different uid", unixCaller(1000), unixCaller(65534)},
|
||||
{"root is not the owner either", unixCaller(1000), unixCaller(0)},
|
||||
{"different sid", windowsCaller("S-1-5-21-1-2-3-1001"), windowsCaller("S-1-5-21-1-2-3-1002")},
|
||||
{"windows caller against a unix owner", unixCaller(0), windowsCaller("S-1-5-18")},
|
||||
{"unix caller against a windows owner", windowsCaller("S-1-5-18"), unixCaller(0)},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
c.store("victim-token", tt.owner, testTTL, c.currentGeneration())
|
||||
|
||||
got, found := c.get(tt.caller)
|
||||
|
||||
assert.False(t, found, "a caller that is not the owner must get a miss")
|
||||
assert.Empty(t, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestJWTCache_EmptyCacheMatchesNobody(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
|
||||
got, found := c.get(unixCaller(0))
|
||||
|
||||
assert.False(t, found)
|
||||
assert.Empty(t, got)
|
||||
}
|
||||
|
||||
// An entry with no recorded owner must match nobody, root included: an
|
||||
// unidentified caller arrives as the zero Identity, which carries uid 0. This
|
||||
// pins the nil-owner guard rather than the comparison, so it sets up an entry
|
||||
// that exists and then drops its owner.
|
||||
func TestJWTCache_UnownedEntryMatchesNobody(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
c.store("token", unixCaller(1000), testTTL, c.currentGeneration())
|
||||
c.owner = nil
|
||||
|
||||
got, found := c.get(unixCaller(0))
|
||||
|
||||
assert.False(t, found)
|
||||
assert.Empty(t, got)
|
||||
}
|
||||
|
||||
// The same user calling once elevated and once not is still the same user, so
|
||||
// hiding their own token from them would be wrong.
|
||||
func TestJWTCache_ElevationDoesNotChangeTheOwner(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
sid := "S-1-5-21-1-2-3-1001"
|
||||
owner := windowsCaller(sid)
|
||||
owner.Elevated = true
|
||||
c.store("token", owner, testTTL, c.currentGeneration())
|
||||
|
||||
got, found := c.get(windowsCaller(sid))
|
||||
|
||||
require.True(t, found)
|
||||
assert.Equal(t, "token", got)
|
||||
}
|
||||
|
||||
func TestJWTCache_Expiry(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
owner := unixCaller(1000)
|
||||
c.store("token", owner, testTTL, c.currentGeneration())
|
||||
c.expiresAt = time.Now().Add(-time.Second)
|
||||
|
||||
_, found := c.get(owner)
|
||||
|
||||
assert.False(t, found)
|
||||
}
|
||||
|
||||
// Logout and SwitchProfile call clear — Down deliberately does not: the NetBird
|
||||
// session the token speaks for is over, so not even its owner may have it back.
|
||||
func TestJWTCache_ClearDropsTheEntry(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
owner := unixCaller(1000)
|
||||
c.store("token", owner, testTTL, c.currentGeneration())
|
||||
|
||||
c.clear()
|
||||
|
||||
_, found := c.get(owner)
|
||||
assert.False(t, found)
|
||||
assert.Nil(t, c.owner, "clear must forget the owner too")
|
||||
assert.Nil(t, c.timer, "clear must stop the expiry timer")
|
||||
}
|
||||
|
||||
// WaitJWTToken polls the IdP unlocked, so a logout or a profile switch can
|
||||
// clear the cache while a flow is still in the air. The token that flow returns
|
||||
// belongs to the session that ended, so it must not land in the cache the new
|
||||
// session is using.
|
||||
func TestJWTCache_StoreFromAnEndedSessionIsDropped(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
owner := unixCaller(1000)
|
||||
|
||||
// The generation a caller takes when its authentication starts.
|
||||
generation := c.currentGeneration()
|
||||
|
||||
c.clear() // logout or profile switch, while the IdP is still being polled
|
||||
|
||||
stored := c.store("stale-token", owner, testTTL, generation)
|
||||
|
||||
assert.False(t, stored, "a token from an ended session must not be cached")
|
||||
_, found := c.get(owner)
|
||||
assert.False(t, found, "the cache must stay empty after the session ended")
|
||||
}
|
||||
|
||||
// The same caller must still be able to store once it re-reads the generation, so
|
||||
// the guard does not wedge the cache after any invalidation.
|
||||
func TestJWTCache_StoreWorksAgainAfterClear(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
owner := unixCaller(1000)
|
||||
|
||||
c.clear()
|
||||
|
||||
require.True(t, c.store("token", owner, testTTL, c.currentGeneration()))
|
||||
|
||||
got, found := c.get(owner)
|
||||
require.True(t, found)
|
||||
assert.Equal(t, "token", got)
|
||||
}
|
||||
|
||||
func TestJWTCache_StoreReplacesThePreviousOwner(t *testing.T) {
|
||||
c := newJWTCache()
|
||||
first := unixCaller(1000)
|
||||
second := unixCaller(1001)
|
||||
|
||||
c.store("first-token", first, testTTL, c.currentGeneration())
|
||||
c.store("second-token", second, testTTL, c.currentGeneration())
|
||||
|
||||
_, found := c.get(first)
|
||||
assert.False(t, found, "the previous owner must not reach the new token")
|
||||
|
||||
got, found := c.get(second)
|
||||
require.True(t, found)
|
||||
assert.Equal(t, "second-token", got)
|
||||
}
|
||||
@@ -0,0 +1,200 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"context"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/grpc/codes"
|
||||
gstatus "google.golang.org/grpc/status"
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal"
|
||||
"github.com/netbirdio/netbird/client/internal/profilemanager"
|
||||
"github.com/netbirdio/netbird/client/proto"
|
||||
)
|
||||
|
||||
// unreachableManagementURL keeps a test that is expected to stop at a gate from
|
||||
// reaching the network if the gate ever regresses: the profiles a logout must
|
||||
// not touch point here, so a leak fails fast instead of contacting a real
|
||||
// management server.
|
||||
const unreachableManagementURL = "https://127.0.0.1:9"
|
||||
|
||||
// enableSSHOnProfile rewrites the profile config at cfgPath with the SSH server
|
||||
// enabled. Deregistering an SSH-enabled profile is a privileged change, so an
|
||||
// unprivileged caller is refused by requirePrivilegeForDeregistration before any
|
||||
// management connection is attempted, which is what keeps these tests offline.
|
||||
func enableSSHOnProfile(t *testing.T, cfgPath string) {
|
||||
t.Helper()
|
||||
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
|
||||
ConfigPath: cfgPath,
|
||||
ManagementURL: "https://api.netbird.io:443",
|
||||
ServerSSHAllowed: boolPtr(true),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
// Logging out of the profile the daemon is already running is a deregistration,
|
||||
// not profile management, so the profiles-disabled kill switch must not block
|
||||
// it. The desktop UI always addresses logout by profile (both the profile menu
|
||||
// and the session-expiration dialog), so gating it left users with
|
||||
// disableProfiles enforced unable to log out at all.
|
||||
func TestLogout_ActiveProfileAllowedWhenProfilesDisabled(t *testing.T) {
|
||||
s, _, activeProfile, username, cfgPath := setupServerWithProfile(t)
|
||||
s.rootCtx = internal.CtxInitState(context.Background())
|
||||
enableSSHOnProfile(t, cfgPath)
|
||||
|
||||
s.profilesDisabled = true
|
||||
|
||||
_, err := s.Logout(userCtx(), &proto.LogoutRequest{
|
||||
ProfileName: &activeProfile,
|
||||
Username: &username,
|
||||
})
|
||||
|
||||
require.Error(t, err, "the SSH privilege gate is expected to refuse this unprivileged caller")
|
||||
require.Equal(t, codes.PermissionDenied, gstatus.Code(err),
|
||||
"logout of the active profile must reach the deregistration path, not be refused as profile management: %v", err)
|
||||
require.NotContains(t, gstatus.Convert(err).Message(), errProfilesDisabled)
|
||||
}
|
||||
|
||||
// A profile-addressed logout that targets some *other* profile does manage
|
||||
// profiles, so it stays gated: with profiles disabled the daemon must not
|
||||
// deregister a peer the user is not currently running.
|
||||
func TestLogout_OtherProfileStaysGatedWhenProfilesDisabled(t *testing.T) {
|
||||
s, _, _, username, _ := setupServerWithProfile(t)
|
||||
s.rootCtx = internal.CtxInitState(context.Background())
|
||||
|
||||
other := "other-profile"
|
||||
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
|
||||
ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, other+".json"),
|
||||
ManagementURL: unreachableManagementURL,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
s.profilesDisabled = true
|
||||
|
||||
_, err = s.Logout(userCtx(), &proto.LogoutRequest{
|
||||
ProfileName: &other,
|
||||
Username: &username,
|
||||
})
|
||||
|
||||
require.Error(t, err)
|
||||
require.Equal(t, codes.Unavailable, gstatus.Code(err), "want the profiles-disabled refusal, got %v", err)
|
||||
require.Contains(t, gstatus.Convert(err).Message(), errProfilesDisabled)
|
||||
}
|
||||
|
||||
// A legacy profile ID is a display name, so two users can hold the same ID in
|
||||
// their own profile directories. Matching on the ID alone would let one user's
|
||||
// logout pass the gate against the other user's active profile, so the username
|
||||
// is part of the comparison.
|
||||
func TestLogout_ForeignUserProfileStaysGatedWhenProfilesDisabled(t *testing.T) {
|
||||
s, _, _, username, _ := setupServerWithProfile(t)
|
||||
s.rootCtx = internal.CtxInitState(context.Background())
|
||||
|
||||
// A legacy-style profile whose ID is its filename stem, and an active state
|
||||
// claiming that same ID for a different user.
|
||||
shared := "shared-legacy-name"
|
||||
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
|
||||
ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, shared+".json"),
|
||||
ManagementURL: unreachableManagementURL,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{
|
||||
ID: profilemanager.ID(shared),
|
||||
Username: "someone-else",
|
||||
}))
|
||||
|
||||
s.profilesDisabled = true
|
||||
|
||||
_, err = s.Logout(userCtx(), &proto.LogoutRequest{
|
||||
ProfileName: &shared,
|
||||
Username: &username,
|
||||
})
|
||||
|
||||
require.Error(t, err)
|
||||
require.Equal(t, codes.Unavailable, gstatus.Code(err),
|
||||
"another user's profile must not pass the gate on an ID match alone: %v", err)
|
||||
}
|
||||
|
||||
// Deregistering a namesake profile must not go out with the running config.
|
||||
// logoutFromProfile reuses the connected client's config when the target is the
|
||||
// active profile, and on an ID-only match a shared legacy ID made it reuse it
|
||||
// for another user's profile, deregistering the active peer instead.
|
||||
func TestLogout_ForeignUserProfileDoesNotUseTheRunningConfig(t *testing.T) {
|
||||
s, _, _, username, cfgPath := setupServerWithProfile(t)
|
||||
s.rootCtx = internal.CtxInitState(context.Background())
|
||||
|
||||
// The running config has the SSH server enabled, so reusing it would be
|
||||
// refused with PermissionDenied. The namesake profile does not, so the
|
||||
// correct path gets as far as dialing its own unreachable management URL.
|
||||
enableSSHOnProfile(t, cfgPath)
|
||||
running, err := profilemanager.GetConfig(cfgPath)
|
||||
require.NoError(t, err)
|
||||
s.config = running
|
||||
s.connectClient = newDummyConnectClient(context.Background())
|
||||
|
||||
shared := "shared-legacy-name"
|
||||
_, err = profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
|
||||
ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, shared+".json"),
|
||||
ManagementURL: unreachableManagementURL,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{
|
||||
ID: profilemanager.ID(shared),
|
||||
Username: "someone-else",
|
||||
}))
|
||||
|
||||
// Bounded so the deregistration the fixed path attempts fails on the dial
|
||||
// rather than sitting in gRPC backoff for the whole test timeout.
|
||||
ctx, cancel := context.WithTimeout(userCtx(), 2*time.Second)
|
||||
t.Cleanup(cancel)
|
||||
|
||||
_, err = s.Logout(ctx, &proto.LogoutRequest{
|
||||
ProfileName: &shared,
|
||||
Username: &username,
|
||||
})
|
||||
|
||||
require.Error(t, err)
|
||||
require.NotEqual(t, codes.PermissionDenied, gstatus.Code(err),
|
||||
"the namesake profile was deregistered with the running config: %v", err)
|
||||
}
|
||||
|
||||
// The connection teardown follows the profile that is active when the logout
|
||||
// completes, not the one seen before it started: Login switches profiles under
|
||||
// guardedConfigMu, which the logout path does not hold, so a login that landed
|
||||
// meanwhile must keep its connection.
|
||||
func TestCleanupAfterProfileLogout_FollowsTheCurrentActiveProfile(t *testing.T) {
|
||||
s, _, activeProfile, username, _ := setupServerWithProfile(t)
|
||||
s.rootCtx = internal.CtxInitState(context.Background())
|
||||
|
||||
state := internal.CtxGetState(s.rootCtx)
|
||||
|
||||
s.cleanupAfterProfileLogout("some-other-profile", username)
|
||||
status, err := state.Status()
|
||||
require.NoError(t, err)
|
||||
require.NotEqual(t, internal.StatusNeedsLogin, status,
|
||||
"logging out of a profile that is not active must not ask for a new login")
|
||||
|
||||
s.cleanupAfterProfileLogout(profilemanager.ID(activeProfile), username)
|
||||
status, err = state.Status()
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, internal.StatusNeedsLogin, status,
|
||||
"logging out of the active profile must ask for a new login")
|
||||
}
|
||||
|
||||
// With profiles enabled the gate is out of the way on both surfaces; the active
|
||||
// profile still reaches the deregistration path.
|
||||
func TestLogout_ActiveProfileAllowedWhenProfilesEnabled(t *testing.T) {
|
||||
s, _, activeProfile, username, cfgPath := setupServerWithProfile(t)
|
||||
s.rootCtx = internal.CtxInitState(context.Background())
|
||||
enableSSHOnProfile(t, cfgPath)
|
||||
|
||||
_, err := s.Logout(userCtx(), &proto.LogoutRequest{
|
||||
ProfileName: &activeProfile,
|
||||
Username: &username,
|
||||
})
|
||||
|
||||
require.Error(t, err, "the SSH privilege gate is expected to refuse this unprivileged caller")
|
||||
require.Equal(t, codes.PermissionDenied, gstatus.Code(err), "want the privilege refusal, got %v", err)
|
||||
}
|
||||
+32
-2
@@ -233,6 +233,24 @@ func conflictString(key, got string) conflictCheck {
|
||||
}
|
||||
}
|
||||
|
||||
// conflictStringPtr is conflictString for optional proto fields, where an
|
||||
// explicit empty value is still a request to change the setting. If p is
|
||||
// nil the field is treated as matching (no override requested); otherwise
|
||||
// the check returns true only when the policy contains the key and its
|
||||
// value equals *p.
|
||||
func conflictStringPtr(key string, p *string) conflictCheck {
|
||||
return conflictCheck{
|
||||
key: key,
|
||||
check: func(pol *mdm.Policy) bool {
|
||||
if p == nil {
|
||||
return true
|
||||
}
|
||||
want, ok := pol.GetString(key)
|
||||
return ok && want == *p
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// conflictInt64 builds a conflictCheck for an integer MDM key. If p is
|
||||
// nil the field is treated as matching; otherwise the check returns
|
||||
// true only when the policy contains the key and its int value equals *p.
|
||||
@@ -297,10 +315,13 @@ func mdmManagedFieldConflicts(msg *proto.SetConfigRequest, policy *mdm.Policy) [
|
||||
conflictBool(mdm.KeyRosenpassPermissive, msg.RosenpassPermissive),
|
||||
conflictBool(mdm.KeyDisableAutoConnect, msg.DisableAutoConnect),
|
||||
conflictBool(mdm.KeyAllowServerSSH, msg.ServerSSHAllowed),
|
||||
conflictBool(mdm.KeyRemoteJobsAllowed, msg.RemoteJobsAllowed),
|
||||
conflictBool(mdm.KeyDisableClientRoutes, msg.DisableClientRoutes),
|
||||
conflictBool(mdm.KeyDisableServerRoutes, msg.DisableServerRoutes),
|
||||
conflictBool(mdm.KeyBlockInbound, msg.BlockInbound),
|
||||
conflictInt64(mdm.KeyWireguardPort, msg.WireguardPort),
|
||||
conflictBool(mdm.KeyEnableLocalMetrics, msg.EnableLocalMetrics),
|
||||
conflictStringPtr(mdm.KeyLocalMetricsAddress, msg.LocalMetricsAddress),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -332,6 +353,7 @@ func setConfigRequestHasConfigOverrides(msg *proto.SetConfigRequest) bool {
|
||||
msg.Mtu != nil ||
|
||||
msg.DisableAutoConnect != nil ||
|
||||
msg.ServerSSHAllowed != nil ||
|
||||
msg.RemoteJobsAllowed != nil ||
|
||||
msg.NetworkMonitor != nil ||
|
||||
msg.DisableClientRoutes != nil ||
|
||||
msg.DisableServerRoutes != nil ||
|
||||
@@ -346,7 +368,9 @@ func setConfigRequestHasConfigOverrides(msg *proto.SetConfigRequest) bool {
|
||||
msg.EnableSSHLocalPortForwarding != nil ||
|
||||
msg.EnableSSHRemotePortForwarding != nil ||
|
||||
msg.DisableSSHAuth != nil ||
|
||||
msg.SshJWTCacheTTL != nil
|
||||
msg.SshJWTCacheTTL != nil ||
|
||||
msg.EnableLocalMetrics != nil ||
|
||||
msg.LocalMetricsAddress != nil
|
||||
}
|
||||
|
||||
// loginRequestHasConfigOverrides reports whether the LoginRequest
|
||||
@@ -370,6 +394,7 @@ func loginRequestHasConfigOverrides(msg *proto.LoginRequest) bool {
|
||||
msg.WireguardPort != nil ||
|
||||
msg.DisableAutoConnect != nil ||
|
||||
msg.ServerSSHAllowed != nil ||
|
||||
msg.RemoteJobsAllowed != nil ||
|
||||
msg.RosenpassPermissive != nil ||
|
||||
len(msg.ExtraIFaceBlacklist) > 0 ||
|
||||
msg.NetworkMonitor != nil ||
|
||||
@@ -381,7 +406,9 @@ func loginRequestHasConfigOverrides(msg *proto.LoginRequest) bool {
|
||||
msg.BlockLanAccess != nil ||
|
||||
msg.DisableNotifications != nil ||
|
||||
len(msg.DnsLabels) > 0 || msg.CleanDNSLabels ||
|
||||
msg.BlockInbound != nil
|
||||
msg.BlockInbound != nil ||
|
||||
msg.EnableLocalMetrics != nil ||
|
||||
msg.LocalMetricsAddress != nil
|
||||
}
|
||||
|
||||
// loginRequestMDMConflicts mirrors mdmManagedFieldConflicts but for the
|
||||
@@ -418,10 +445,13 @@ func loginRequestMDMConflicts(msg *proto.LoginRequest, policy *mdm.Policy) []str
|
||||
conflictBool(mdm.KeyRosenpassPermissive, msg.RosenpassPermissive),
|
||||
conflictBool(mdm.KeyDisableAutoConnect, msg.DisableAutoConnect),
|
||||
conflictBool(mdm.KeyAllowServerSSH, msg.ServerSSHAllowed),
|
||||
conflictBool(mdm.KeyRemoteJobsAllowed, msg.RemoteJobsAllowed),
|
||||
conflictBool(mdm.KeyDisableClientRoutes, msg.DisableClientRoutes),
|
||||
conflictBool(mdm.KeyDisableServerRoutes, msg.DisableServerRoutes),
|
||||
conflictBool(mdm.KeyBlockInbound, msg.BlockInbound),
|
||||
conflictInt64(mdm.KeyWireguardPort, msg.WireguardPort),
|
||||
conflictBool(mdm.KeyEnableLocalMetrics, msg.EnableLocalMetrics),
|
||||
conflictStringPtr(mdm.KeyLocalMetricsAddress, msg.LocalMetricsAddress),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -232,4 +232,3 @@ func toNetIDs(routes []string) []route.NetID {
|
||||
}
|
||||
return netIDs
|
||||
}
|
||||
|
||||
|
||||
+186
-68
@@ -23,6 +23,10 @@ import (
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/auth"
|
||||
"github.com/netbirdio/netbird/client/internal/expose"
|
||||
"github.com/netbirdio/netbird/client/internal/ipcauth"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/localmetrics"
|
||||
"github.com/netbirdio/netbird/client/internal/profilemanager"
|
||||
sleephandler "github.com/netbirdio/netbird/client/internal/sleep/handler"
|
||||
"github.com/netbirdio/netbird/client/mdm"
|
||||
@@ -35,6 +39,7 @@ import (
|
||||
"github.com/netbirdio/netbird/client/internal/statemanager"
|
||||
"github.com/netbirdio/netbird/client/internal/updater"
|
||||
"github.com/netbirdio/netbird/client/proto"
|
||||
"github.com/netbirdio/netbird/util"
|
||||
"github.com/netbirdio/netbird/util/capture"
|
||||
"github.com/netbirdio/netbird/version"
|
||||
)
|
||||
@@ -108,6 +113,7 @@ type Server struct {
|
||||
|
||||
statusRecorder *peer.Status
|
||||
sessionWatcher *internal.SessionWatcher
|
||||
localMetrics *localmetrics.Manager
|
||||
|
||||
probeThrottle *probeThrottle
|
||||
persistSyncResponse bool
|
||||
@@ -145,9 +151,17 @@ type Server struct {
|
||||
}
|
||||
|
||||
type oauthAuthFlow struct {
|
||||
expiresAt time.Time
|
||||
flow auth.OAuthFlow
|
||||
info auth.AuthFlowInfo
|
||||
expiresAt time.Time
|
||||
flow auth.OAuthFlow
|
||||
info auth.AuthFlowInfo
|
||||
|
||||
// cacheGeneration is the SSH JWT cache's generation as of the start of the
|
||||
// request that created this flow. The flow outlives a profile switch, so
|
||||
// reading the generation any later — when the IdP has answered, or when the
|
||||
// token finally arrives — would read the new session's one and let the old
|
||||
// session's token into the new session's cache.
|
||||
cacheGeneration uint64
|
||||
|
||||
waitCancel context.CancelFunc
|
||||
}
|
||||
|
||||
@@ -171,9 +185,28 @@ func New(ctx context.Context, logFile string, configFile string, profilesDisable
|
||||
s.sleepHandler = sleephandler.New(agent)
|
||||
s.startSleepDetector()
|
||||
|
||||
s.localMetrics = localmetrics.NewManager(ctx, s.statusRecorder, s.clientMetricsGatherer)
|
||||
|
||||
return s
|
||||
}
|
||||
|
||||
// clientMetricsGatherer returns the Prometheus gatherer of the running
|
||||
// engine's client metrics, or nil when no engine is running.
|
||||
func (s *Server) clientMetricsGatherer() prometheus.Gatherer {
|
||||
s.mutex.Lock()
|
||||
connectClient := s.connectClient
|
||||
s.mutex.Unlock()
|
||||
|
||||
if connectClient == nil {
|
||||
return nil
|
||||
}
|
||||
engine := connectClient.Engine()
|
||||
if engine == nil {
|
||||
return nil
|
||||
}
|
||||
return engine.GetClientMetrics().PrometheusGatherer()
|
||||
}
|
||||
|
||||
func (s *Server) Start() error {
|
||||
s.mutex.Lock()
|
||||
defer s.mutex.Unlock()
|
||||
@@ -254,6 +287,7 @@ func (s *Server) Start() error {
|
||||
|
||||
s.statusRecorder.UpdateManagementAddress(config.ManagementURL.String())
|
||||
s.statusRecorder.UpdateRosenpass(config.RosenpassEnabled, config.RosenpassPermissive)
|
||||
s.localMetrics.Reconcile(config.LocalMetricsEnabled, config.LocalMetricsAddress)
|
||||
|
||||
if s.sessionWatcher == nil {
|
||||
s.sessionWatcher = internal.NewSessionWatcher(s.rootCtx, s.statusRecorder)
|
||||
@@ -477,11 +511,18 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if _, err := profilemanager.UpdateConfig(config); err != nil {
|
||||
updatedConf, err := profilemanager.UpdateConfig(config)
|
||||
if err != nil {
|
||||
log.Errorf("failed to update profile config: %v", err)
|
||||
return nil, fmt.Errorf("failed to update profile config: %w", err)
|
||||
}
|
||||
|
||||
if activeProf, err := s.profileManager.GetActiveProfileState(); err == nil {
|
||||
if activePath, err := activeProf.FilePath(); err == nil && activePath == config.ConfigPath {
|
||||
s.localMetrics.Reconcile(updatedConf.LocalMetricsEnabled, updatedConf.LocalMetricsAddress)
|
||||
}
|
||||
}
|
||||
|
||||
return &proto.SetConfigResponse{}, nil
|
||||
}
|
||||
|
||||
@@ -551,8 +592,11 @@ func (s *Server) setConfigInputFromRequest(msg *proto.SetConfigRequest) (profile
|
||||
|
||||
config.RosenpassEnabled = msg.RosenpassEnabled
|
||||
config.RosenpassPermissive = msg.RosenpassPermissive
|
||||
config.LocalMetricsEnabled = msg.EnableLocalMetrics
|
||||
config.LocalMetricsAddress = msg.LocalMetricsAddress
|
||||
config.DisableAutoConnect = msg.DisableAutoConnect
|
||||
config.ServerSSHAllowed = msg.ServerSSHAllowed
|
||||
config.RemoteJobsAllowed = msg.RemoteJobsAllowed
|
||||
config.NetworkMonitor = msg.NetworkMonitor
|
||||
config.DisableClientRoutes = msg.DisableClientRoutes
|
||||
config.DisableServerRoutes = msg.DisableServerRoutes
|
||||
@@ -618,6 +662,11 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
|
||||
}
|
||||
|
||||
state := internal.CtxGetState(s.rootCtx)
|
||||
status := state.CurrentStatus()
|
||||
if status == internal.StatusConnected {
|
||||
return &proto.LoginResponse{}, nil
|
||||
}
|
||||
|
||||
defer func() {
|
||||
status, err := state.Status()
|
||||
if err != nil || (status != internal.StatusNeedsLogin && status != internal.StatusLoginFailed) {
|
||||
@@ -657,6 +706,8 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
|
||||
s.config = config
|
||||
s.mutex.Unlock()
|
||||
|
||||
s.localMetrics.Reconcile(config.LocalMetricsEnabled, config.LocalMetricsAddress)
|
||||
|
||||
// A probe that errors leaves the login undecided: Management unreachable, a
|
||||
// restart mid-request, an internal error. Those are returned for the caller
|
||||
// to retry, because turning them into an SSO prompt asks the user to solve
|
||||
@@ -675,7 +726,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
|
||||
}
|
||||
|
||||
if msg.SetupKey == "" {
|
||||
return s.ssoLogin(ctx, config, msg)
|
||||
return s.beginSSOLogin(ctx, config, msg)
|
||||
}
|
||||
|
||||
// Setup-key path: we are about to dial Management with the key, so the
|
||||
@@ -691,17 +742,15 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
|
||||
return &proto.LoginResponse{}, nil
|
||||
}
|
||||
|
||||
// ssoLogin starts the browser leg of a login that carries no setup key: it builds the OAuth
|
||||
// flow, reuses the one already in flight when it targets the same client, and returns the
|
||||
// verification URI for the caller to open.
|
||||
func (s *Server) ssoLogin(ctx context.Context, config *profilemanager.Config, msg *proto.LoginRequest) (*proto.LoginResponse, error) {
|
||||
// beginSSOLogin starts the browser leg of a login that carries no setup key and
|
||||
// returns the response that parks the caller on it.
|
||||
func (s *Server) beginSSOLogin(ctx context.Context, config *profilemanager.Config, msg *proto.LoginRequest) (*proto.LoginResponse, error) {
|
||||
state := internal.CtxGetState(s.rootCtx)
|
||||
|
||||
var hint string
|
||||
hint := ""
|
||||
if msg.Hint != nil {
|
||||
hint = *msg.Hint
|
||||
}
|
||||
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint)
|
||||
if err != nil {
|
||||
state.Set(internal.StatusLoginFailed)
|
||||
@@ -713,7 +762,7 @@ func (s *Server) ssoLogin(ctx context.Context, config *profilemanager.Config, ms
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if resp := s.reusableOAuthFlow(ctx, oAuthFlow); resp != nil {
|
||||
if resp := s.pendingOAuthFlowResponse(ctx, oAuthFlow); resp != nil {
|
||||
state.Set(internal.StatusNeedsLogin)
|
||||
return resp, nil
|
||||
}
|
||||
@@ -740,30 +789,32 @@ func (s *Server) ssoLogin(ctx context.Context, config *profilemanager.Config, ms
|
||||
}, nil
|
||||
}
|
||||
|
||||
// reusableOAuthFlow returns the response for a flow that is still in flight for the same client,
|
||||
// or nil when there is none to reuse. A flow too close to expiry is dropped along with the
|
||||
// execution waiting on it, so the caller starts a fresh one.
|
||||
func (s *Server) reusableOAuthFlow(ctx context.Context, oAuthFlow auth.OAuthFlow) *proto.LoginResponse {
|
||||
// pendingOAuthFlowResponse returns the in-flight flow's response when it
|
||||
// targets the same IdP client and has enough time left for the user to finish
|
||||
// the browser leg, so a second login joins the pending flow instead of opening
|
||||
// a competing one. A flow too close to expiry has its waiter cancelled and nil
|
||||
// returned, leaving the caller to start a fresh flow.
|
||||
func (s *Server) pendingOAuthFlowResponse(ctx context.Context, oAuthFlow auth.OAuthFlow) *proto.LoginResponse {
|
||||
if s.oauthAuthFlow.flow == nil || s.oauthAuthFlow.flow.GetClientID(ctx) != oAuthFlow.GetClientID(ctx) {
|
||||
return nil
|
||||
}
|
||||
|
||||
if !s.oauthAuthFlow.expiresAt.After(time.Now().Add(90 * time.Second)) {
|
||||
log.Warnf("canceling previous waiting execution")
|
||||
if s.oauthAuthFlow.waitCancel != nil {
|
||||
s.oauthAuthFlow.waitCancel()
|
||||
if s.oauthAuthFlow.expiresAt.After(time.Now().Add(90 * time.Second)) {
|
||||
log.Debugf("using previous oauth flow info")
|
||||
return &proto.LoginResponse{
|
||||
NeedsSSOLogin: true,
|
||||
VerificationURI: s.oauthAuthFlow.info.VerificationURI,
|
||||
VerificationURIComplete: s.oauthAuthFlow.info.VerificationURIComplete,
|
||||
UserCode: s.oauthAuthFlow.info.UserCode,
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
log.Debugf("using previous oauth flow info")
|
||||
|
||||
return &proto.LoginResponse{
|
||||
NeedsSSOLogin: true,
|
||||
VerificationURI: s.oauthAuthFlow.info.VerificationURI,
|
||||
VerificationURIComplete: s.oauthAuthFlow.info.VerificationURIComplete,
|
||||
UserCode: s.oauthAuthFlow.info.UserCode,
|
||||
log.Warnf("canceling previous waiting execution")
|
||||
if s.oauthAuthFlow.waitCancel != nil {
|
||||
s.oauthAuthFlow.waitCancel()
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// WaitSSOLogin validates the supplied userCode against the in-flight OAuth
|
||||
@@ -1035,6 +1086,7 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR
|
||||
|
||||
s.statusRecorder.UpdateManagementAddress(s.config.ManagementURL.String())
|
||||
s.statusRecorder.UpdateRosenpass(s.config.RosenpassEnabled, s.config.RosenpassPermissive)
|
||||
s.localMetrics.Reconcile(s.config.LocalMetricsEnabled, s.config.LocalMetricsAddress)
|
||||
|
||||
s.clientRunning = true
|
||||
s.clientRunningChan = make(chan struct{})
|
||||
@@ -1212,6 +1264,9 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi
|
||||
}
|
||||
|
||||
s.config = config
|
||||
s.localMetrics.Reconcile(config.LocalMetricsEnabled, config.LocalMetricsAddress)
|
||||
|
||||
s.jwtCache.clear()
|
||||
|
||||
if msg != nil && msg.ProfileName != nil {
|
||||
s.publishProfileListChanged(*msg.ProfileName)
|
||||
@@ -1338,11 +1393,16 @@ func (s *Server) handleProfileLogout(ctx context.Context, msg *proto.LogoutReque
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := s.validateProfileOperation(resolved.ID, true); err != nil {
|
||||
activeProf, err := s.profileManager.GetActiveProfileState()
|
||||
if err != nil {
|
||||
return nil, gstatus.Errorf(codes.FailedPrecondition, "failed to get active profile state: %v", err)
|
||||
}
|
||||
|
||||
if err := s.validateProfileLogout(resolved.ID, isActiveProfile(activeProf, resolved.ID, username)); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := s.logoutFromProfile(ctx, resolved); err != nil {
|
||||
if err := s.logoutFromProfile(ctx, resolved, username); err != nil {
|
||||
log.Errorf("failed to logout from profile %s: %v", resolved.ID, err)
|
||||
// A refused deregistration is already a status error carrying the reason
|
||||
// and the command to run; rewrapping it as Internal would flatten both
|
||||
@@ -1353,18 +1413,36 @@ func (s *Server) handleProfileLogout(ctx context.Context, msg *proto.LogoutReque
|
||||
return nil, gstatus.Errorf(codes.Internal, "logout: %v", err)
|
||||
}
|
||||
|
||||
activeProf, _ := s.profileManager.GetActiveProfileState()
|
||||
if activeProf != nil && activeProf.ID == resolved.ID {
|
||||
if err := s.cleanupConnection(); err != nil && !errors.Is(err, ErrServiceNotUp) {
|
||||
log.Errorf("failed to cleanup connection: %v", err)
|
||||
}
|
||||
state := internal.CtxGetState(s.rootCtx)
|
||||
state.Set(internal.StatusNeedsLogin)
|
||||
}
|
||||
s.cleanupAfterProfileLogout(resolved.ID, username)
|
||||
|
||||
return &proto.LogoutResponse{}, nil
|
||||
}
|
||||
|
||||
// cleanupAfterProfileLogout tears the connection down and asks for a new login
|
||||
// when the profile that was just deregistered is the one the daemon is running.
|
||||
// The active profile is read again here rather than reused from the pre-flight
|
||||
// check: Login switches profiles under guardedConfigMu, which this path does not
|
||||
// hold, so a login that landed meanwhile must not have its fresh connection
|
||||
// dropped by a logout that targeted the profile it replaced.
|
||||
func (s *Server) cleanupAfterProfileLogout(id profilemanager.ID, username string) {
|
||||
activeProf, err := s.profileManager.GetActiveProfileState()
|
||||
if err != nil {
|
||||
log.Errorf("failed to get active profile state after logout from profile %s: %v", id, err)
|
||||
return
|
||||
}
|
||||
|
||||
if !isActiveProfile(activeProf, id, username) {
|
||||
return
|
||||
}
|
||||
|
||||
if err := s.cleanupConnection(); err != nil && !errors.Is(err, ErrServiceNotUp) {
|
||||
log.Errorf("failed to cleanup connection: %v", err)
|
||||
}
|
||||
s.jwtCache.clear()
|
||||
state := internal.CtxGetState(s.rootCtx)
|
||||
state.Set(internal.StatusNeedsLogin)
|
||||
}
|
||||
|
||||
func (s *Server) handleActiveProfileLogout(ctx context.Context) (*proto.LogoutResponse, error) {
|
||||
if s.config == nil {
|
||||
activeProf, err := s.profileManager.GetActiveProfileState()
|
||||
@@ -1389,6 +1467,7 @@ func (s *Server) handleActiveProfileLogout(ctx context.Context) (*proto.LogoutRe
|
||||
log.Errorf("failed to cleanup connection: %v", err)
|
||||
return nil, err
|
||||
}
|
||||
s.jwtCache.clear()
|
||||
|
||||
state := internal.CtxGetState(s.rootCtx)
|
||||
state.Set(internal.StatusNeedsLogin)
|
||||
@@ -1416,40 +1495,47 @@ func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*prof
|
||||
return config, configExisted, nil
|
||||
}
|
||||
|
||||
func (s *Server) canRemoveProfile(id profilemanager.ID) error {
|
||||
if id == profilemanager.DefaultProfileName {
|
||||
return fmt.Errorf("remove profile with reserved name: %s", profilemanager.DefaultProfileName)
|
||||
}
|
||||
|
||||
activeProf, err := s.profileManager.GetActiveProfileState()
|
||||
if err == nil && activeProf.ID == id {
|
||||
return fmt.Errorf("remove active profile: %s", id)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *Server) validateProfileOperation(id profilemanager.ID, allowActiveProfile bool) error {
|
||||
if s.checkProfilesDisabled() {
|
||||
return gstatus.Errorf(codes.Unavailable, errProfilesDisabled)
|
||||
}
|
||||
|
||||
// validateProfileLogout gates a profile-addressed logout. Deregistering the
|
||||
// profile the daemon already runs is what a plain `netbird logout` does, so the
|
||||
// profiles-disabled kill switch must not block it. Logging out of any other
|
||||
// profile is profile management and stays gated.
|
||||
func (s *Server) validateProfileLogout(id profilemanager.ID, isActive bool) error {
|
||||
if id == "" {
|
||||
return gstatus.Errorf(codes.InvalidArgument, "profile name must be provided")
|
||||
}
|
||||
|
||||
if !allowActiveProfile {
|
||||
if err := s.canRemoveProfile(id); err != nil {
|
||||
return gstatus.Errorf(codes.InvalidArgument, "%v", err)
|
||||
}
|
||||
if isActive {
|
||||
return nil
|
||||
}
|
||||
|
||||
if s.checkProfilesDisabled() {
|
||||
return gstatus.Errorf(codes.Unavailable, errProfilesDisabled)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *Server) logoutFromProfile(ctx context.Context, profile *profilemanager.Profile) error {
|
||||
// isActiveProfile reports whether id is the profile the daemon runs for
|
||||
// username. The username is part of the comparison because legacy profile IDs
|
||||
// are display names, which two users can both hold; the default profile is
|
||||
// shared by every user and carries no username.
|
||||
func isActiveProfile(activeProf *profilemanager.ActiveProfileState, id profilemanager.ID, username string) bool {
|
||||
if activeProf == nil || activeProf.ID != id {
|
||||
return false
|
||||
}
|
||||
|
||||
return id == profilemanager.DefaultProfileName || activeProf.Username == username
|
||||
}
|
||||
|
||||
// logoutFromProfile deregisters profile, reusing the running config when
|
||||
// profile is the one the daemon is connected with. The username takes part in
|
||||
// that decision for the same reason it does in the logout gate: a legacy
|
||||
// profile ID is a display name two users can share, and sending the running
|
||||
// config for a namesake would deregister the active peer instead of the
|
||||
// requested one.
|
||||
func (s *Server) logoutFromProfile(ctx context.Context, profile *profilemanager.Profile, username string) error {
|
||||
activeProf, err := s.profileManager.GetActiveProfileState()
|
||||
if err == nil && activeProf.ID == profile.ID && s.connectClient != nil {
|
||||
if err == nil && isActiveProfile(activeProf, profile.ID, username) && s.connectClient != nil {
|
||||
return s.sendLogoutRequest(ctx)
|
||||
}
|
||||
|
||||
@@ -1713,6 +1799,20 @@ func (s *Server) getJWTCacheTTL() time.Duration {
|
||||
return ttl
|
||||
}
|
||||
|
||||
// cachedJWT returns the cached SSH JWT to the identity that obtained it, and a
|
||||
// miss on a control channel that carries no caller identity.
|
||||
func (s *Server) cachedJWT(ctx context.Context) (string, bool) {
|
||||
caller, ok := ipcauth.CallerIdentity(ctx)
|
||||
if !ok {
|
||||
// Expected and handled on a control channel with no peer identity: the
|
||||
// caller re-authenticates. daemonServerOptions warns about it once at
|
||||
// startup, so this stays out of the per-request log.
|
||||
log.Debug("not serving the cached SSH JWT: the caller's identity cannot be verified on this control channel")
|
||||
return "", false
|
||||
}
|
||||
return s.jwtCache.get(caller)
|
||||
}
|
||||
|
||||
// RequestJWTAuth initiates JWT authentication flow for SSH
|
||||
func (s *Server) RequestJWTAuth(
|
||||
ctx context.Context,
|
||||
@@ -1722,8 +1822,14 @@ func (s *Server) RequestJWTAuth(
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
|
||||
// The generation is read here, with the config and under the same lock, not
|
||||
// where the flow is stored below: RequestAuthInfo talks to the IdP in
|
||||
// between, and a switch or a logout during that call would otherwise be
|
||||
// read as the generation this flow belongs to. SwitchProfile holds
|
||||
// s.mutex across its own clear(), so the pair cannot be torn.
|
||||
s.mutex.Lock()
|
||||
config := s.config
|
||||
cacheGeneration := s.jwtCache.currentGeneration()
|
||||
s.mutex.Unlock()
|
||||
|
||||
if config == nil {
|
||||
@@ -1732,7 +1838,7 @@ func (s *Server) RequestJWTAuth(
|
||||
|
||||
jwtCacheTTL := s.getJWTCacheTTL()
|
||||
if jwtCacheTTL > 0 {
|
||||
if cachedToken, found := s.jwtCache.get(); found {
|
||||
if cachedToken, found := s.cachedJWT(ctx); found {
|
||||
log.Debugf("JWT token found in cache, returning cached token for SSH authentication")
|
||||
|
||||
return &proto.RequestJWTAuthResponse{
|
||||
@@ -1766,6 +1872,7 @@ func (s *Server) RequestJWTAuth(
|
||||
s.oauthAuthFlow.flow = oAuthFlow
|
||||
s.oauthAuthFlow.info = authInfo
|
||||
s.oauthAuthFlow.expiresAt = time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second)
|
||||
s.oauthAuthFlow.cacheGeneration = cacheGeneration
|
||||
s.mutex.Unlock()
|
||||
|
||||
return &proto.RequestJWTAuthResponse{
|
||||
@@ -1790,6 +1897,10 @@ func (s *Server) WaitJWTToken(
|
||||
s.mutex.Lock()
|
||||
oAuthFlow := s.oauthAuthFlow.flow
|
||||
authInfo := s.oauthAuthFlow.info
|
||||
// Recorded when the flow was created, not read here: the flow survives a
|
||||
// profile switch, and everything from RequestJWTAuth to the IdP answering
|
||||
// has to count as the same session for the cache.
|
||||
generation := s.oauthAuthFlow.cacheGeneration
|
||||
s.mutex.Unlock()
|
||||
|
||||
if oAuthFlow == nil || authInfo.DeviceCode != req.DeviceCode {
|
||||
@@ -1804,11 +1915,17 @@ func (s *Server) WaitJWTToken(
|
||||
token := tokenInfo.GetTokenToUse()
|
||||
|
||||
jwtCacheTTL := s.getJWTCacheTTL()
|
||||
if jwtCacheTTL > 0 {
|
||||
s.jwtCache.store(token, jwtCacheTTL)
|
||||
log.Debugf("JWT token cached for SSH authentication, TTL: %v", jwtCacheTTL)
|
||||
} else {
|
||||
switch caller, ok := ipcauth.CallerIdentity(ctx); {
|
||||
case jwtCacheTTL <= 0:
|
||||
log.Debug("JWT caching disabled, not storing token")
|
||||
case !ok:
|
||||
log.Debug("not caching the SSH JWT: the caller's identity cannot be verified on this control channel")
|
||||
default:
|
||||
if s.jwtCache.store(token, caller, jwtCacheTTL, generation) {
|
||||
log.Debugf("JWT token cached for SSH authentication, TTL: %v", jwtCacheTTL)
|
||||
} else {
|
||||
log.Debug("not caching the SSH JWT: the session it was obtained under ended while the IdP was polled")
|
||||
}
|
||||
}
|
||||
|
||||
s.mutex.Lock()
|
||||
@@ -2128,6 +2245,7 @@ func (s *Server) GetConfig(ctx context.Context, req *proto.GetConfigRequest) (*p
|
||||
Mtu: int64(cfg.MTU),
|
||||
DisableAutoConnect: cfg.DisableAutoConnect,
|
||||
ServerSSHAllowed: *cfg.ServerSSHAllowed,
|
||||
RemoteJobsAllowed: util.ReturnBoolWithDefaultFalse(cfg.RemoteJobsAllowed),
|
||||
RosenpassEnabled: cfg.RosenpassEnabled,
|
||||
RosenpassPermissive: cfg.RosenpassPermissive,
|
||||
BlockInbound: cfg.BlockInbound,
|
||||
@@ -2218,7 +2336,7 @@ func (s *Server) RemoveProfile(ctx context.Context, msg *proto.RemoveProfileRequ
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if err := s.logoutFromProfile(ctx, resolved); err != nil {
|
||||
if err := s.logoutFromProfile(ctx, resolved, msg.Username); err != nil {
|
||||
// Deregistration is best-effort here: the local profile is removed
|
||||
// either way, so an unprivileged caller leaves the peer registered on
|
||||
// the management server rather than being blocked from removing it.
|
||||
|
||||
@@ -18,6 +18,10 @@ func newTestServer() *Server {
|
||||
return &Server{
|
||||
rootCtx: context.Background(),
|
||||
statusRecorder: peer.NewRecorder(""),
|
||||
// New always populates the SSH JWT cache and the logout and
|
||||
// profile-switch paths call into it unconditionally, so a Server
|
||||
// assembled field by field has to populate it too.
|
||||
jwtCache: newJWTCache(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/auth"
|
||||
"github.com/netbirdio/netbird/client/internal/localmetrics"
|
||||
"github.com/netbirdio/netbird/client/internal/profilemanager"
|
||||
"github.com/netbirdio/netbird/client/proto"
|
||||
)
|
||||
|
||||
// These cover the RPC side of the cache: the cache itself is exercised in
|
||||
// jwt_cache_test.go, but a correct cache buys nothing if the handlers around it
|
||||
// consult the wrong identity or forget to clear it.
|
||||
|
||||
func TestCachedJWT_ServesTheOwner(t *testing.T) {
|
||||
s := newTestServer()
|
||||
owner := unprivilegedIdentity()
|
||||
s.jwtCache.store("token", owner, testTTL, s.jwtCache.currentGeneration())
|
||||
|
||||
got, found := s.cachedJWT(ctxWithIdentity(owner))
|
||||
|
||||
require.True(t, found, "the identity that obtained the token must get it back")
|
||||
assert.Equal(t, "token", got)
|
||||
}
|
||||
|
||||
func TestCachedJWT_RefusesAnotherCaller(t *testing.T) {
|
||||
s := newTestServer()
|
||||
s.jwtCache.store("token", unprivilegedIdentity(), testTTL, s.jwtCache.currentGeneration())
|
||||
|
||||
got, found := s.cachedJWT(ctxWithIdentity(privilegedIdentity()))
|
||||
|
||||
assert.False(t, found, "a caller that did not obtain the token must get a miss")
|
||||
assert.Empty(t, got)
|
||||
}
|
||||
|
||||
// A control channel that carries no caller identity — a TCP daemon socket, or a
|
||||
// platform with no peer-credential primitive — cannot tell one local user from
|
||||
// another, so cachedJWT must fail closed there.
|
||||
func TestCachedJWT_WithoutCallerIdentity(t *testing.T) {
|
||||
s := newTestServer()
|
||||
s.jwtCache.store("token", unprivilegedIdentity(), testTTL, s.jwtCache.currentGeneration())
|
||||
|
||||
got, found := s.cachedJWT(context.Background())
|
||||
|
||||
assert.False(t, found)
|
||||
assert.Empty(t, got)
|
||||
}
|
||||
|
||||
// profileFixture points the profile globals at a temp dir holding a single
|
||||
// default profile, which is the one ActiveProfileState.FilePath resolves
|
||||
// without consulting the current OS user.
|
||||
func profileFixture(t *testing.T) string {
|
||||
t.Helper()
|
||||
|
||||
dir := t.TempDir()
|
||||
defaultConfig := filepath.Join(dir, "default.json")
|
||||
require.NoError(t, os.WriteFile(defaultConfig, []byte("{}"), 0o600))
|
||||
|
||||
origDir := profilemanager.DefaultConfigPathDir
|
||||
origDefault := profilemanager.DefaultConfigPath
|
||||
origState := profilemanager.ActiveProfileStatePath
|
||||
origOverride := profilemanager.ConfigDirOverride
|
||||
|
||||
profilemanager.DefaultConfigPathDir = dir
|
||||
profilemanager.DefaultConfigPath = defaultConfig
|
||||
profilemanager.ActiveProfileStatePath = filepath.Join(dir, "active_profile.json")
|
||||
profilemanager.ConfigDirOverride = dir
|
||||
|
||||
t.Cleanup(func() {
|
||||
profilemanager.DefaultConfigPathDir = origDir
|
||||
profilemanager.DefaultConfigPath = origDefault
|
||||
profilemanager.ActiveProfileStatePath = origState
|
||||
profilemanager.ConfigDirOverride = origOverride
|
||||
})
|
||||
|
||||
return defaultConfig
|
||||
}
|
||||
|
||||
// A profile carries its own NetBird account, so a token obtained under the
|
||||
// previous one must not survive the switch even for the local user who
|
||||
// obtained it.
|
||||
func TestSwitchProfile_ClearsJWTCache(t *testing.T) {
|
||||
defaultConfig := profileFixture(t)
|
||||
|
||||
// localmetrics.NewManager runs until its context is done, so the manager
|
||||
// must not outlive the test.
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
|
||||
s := newTestServer()
|
||||
s.profileManager = profilemanager.NewServiceManager(defaultConfig)
|
||||
s.localMetrics = localmetrics.NewManager(ctx, s.statusRecorder, nil)
|
||||
|
||||
// A second profile to move to, so the request goes through
|
||||
// switchProfileIfNeeded rather than the no-op path a nil request takes.
|
||||
const target = "second"
|
||||
username := "tester"
|
||||
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
|
||||
ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, target+".json"),
|
||||
ManagementURL: "https://api.netbird.io:443",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
owner := unprivilegedIdentity()
|
||||
s.jwtCache.store("token", owner, testTTL, s.jwtCache.currentGeneration())
|
||||
|
||||
name := target
|
||||
_, err = s.SwitchProfile(ctx, &proto.SwitchProfileRequest{ProfileName: &name, Username: &username})
|
||||
require.NoError(t, err)
|
||||
|
||||
active, err := s.profileManager.GetActiveProfileState()
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, profilemanager.ID(target), active.ID, "the profile must actually have changed")
|
||||
|
||||
_, found := s.jwtCache.get(owner)
|
||||
assert.False(t, found, "switching profile must drop the cached SSH JWT")
|
||||
}
|
||||
|
||||
// Down ends the connection, not the session: the peer stays enrolled and the
|
||||
// token still belongs to the same NetBird identity, so `down` followed by `up`
|
||||
// must not cost the owner a fresh device-code flow.
|
||||
//
|
||||
// The logout handlers do call cleanupConnection, and SwitchProfile does not;
|
||||
// what they have in common is that each clears the cache itself, right after,
|
||||
// so tearing the connection down is no longer what decides the token's fate.
|
||||
func TestCleanupConnection_KeepsJWTCache(t *testing.T) {
|
||||
s := newTestServer()
|
||||
_, cancel := context.WithCancel(context.Background())
|
||||
s.actCancel = cancel
|
||||
|
||||
owner := unprivilegedIdentity()
|
||||
s.jwtCache.store("token", owner, testTTL, s.jwtCache.currentGeneration())
|
||||
|
||||
require.NoError(t, s.cleanupConnection())
|
||||
|
||||
got, found := s.jwtCache.get(owner)
|
||||
require.True(t, found, "going down must not drop the cached SSH JWT")
|
||||
assert.Equal(t, "token", got)
|
||||
}
|
||||
|
||||
// fakeOAuthFlow stands in for the IdP round trip so a test can drive
|
||||
// WaitJWTToken without a real device-code flow.
|
||||
type fakeOAuthFlow struct {
|
||||
token string
|
||||
}
|
||||
|
||||
func (f *fakeOAuthFlow) RequestAuthInfo(context.Context) (auth.AuthFlowInfo, error) {
|
||||
return auth.AuthFlowInfo{DeviceCode: "device-code"}, nil
|
||||
}
|
||||
|
||||
func (f *fakeOAuthFlow) WaitToken(context.Context, auth.AuthFlowInfo) (auth.TokenInfo, error) {
|
||||
return auth.TokenInfo{AccessToken: f.token}, nil
|
||||
}
|
||||
|
||||
func (f *fakeOAuthFlow) GetClientID(context.Context) string { return "client-id" }
|
||||
|
||||
// The flow outlives a profile switch, because SwitchProfile does not reset
|
||||
// s.oauthAuthFlow. A switch between RequestJWTAuth and the IdP answering must
|
||||
// still keep the token out of the cache the new profile uses, and the
|
||||
// generation the flow carries is what decides it: reading the cache's own
|
||||
// generation at store time would already be the new one.
|
||||
func TestWaitJWTToken_DropsTokenFromASessionThatEndedBeforeTheWait(t *testing.T) {
|
||||
s := newTestServer()
|
||||
owner := unprivilegedIdentity()
|
||||
ttl := int(testTTL.Seconds())
|
||||
s.config = &profilemanager.Config{SSHJWTCacheTTL: &ttl}
|
||||
|
||||
// RequestJWTAuth ran under the previous session and recorded its generation.
|
||||
s.oauthAuthFlow.flow = &fakeOAuthFlow{token: "token-from-the-old-session"}
|
||||
s.oauthAuthFlow.info = auth.AuthFlowInfo{DeviceCode: "device-code"}
|
||||
s.oauthAuthFlow.cacheGeneration = s.jwtCache.currentGeneration()
|
||||
|
||||
// A profile switch or a logout lands before the caller reaches WaitJWTToken.
|
||||
s.jwtCache.clear()
|
||||
|
||||
_, err := s.WaitJWTToken(ctxWithIdentity(owner), &proto.WaitJWTTokenRequest{DeviceCode: "device-code"})
|
||||
require.NoError(t, err)
|
||||
|
||||
_, found := s.jwtCache.get(owner)
|
||||
assert.False(t, found, "a token whose flow started under the previous session must not be cached")
|
||||
}
|
||||
@@ -200,7 +200,7 @@ func startManagement(t *testing.T, signalAddr string, counter *int) (*grpc.Serve
|
||||
|
||||
requestBuffer := server.NewAccountRequestBuffer(context.Background(), store)
|
||||
peersUpdateManager := update_channel.NewPeersUpdateManager(metrics)
|
||||
networkMapController := controller.NewController(context.Background(), store, metrics, peersUpdateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock(), manager.NewEphemeralManager(store, peersManager), config)
|
||||
networkMapController := controller.NewController(context.Background(), store, metrics, peersUpdateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock(), manager.NewEphemeralManager(store, peersManager), config, nil)
|
||||
accountManager, err := server.BuildManager(context.Background(), config, store, networkMapController, jobManager, nil, "", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock, false, cacheStore)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
|
||||
@@ -136,6 +136,51 @@ func TestSetConfig_MDMReject_MultipleFields(t *testing.T) {
|
||||
}, v.GetFields())
|
||||
}
|
||||
|
||||
func TestSetConfig_MDMReject_LocalMetrics(t *testing.T) {
|
||||
withMDMPolicy(t, mdm.NewPolicy(map[string]any{
|
||||
mdm.KeyEnableLocalMetrics: true,
|
||||
mdm.KeyLocalMetricsAddress: "127.0.0.1:9191",
|
||||
}))
|
||||
|
||||
s, ctx, profName, username, _ := setupServerWithProfile(t)
|
||||
|
||||
enabled := false
|
||||
addr := "0.0.0.0:9999"
|
||||
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
|
||||
ProfileName: profName,
|
||||
Username: username,
|
||||
EnableLocalMetrics: &enabled,
|
||||
LocalMetricsAddress: &addr,
|
||||
})
|
||||
|
||||
v := extractViolation(t, err)
|
||||
assert.ElementsMatch(t, []string{
|
||||
mdm.KeyEnableLocalMetrics,
|
||||
mdm.KeyLocalMetricsAddress,
|
||||
}, v.GetFields())
|
||||
}
|
||||
|
||||
// An explicitly empty address still changes the effective listen address
|
||||
// (the manager falls back to the default), so presence must be honored
|
||||
// rather than collapsed to "field not set".
|
||||
func TestSetConfig_MDMReject_LocalMetricsEmptyAddress(t *testing.T) {
|
||||
withMDMPolicy(t, mdm.NewPolicy(map[string]any{
|
||||
mdm.KeyLocalMetricsAddress: "127.0.0.1:9999",
|
||||
}))
|
||||
|
||||
s, ctx, profName, username, _ := setupServerWithProfile(t)
|
||||
|
||||
addr := ""
|
||||
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
|
||||
ProfileName: profName,
|
||||
Username: username,
|
||||
LocalMetricsAddress: &addr,
|
||||
})
|
||||
|
||||
v := extractViolation(t, err)
|
||||
assert.ElementsMatch(t, []string{mdm.KeyLocalMetricsAddress}, v.GetFields())
|
||||
}
|
||||
|
||||
func TestSetConfig_MDMReject_AllOrNothing(t *testing.T) {
|
||||
// MDM enforces ManagementURL only; user request touches both the
|
||||
// enforced field AND a non-enforced field (RosenpassEnabled).
|
||||
|
||||
@@ -61,6 +61,7 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) {
|
||||
rosenpassEnabled := true
|
||||
rosenpassPermissive := true
|
||||
serverSSHAllowed := true
|
||||
remoteJobsAllowed := true
|
||||
interfaceName := "utun100"
|
||||
wireguardPort := int64(51820)
|
||||
preSharedKey := "test-psk"
|
||||
@@ -76,6 +77,8 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) {
|
||||
disableIPv6 := true
|
||||
mtu := int64(1280)
|
||||
sshJWTCacheTTL := int32(300)
|
||||
enableLocalMetrics := true
|
||||
localMetricsAddress := "127.0.0.1:9292"
|
||||
|
||||
req := &proto.SetConfigRequest{
|
||||
ProfileName: profName,
|
||||
@@ -85,6 +88,7 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) {
|
||||
RosenpassEnabled: &rosenpassEnabled,
|
||||
RosenpassPermissive: &rosenpassPermissive,
|
||||
ServerSSHAllowed: &serverSSHAllowed,
|
||||
RemoteJobsAllowed: &remoteJobsAllowed,
|
||||
InterfaceName: &interfaceName,
|
||||
WireguardPort: &wireguardPort,
|
||||
OptionalPreSharedKey: &preSharedKey,
|
||||
@@ -107,6 +111,8 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) {
|
||||
DnsRouteInterval: durationpb.New(2 * time.Minute),
|
||||
Mtu: &mtu,
|
||||
SshJWTCacheTTL: &sshJWTCacheTTL,
|
||||
EnableLocalMetrics: &enableLocalMetrics,
|
||||
LocalMetricsAddress: &localMetricsAddress,
|
||||
}
|
||||
|
||||
_, err = s.SetConfig(ctx, req)
|
||||
@@ -128,6 +134,8 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) {
|
||||
require.Equal(t, rosenpassPermissive, cfg.RosenpassPermissive)
|
||||
require.NotNil(t, cfg.ServerSSHAllowed)
|
||||
require.Equal(t, serverSSHAllowed, *cfg.ServerSSHAllowed)
|
||||
require.NotNil(t, cfg.RemoteJobsAllowed)
|
||||
require.Equal(t, remoteJobsAllowed, *cfg.RemoteJobsAllowed)
|
||||
require.Equal(t, interfaceName, cfg.WgIface)
|
||||
require.Equal(t, int(wireguardPort), cfg.WgPort)
|
||||
require.Equal(t, preSharedKey, cfg.PreSharedKey)
|
||||
@@ -153,6 +161,8 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) {
|
||||
require.Equal(t, uint16(mtu), cfg.MTU)
|
||||
require.NotNil(t, cfg.SSHJWTCacheTTL)
|
||||
require.Equal(t, int(sshJWTCacheTTL), *cfg.SSHJWTCacheTTL)
|
||||
require.Equal(t, enableLocalMetrics, cfg.LocalMetricsEnabled)
|
||||
require.Equal(t, localMetricsAddress, cfg.LocalMetricsAddress)
|
||||
|
||||
verifyAllFieldsCovered(t, req)
|
||||
}
|
||||
@@ -180,6 +190,7 @@ func verifyAllFieldsCovered(t *testing.T, req *proto.SetConfigRequest) {
|
||||
"RosenpassEnabled": true,
|
||||
"RosenpassPermissive": true,
|
||||
"ServerSSHAllowed": true,
|
||||
"RemoteJobsAllowed": true,
|
||||
"InterfaceName": true,
|
||||
"WireguardPort": true,
|
||||
"OptionalPreSharedKey": true,
|
||||
@@ -205,6 +216,8 @@ func verifyAllFieldsCovered(t *testing.T, req *proto.SetConfigRequest) {
|
||||
"EnableSSHRemotePortForwarding": true,
|
||||
"DisableSSHAuth": true,
|
||||
"SshJWTCacheTTL": true,
|
||||
"EnableLocalMetrics": true,
|
||||
"LocalMetricsAddress": true,
|
||||
}
|
||||
|
||||
val := reflect.ValueOf(req).Elem()
|
||||
@@ -240,6 +253,7 @@ func TestCLIFlags_MappedToSetConfig(t *testing.T) {
|
||||
"enable-rosenpass": "RosenpassEnabled",
|
||||
"rosenpass-permissive": "RosenpassPermissive",
|
||||
"allow-server-ssh": "ServerSSHAllowed",
|
||||
"allow-remote-jobs": "RemoteJobsAllowed",
|
||||
"interface-name": "InterfaceName",
|
||||
"wireguard-port": "WireguardPort",
|
||||
"preshared-key": "OptionalPreSharedKey",
|
||||
@@ -264,6 +278,8 @@ func TestCLIFlags_MappedToSetConfig(t *testing.T) {
|
||||
"enable-ssh-remote-port-forwarding": "EnableSSHRemotePortForwarding",
|
||||
"disable-ssh-auth": "DisableSSHAuth",
|
||||
"ssh-jwt-cache-ttl": "SshJWTCacheTTL",
|
||||
"enable-local-metrics": "EnableLocalMetrics",
|
||||
"local-metrics-address": "LocalMetricsAddress",
|
||||
}
|
||||
|
||||
// SetConfigRequest fields that don't have CLI flags (settable only via UI or other means).
|
||||
|
||||
+81
-12
@@ -14,6 +14,7 @@ import (
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/daemonaddr"
|
||||
"github.com/netbirdio/netbird/client/internal/ipcauth"
|
||||
"github.com/netbirdio/netbird/client/internal/localmetrics"
|
||||
"github.com/netbirdio/netbird/client/internal/profilemanager"
|
||||
"github.com/netbirdio/netbird/client/proto"
|
||||
"github.com/netbirdio/netbird/util"
|
||||
@@ -30,6 +31,8 @@ import (
|
||||
// management identity hands SSH authorization decisions, including which
|
||||
// keys and users are accepted, to whoever controls that identity. Changing
|
||||
// the management URL and deregistering the peer are both ways to do that.
|
||||
// - Binding the local metrics endpoint to a non-loopback address publishes
|
||||
// peer names and connectivity state to the network without authentication.
|
||||
//
|
||||
// Everything else stays unauthenticated, so this is not an authorization model:
|
||||
// it only refuses the changes that would let a local user become root. A caller
|
||||
@@ -39,27 +42,36 @@ import (
|
||||
// user-to-root boundary. Fields are nil or empty when the request leaves them
|
||||
// untouched.
|
||||
type privilegedConfigChange struct {
|
||||
managementURL string
|
||||
serverSSHAllowed *bool
|
||||
enableSSHRoot *bool
|
||||
disableSSHAuth *bool
|
||||
managementURL string
|
||||
serverSSHAllowed *bool
|
||||
remoteJobsAllowed *bool
|
||||
enableSSHRoot *bool
|
||||
disableSSHAuth *bool
|
||||
enableLocalMetrics *bool
|
||||
localMetricsAddress *string
|
||||
}
|
||||
|
||||
func privilegedChangeFromSetConfig(msg *proto.SetConfigRequest) privilegedConfigChange {
|
||||
return privilegedConfigChange{
|
||||
managementURL: msg.GetManagementUrl(),
|
||||
serverSSHAllowed: msg.ServerSSHAllowed,
|
||||
enableSSHRoot: msg.EnableSSHRoot,
|
||||
disableSSHAuth: msg.DisableSSHAuth,
|
||||
managementURL: msg.GetManagementUrl(),
|
||||
serverSSHAllowed: msg.ServerSSHAllowed,
|
||||
remoteJobsAllowed: msg.RemoteJobsAllowed,
|
||||
enableSSHRoot: msg.EnableSSHRoot,
|
||||
disableSSHAuth: msg.DisableSSHAuth,
|
||||
enableLocalMetrics: msg.EnableLocalMetrics,
|
||||
localMetricsAddress: msg.LocalMetricsAddress,
|
||||
}
|
||||
}
|
||||
|
||||
func privilegedChangeFromLogin(msg *proto.LoginRequest) privilegedConfigChange {
|
||||
return privilegedConfigChange{
|
||||
managementURL: msg.GetManagementUrl(),
|
||||
serverSSHAllowed: msg.ServerSSHAllowed,
|
||||
enableSSHRoot: msg.EnableSSHRoot,
|
||||
disableSSHAuth: msg.DisableSSHAuth,
|
||||
managementURL: msg.GetManagementUrl(),
|
||||
serverSSHAllowed: msg.ServerSSHAllowed,
|
||||
remoteJobsAllowed: msg.RemoteJobsAllowed,
|
||||
enableSSHRoot: msg.EnableSSHRoot,
|
||||
disableSSHAuth: msg.DisableSSHAuth,
|
||||
enableLocalMetrics: msg.EnableLocalMetrics,
|
||||
localMetricsAddress: msg.LocalMetricsAddress,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,6 +95,21 @@ func requirePrivilegeForConfigChange(ctx context.Context, stored *profilemanager
|
||||
return denyPrivileged(ctx, "enabling the NetBird SSH server", ipcauth.UpCommand("--allow-server-ssh"))
|
||||
}
|
||||
|
||||
// Enabling remote jobs lets the management server run jobs (e.g. debug
|
||||
// bundles) on this host, so turning it on crosses the user-to-root
|
||||
// boundary the same way enabling the SSH server does. The stored value
|
||||
// defaults to off (nil = off), so a legacy config is correctly seen as
|
||||
// off and turning it on requires privilege.
|
||||
if enables(storedFlag(stored, func(c *profilemanager.Config) *bool { return c.RemoteJobsAllowed }), change.remoteJobsAllowed) {
|
||||
return denyPrivileged(ctx, "enabling remote jobs", ipcauth.UpCommand("--allow-remote-jobs"))
|
||||
}
|
||||
|
||||
if addr, exposes := exposesLocalMetrics(stored, change); exposes {
|
||||
return denyPrivileged(ctx,
|
||||
"exposing the local metrics endpoint on a non-loopback address",
|
||||
ipcauth.UpCommand("--enable-local-metrics --local-metrics-address "+addr))
|
||||
}
|
||||
|
||||
// Only guard the management binding while the SSH server is enabled: that is
|
||||
// when the management identity decides who may open a shell here.
|
||||
if !sshServerEnabled(stored) {
|
||||
@@ -245,6 +272,48 @@ func sshServerCurrentlyAllowed(cfg *profilemanager.Config) *bool {
|
||||
return &enabled
|
||||
}
|
||||
|
||||
// exposesLocalMetrics reports whether the change would leave the metrics
|
||||
// endpoint enabled on an address that is not confirmed loopback, and returns
|
||||
// that address. A request that restates the stored state is not a change, so a
|
||||
// settings form resubmitted after an administrator opened the endpoint is not
|
||||
// refused.
|
||||
func exposesLocalMetrics(stored *profilemanager.Config, change privilegedConfigChange) (string, bool) {
|
||||
storedEnabled, storedAddr := storedLocalMetrics(stored)
|
||||
|
||||
enabled := storedEnabled
|
||||
if change.enableLocalMetrics != nil {
|
||||
enabled = *change.enableLocalMetrics
|
||||
}
|
||||
addr := storedAddr
|
||||
if change.localMetricsAddress != nil {
|
||||
addr = metricsAddrOrDefault(*change.localMetricsAddress)
|
||||
}
|
||||
|
||||
if !enabled || localmetrics.IsLoopback(addr) {
|
||||
return "", false
|
||||
}
|
||||
if storedEnabled && storedAddr == addr {
|
||||
return "", false
|
||||
}
|
||||
return addr, true
|
||||
}
|
||||
|
||||
// storedLocalMetrics reads the metrics settings from the stored config,
|
||||
// tolerating a config that does not exist yet.
|
||||
func storedLocalMetrics(cfg *profilemanager.Config) (bool, string) {
|
||||
if cfg == nil {
|
||||
return false, localmetrics.DefaultListenAddress
|
||||
}
|
||||
return cfg.LocalMetricsEnabled, metricsAddrOrDefault(cfg.LocalMetricsAddress)
|
||||
}
|
||||
|
||||
func metricsAddrOrDefault(addr string) string {
|
||||
if addr == "" {
|
||||
return localmetrics.DefaultListenAddress
|
||||
}
|
||||
return addr
|
||||
}
|
||||
|
||||
// sameManagementURL reports whether requested addresses the same management
|
||||
// server as stored, comparing scheme, host and effective port so that an
|
||||
// equivalent spelling ("https://api.netbird.io" for a stored
|
||||
|
||||
@@ -61,6 +61,8 @@ func noIdentityCtx() context.Context { return context.Background() }
|
||||
|
||||
func boolPtr(v bool) *bool { return &v }
|
||||
|
||||
func strPtr(v string) *string { return &v }
|
||||
|
||||
func mustURL(t *testing.T, raw string) *url.URL {
|
||||
t.Helper()
|
||||
u, err := url.Parse(raw)
|
||||
@@ -171,6 +173,34 @@ func TestRequirePrivilegeForConfigChange_SSHFlags(t *testing.T) {
|
||||
stored: &profilemanager.Config{DisableSSHAuth: boolPtr(true)},
|
||||
change: privilegedConfigChange{disableSSHAuth: boolPtr(false)},
|
||||
},
|
||||
{
|
||||
name: "enabling remote jobs unprivileged is refused",
|
||||
stored: &profilemanager.Config{RemoteJobsAllowed: boolPtr(false)},
|
||||
change: privilegedConfigChange{remoteJobsAllowed: boolPtr(true)},
|
||||
wantDeny: true,
|
||||
},
|
||||
{
|
||||
name: "enabling remote jobs as root is allowed",
|
||||
stored: &profilemanager.Config{RemoteJobsAllowed: boolPtr(false)},
|
||||
change: privilegedConfigChange{remoteJobsAllowed: boolPtr(true)},
|
||||
privileged: true,
|
||||
},
|
||||
{
|
||||
name: "a profile with no config yet counts as off, so enabling remote jobs is refused",
|
||||
stored: nil,
|
||||
change: privilegedConfigChange{remoteJobsAllowed: boolPtr(true)},
|
||||
wantDeny: true,
|
||||
},
|
||||
{
|
||||
name: "restating already-enabled remote jobs is not a change",
|
||||
stored: &profilemanager.Config{RemoteJobsAllowed: boolPtr(true)},
|
||||
change: privilegedConfigChange{remoteJobsAllowed: boolPtr(true)},
|
||||
},
|
||||
{
|
||||
name: "turning remote jobs off is not guarded",
|
||||
stored: &profilemanager.Config{RemoteJobsAllowed: boolPtr(true)},
|
||||
change: privilegedConfigChange{remoteJobsAllowed: boolPtr(false)},
|
||||
},
|
||||
{
|
||||
name: "a request that touches none of the guarded fields is allowed",
|
||||
stored: &profilemanager.Config{ServerSSHAllowed: boolPtr(false)},
|
||||
@@ -194,6 +224,102 @@ func TestRequirePrivilegeForConfigChange_SSHFlags(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequirePrivilegeForConfigChange_LocalMetrics(t *testing.T) {
|
||||
exposed := &profilemanager.Config{LocalMetricsEnabled: true, LocalMetricsAddress: "0.0.0.0:9191"}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
stored *profilemanager.Config
|
||||
change privilegedConfigChange
|
||||
privileged bool
|
||||
wantDeny bool
|
||||
}{
|
||||
{
|
||||
name: "binding a non-loopback address unprivileged is refused",
|
||||
stored: &profilemanager.Config{},
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true), localMetricsAddress: strPtr("0.0.0.0:9191")},
|
||||
wantDeny: true,
|
||||
},
|
||||
{
|
||||
name: "binding a non-loopback address as root is allowed",
|
||||
stored: &profilemanager.Config{},
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true), localMetricsAddress: strPtr("0.0.0.0:9191")},
|
||||
privileged: true,
|
||||
},
|
||||
{
|
||||
name: "enabling on the default loopback address is not guarded",
|
||||
stored: &profilemanager.Config{},
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true)},
|
||||
},
|
||||
{
|
||||
name: "enabling on an explicit loopback address is not guarded",
|
||||
stored: &profilemanager.Config{},
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true), localMetricsAddress: strPtr("127.0.0.1:9999")},
|
||||
},
|
||||
{
|
||||
name: "enabling on the IPv6 loopback address is not guarded",
|
||||
stored: &profilemanager.Config{},
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true), localMetricsAddress: strPtr("[::1]:9191")},
|
||||
},
|
||||
{
|
||||
// The address alone does nothing while the endpoint stays off.
|
||||
name: "a non-loopback address without enabling is not guarded",
|
||||
stored: &profilemanager.Config{},
|
||||
change: privilegedConfigChange{localMetricsAddress: strPtr("0.0.0.0:9191")},
|
||||
},
|
||||
{
|
||||
name: "widening an already enabled loopback endpoint is refused",
|
||||
stored: &profilemanager.Config{LocalMetricsEnabled: true, LocalMetricsAddress: "127.0.0.1:9191"},
|
||||
change: privilegedConfigChange{localMetricsAddress: strPtr("0.0.0.0:9191")},
|
||||
wantDeny: true,
|
||||
},
|
||||
{
|
||||
name: "restating an already exposed endpoint is not a change",
|
||||
stored: exposed,
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true), localMetricsAddress: strPtr("0.0.0.0:9191")},
|
||||
},
|
||||
{
|
||||
name: "turning an exposed endpoint off is not guarded",
|
||||
stored: exposed,
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(false)},
|
||||
},
|
||||
{
|
||||
name: "re-enabling an exposed endpoint that was turned off is refused",
|
||||
stored: &profilemanager.Config{LocalMetricsEnabled: false, LocalMetricsAddress: "0.0.0.0:9191"},
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true)},
|
||||
wantDeny: true,
|
||||
},
|
||||
{
|
||||
// Fail closed: an address that cannot be parsed is not confirmed loopback.
|
||||
name: "an unparseable address is refused",
|
||||
stored: &profilemanager.Config{},
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true), localMetricsAddress: strPtr("not-an-address")},
|
||||
wantDeny: true,
|
||||
},
|
||||
{
|
||||
name: "a profile with no config yet counts as off, so exposing is refused",
|
||||
stored: nil,
|
||||
change: privilegedConfigChange{enableLocalMetrics: boolPtr(true), localMetricsAddress: strPtr("0.0.0.0:9191")},
|
||||
wantDeny: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
ctx := userCtx()
|
||||
if tt.privileged {
|
||||
ctx = rootCtx()
|
||||
}
|
||||
err := requirePrivilegeForConfigChange(ctx, tt.stored, tt.change)
|
||||
if tt.wantDeny {
|
||||
assertDenied(t, err)
|
||||
return
|
||||
}
|
||||
assertAllowed(t, err)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequirePrivilegeForConfigChange_ManagementURL(t *testing.T) {
|
||||
sshOn := func(raw string) *profilemanager.Config {
|
||||
return &profilemanager.Config{ServerSSHAllowed: boolPtr(true), ManagementURL: mustURL(t, raw)}
|
||||
|
||||
Reference in New Issue
Block a user