mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-06 23:11:28 +02:00
[client] Bind the cached SSH JWT to the local caller that obtained it (#7378)
* [client] Bind the cached SSH JWT to the local caller that obtained it Record the identity that obtained the token and return it only to that same identity, comparing the account alone: the group set and the elevation flag describe what a token may do rather than who it belongs to, and the same user may call once elevated and once not. A control channel that carries no caller identity gets a miss on read and stores nothing on write, matching how the other ipcauth consumers fail closed. Clear the entry when the session it speaks for ends: logout, down and profile switch. * [client] Cover the profile-switch path of the SSH JWT cache The cache being correct buys nothing if a handler around it forgets to clear it, and SwitchProfile had no test at all. Point 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, and call SwitchProfile with no request so neither the switch itself nor the profile-list event is involved. * [client] Report the SSH JWT cache in the no-identity startup warning daemonServerOptions already warns once, at startup, about what a control channel with no caller identity gives up. Name the SSH JWT cache there too, on both the TCP and the no-peer-identity-primitive paths. The per-request logs in cachedJWT and WaitJWTToken drop to Debug: the condition is expected and handled on such a channel, the caller simply re-authenticates, and repeating it on every SSH authentication buried the one message that is actionable. * [client] Stop the local-metrics manager leaking out of the profile test localmetrics.NewManager runs a goroutine until its context is done, and the test handed it context.Background(), so the manager outlived the test and stayed in the test binary for every case that followed. * [client] Keep the cached SSH JWT across a down/up cycle Clearing the cache in cleanupConnection also caught Down, which ends the connection and not the session: the peer stays enrolled, `up` reconnects without going back to the IdP, and the token still belongs to the same NetBird identity. With a long cache TTL that cost the owner a fresh device-code flow for nothing, since the owner binding is what keeps the token away from other local accounts. Clear it on the two paths where the session really ends and the next one may belong to a different NetBird user: profile logout when the profile is the active one, and active-profile logout. SwitchProfile already cleared it on its own. * [client] Resolve the merge conflict in the profile-logout cleanup main extracted the inline profile-logout cleanup into cleanupAfterProfileLogout, which this branch had edited in place to clear the SSH JWT cache. Take main's helper and move the clear inside it. The helper returns early when the profile that was deregistered is not the active one, so the cache is still only cleared when the session that owns the token actually ends. * [client] Do not cache an SSH JWT obtained under a session that ended WaitJWTToken polls the IdP with s.mutex released, and that wait can run for as long as the user takes in the browser. A logout or a profile switch in the meantime clears the cache, but the poll then completed and stored its token anyway, so the entry the next session read belonged to the previous one. Give the cache a generation that clear advances. WaitJWTToken takes the generation before the wait and hands it back to store, which keeps the token only while the generation still matches. The two mutexes are distinct, so this was never a data race and the race detector could not have found it: the window is between two separately locked sections. * [client] Make the profile-switch test switch a profile SwitchProfile with a nil request skips switchProfileIfNeeded, so the test only covered the no-op path and would have passed with profile-transition invalidation broken. Create a second profile and name it in the request, then assert the active profile actually moved before checking the cache. Also correct the comment on the Down test: the logout handlers do call cleanupConnection. What changed is that clearing the cache is no longer one of the things cleanupConnection does. * [client] Take the SSH JWT cache generation when the flow is created WaitJWTToken read the generation after validating the device code, but the flow it belongs to is created earlier, in RequestJWTAuth, and SwitchProfile does not reset s.oauthAuthFlow. A profile switch between the two therefore advanced the generation before it was ever read: the guard compared the new session against itself and let the token through, which is the case it exists to stop. Record the generation on the flow when RequestJWTAuth creates it, and read it from there. The whole span from the request to the IdP answering now counts as one session for the cache. * [client] Correct two test comments the clear-on-Down change invalidated Moving the clear out of cleanupConnection left two comments describing the old behaviour: newTestServer said cleanupConnection clears the cache, and the comment above TestJWTCache_ClearDropsTheEntry listed Down among the callers of clear. Neither is true any more. * [client] Read the SSH JWT cache generation before the IdP round trip RequestJWTAuth read the generation where it stored the flow, which is after RequestAuthInfo has talked to the IdP. A logout or a profile switch during that call advanced the generation first, so the flow recorded the new session's value and the later store was accepted: the window moved rather than closed. Read it with the config, under the same s.mutex section. SwitchProfile holds that mutex across its own clear(), so the config and the generation cannot be torn apart by a switch.
This commit is contained in:
@@ -41,13 +41,15 @@ func daemonServerOptions(network string) []grpc.ServerOption {
|
||||
if network == "tcp" {
|
||||
log.Warnf("daemon is listening on TCP (%s): callers carry no verifiable identity over TCP, "+
|
||||
"so privileged operations (SSH root login, SSH auth, enabling the SSH server, management URL changes, "+
|
||||
"deregistration) will be denied. Use a unix socket, or npipe:// on Windows", daemonAddr)
|
||||
"deregistration) will be denied, and the SSH JWT cache is neither filled nor served. "+
|
||||
"Use a unix socket, or npipe:// on Windows", daemonAddr)
|
||||
return nil
|
||||
}
|
||||
|
||||
creds := ipcauth.NewTransportCredentials() //nolint:staticcheck
|
||||
if creds == nil { //nolint:staticcheck // nil only on platforms without a peer-identity primitive
|
||||
log.Warnf("daemon IPC has no peer-identity primitive on %s: privileged operations will be denied", runtime.GOOS)
|
||||
log.Warnf("daemon IPC has no peer-identity primitive on %s: privileged operations will be denied "+
|
||||
"and the SSH JWT cache is neither filled nor served", runtime.GOOS)
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -91,6 +91,19 @@ func (i Identity) IsPrivileged() bool {
|
||||
return slices.Contains(i.Groups, sidAdministrators)
|
||||
}
|
||||
|
||||
// SameUser reports whether two identities are the same local principal. Only
|
||||
// the account is compared: the group set and the elevation flag describe what a
|
||||
// token may do, not who it belongs to. A SID on either side decides the
|
||||
// comparison, so a Windows principal never matches a Unix one on the UID both
|
||||
// happen to leave at zero. The zero Identity carries uid 0, so callers must
|
||||
// establish that both identities are real before the answer means anything.
|
||||
func (i Identity) SameUser(other Identity) bool {
|
||||
if i.SID != "" || other.SID != "" {
|
||||
return i.SID == other.SID
|
||||
}
|
||||
return i.UID == other.UID
|
||||
}
|
||||
|
||||
// String renders the identity for audit logs and denial messages.
|
||||
func (i Identity) String() string {
|
||||
if i.IsWindows() {
|
||||
|
||||
66
client/internal/ipcauth/identity_sameuser_test.go
Normal file
66
client/internal/ipcauth/identity_sameuser_test.go
Normal file
@@ -0,0 +1,66 @@
|
||||
package ipcauth
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestIdentitySameUser(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
a Identity
|
||||
b Identity
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "same uid",
|
||||
a: Identity{UID: 1000, GID: 1000},
|
||||
b: Identity{UID: 1000, GID: 1000},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "same uid, different gid and pid still the same user",
|
||||
a: Identity{UID: 1000, GID: 1000, PID: 11},
|
||||
b: Identity{UID: 1000, GID: 27, PID: 22},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "different uid",
|
||||
a: Identity{UID: 1000},
|
||||
b: Identity{UID: 1001},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "same sid",
|
||||
a: Identity{SID: "S-1-5-21-1-2-3-1001"},
|
||||
b: Identity{SID: "S-1-5-21-1-2-3-1001"},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "same sid, elevation and groups differ",
|
||||
a: Identity{SID: "S-1-5-21-1-2-3-1001", Elevated: true, Groups: []string{sidAdministrators}},
|
||||
b: Identity{SID: "S-1-5-21-1-2-3-1001"},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "different sid",
|
||||
a: Identity{SID: "S-1-5-21-1-2-3-1001"},
|
||||
b: Identity{SID: "S-1-5-21-1-2-3-1002"},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "a windows principal is never a unix one",
|
||||
a: Identity{SID: "S-1-5-18"},
|
||||
b: Identity{UID: 0},
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equal(t, tt.want, tt.a.SameUser(tt.b))
|
||||
assert.Equal(t, tt.want, tt.b.SameUser(tt.a), "SameUser must be symmetric")
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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{}
|
||||
}
|
||||
|
||||
176
client/server/jwt_cache_test.go
Normal file
176
client/server/jwt_cache_test.go
Normal file
@@ -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)
|
||||
}
|
||||
@@ -23,6 +23,7 @@ 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"
|
||||
@@ -150,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
|
||||
}
|
||||
|
||||
@@ -1252,6 +1261,8 @@ 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)
|
||||
}
|
||||
@@ -1422,6 +1433,7 @@ func (s *Server) cleanupAfterProfileLogout(id profilemanager.ID, username string
|
||||
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)
|
||||
}
|
||||
@@ -1450,6 +1462,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)
|
||||
@@ -1781,6 +1794,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,
|
||||
@@ -1790,8 +1817,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 {
|
||||
@@ -1800,7 +1833,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{
|
||||
@@ -1834,6 +1867,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{
|
||||
@@ -1858,6 +1892,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 {
|
||||
@@ -1872,11 +1910,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()
|
||||
|
||||
@@ -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(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
188
client/server/server_jwt_test.go
Normal file
188
client/server/server_jwt_test.go
Normal file
@@ -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")
|
||||
}
|
||||
Reference in New Issue
Block a user