diff --git a/client/internal/ipcauth/authz_gate.go b/client/internal/ipcauth/authz_gate.go index f50671fde..f3173c9e9 100644 --- a/client/internal/ipcauth/authz_gate.go +++ b/client/internal/ipcauth/authz_gate.go @@ -87,6 +87,10 @@ func (g *AuthzGate) resolveLevel(id Identity, target Target) AuthzLevel { if !target.Owned { return AuthzLevelIdentified } + // With ownership off, reaching the profile is also reaching its session. + if ProfileOwnershipDisabled() { + return AuthzLevelSessionHolder + } if holder, running := g.st.SessionHolder(); !running || holder.Matches(id) { return AuthzLevelSessionHolder } diff --git a/client/internal/ipcauth/authz_gate_test.go b/client/internal/ipcauth/authz_gate_test.go index b9f53fbff..c4f65a76a 100644 --- a/client/internal/ipcauth/authz_gate_test.go +++ b/client/internal/ipcauth/authz_gate_test.go @@ -176,6 +176,19 @@ func TestAuthorizeCarriesTheTargetForAPrivilegedCaller(t *testing.T) { assert.Equal(t, "/profiles/abcd1111.json", got) } +// With ownership off, somebody else's live session is not a reason to refuse. +func TestAuthorizeIgnoresAHeldSessionWhenOwnershipDisabled(t *testing.T) { + t.Setenv(EnvDisableProfileOwnership, "true") + g := gateFor(t, stubState{ + target: Target{Path: "/profiles/mine.json", Owned: true}, + running: true, + holder: Principal{Kind: KindUID, Value: "4242"}, + }) + + _, err := g.authorize(transportCtx(unprivUser, nil), servicePath+"Up", &proto.UpRequest{}) + assert.NoError(t, err) +} + func TestAuthorizeBlamesAHeldSession(t *testing.T) { g := gateFor(t, stubState{ target: Target{Path: "/profiles/mine.json", Owned: true}, diff --git a/client/internal/ipcauth/identity.go b/client/internal/ipcauth/identity.go index 97fa517e9..576605108 100644 --- a/client/internal/ipcauth/identity.go +++ b/client/internal/ipcauth/identity.go @@ -37,9 +37,15 @@ const EnvDisableProfileOwnership = "NB_DISABLE_PROFILE_OWNERSHIP" var logProfileOwnershipDisabledOrError sync.Once -// defaultProfileClaimDisabled reports whether the environment turns off -// profile ownership. A ownership check will always return true. -func isProfileOwnershipDisabled() bool { +// ProfileOwnershipDisabled reports whether profile ownership is turned off, so +// every identified caller reaches every profile and controls its session. +// +// Mobile records no owners. The app is the only caller and the platform sandbox +// already isolates one install from another. +func ProfileOwnershipDisabled() bool { + if runtime.GOOS == "android" || runtime.GOOS == "ios" { + return true + } val := os.Getenv(EnvDisableProfileOwnership) if val == "" { return false @@ -310,9 +316,6 @@ func looksLikeSID(v string) bool { // A principal is a config value, not a caller, so it is never converted into an // Identity. func (p Principal) Matches(id Identity) bool { - if isProfileOwnershipDisabled() { - return true - } if !id.Known() { return false } diff --git a/client/internal/profilemanager/migration.go b/client/internal/profilemanager/migration.go index ebe1b94a4..12696d152 100644 --- a/client/internal/profilemanager/migration.go +++ b/client/internal/profilemanager/migration.go @@ -1,3 +1,5 @@ +//go:build !ios && !android + package profilemanager import ( diff --git a/client/internal/profilemanager/migration_mobile.go b/client/internal/profilemanager/migration_mobile.go new file mode 100644 index 000000000..b3ef2c433 --- /dev/null +++ b/client/internal/profilemanager/migration_mobile.go @@ -0,0 +1,23 @@ +//go:build ios || android + +package profilemanager + +import ( + "os/user" + "strconv" + + "github.com/netbirdio/netbird/client/internal/ipcauth" +) + +// MigrateLegacyProfiles is a no-op on mobile +func (s *ServiceManager) MigrateLegacyProfiles() error { + return nil +} + +// PrincipalForUser turns a resolved account into an owner principal. +func PrincipalForUser(u *user.User) (string, bool) { + if uid, err := strconv.ParseUint(u.Uid, 10, 32); err == nil { + return ipcauth.UIDPrincipal(uint32(uid)), true + } + return "", false +} diff --git a/client/internal/profilemanager/migration_test.go b/client/internal/profilemanager/migration_test.go index 597cb291e..baff471a2 100644 --- a/client/internal/profilemanager/migration_test.go +++ b/client/internal/profilemanager/migration_test.go @@ -1,3 +1,5 @@ +//go:build !ios && !android + package profilemanager import ( diff --git a/client/internal/profilemanager/profilemanager.go b/client/internal/profilemanager/profilemanager.go index b3d287a48..7c2be498d 100644 --- a/client/internal/profilemanager/profilemanager.go +++ b/client/internal/profilemanager/profilemanager.go @@ -44,6 +44,9 @@ func (p *Profile) AccessibleBy(id ipcauth.Identity) bool { if !id.Known() { return false } + if ipcauth.ProfileOwnershipDisabled() { + return true + } if ipcauth.IsPrivilegedCaller(id) { return true } diff --git a/client/internal/profilemanager/profilemanager_test.go b/client/internal/profilemanager/profilemanager_test.go index 811a00a6f..d132b77dd 100644 --- a/client/internal/profilemanager/profilemanager_test.go +++ b/client/internal/profilemanager/profilemanager_test.go @@ -7,8 +7,19 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/client/internal/ipcauth" ) +func TestAccessibleByReachesUnownedProfileWhenOwnershipDisabled(t *testing.T) { + t.Setenv(ipcauth.EnvDisableProfileOwnership, "true") + + unowned := Profile{} + caller := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + + assert.True(t, unowned.AccessibleBy(caller), "an unowned profile is reachable with ownership off") +} + func withTempConfigDir(t *testing.T, testFunc func(configDir string)) { t.Helper() tempDir := t.TempDir() diff --git a/client/mobile/profile_manager.go b/client/mobile/profile_manager.go index d0384bce5..1f722b2d7 100644 --- a/client/mobile/profile_manager.go +++ b/client/mobile/profile_manager.go @@ -60,9 +60,8 @@ type Profile struct { type ProfileManager struct { configDir string username string - // identity scopes profile ownership. There is no IPC hop on mobile: the - // manager runs inside the app, so the owner of a profile is the app process - // itself, and the device has a single user anyway. + // identity is this app process, which every profile call is made as. Mobile + // records no owners, see ipcauth.ProfileOwnershipDisabled. identity ipcauth.Identity serviceMgr *profilemanager.ServiceManager mdmLoader *mdm.Loader @@ -167,7 +166,7 @@ func (pm *ProfileManager) AddProfile(displayName string) (*Profile, error) { if err := pm.checkProfilesAllowed(); err != nil { return nil, err } - profile, err := pm.serviceMgr.AddProfile(displayName, &pm.identity) + profile, err := pm.serviceMgr.AddProfile(displayName, nil) if err != nil { return nil, fmt.Errorf("add profile: %w", err) }