Fix session holder logic for profiles without owner.

This commit is contained in:
Theodor S. Midtlien
2026-10-27 13:21:59 +01:00
parent 4ab791ba23
commit 748bcb072e
3 changed files with 38 additions and 3 deletions
@@ -206,3 +206,32 @@ func TestAuthorizeBlamesAHeldSession(t *testing.T) {
assert.Contains(t, denial.Command, "netbird down",
"taking the connection down is the remedy this refusal points at")
}
// A live session on a profile that has no owner is held by the zero Principal.
// An owner of the target profile must not be taken for its holder as we want
// to fail close.
func TestResolveLevelUnownedLiveSessionFailsClosed(t *testing.T) {
g := gateFor(t, stubState{
target: Target{Path: "/profiles/mine.json", Owned: true},
running: true,
})
_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"Up", &proto.UpRequest{})
require.Error(t, err, "a caller reached a live session nobody owns")
denial, ok := DenialFrom(err)
require.True(t, ok, "the refusal carries no explanation")
assert.Equal(t, ErrorReasonSessionHeld, denial.Reason,
"the caller owns the target, so the refusal is about the live session")
}
// With no session running there is nothing to hold, so the owner of the target
// profile may act on it.
func TestResolveLevelIdleDaemonGrantsSessionHolder(t *testing.T) {
g := gateFor(t, stubState{
target: Target{Path: "/profiles/mine.json", Owned: true},
})
_, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"Up", &proto.UpRequest{})
assert.NoError(t, err)
}
+7 -1
View File
@@ -3113,10 +3113,16 @@ func (s *Server) SessionHolder() (ipcauth.Principal, bool) {
s.mutex.Lock()
defer s.mutex.Unlock()
if !s.clientRunning || len(s.config.Owners) == 0 {
if !s.clientRunning {
return ipcauth.Principal{}, false
}
// A session with no ownership is held by the zero Principal, which matches
// nobody and fail close.
if len(s.config.Owners) == 0 {
return ipcauth.Principal{}, true
}
// The zero Principal matches nobody, so an unparseable owner locks the
// session rather than opening it.
principal, ok := ipcauth.ParsePrincipal(s.config.Owners[0])
+2 -2
View File
@@ -44,8 +44,8 @@ func TestResolveTarget_RefreshesActiveConfigForAnyHandle(t *testing.T) {
// The daemon's copy as it stood before the claim landed on disk.
s.config = &profilemanager.Config{}
s.clientRunning = true
_, running := s.SessionHolder()
require.False(t, running, "fixture is wrong: the stale copy already names an owner")
stale, _ := s.SessionHolder()
require.False(t, stale.Matches(owner), "fixture is wrong: the stale copy already names an owner")
target, err := s.ResolveTarget(owner, tc.handle)
require.NoError(t, err)