mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-10 23:49:09 +02:00
Read user certificates only from the session of the active profile's owner
This commit is contained in:
@@ -88,8 +88,11 @@ cmd.SysProcAttr = &syscall.SysProcAttr{Token: syscall.Token(token), CreationFlag
|
||||
`CREATE_NO_WINDOW` matters: without it a console window flashes on the user's desktop on
|
||||
every sync.
|
||||
|
||||
Session selection prefers the physical console, then falls back to any active session,
|
||||
so remote desktop and VDI hosts work. `WTSQueryUserToken` needs `SE_TCB_NAME`, which
|
||||
The session asked is one belonging to the account that owns the active profile, matched
|
||||
by SID: the console first, then active remote sessions, then disconnected ones, whose
|
||||
user is still signed in. Session 0 hosts services and is never asked. A profile without
|
||||
an owner asks the console user only. Nobody else who happens to be signed in to a
|
||||
terminal server or VDI host can therefore decide the result. `WTSQueryUserToken` needs `SE_TCB_NAME`, which
|
||||
LocalSystem holds and an ordinary process does not, so a user-run `netbird up` skips the
|
||||
helper and reads the machine store alone.
|
||||
|
||||
@@ -221,11 +224,11 @@ currently open**. Consequences worth designing around:
|
||||
- **Signing out changes the answer.** Posture can flip between compliant and
|
||||
non-compliant across a sign-out, so management should treat "no proof" as its own
|
||||
state rather than as a failed check, or users get disconnected at the sign-in screen.
|
||||
- **One session is asked, not all of them.** macOS asks the console user, so other
|
||||
fast-user-switched accounts are skipped even though their keychains are unlocked.
|
||||
Windows prefers the console and otherwise takes the first active session. If you ever
|
||||
need every signed-in user, both platforms would have to enumerate sessions and ask
|
||||
each one.
|
||||
- **Only the profile owner is asked.** The user certificate belongs to whoever owns the
|
||||
active NetBird profile. macOS asks the console user only when that user owns the
|
||||
profile, so a fast-user-switched account never answers for someone else. Windows asks
|
||||
a session of the owner wherever it is, console or remote, and with no owner recorded
|
||||
only the console user.
|
||||
- **A locked keychain still blocks signing.** A user can be logged in with their
|
||||
keychain locked (locked on sleep, or manually). The helper then needs an unlock prompt
|
||||
and may block, which is why the spawn has a 30s timeout and a failure is reported as
|
||||
|
||||
@@ -23,7 +23,7 @@ const helperTimeout = 30 * time.Second
|
||||
// installs device identities, and reaches the console user's login keychain only by
|
||||
// launching a helper into that user's session. A Mac sitting at the login window
|
||||
// therefore yields device proofs alone.
|
||||
func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, _ Config) []certposture.Proof {
|
||||
func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, cfg Config) []certposture.Proof {
|
||||
challenges := certificateChallenges(checks)
|
||||
if len(challenges) == 0 {
|
||||
logNoChallenges(checks)
|
||||
@@ -38,7 +38,7 @@ func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte,
|
||||
|
||||
proofs := CollectChallenges(ctx, DefaultStore(), challenges, peerKey)
|
||||
|
||||
userProofs, err := collectAsConsoleUser(ctx, challenges, peerKey)
|
||||
userProofs, err := collectAsConsoleUser(ctx, cfg.ProfileOwner, challenges, peerKey)
|
||||
if err != nil {
|
||||
log.Debugf("certificate posture: console user keychain unavailable: %v", err)
|
||||
}
|
||||
@@ -49,11 +49,15 @@ func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte,
|
||||
// user. Dropping to their uid is not enough: keychain access is an XPC call to a
|
||||
// per-session securityd, so the helper has to enter their Mach bootstrap namespace,
|
||||
// which is what launchctl asuser does.
|
||||
func collectAsConsoleUser(ctx context.Context, challenges []*proto.CertificateChallenge, peerKey []byte) ([]certposture.Proof, error) {
|
||||
func collectAsConsoleUser(ctx context.Context, owner string, challenges []*proto.CertificateChallenge, peerKey []byte) ([]certposture.Proof, error) {
|
||||
user, ok := CurrentConsoleUser()
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
if !user.isOwner(owner) {
|
||||
log.Debugf("certificate posture: console user %s does not own the active profile, no user keychain is asked", user.Name)
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
binary, err := os.Executable()
|
||||
if err != nil {
|
||||
|
||||
@@ -22,7 +22,7 @@ const helperTimeout = 30 * time.Second
|
||||
// Intune enrol device certificates, and reaches the signed-in user's store by launching
|
||||
// a helper with that session's token. A machine at the sign-in screen therefore proves
|
||||
// device certificates alone.
|
||||
func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, _ Config) []certposture.Proof {
|
||||
func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, cfg Config) []certposture.Proof {
|
||||
challenges := certificateChallenges(checks)
|
||||
if len(challenges) == 0 {
|
||||
logNoChallenges(checks)
|
||||
@@ -37,7 +37,7 @@ func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte,
|
||||
return proofs
|
||||
}
|
||||
|
||||
userProofs, err := collectAsDesktopUser(ctx, challenges, peerKey)
|
||||
userProofs, err := collectAsDesktopUser(ctx, cfg.ProfileOwner, challenges, peerKey)
|
||||
if err != nil {
|
||||
log.Debugf("certificate posture: user certificate store unavailable: %v", err)
|
||||
}
|
||||
@@ -53,8 +53,8 @@ func helperStore() Store {
|
||||
// collectAsDesktopUser runs the helper inside the interactive session of the signed-in
|
||||
// user. Unlike a keychain on macOS, a Windows service can assume a user identity
|
||||
// directly, so the session token goes straight into the child process.
|
||||
func collectAsDesktopUser(ctx context.Context, challenges []*proto.CertificateChallenge, peerKey []byte) ([]certposture.Proof, error) {
|
||||
user, ok := CurrentDesktopUser()
|
||||
func collectAsDesktopUser(ctx context.Context, owner string, challenges []*proto.CertificateChallenge, peerKey []byte) ([]certposture.Proof, error) {
|
||||
user, ok := CurrentDesktopUser(owner)
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ package certproof
|
||||
import (
|
||||
"bytes"
|
||||
"fmt"
|
||||
"strconv"
|
||||
"sync"
|
||||
|
||||
"github.com/ebitengine/purego"
|
||||
@@ -69,6 +70,13 @@ func (u ConsoleUser) hasDesktop() bool {
|
||||
return u.UID != 0
|
||||
}
|
||||
|
||||
// isOwner reports whether the console user is owner, the account of the active profile,
|
||||
// which is recorded as a short user name or, for an account without one, a numeric uid.
|
||||
// With no owner the console user counts, as macOS has a single console user.
|
||||
func (u ConsoleUser) isOwner(owner string) bool {
|
||||
return owner == "" || owner == u.Name || owner == strconv.FormatUint(uint64(u.UID), 10)
|
||||
}
|
||||
|
||||
func cfString(str uintptr) string {
|
||||
buf := make([]byte, consoleNameBufSize)
|
||||
if !cfStringGetCString(str, &buf[0], len(buf), encodingUTF8) {
|
||||
|
||||
@@ -39,3 +39,13 @@ func TestCurrentConsoleUser_AgreesWithItself(t *testing.T) {
|
||||
assert.NotZero(t, user.UID, "a desktop session never belongs to uid 0")
|
||||
assert.True(t, user.hasDesktop(), "a reported console user must be a desktop session")
|
||||
}
|
||||
|
||||
func TestConsoleUser_IsOwner(t *testing.T) {
|
||||
user := ConsoleUser{Name: "maycon", UID: 501, GID: 20}
|
||||
|
||||
assert.True(t, user.isOwner(""), "a profile without owner accepts the single console user")
|
||||
assert.True(t, user.isOwner("maycon"), "the owner by short name")
|
||||
assert.True(t, user.isOwner("501"), "the owner recorded as a numeric uid")
|
||||
assert.False(t, user.isOwner("viktor"), "another account's profile must not read this user's keychain")
|
||||
assert.False(t, user.isOwner("502"), "another uid is another account")
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ package certproof
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"unsafe"
|
||||
|
||||
log "github.com/sirupsen/logrus"
|
||||
@@ -10,11 +11,14 @@ import (
|
||||
|
||||
const (
|
||||
noActiveSession = 0xFFFFFFFF
|
||||
servicesSession = 0
|
||||
|
||||
// wtsCurrentServer is WTS_CURRENT_SERVER_HANDLE and wtsActive is WTSActive of
|
||||
// WTS_CONNECTSTATE_CLASS. Neither is exported by x/sys/windows.
|
||||
// wtsCurrentServer is WTS_CURRENT_SERVER_HANDLE; wtsActive and wtsDisconnected are
|
||||
// WTSActive and WTSDisconnected of WTS_CONNECTSTATE_CLASS. None is exported by
|
||||
// x/sys/windows.
|
||||
wtsCurrentServer = windows.Handle(0)
|
||||
wtsActive = 0
|
||||
wtsDisconnected = 4
|
||||
)
|
||||
|
||||
// DesktopUser is an interactive session and the account signed into it. The user's
|
||||
@@ -33,37 +37,91 @@ func (u DesktopUser) Close() {
|
||||
}
|
||||
}
|
||||
|
||||
// CurrentDesktopUser returns a token for the interactive user whose certificate store
|
||||
// should be asked. The physical console comes first, and an active remote desktop
|
||||
// session is used when nobody is at the console, which is how servers and VDI hosts are
|
||||
// normally reached. The second return is false at the sign-in screen, where no
|
||||
// interactive session exists and only machine certificates can be proven.
|
||||
// CurrentDesktopUser returns a token for the session whose certificate store should be
|
||||
// asked: a session of owner, the account the active profile belongs to, with the
|
||||
// physical console preferred over remote sessions. With no owner only the console user
|
||||
// counts. Picking any signed-in user instead would let whoever else is logged in to a
|
||||
// terminal server or VDI host decide the result. The second return is false when no
|
||||
// such session exists, and only machine certificates can then be proven.
|
||||
//
|
||||
// Obtaining the token needs SE_TCB_NAME, which the LocalSystem service has and an
|
||||
// ordinary process does not.
|
||||
func CurrentDesktopUser() (DesktopUser, bool) {
|
||||
if session := windows.WTSGetActiveConsoleSessionId(); session != noActiveSession {
|
||||
if user, ok := desktopUser(session); ok {
|
||||
return user, true
|
||||
}
|
||||
log.Debugf("console session %d has nobody signed in, looking for an active remote session", session)
|
||||
func CurrentDesktopUser(owner string) (DesktopUser, bool) {
|
||||
console := windows.WTSGetActiveConsoleSessionId()
|
||||
if owner == "" {
|
||||
return consoleUser(console)
|
||||
}
|
||||
|
||||
sessions, err := activeSessions()
|
||||
match, err := ownerMatcher(owner)
|
||||
if err != nil {
|
||||
log.Debugf("certificate posture: %v", err)
|
||||
return DesktopUser{}, false
|
||||
}
|
||||
|
||||
sessions, err := userSessions(console)
|
||||
if err != nil {
|
||||
log.Debugf("cannot enumerate terminal sessions: %v", err)
|
||||
return DesktopUser{}, false
|
||||
}
|
||||
for _, session := range sessions {
|
||||
if user, ok := desktopUser(session); ok {
|
||||
user, ok := desktopUser(session)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
if match(user) {
|
||||
return user, true
|
||||
}
|
||||
user.Close()
|
||||
}
|
||||
|
||||
log.Debug("no interactive session is signed in, no user certificate store is reachable")
|
||||
log.Debugf("certificate posture: profile owner %s has no signed-in session, no user certificate store is reachable", owner)
|
||||
return DesktopUser{}, false
|
||||
}
|
||||
|
||||
func consoleUser(console uint32) (DesktopUser, bool) {
|
||||
if console == noActiveSession || console == servicesSession {
|
||||
log.Debug("no console session, no user certificate store is reachable")
|
||||
return DesktopUser{}, false
|
||||
}
|
||||
user, ok := desktopUser(console)
|
||||
if !ok {
|
||||
log.Debugf("console session %d has nobody signed in, no user certificate store is reachable", console)
|
||||
}
|
||||
return user, ok
|
||||
}
|
||||
|
||||
// ownerMatcher reports whether a session user is owner. Accounts are compared by SID,
|
||||
// which is what identifies a Windows account; the name comparison is a fallback for an
|
||||
// owner name that no longer resolves, and is case-insensitive like Windows account names.
|
||||
func ownerMatcher(owner string) (func(DesktopUser) bool, error) {
|
||||
ownerSID, _, _, err := windows.LookupSID("", owner)
|
||||
if err != nil {
|
||||
log.Debugf("certificate posture: resolving profile owner %s: %v, matching by name", owner, err)
|
||||
return func(user DesktopUser) bool { return sameAccountName(user.Name, owner) }, nil
|
||||
}
|
||||
return func(user DesktopUser) bool {
|
||||
tokenUser, err := user.Token.GetTokenUser()
|
||||
if err != nil {
|
||||
log.Debugf("failed reading token user of session %d: %v", user.Session, err)
|
||||
return false
|
||||
}
|
||||
return tokenUser.User.Sid.Equals(ownerSID)
|
||||
}, nil
|
||||
}
|
||||
|
||||
// sameAccountName compares DOMAIN\account names case-insensitively, and an owner given
|
||||
// without a domain against the account part alone.
|
||||
func sameAccountName(sessionName, owner string) bool {
|
||||
if strings.EqualFold(sessionName, owner) {
|
||||
return true
|
||||
}
|
||||
if strings.Contains(owner, `\`) {
|
||||
return false
|
||||
}
|
||||
_, account, found := strings.Cut(sessionName, `\`)
|
||||
return found && strings.EqualFold(account, owner)
|
||||
}
|
||||
|
||||
func desktopUser(session uint32) (DesktopUser, bool) {
|
||||
var token windows.Token
|
||||
if err := windows.WTSQueryUserToken(session, &token); err != nil {
|
||||
@@ -97,7 +155,10 @@ func tokenAccount(token windows.Token) (string, error) {
|
||||
return domain + `\` + account, nil
|
||||
}
|
||||
|
||||
func activeSessions() ([]uint32, error) {
|
||||
// userSessions lists the sessions a user can be signed in to, console first, then
|
||||
// active remote sessions, then disconnected ones, whose user is still signed in. Session
|
||||
// 0 is skipped: it hosts services and never belongs to an interactive user.
|
||||
func userSessions(console uint32) ([]uint32, error) {
|
||||
var info *windows.WTS_SESSION_INFO
|
||||
var count uint32
|
||||
if err := windows.WTSEnumerateSessions(wtsCurrentServer, 0, 1, &info, &count); err != nil {
|
||||
@@ -105,13 +166,19 @@ func activeSessions() ([]uint32, error) {
|
||||
}
|
||||
defer windows.WTSFreeMemory(uintptr(unsafe.Pointer(info)))
|
||||
|
||||
var active []uint32
|
||||
for _, session := range unsafe.Slice(info, count) {
|
||||
if session.State == wtsActive {
|
||||
active = append(active, session.SessionID)
|
||||
var sessions []uint32
|
||||
if console != noActiveSession && console != servicesSession {
|
||||
sessions = append(sessions, console)
|
||||
}
|
||||
for _, state := range []uint32{wtsActive, wtsDisconnected} {
|
||||
for _, session := range unsafe.Slice(info, count) {
|
||||
if session.SessionID == servicesSession || session.SessionID == console || session.State != state {
|
||||
continue
|
||||
}
|
||||
sessions = append(sessions, session.SessionID)
|
||||
}
|
||||
}
|
||||
return active, nil
|
||||
return sessions, nil
|
||||
}
|
||||
|
||||
// runningAsLocalSystem reports whether this process is the service. The helper runs as
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
package certproof
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestSameAccountName(t *testing.T) {
|
||||
tests := []struct {
|
||||
session, owner string
|
||||
want bool
|
||||
}{
|
||||
{`CORP\alice`, `CORP\alice`, true},
|
||||
{`CORP\alice`, `corp\ALICE`, true},
|
||||
{`CORP\alice`, `alice`, true},
|
||||
{`CORP\alice`, `OTHER\alice`, false},
|
||||
{`CORP\alice`, `bob`, false},
|
||||
{`alice`, `alice`, true},
|
||||
{`CORP\alice`, `CORP\alic`, false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
assert.Equal(t, tt.want, sameAccountName(tt.session, tt.owner), "session %q, owner %q", tt.session, tt.owner)
|
||||
}
|
||||
}
|
||||
|
||||
// CurrentDesktopUser against the real session manager: an owner no session belongs to
|
||||
// must never yield a session, whoever else is signed in.
|
||||
func TestCurrentDesktopUser_UnknownOwnerHasNoSession(t *testing.T) {
|
||||
user, ok := CurrentDesktopUser(`NO-SUCH-DOMAIN\no-such-user-netbird`)
|
||||
if ok {
|
||||
user.Close()
|
||||
}
|
||||
assert.False(t, ok, "a profile owner without a session must not borrow another user's store")
|
||||
}
|
||||
@@ -47,12 +47,18 @@ type Store interface {
|
||||
Candidates(ctx context.Context) ([]Candidate, error)
|
||||
}
|
||||
|
||||
// Config selects where the Linux daemon looks for certificates: Dir is the PEM directory,
|
||||
// Config selects where the daemon looks for certificates. Dir is the Linux PEM directory,
|
||||
// empty for NB_CERT_STORE_DIR or /etc/netbird/certs, and PKCS11 names a token whose keys
|
||||
// sign for certificates on the token or in that directory.
|
||||
//
|
||||
// ProfileOwner is the OS account the active profile belongs to. On macOS and Windows only
|
||||
// that account's certificate store is consulted for user certificates, so on a machine
|
||||
// with several people signed in the result does not depend on who else is logged in.
|
||||
// Empty means the profile has no owner, and only the user at the physical console counts.
|
||||
type Config struct {
|
||||
Dir string
|
||||
PKCS11 PKCS11Config
|
||||
Dir string
|
||||
PKCS11 PKCS11Config
|
||||
ProfileOwner string
|
||||
}
|
||||
|
||||
func (c Config) dir() string {
|
||||
|
||||
@@ -76,6 +76,8 @@ type ConnectClient struct {
|
||||
// netMgr gates every reconnection loop on OS-reported network
|
||||
// availability and sweeps connections on network change.
|
||||
netMgr *netevents.Manager
|
||||
|
||||
profileOwner string
|
||||
}
|
||||
|
||||
// ConnectClientOption configures optional ConnectClient behavior.
|
||||
@@ -86,6 +88,12 @@ func WithNetEvents(events *netevents.Manager) ConnectClientOption {
|
||||
return func(c *ConnectClient) { c.netMgr = events }
|
||||
}
|
||||
|
||||
// WithProfileOwner names the OS account the active profile belongs to, whose own
|
||||
// certificate store answers user certificate posture checks.
|
||||
func WithProfileOwner(username string) ConnectClientOption {
|
||||
return func(c *ConnectClient) { c.profileOwner = username }
|
||||
}
|
||||
|
||||
func NewConnectClient(
|
||||
ctx context.Context,
|
||||
config *profilemanager.Config,
|
||||
@@ -417,6 +425,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan
|
||||
return wrapErr(err)
|
||||
}
|
||||
engineConfig.TempDir = mobileDependency.TempDir
|
||||
engineConfig.CertStore.ProfileOwner = c.profileOwner
|
||||
// Leave StateDir empty when there is no state path so a disk-backed
|
||||
// syncstore falls back to os.TempDir() instead of filepath.Dir("") == ".".
|
||||
if path != "" {
|
||||
|
||||
+15
-1
@@ -2515,9 +2515,23 @@ func (s *Server) checkDisableAdvancedView() *bool {
|
||||
return nil
|
||||
}
|
||||
|
||||
// profileOwnerOption passes the OS account of the active profile to the connect client,
|
||||
// which reads that account's certificate store for user certificate posture checks.
|
||||
func (s *Server) profileOwnerOption() []internal.ConnectClientOption {
|
||||
if s.profileManager == nil {
|
||||
return nil
|
||||
}
|
||||
activeProf, err := s.profileManager.GetActiveProfileState()
|
||||
if err != nil {
|
||||
log.Debugf("no active profile owner for certificate posture: %v", err)
|
||||
return nil
|
||||
}
|
||||
return []internal.ConnectClientOption{internal.WithProfileOwner(activeProf.Username)}
|
||||
}
|
||||
|
||||
func (s *Server) connect(ctx context.Context, config *profilemanager.Config, statusRecorder *peer.Status, runningChan chan struct{}) error {
|
||||
log.Tracef("running client connection")
|
||||
client := internal.NewConnectClient(ctx, config, statusRecorder)
|
||||
client := internal.NewConnectClient(ctx, config, statusRecorder, s.profileOwnerOption()...)
|
||||
client.SetUpdateManager(s.updateManager)
|
||||
client.SetSyncResponsePersistence(s.persistSyncResponse)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user