From 6495ad8687339e8b7a43893cd5dbfadd17233ddb Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 5 Aug 2026 10:50:21 +0200 Subject: [PATCH] Ask the operating system for privileges when a guarded SSH setting is changed --- .goreleaser_ui.yaml | 10 + client/internal/daemonaddr/identity.go | 14 + client/internal/daemonaddr/identity_test.go | 27 ++ client/internal/elevate/elevate.go | 74 ++++ client/internal/elevate/run_darwin.go | 355 ++++++++++++++++++ client/internal/elevate/run_darwin_test.go | 141 +++++++ client/internal/elevate/run_unix.go | 112 ++++++ client/internal/elevate/run_unix_test.go | 105 ++++++ client/internal/elevate/run_unsupported.go | 15 + client/internal/elevate/run_windows.go | 193 ++++++++++ client/internal/elevate/trusted.go | 40 ++ .../internal/elevate/trusted_group_darwin.go | 10 + client/internal/elevate/trusted_group_unix.go | 9 + client/internal/elevate/trusted_unix.go | 98 +++++ client/internal/elevate/trusted_unix_test.go | 149 ++++++++ client/internal/elevate/trusted_windows.go | 150 ++++++++ client/internal/ipcauth/privileged.go | 16 + client/ui/build/linux/netbird-ui.desktop | 7 +- client/ui/build/linux/netbird.desktop | 3 +- client/ui/build/linux/nfpm/nfpm.yaml | 5 + .../linux/polkit/io.netbird.settings.policy | 56 +++ .../frontend/src/contexts/SettingsContext.tsx | 86 ++++- client/ui/frontend/src/hooks/usePrivilege.ts | 2 +- .../src/modules/settings/SettingsSSH.tsx | 157 ++++++-- client/ui/i18n/locales/de/common.json | 30 +- client/ui/i18n/locales/en/common.json | 28 +- client/ui/i18n/locales/es/common.json | 30 +- client/ui/i18n/locales/fr/common.json | 30 +- client/ui/i18n/locales/hu/common.json | 30 +- client/ui/i18n/locales/it/common.json | 30 +- client/ui/i18n/locales/ja/common.json | 27 ++ client/ui/i18n/locales/pt/common.json | 30 +- client/ui/i18n/locales/ru/common.json | 30 +- client/ui/i18n/locales/zh-CN/common.json | 30 +- client/ui/main.go | 9 + client/ui/privileged_settings.go | 27 ++ client/ui/services/guarded.go | 233 ++++++++++++ client/ui/services/guarded_test.go | 290 ++++++++++++++ client/ui/services/oneshot.go | 233 ++++++++++++ client/ui/services/oneshot_test.go | 166 ++++++++ client/ui/services/settings.go | 114 +++++- 41 files changed, 3105 insertions(+), 96 deletions(-) create mode 100644 client/internal/daemonaddr/identity.go create mode 100644 client/internal/daemonaddr/identity_test.go create mode 100644 client/internal/elevate/elevate.go create mode 100644 client/internal/elevate/run_darwin.go create mode 100644 client/internal/elevate/run_darwin_test.go create mode 100644 client/internal/elevate/run_unix.go create mode 100644 client/internal/elevate/run_unix_test.go create mode 100644 client/internal/elevate/run_unsupported.go create mode 100644 client/internal/elevate/run_windows.go create mode 100644 client/internal/elevate/trusted.go create mode 100644 client/internal/elevate/trusted_group_darwin.go create mode 100644 client/internal/elevate/trusted_group_unix.go create mode 100644 client/internal/elevate/trusted_unix.go create mode 100644 client/internal/elevate/trusted_unix_test.go create mode 100644 client/internal/elevate/trusted_windows.go create mode 100644 client/ui/build/linux/polkit/io.netbird.settings.policy create mode 100644 client/ui/privileged_settings.go create mode 100644 client/ui/services/guarded.go create mode 100644 client/ui/services/guarded_test.go create mode 100644 client/ui/services/oneshot.go create mode 100644 client/ui/services/oneshot_test.go diff --git a/.goreleaser_ui.yaml b/.goreleaser_ui.yaml index ca5148823..b630f8dd0 100644 --- a/.goreleaser_ui.yaml +++ b/.goreleaser_ui.yaml @@ -92,6 +92,11 @@ nfpms: dst: /usr/share/applications/org.wails.netbird.desktop - src: client/ui/build/appicon.png dst: /usr/share/pixmaps/netbird.png + # Names the polkit action for the elevation prompt the app raises when an + # unprivileged user changes a privileged setting; without it the dialog + # shows a raw command line. + - src: client/ui/build/linux/polkit/io.netbird.settings.policy + dst: /usr/share/polkit-1/actions/io.netbird.settings.policy dependencies: - netbird (>= 0.75.0) - libgtk-4-1 (>= 4.14) @@ -115,6 +120,11 @@ nfpms: dst: /usr/share/applications/org.wails.netbird.desktop - src: client/ui/build/appicon.png dst: /usr/share/pixmaps/netbird.png + # Names the polkit action for the elevation prompt the app raises when an + # unprivileged user changes a privileged setting; without it the dialog + # shows a raw command line. + - src: client/ui/build/linux/polkit/io.netbird.settings.policy + dst: /usr/share/polkit-1/actions/io.netbird.settings.policy dependencies: - netbird >= 0.75.0 - (gtk4 >= 4.14 or libgtk-4-1 >= 4.14) diff --git a/client/internal/daemonaddr/identity.go b/client/internal/daemonaddr/identity.go new file mode 100644 index 000000000..c4a02ee5c --- /dev/null +++ b/client/internal/daemonaddr/identity.go @@ -0,0 +1,14 @@ +package daemonaddr + +import "strings" + +// CarriesIdentity reports whether the control channel at addr conveys the +// connecting process's identity to the daemon. A Unix socket carries peer +// credentials and a named pipe carries the client's token; loopback TCP carries +// neither, so on such an address the daemon cannot authorize a privileged +// operation for anybody. A client uses this to tell whether becoming privileged +// would get it anywhere: on an identity-less address it would not, and the only +// way forward is to move the daemon onto one that carries identity. +func CarriesIdentity(addr string) bool { + return strings.HasPrefix(addr, "unix://") || strings.HasPrefix(addr, pipeScheme) +} diff --git a/client/internal/daemonaddr/identity_test.go b/client/internal/daemonaddr/identity_test.go new file mode 100644 index 000000000..73ba39f3d --- /dev/null +++ b/client/internal/daemonaddr/identity_test.go @@ -0,0 +1,27 @@ +package daemonaddr + +import "testing" + +func TestCarriesIdentity(t *testing.T) { + tests := []struct { + addr string + want bool + }{ + {"unix:///var/run/netbird.sock", true}, + {"unix:///var/run/netbird/default.sock", true}, + {"npipe://netbird", true}, + {`npipe://\\.\pipe\ProtectedPrefix\Administrators\netbird`, true}, + {"tcp://127.0.0.1:41731", false}, + {"tcp://localhost:41731", false}, + {"", false}, + {"/var/run/netbird.sock", false}, + } + + for _, tt := range tests { + t.Run(tt.addr, func(t *testing.T) { + if got := CarriesIdentity(tt.addr); got != tt.want { + t.Errorf("CarriesIdentity(%q) = %v, want %v", tt.addr, got, tt.want) + } + }) + } +} diff --git a/client/internal/elevate/elevate.go b/client/internal/elevate/elevate.go new file mode 100644 index 000000000..aa5a5da78 --- /dev/null +++ b/client/internal/elevate/elevate.go @@ -0,0 +1,74 @@ +// Package elevate re-runs this very executable under the operating system's own +// privilege-elevation mechanism and waits for it to finish. +// +// It exists so that a change the daemon restricts to root/administrator can be +// authorized from the GUI, by the user, at the moment they ask for it: Windows +// shows the UAC consent dialog, macOS the system authentication dialog, and +// Linux/FreeBSD the session's polkit agent. The credentials, where any are +// asked for, are collected by the operating system and never pass through +// NetBird. +// +// What the elevated process then does is the caller's business: it is the same +// binary, in a one-shot mode, and it is authorized by the daemon exactly like +// any other privileged caller, from the identity the kernel reports on the +// control channel. Nothing here grants privilege, and the daemon gains no new +// way to be talked into something: elevation only changes who is calling it. +package elevate + +import ( + "context" + "errors" + + log "github.com/sirupsen/logrus" +) + +// AppliedMarker is what the elevated process prints on standard output once it has +// done what it was run for. +// +// macOS's AuthorizationExecuteWithPrivileges reports no exit status and does not +// say which process it started, so there this line is the only evidence that the +// change was applied. The other platforms have an exit code and ignore it. +const AppliedMarker = "netbird-elevated: applied" + +var ( + // ErrDeclined reports that the user dismissed the prompt or did not + // authenticate. Nothing happened and nothing is wrong: a caller undoes its + // optimistic update and stays quiet. + ErrDeclined = errors.New("authorization declined") + + // ErrUnavailable reports that this host has no elevation mechanism we can + // drive: no polkit on a Unix desktop, or an executable we decline to run as + // root. A caller falls back to telling the user which command to run. + ErrUnavailable = errors.New("no privilege elevation mechanism available") +) + +// Run runs this executable with args under the platform's elevation mechanism +// and waits for it to exit. A non-zero exit is returned as an error, so the +// caller can treat a completed Run as the operation having succeeded. +// +// The args are the caller's own command line, so they cross no privilege +// boundary: only a user who has just authenticated as an administrator can get +// them run at all. +func Run(ctx context.Context, args ...string) error { + self, err := trustedSelf() + if err != nil { + return err + } + return run(ctx, self, args) +} + +// Available reports whether Run has a mechanism to use on this host, so a caller +// can offer the prompt only when there is one and otherwise fall back to +// guidance the user can act on. It answers from what is installed, not from what +// the user is allowed to do: an administrator's password may still be required +// and may still not be given, which is ErrDeclined from Run. +func Available() bool { + if _, err := trustedSelf(); err != nil { + // Worth a line: this is also what a build run from a group-writable + // directory hits, and there is nothing in the UI to say why the offer is + // missing. + log.Debugf("not offering privilege elevation: %v", err) + return false + } + return mechanismAvailable() +} diff --git a/client/internal/elevate/run_darwin.go b/client/internal/elevate/run_darwin.go new file mode 100644 index 000000000..858e90793 --- /dev/null +++ b/client/internal/elevate/run_darwin.go @@ -0,0 +1,355 @@ +package elevate + +import ( + "context" + "fmt" + "os" + "runtime" + "strings" + "sync" + "syscall" + "unsafe" + + "github.com/ebitengine/purego" + log "github.com/sirupsen/logrus" +) + +// Authorization Services, reached through purego rather than cgo so the released +// binaries keep building with CGO_ENABLED=0. +// +// The prompt belongs to this process, which is what makes it carry the +// application's name and our own explanation. Going through osascript instead puts +// the very same trampoline behind a dialog attributed to osascript, and means +// handing a shell a command line to re-parse. +// +// # On AuthorizationExecuteWithPrivileges +// +// It is deprecated, and Apple's guidance (Quinn, "BSD Privilege Escalation on +// macOS", developer.apple.com/forums/thread/708765) is "while it still works, it's +// been deprecated for many years. Do not use it in a widely distributed product." +// It is used here anyway, knowingly, because the alternatives Apple offers are for +// *obtaining* ongoing privileges — an installer package, SMAppService, SMJobBless — +// and NetBird already has what they would install: a launchd daemon running as +// root. What is missing is only a way for an unprivileged client to ask it to act. +// +// The way to that without a deprecated call is to authorize the client instead of +// elevating one: the app takes the right with AuthorizationCreate, passes the +// AuthorizationExternalForm to the daemon, and the daemon checks it with +// AuthorizationCopyRights before acting — none of which is deprecated. It is the +// better design and it is where this should end up. It also means the daemon +// accepting an authorization over its control socket, which is a new way to be +// asked for privileged work and wants reviewing as such, so it is deliberately not +// bundled in with the rest of this. +// +// Until then, three things keep the deprecation from being a trap. Every symbol is +// resolved with an error rather than a panic, so a macOS that has dropped this +// function leaves the app offering the user a command instead of crashing on the +// way to a prompt. A failure to run the tool is reported as ErrUnavailable, so the +// fallback is the same one an agent-less Linux session gets. And the whole path +// runs under guard, which turns a panic out of the FFI layer into that same +// fallback. +// +// One thing that is not optional: the elevated process must be signed with the +// hardened runtime, which is what stops DYLD_INSERT_LIBRARIES in the environment +// the trampoline passes on from loading somebody's library into a root process. The +// released app is signed and notarised, so it is; see also trustedSelf, which +// refuses to elevate an executable others can write. + +const ( + securityFramework = "/System/Library/Frameworks/Security.framework/Security" + libSystem = "/usr/lib/libSystem.B.dylib" + + // trampoline is what the framework hands the tool to. Present on every macOS, + // and worth confirming before offering a prompt rather than mid-prompt. + trampoline = "/usr/libexec/security_authtrampoline" +) + +// rightExecute is the right an administrator holds, and what +// AuthorizationExecuteWithPrivileges requires of us. +const rightExecute = "system.privilege.admin" + +// promptKey is kAuthorizationEnvironmentPrompt, which puts a sentence of ours above +// the system's in the dialog. It is about the change rather than the mechanism. +const ( + promptKey = "prompt" + promptText = "NetBird needs to change a setting that grants SSH access to this computer." +) + +// OSStatus values from SecBase.h that mean something to us; anything else is +// reported as it comes. +const ( + errAuthorizationSuccess = 0 + errAuthorizationDenied = -60005 + errAuthorizationCanceled = -60006 + errAuthorizationInteractionNotAllowed = -60007 + errAuthorizationToolExecuteFailure = -60031 + errAuthorizationToolEnvironmentError = -60032 +) + +// AuthorizationFlags from Authorization.h. +const ( + flagDefaults = 0 + flagInteractionAllowed = 1 << 0 + flagExtendRights = 1 << 1 + flagDestroyRights = 1 << 3 + flagPreAuthorize = 1 << 4 +) + +// authorizationItem mirrors AuthorizationItem: a name, and a value the name gives +// meaning to. 32 bytes on both amd64 and arm64. +type authorizationItem struct { + name *byte + valueLength uintptr + value unsafe.Pointer + // flags is reserved by the API and always zero. Declared because the layout + // is the contract: without it the struct is 24 bytes where C reads 32. + flags uint32 //nolint:unused // part of the C layout +} + +// authorizationItemSet mirrors AuthorizationItemSet, which serves as both an +// AuthorizationRights and an AuthorizationEnvironment. +type authorizationItemSet struct { + count uint32 + items *authorizationItem +} + +var ( + authorizationCreate func(rights, environment *authorizationItemSet, flags uint32, authorization *uintptr) int32 + authorizationExecuteWithPrivileges func(authorization uintptr, pathToTool string, options uint32, arguments *uintptr, communicationsPipe *uintptr) int32 + authorizationFree func(authorization uintptr, flags uint32) int32 + fileno func(stream uintptr) int32 + fclose func(stream uintptr) int32 + + loadOnce sync.Once + loadErr error +) + +// load resolves the functions once. A framework that cannot be opened, or a symbol +// that is no longer there, leaves the host without a mechanism rather than taking +// the process down with it: see the note on deprecation above. +func load() error { + loadOnce.Do(func() { loadErr = guard("loading Security.framework", resolve) }) + return loadErr +} + +// guard turns a panic out of the FFI layer into an error, so an API that has +// changed under us costs the user a prompt rather than the window they were +// clicking in. purego panics on a signature it cannot map, and this is the one +// place in the client that calls a deprecated system function. +// +// It catches Go panics, which is what purego raises. A fault inside the framework +// itself is not a panic and not recoverable; the layout the tests pin down is what +// stands between us and that. +func guard(what string, fn func() error) (err error) { + defer func() { + r := recover() + if r == nil { + return + } + log.Errorf("%s panicked: %v", what, r) + err = fmt.Errorf("%w: %s: %v", ErrUnavailable, what, r) + }() + return fn() +} + +func resolve() error { + security, err := purego.Dlopen(securityFramework, purego.RTLD_LAZY|purego.RTLD_GLOBAL) + if err != nil { + return fmt.Errorf("open %s: %w", securityFramework, err) + } + system, err := purego.Dlopen(libSystem, purego.RTLD_LAZY|purego.RTLD_GLOBAL) + if err != nil { + return fmt.Errorf("open %s: %w", libSystem, err) + } + + // purego.RegisterLibFunc panics on a symbol it cannot find, which is not how a + // deprecated function's disappearance should reach the user. + for _, fn := range []struct { + ptr any + handle uintptr + name string + }{ + {&authorizationCreate, security, "AuthorizationCreate"}, + {&authorizationExecuteWithPrivileges, security, "AuthorizationExecuteWithPrivileges"}, + {&authorizationFree, security, "AuthorizationFree"}, + {&fileno, system, "fileno"}, + {&fclose, system, "fclose"}, + } { + symbol, err := purego.Dlsym(fn.handle, fn.name) + if err != nil { + return fmt.Errorf("resolve %s: %w", fn.name, err) + } + if symbol == 0 { + return fmt.Errorf("resolve %s: not present on this system", fn.name) + } + purego.RegisterFunc(fn.ptr, symbol) + } + return nil +} + +// run asks the system to run self as root: first for the right, which is what puts +// up the authentication dialog and collects the password or takes the Touch ID, +// then for the tool. The credentials go to the system's authorization trampoline +// and never to us. +// +// The context bounds only our own waiting; the dialog belongs to the system and +// closes when the user answers it. +func run(ctx context.Context, self string, args []string) error { + if err := load(); err != nil { + return fmt.Errorf("%w: %v", ErrUnavailable, err) + } + + return guard("asking for privileges", func() error { + authorization, err := authorize() + if err != nil { + return err + } + defer authorizationFree(authorization, flagDestroyRights) + + return execute(ctx, authorization, self, args) + }) +} + +func mechanismAvailable() bool { + if err := load(); err != nil { + return false + } + info, err := os.Stat(trampoline) + return err == nil && !info.IsDir() +} + +// authorize obtains the right, prompting for it. A dismissed dialog comes back as +// errAuthorizationCanceled and a password given up on as errAuthorizationDenied; +// both are the user's answer rather than a failure. +func authorize() (uintptr, error) { + var pinner runtime.Pinner + defer pinner.Unpin() + + rights := itemSet(&pinner, authorizationItem{name: cString(&pinner, rightExecute)}) + environment := itemSet(&pinner, promptItem(&pinner)) + + var authorization uintptr + status := authorizationCreate(rights, environment, + flagDefaults|flagInteractionAllowed|flagPreAuthorize|flagExtendRights, &authorization) + + switch status { + case errAuthorizationSuccess: + return authorization, nil + case errAuthorizationCanceled, errAuthorizationDenied: + return 0, ErrDeclined + case errAuthorizationInteractionNotAllowed: + // Nowhere to put a dialog, so there is nobody to ask: a launch daemon, or + // a session with no window server. + return 0, fmt.Errorf("%w: this session cannot show an authorization prompt", ErrUnavailable) + default: + return 0, fmt.Errorf("request %s: OSStatus %d", rightExecute, status) + } +} + +// execute runs the tool with the right in hand and waits for it by reading the pipe +// it is given until the tool closes it. +// +// AuthorizationExecuteWithPrivileges reports no exit status and does not say what +// process it started, which is why the one-shot says so itself: what it prints is +// the only evidence that the change was applied. +func execute(ctx context.Context, authorization uintptr, self string, args []string) error { + var pinner runtime.Pinner + defer pinner.Unpin() + + argv := make([]uintptr, 0, len(args)+1) + for _, arg := range args { + argv = append(argv, uintptr(unsafe.Pointer(cString(&pinner, arg)))) + } + argv = append(argv, 0) + pinner.Pin(&argv[0]) + + var pipe uintptr + status := authorizationExecuteWithPrivileges(authorization, self, flagDefaults, &argv[0], &pipe) + switch status { + case errAuthorizationSuccess: + case errAuthorizationCanceled: + return ErrDeclined + case errAuthorizationToolExecuteFailure, errAuthorizationToolEnvironmentError: + // The right was granted and the tool still did not start. Nothing the user + // can do about it from here, so point them at the command instead. + return fmt.Errorf("%w: the system would not run %s elevated (OSStatus %d)", ErrUnavailable, self, status) + default: + return fmt.Errorf("run %s elevated: OSStatus %d", self, status) + } + + out, err := readPipe(ctx, pipe) + if err != nil { + return err + } + if !strings.Contains(out, AppliedMarker) { + return fmt.Errorf("elevated netbird did not report the change as applied: %s", firstLine(out)) + } + return nil +} + +// readPipe drains the tool's output, which ends when the tool exits and is +// therefore also how we wait for it. +func readPipe(ctx context.Context, pipe uintptr) (string, error) { + if pipe == 0 { + return "", nil + } + defer fclose(pipe) + + fd := int(fileno(pipe)) + if fd < 0 { + return "", nil + } + + var out strings.Builder + buf := make([]byte, 4096) + for { + if err := ctx.Err(); err != nil { + return out.String(), err + } + n, err := syscall.Read(fd, buf) + if n > 0 { + out.Write(buf[:n]) + } + if n <= 0 || err != nil { + return out.String(), nil + } + } +} + +// itemSet builds an AuthorizationItemSet over items, pinned for the call. +func itemSet(pinner *runtime.Pinner, items ...authorizationItem) *authorizationItemSet { + pinner.Pin(&items[0]) + set := &authorizationItemSet{count: uint32(len(items)), items: &items[0]} + pinner.Pin(set) + return set +} + +// promptItem is the environment entry carrying our sentence for the dialog. +func promptItem(pinner *runtime.Pinner) authorizationItem { + value := []byte(promptText) + pinner.Pin(&value[0]) + return authorizationItem{ + name: cString(pinner, promptKey), + valueLength: uintptr(len(value)), + value: unsafe.Pointer(&value[0]), + } +} + +// cString returns a NUL-terminated copy of s, pinned so the C side may hold it for +// the duration of the call. +func cString(pinner *runtime.Pinner, s string) *byte { + b := append([]byte(s), 0) + pinner.Pin(&b[0]) + return &b[0] +} + +func firstLine(s string) string { + s = strings.TrimSpace(s) + if s == "" { + return "no output" + } + if i := strings.IndexByte(s, '\n'); i >= 0 { + return s[:i] + } + return s +} diff --git a/client/internal/elevate/run_darwin_test.go b/client/internal/elevate/run_darwin_test.go new file mode 100644 index 000000000..5b8cbba2f --- /dev/null +++ b/client/internal/elevate/run_darwin_test.go @@ -0,0 +1,141 @@ +package elevate + +import ( + "errors" + "runtime" + "strings" + "testing" +) + +// The framework has to load and the symbols has to resolve, or nothing else here +// means anything. +func TestSecurityFrameworkLoads(t *testing.T) { + if err := load(); err != nil { + t.Fatalf("load() = %v, want the framework to open", err) + } + for name, fn := range map[string]any{ + "AuthorizationCreate": authorizationCreate, + "AuthorizationExecuteWithPrivileges": authorizationExecuteWithPrivileges, + "AuthorizationFree": authorizationFree, + "fileno": fileno, + "fclose": fclose, + } { + if fn == nil { + t.Errorf("%s did not resolve", name) + } + } +} + +// A request with no interaction allowed exercises the whole call — the rights and +// environment structs, and the OSStatus that comes back — without a dialog anybody +// has to answer. What the system decides is its business; that it decides at all is +// what this asserts. +func TestAuthorizationCreateWithoutInteraction(t *testing.T) { + if err := load(); err != nil { + t.Skipf("Security.framework did not open: %v", err) + } + + var pinner runtime.Pinner + defer pinner.Unpin() + + rights := itemSet(&pinner, authorizationItem{name: cString(&pinner, rightExecute)}) + environment := itemSet(&pinner, promptItem(&pinner)) + + if got := rights.count; got != 1 { + t.Fatalf("rights.count = %d, want 1: the struct layout is wrong", got) + } + + var authorization uintptr + status := authorizationCreate(rights, environment, flagDefaults|flagExtendRights, &authorization) + + switch status { + case errAuthorizationSuccess: + // Credentials were already cached for this session. + authorizationFree(authorization, flagDestroyRights) + case errAuthorizationDenied, errAuthorizationInteractionNotAllowed: + // The expected answers when nobody may be asked. + default: + t.Fatalf("AuthorizationCreate returned OSStatus %d, want a known one", status) + } +} + +// Asking with a right nobody has must not be mistaken for a declined prompt: the +// caller would report nothing at all. +func TestAuthorizeUnknownRightIsNotDeclined(t *testing.T) { + if err := load(); err != nil { + t.Skipf("Security.framework did not open: %v", err) + } + + var pinner runtime.Pinner + defer pinner.Unpin() + + rights := itemSet(&pinner, authorizationItem{name: cString(&pinner, "io.netbird.right.that.does.not.exist")}) + + var authorization uintptr + status := authorizationCreate(rights, nil, flagDefaults|flagExtendRights, &authorization) + if status == errAuthorizationSuccess { + authorizationFree(authorization, flagDestroyRights) + t.Fatal("a right that does not exist was granted") + } +} + +func TestMechanismAvailable(t *testing.T) { + if !mechanismAvailable() { + t.Error("mechanismAvailable() = false on macOS, where the trampoline always exists") + } +} + +// The one-shot's report is what stands in for an exit status here, so a run that +// says nothing must not read as success. +func TestExecuteRequiresTheAppliedMarker(t *testing.T) { + if !strings.Contains(AppliedMarker, "netbird") { + t.Errorf("AppliedMarker = %q, want something the one-shot would not print by accident", AppliedMarker) + } +} + +// A panic out of the FFI layer has to reach the caller as "no mechanism", which is +// the outcome that offers the user the command instead of taking the window down. +func TestGuardTurnsAPanicIntoUnavailable(t *testing.T) { + err := guard("pretending to call something", func() error { + panic("purego: signature it cannot map") + }) + + if !errors.Is(err, ErrUnavailable) { + t.Fatalf("guard() = %v, want it to be ErrUnavailable", err) + } + if !strings.Contains(err.Error(), "pretending to call something") { + t.Errorf("guard() = %v, want it to name what panicked", err) + } +} + +func TestGuardPassesErrorsThrough(t *testing.T) { + sentinel := errors.New("the call itself failed") + if err := guard("calling", func() error { return sentinel }); !errors.Is(err, sentinel) { + t.Errorf("guard() = %v, want the error it was given", err) + } + if err := guard("calling", func() error { return nil }); err != nil { + t.Errorf("guard() = %v, want nil", err) + } +} + +func TestFirstLine(t *testing.T) { + tests := []struct{ in, want string }{ + {in: "", want: "no output"}, + {in: " \n ", want: "no output"}, + {in: "one line", want: "one line"}, + {in: "first\nsecond", want: "first"}, + } + for _, tt := range tests { + if got := firstLine(tt.in); got != tt.want { + t.Errorf("firstLine(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} + +// Declined has to stay distinguishable after wrapping, which is what the callers +// switch on. +func TestErrDeclinedSurvivesWrapping(t *testing.T) { + if !errors.Is(errors.Join(ErrDeclined), ErrDeclined) { + t.Error("ErrDeclined does not match itself through errors.Is") + } +} diff --git a/client/internal/elevate/run_unix.go b/client/internal/elevate/run_unix.go new file mode 100644 index 000000000..6ffb245cc --- /dev/null +++ b/client/internal/elevate/run_unix.go @@ -0,0 +1,112 @@ +//go:build linux || freebsd + +package elevate + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "os/exec" + "strings" +) + +// pkexec exit codes that are about the authorization rather than about the program +// we asked it to run. The manual page reserves both. +const ( + // exitDismissed is returned when the user dismissed the authentication + // dialog. + exitDismissed = 126 + // exitNotAuthorized is returned when the authorization was not obtained. That + // covers the user saying no as well as pkexec having had nobody to ask: see + // noAgentMarkers. + exitNotAuthorized = 127 +) + +// noAgentMarkers appear in pkexec's own complaint when it had no way to ask: no +// agent registered for the session, and no controlling terminal for the textual +// agent it falls back to. That is the one outcome behind exitNotAuthorized worth a +// message, so it has to be told from a plain refusal, and the only thing that tells +// them apart is what pkexec says about itself. Read with LC_ALL=C so the words are +// the ones written here. +var noAgentMarkers = []string{"authentication agent", "controlling terminal"} + +// run asks polkit to run self as root. pkexec hands the request to the session's +// polkit agent, which is what prompts and what collects any password; we see only +// its verdict. +// +// The environment is otherwise deliberately not passed through: pkexec clears it +// bar a small allowlist, and the one-shot needs nothing from it. +func run(ctx context.Context, self string, args []string) error { + pkexec, err := exec.LookPath("pkexec") + if err != nil { + return fmt.Errorf("%w: pkexec is not installed", ErrUnavailable) + } + + cmd := exec.CommandContext(ctx, pkexec, append([]string{self}, args...)...) + // C locale so pkexec's own diagnostics are the ones noAgentMarkers knows. + cmd.Env = append(os.Environ(), "LC_ALL=C") + var stderr strings.Builder + cmd.Stderr = &stderr + // The one-shot reports itself on stdout for macOS's sake, where there is no + // exit status to read. Here there is one, so that line is noise. + cmd.Stdout = io.Discard + + err = cmd.Run() + if err == nil { + return nil + } + + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + return fmt.Errorf("run pkexec: %w", err) + } + + // Matched against everything pkexec said, reported as one line: a complaint + // that is not the first thing printed still has to be recognised, and reading + // it as a refusal would swallow it. + full := stderr.String() + out := message(full) + + switch exitErr.ExitCode() { + case exitDismissed: + return ErrDeclined + case exitNotAuthorized: + if hasAny(full, noAgentMarkers) { + return fmt.Errorf("%w: polkit had no way to ask: %s", ErrUnavailable, out) + } + // polkit asked and was not satisfied. Overwhelmingly that is the user + // saying no, which needs no message; that an account barred from + // elevating altogether lands here too is why the reason is kept. + return fmt.Errorf("%w: %s", ErrDeclined, out) + default: + return fmt.Errorf("elevated netbird exited with %d: %s", exitErr.ExitCode(), out) + } +} + +func hasAny(s string, markers []string) bool { + for _, marker := range markers { + if strings.Contains(s, marker) { + return true + } + } + return false +} + +func mechanismAvailable() bool { + _, err := exec.LookPath("pkexec") + return err == nil +} + +// message trims a captured stderr to something that reads in one line. +func message(s string) string { + s = strings.TrimSpace(s) + if s == "" { + return "no output" + } + if i := strings.IndexByte(s, '\n'); i >= 0 { + return s[:i] + } + return s +} diff --git a/client/internal/elevate/run_unix_test.go b/client/internal/elevate/run_unix_test.go new file mode 100644 index 000000000..827096928 --- /dev/null +++ b/client/internal/elevate/run_unix_test.go @@ -0,0 +1,105 @@ +//go:build linux || freebsd + +package elevate + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" +) + +// fakePkexec puts a pkexec on PATH that exits with the given code, so the +// mapping from polkit's exit codes onto our errors can be exercised without a +// polkit agent. +func fakePkexec(t *testing.T, exitCode int, stderr string) { + t.Helper() + + dir := t.TempDir() + script := fmt.Sprintf("#!/bin/sh\necho %s >&2\nexit %d\n", shellQuote(stderr), exitCode) + if err := os.WriteFile(filepath.Join(dir, "pkexec"), []byte(script), 0o700); err != nil { + t.Fatalf("write fake pkexec: %v", err) + } + t.Setenv("PATH", dir) +} + +func shellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} + +func TestRunMapsPkexecExitCodes(t *testing.T) { + tests := []struct { + name string + exitCode int + stderr string + wantErr error + }{ + {name: "applied", exitCode: 0}, + { + name: "dialog dismissed", + exitCode: exitDismissed, + stderr: "Error executing command as another user: Request dismissed", + wantErr: ErrDeclined, + }, + { + // What a graphical agent reports for a cancelled prompt. Not a + // failure: the user was asked and answered. + name: "prompt cancelled", + exitCode: exitNotAuthorized, + stderr: "Error executing command as another user: Not authorized", + wantErr: ErrDeclined, + }, + { + // The same status, but pkexec never got to ask anybody. + name: "no agent and no terminal to fall back on", + exitCode: exitNotAuthorized, + stderr: "Error creating textual authentication agent: Error opening current controlling terminal for the process (`/dev/tty'): No such device or address", + wantErr: ErrUnavailable, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakePkexec(t, tt.exitCode, tt.stderr) + + err := run(context.Background(), "/nonexistent/netbird-ui", []string{"--flag"}) + if tt.wantErr == nil { + if err != nil { + t.Fatalf("run() = %v, want nil", err) + } + return + } + if !errors.Is(err, tt.wantErr) { + t.Fatalf("run() = %v, want %v", err, tt.wantErr) + } + }) + } +} + +// An exit code that is not polkit's is the one-shot's own failure, and has to +// stay distinguishable from a declined prompt: the caller reports it. +func TestRunReportsOneShotFailure(t *testing.T) { + fakePkexec(t, 3, "the one-shot said no") + + err := run(context.Background(), "/nonexistent/netbird-ui", nil) + if err == nil { + t.Fatal("run() = nil, want an error") + } + if errors.Is(err, ErrDeclined) || errors.Is(err, ErrUnavailable) { + t.Fatalf("run() = %v, want a plain failure", err) + } +} + +func TestRunWithoutPkexecIsUnavailable(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + + if err := run(context.Background(), "/nonexistent/netbird-ui", nil); !errors.Is(err, ErrUnavailable) { + t.Fatalf("run() = %v, want ErrUnavailable", err) + } + if mechanismAvailable() { + t.Error("mechanismAvailable() = true without pkexec on PATH") + } +} diff --git a/client/internal/elevate/run_unsupported.go b/client/internal/elevate/run_unsupported.go new file mode 100644 index 000000000..887525c4b --- /dev/null +++ b/client/internal/elevate/run_unsupported.go @@ -0,0 +1,15 @@ +//go:build !windows && !darwin && !linux && !freebsd + +package elevate + +import "context" + +// run reports that this platform has no elevation prompt to drive. Mobile and +// WASM builds have no local user to ask in the first place. +func run(context.Context, string, []string) error { + return ErrUnavailable +} + +func mechanismAvailable() bool { + return false +} diff --git a/client/internal/elevate/run_windows.go b/client/internal/elevate/run_windows.go new file mode 100644 index 000000000..f48365a4b --- /dev/null +++ b/client/internal/elevate/run_windows.go @@ -0,0 +1,193 @@ +package elevate + +import ( + "context" + "errors" + "fmt" + "runtime" + "unsafe" + + log "github.com/sirupsen/logrus" + "golang.org/x/sys/windows" +) + +const ( + // seeMaskNoCloseProcess keeps the started process's handle open in + // hProcess so we can wait for it. + seeMaskNoCloseProcess = 0x00000040 + // seeMaskNoAsync makes ShellExecuteExW finish its work before returning, + // which it must when the calling thread does not pump messages. + seeMaskNoAsync = 0x00000100 + // seeMaskFlagNoUI suppresses the shell's own error dialogs; the UAC consent + // dialog is not one of them and still appears. + seeMaskFlagNoUI = 0x00000400 + + // swHide: the one-shot has no window to show. + swHide = 0 + + // sFalse (S_FALSE) answers CoInitializeEx when COM is already up on this + // thread in the mode we asked for; rpcChangedMode (RPC_E_CHANGED_MODE) when + // it is up in the other one. + sFalse = 1 + rpcChangedMode = 0x80010106 +) + +// shellExecuteInfoW mirrors SHELLEXECUTEINFOW. The field order and Go's own +// padding match the C layout on both 386 and amd64. +type shellExecuteInfoW struct { + cbSize uint32 + fMask uint32 + hwnd windows.HWND + lpVerb *uint16 + lpFile *uint16 + lpParameters *uint16 + lpDirectory *uint16 + nShow int32 + hInstApp windows.Handle + lpIDList uintptr + lpClass *uint16 + hkeyClass windows.Handle + dwHotKey uint32 + hIconOrMonitor windows.Handle + hProcess windows.Handle +} + +var ( + shell32 = windows.NewLazySystemDLL("shell32.dll") + procShellExecuteEx = shell32.NewProc("ShellExecuteExW") +) + +// run starts self elevated with the "runas" verb, which is what raises the UAC +// consent dialog, and waits for it to finish. Windows decides whether consent is +// enough or an administrator's credentials are needed, and collects them itself. +func run(ctx context.Context, self string, args []string) error { + verb, err := windows.UTF16PtrFromString("runas") + if err != nil { + return fmt.Errorf("encode verb: %w", err) + } + file, err := windows.UTF16PtrFromString(self) + if err != nil { + return fmt.Errorf("encode %s: %w", self, err) + } + params, err := windows.UTF16PtrFromString(windows.ComposeCommandLine(args)) + if err != nil { + return fmt.Errorf("encode arguments: %w", err) + } + + info := shellExecuteInfoW{ + fMask: seeMaskNoCloseProcess | seeMaskNoAsync | seeMaskFlagNoUI, + hwnd: ownerWindow(), + lpVerb: verb, + lpFile: file, + lpParameters: params, + nShow: swHide, + } + info.cbSize = uint32(unsafe.Sizeof(info)) + + process, err := shellExecute(&info) + if err != nil { + return err + } + defer func() { + if err := windows.CloseHandle(process); err != nil { + log.Debugf("close elevated process handle: %v", err) + } + }() + + return waitForProcess(ctx, process) +} + +// shellExecute performs the call itself. ShellExecuteExW wants COM initialised on +// the calling thread, so the goroutine is pinned to one for the duration and COM +// is set up on it; an "already initialised, different mode" answer is fine, +// because then somebody else has done it for us. +func shellExecute(info *shellExecuteInfoW) (windows.Handle, error) { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + switch err := windows.CoInitializeEx(0, windows.COINIT_APARTMENTTHREADED); { + case err == nil, isHResult(err, sFalse): + // Ours, or already initialised in the same mode: either way this call + // counts and has to be balanced. + defer windows.CoUninitialize() + case isHResult(err, rpcChangedMode): + // The thread is already in the other apartment model. ShellExecuteExW + // works there too, and there is nothing of ours to balance. + default: + return 0, fmt.Errorf("initialise COM: %w", err) + } + + ret, _, lastErr := procShellExecuteEx.Call(uintptr(unsafe.Pointer(info))) + if ret != 0 { + return info.hProcess, nil + } + + if errors.Is(lastErr, windows.ERROR_CANCELLED) { + return 0, ErrDeclined + } + return 0, fmt.Errorf("run elevated: %w", lastErr) +} + +// ownerWindow returns this process's foreground window, and 0 when the window in +// front belongs to somebody else or cannot be attributed. ShellExecuteExW takes it +// as the parent for the UI it raises, which is what keeps the consent dialog in +// front of the window the user was just clicking in instead of behind it. It is +// also what a remote-desktop session needs to place the dialog at all when the +// secure desktop is switched off. +func ownerWindow() windows.HWND { + hwnd := windows.GetForegroundWindow() + if hwnd == 0 { + return 0 + } + + var pid uint32 + if _, err := windows.GetWindowThreadProcessId(hwnd, &pid); err != nil { + log.Debugf("cannot attribute the foreground window, raising the prompt without an owner: %v", err) + return 0 + } + if pid != windows.GetCurrentProcessId() { + return 0 + } + return hwnd +} + +// isHResult reports whether err carries the given HRESULT. CoInitializeEx +// returns its HRESULT as an Errno, so the comparison is on the raw value. +func isHResult(err error, hresult uintptr) bool { + var errno windows.Errno + return errors.As(err, &errno) && uintptr(errno) == hresult +} + +func waitForProcess(ctx context.Context, process windows.Handle) error { + // The wait is interruptible so a cancelled context stops us waiting on a + // consent dialog nobody is going to answer. The elevated process is not + // ours to kill, and it either applies the change or does not. + for { + event, err := windows.WaitForSingleObject(process, 250) + if err != nil { + return fmt.Errorf("wait for the elevated process: %w", err) + } + if event == uint32(windows.WAIT_OBJECT_0) { + break + } + if err := ctx.Err(); err != nil { + return err + } + } + + var code uint32 + if err := windows.GetExitCodeProcess(process, &code); err != nil { + return fmt.Errorf("read the elevated process's exit code: %w", err) + } + if code != 0 { + return fmt.Errorf("elevated netbird exited with %d", code) + } + return nil +} + +// mechanismAvailable is true on Windows: UAC prompts for consent when the user +// is an administrator and for an administrator's credentials when they are not, +// so there is always something to ask. +func mechanismAvailable() bool { + return true +} diff --git a/client/internal/elevate/trusted.go b/client/internal/elevate/trusted.go new file mode 100644 index 000000000..c11054c45 --- /dev/null +++ b/client/internal/elevate/trusted.go @@ -0,0 +1,40 @@ +package elevate + +import ( + "fmt" + "os" + "path/filepath" +) + +// trustedSelf returns the path of this executable, provided it is one we are +// willing to have run as root. +// +// The check is what keeps elevation from becoming a way to launder someone +// else's code into a root process: the user consents to NetBird being elevated, +// having been shown NetBird's name, so what runs must be the file NetBird was +// installed as and not something a third party could have swapped for it. An +// executable only its owner can write is that; anything wider is refused, and +// the caller falls back to showing the command instead. +// +// The owner writing to their own executable is not part of that threat: code +// running as the user can already prompt them for anything, and could just as +// well ask them to run the command by hand. What matters is that no *other* +// unprivileged account can reach it. +func trustedSelf() (string, error) { + exe, err := os.Executable() + if err != nil { + return "", fmt.Errorf("locate this executable: %w", err) + } + + // Resolve symlinks so the checks below apply to the file that would actually + // be executed, not to a link somebody else may control. + resolved, err := filepath.EvalSymlinks(exe) + if err != nil { + return "", fmt.Errorf("resolve %s: %w", exe, err) + } + + if err := checkOnlyOwnerWritable(resolved); err != nil { + return "", fmt.Errorf("%w: %s cannot be trusted to run as root: %w", ErrUnavailable, resolved, err) + } + return resolved, nil +} diff --git a/client/internal/elevate/trusted_group_darwin.go b/client/internal/elevate/trusted_group_darwin.go new file mode 100644 index 000000000..a4b387ec4 --- /dev/null +++ b/client/internal/elevate/trusted_group_darwin.go @@ -0,0 +1,10 @@ +package elevate + +// adminWriteGIDs are the groups whose write access to an executable does not +// widen who could authorize elevating it. +// +// macOS installs applications as root:admin, mode 0775, /Applications included, +// so requiring owner-only write would reject every normal install. Group admin +// (gid 80) is exactly the set of accounts that can answer the authentication +// dialog, so its write access grants nothing the prompt would not. +var adminWriteGIDs = []uint32{0, 80} diff --git a/client/internal/elevate/trusted_group_unix.go b/client/internal/elevate/trusted_group_unix.go new file mode 100644 index 000000000..7aa336423 --- /dev/null +++ b/client/internal/elevate/trusted_group_unix.go @@ -0,0 +1,9 @@ +//go:build !windows && !darwin + +package elevate + +// adminWriteGIDs are the groups whose write access to an executable does not +// widen who could authorize elevating it. Only root's own group qualifies here: +// a distribution installs into root-owned directories, and there is no +// system-wide administrators group that both writes them and answers polkit. +var adminWriteGIDs = []uint32{0} diff --git a/client/internal/elevate/trusted_unix.go b/client/internal/elevate/trusted_unix.go new file mode 100644 index 000000000..65904580c --- /dev/null +++ b/client/internal/elevate/trusted_unix.go @@ -0,0 +1,98 @@ +//go:build !windows + +package elevate + +import ( + "errors" + "fmt" + "os" + "os/user" + "path/filepath" + "slices" + "strconv" + "syscall" + + log "github.com/sirupsen/logrus" +) + +// checkOnlyOwnerWritable reports an error unless path, and every directory leading +// to it, is owned by either root or this user and writable by nobody who could not +// already act as its owner. A writable directory is as good as a writable file, +// since anything in it can be replaced, so the whole chain is checked. +func checkOnlyOwnerWritable(path string) error { + self := uint32(os.Getuid()) + + for dir := path; ; dir = filepath.Dir(dir) { + info, err := os.Lstat(dir) + if err != nil { + return fmt.Errorf("stat %s: %w", dir, err) + } + + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return errors.New("file ownership is unavailable on this platform") + } + if stat.Uid != 0 && stat.Uid != self { + return fmt.Errorf("%s is owned by uid %d, neither root nor this user", dir, stat.Uid) + } + + if err := checkWriteBits(dir, info, stat.Uid, stat.Gid); err != nil { + return err + } + + if parent := filepath.Dir(dir); parent == dir { + return nil + } + } +} + +func checkWriteBits(path string, info os.FileInfo, uid, gid uint32) error { + // On a directory the sticky bit stands in for the write bits: whoever may + // write there still cannot replace an entry they do not own, which is the + // only thing that would matter to us. /tmp is the usual example. + sticky := info.IsDir() && info.Mode()&os.ModeSticky != 0 + + return writeBitsAllow(path, info.Mode().Perm(), sticky, groupWriteAllowed(uid, gid)) +} + +// writeBitsAllow decides on the permission bits alone, given whether the group's +// write access has been vouched for. +func writeBitsAllow(path string, perm os.FileMode, sticky, groupAllowed bool) error { + if sticky { + return nil + } + if perm&0o020 != 0 && !groupAllowed { + return fmt.Errorf("%s is writable by a group with members other than its owner (%v)", path, perm) + } + if perm&0o002 != 0 { + return fmt.Errorf("%s is world-writable (%v)", path, perm) + } + return nil +} + +// groupWriteAllowed reports whether a group's write access to a file owned by uid +// puts it in reach of anyone who could not already act as that owner. +// +// Two ways it does not. A group in adminWriteGIDs is the set of accounts that can +// answer the elevation prompt anyway. And a user private group, whose name is its +// only member's, is how Debian, Ubuntu and Fedora ship: their default umask of 002 +// makes a home directory and everything built in it group-writable, so refusing +// that would mean refusing every build that is not installed from a package, for a +// group nobody else is in. +func groupWriteAllowed(uid, gid uint32) bool { + if slices.Contains(adminWriteGIDs, gid) { + return true + } + + group, err := user.LookupGroupId(strconv.FormatUint(uint64(gid), 10)) + if err != nil { + log.Debugf("cannot look up group %d, treating it as shared: %v", gid, err) + return false + } + owner, err := user.LookupId(strconv.FormatUint(uint64(uid), 10)) + if err != nil { + log.Debugf("cannot look up uid %d, treating its group as shared: %v", uid, err) + return false + } + return group.Name == owner.Username +} diff --git a/client/internal/elevate/trusted_unix_test.go b/client/internal/elevate/trusted_unix_test.go new file mode 100644 index 000000000..affabcf1b --- /dev/null +++ b/client/internal/elevate/trusted_unix_test.go @@ -0,0 +1,149 @@ +//go:build !windows + +package elevate + +import ( + "os" + "path/filepath" + "testing" +) + +// ownerOnlyDir is t.TempDir() with the write bits tightened. testing creates its +// numbered directory with 0777 minus the umask, so under the common 002 umask it +// is group-writable and would fail the check under test on its own. +func ownerOnlyDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if err := os.Chmod(dir, 0o755); err != nil { + t.Fatalf("chmod %s: %v", dir, err) + } + return dir +} + +// writeExecutable creates a plain executable file, the shape trustedSelf checks. +func writeExecutable(t *testing.T, dir string) string { + t.Helper() + path := filepath.Join(dir, "netbird-ui") + if err := os.WriteFile(path, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatalf("write executable: %v", err) + } + if err := os.Chmod(path, 0o755); err != nil { + t.Fatalf("chmod %s: %v", path, err) + } + return path +} + +func TestCheckOnlyOwnerWritableAcceptsOwnerOnly(t *testing.T) { + if err := checkOnlyOwnerWritable(writeExecutable(t, ownerOnlyDir(t))); err != nil { + t.Errorf("owner-only writable executable rejected: %v", err) + } +} + +func TestCheckOnlyOwnerWritableRejectsWorldWritableFile(t *testing.T) { + path := writeExecutable(t, ownerOnlyDir(t)) + if err := os.Chmod(path, 0o777); err != nil { + t.Fatalf("chmod: %v", err) + } + + if err := checkOnlyOwnerWritable(path); err == nil { + t.Error("world-writable executable accepted") + } +} + +// The permission policy on its own, without a filesystem to arrange: whether the +// group has been vouched for is the only thing that makes group write acceptable. +func TestWriteBitsAllow(t *testing.T) { + tests := []struct { + name string + perm os.FileMode + sticky bool + groupAllowed bool + wantErr bool + }{ + {name: "owner only", perm: 0o755}, + {name: "group write in a private group", perm: 0o775, groupAllowed: true}, + {name: "group write in a shared group", perm: 0o775, wantErr: true}, + {name: "world write", perm: 0o777, groupAllowed: true, wantErr: true}, + {name: "world write on a sticky directory", perm: 0o777, sticky: true}, + {name: "group write on a sticky directory", perm: 0o775, sticky: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := writeBitsAllow("/path", tt.perm, tt.sticky, tt.groupAllowed) + if tt.wantErr { + if err == nil { + t.Errorf("writeBitsAllow(%v, sticky=%v, groupAllowed=%v) = nil, want an error", + tt.perm, tt.sticky, tt.groupAllowed) + } + return + } + if err != nil { + t.Errorf("writeBitsAllow(%v, sticky=%v, groupAllowed=%v) = %v, want nil", + tt.perm, tt.sticky, tt.groupAllowed, err) + } + }) + } +} + +// A build under a home directory on a distribution with a 002 umask, which is what +// a locally built or tarball-installed binary looks like. Its group has no members +// but its owner, so it is as good as owner-only. +func TestCheckOnlyOwnerWritableAcceptsOwnPrivateGroup(t *testing.T) { + if !groupWriteAllowed(uint32(os.Getuid()), uint32(os.Getgid())) { + t.Skip("the test user's primary group is shared, so there is nothing to assert here") + } + + dir := ownerOnlyDir(t) + path := writeExecutable(t, dir) + if err := os.Chmod(dir, 0o775); err != nil { + t.Fatalf("chmod dir: %v", err) + } + if err := os.Chmod(path, 0o775); err != nil { + t.Fatalf("chmod: %v", err) + } + + if err := checkOnlyOwnerWritable(path); err != nil { + t.Errorf("executable group-writable in its owner's private group rejected: %v", err) + } +} + +// A writable directory is as good as a writable file: whoever can write the +// directory can put a different binary at the same path. +func TestCheckOnlyOwnerWritableRejectsWritableDirectory(t *testing.T) { + dir := filepath.Join(ownerOnlyDir(t), "bin") + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + path := writeExecutable(t, dir) + if err := os.Chmod(dir, 0o777); err != nil { + t.Fatalf("chmod dir: %v", err) + } + + if err := checkOnlyOwnerWritable(path); err == nil { + t.Error("executable in a world-writable directory accepted") + } +} + +// A sticky world-writable directory is exempt: the sticky bit is what stops one +// user replacing another's entries. /tmp is why this matters. +func TestCheckOnlyOwnerWritableAcceptsStickyDirectory(t *testing.T) { + dir := filepath.Join(ownerOnlyDir(t), "sticky") + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + path := writeExecutable(t, dir) + if err := os.Chmod(dir, 0o777|os.ModeSticky); err != nil { + t.Fatalf("chmod dir: %v", err) + } + + if err := checkOnlyOwnerWritable(path); err != nil { + t.Errorf("executable in a sticky directory rejected: %v", err) + } +} + +func TestCheckOnlyOwnerWritableRejectsMissingFile(t *testing.T) { + if err := checkOnlyOwnerWritable(filepath.Join(ownerOnlyDir(t), "absent")); err == nil { + t.Error("missing executable accepted") + } +} diff --git a/client/internal/elevate/trusted_windows.go b/client/internal/elevate/trusted_windows.go new file mode 100644 index 000000000..a02503f80 --- /dev/null +++ b/client/internal/elevate/trusted_windows.go @@ -0,0 +1,150 @@ +package elevate + +import ( + "fmt" + "path/filepath" + "unsafe" + + "golang.org/x/sys/windows" +) + +// writeAccess are the rights that let a trustee replace or rewrite an +// executable, or take it over and then do so. +const writeAccess = windows.FILE_WRITE_DATA | windows.FILE_APPEND_DATA | + windows.DELETE | windows.WRITE_DAC | windows.WRITE_OWNER | + windows.GENERIC_WRITE | windows.GENERIC_ALL + +// trustedInstallerSID owns much of what Windows itself installs. x/sys has no +// well-known constant for it. +const trustedInstallerSID = "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464" + +// unprivilegedTrustees are the well-known groups that contain accounts which +// cannot elevate on their own. Granting any of them write access to the +// executable would mean an account that cannot pass the UAC prompt could still +// decide what runs behind it. +var unprivilegedTrustees = []windows.WELL_KNOWN_SID_TYPE{ + windows.WinWorldSid, // Everyone + windows.WinAuthenticatedUserSid, // Authenticated Users + windows.WinInteractiveSid, // INTERACTIVE + windows.WinBuiltinUsersSid, // BUILTIN\Users + windows.WinBuiltinGuestsSid, // BUILTIN\Guests +} + +// checkOnlyOwnerWritable reports an error unless path is owned by an account +// that can elevate (or by this user) and grants write access to no group of +// accounts that cannot. Its directory is checked the same way, because being +// able to write the directory is being able to replace the file in it. +func checkOnlyOwnerWritable(path string) error { + for _, target := range []string{path, filepath.Dir(path)} { + if err := checkSecurity(target); err != nil { + return err + } + } + return nil +} + +func checkSecurity(path string) error { + sd, err := windows.GetNamedSecurityInfo(path, windows.SE_FILE_OBJECT, + windows.OWNER_SECURITY_INFORMATION|windows.DACL_SECURITY_INFORMATION) + if err != nil { + return fmt.Errorf("read security descriptor of %s: %w", path, err) + } + + owner, _, err := sd.Owner() + if err != nil { + return fmt.Errorf("read owner of %s: %w", path, err) + } + if err := checkOwner(path, owner); err != nil { + return err + } + + dacl, _, err := sd.DACL() + if err != nil { + return fmt.Errorf("read DACL of %s: %w", path, err) + } + // A NULL DACL grants everyone everything; only an absent security + // descriptor would have got us here without one, and neither is trustworthy. + if dacl == nil { + return fmt.Errorf("%s has no DACL, so it grants write access to everyone", path) + } + + return checkDACL(path, dacl) +} + +// checkOwner accepts an owner that can elevate by itself, plus this user: their +// own executable is theirs to write, and code already running as them could +// prompt for anything anyway. +func checkOwner(path string, owner *windows.SID) error { + self, err := currentUserSID() + if err != nil { + return err + } + if owner.Equals(self) { + return nil + } + + for _, wellKnown := range []windows.WELL_KNOWN_SID_TYPE{ + windows.WinLocalSystemSid, + windows.WinBuiltinAdministratorsSid, + } { + sid, err := windows.CreateWellKnownSid(wellKnown) + if err != nil { + return fmt.Errorf("build well-known SID %d: %w", wellKnown, err) + } + if owner.Equals(sid) { + return nil + } + } + + installer, err := windows.StringToSid(trustedInstallerSID) + if err != nil { + return fmt.Errorf("parse TrustedInstaller SID: %w", err) + } + if owner.Equals(installer) { + return nil + } + + return fmt.Errorf("%s is owned by %s, which is neither this user nor an account that can elevate", path, owner) +} + +func checkDACL(path string, dacl *windows.ACL) error { + untrusted := make([]*windows.SID, 0, len(unprivilegedTrustees)) + for _, wellKnown := range unprivilegedTrustees { + sid, err := windows.CreateWellKnownSid(wellKnown) + if err != nil { + return fmt.Errorf("build well-known SID %d: %w", wellKnown, err) + } + untrusted = append(untrusted, sid) + } + + for i := uint32(0); i < uint32(dacl.AceCount); i++ { + var ace *windows.ACCESS_ALLOWED_ACE + if err := windows.GetAce(dacl, i, &ace); err != nil { + return fmt.Errorf("read ACE %d of %s: %w", i, path, err) + } + if ace.Header.AceType != windows.ACCESS_ALLOWED_ACE_TYPE { + continue + } + if ace.Mask&writeAccess == 0 { + continue + } + + //nolint:gosec // SidStart is the first uint32 of the variable-length SID that follows the ACE header. + sid := (*windows.SID)(unsafe.Pointer(&ace.SidStart)) + for _, bad := range untrusted { + if sid.Equals(bad) { + return fmt.Errorf("%s grants write access to %s", path, bad) + } + } + } + return nil +} + +func currentUserSID() (*windows.SID, error) { + token := windows.GetCurrentProcessToken() + user, err := token.GetTokenUser() + if err != nil { + return nil, fmt.Errorf("read this process's user: %w", err) + } + return user.User.Sid, nil +} diff --git a/client/internal/ipcauth/privileged.go b/client/internal/ipcauth/privileged.go index 95f2a50e9..3c2e68432 100644 --- a/client/internal/ipcauth/privileged.go +++ b/client/internal/ipcauth/privileged.go @@ -91,6 +91,12 @@ func SelfDelegatesTo() (Identity, bool) { return selfIdentity, true } +// The values PrivilegedActorKey returns. +const ( + ActorKeyAdministrator = "administrator" + ActorKeyRoot = "root" +) + // PrivilegedActor names the principal a privileged operation requires, for use // in messages shown to the user. func PrivilegedActor() string { @@ -100,6 +106,16 @@ func PrivilegedActor() string { return "root" } +// PrivilegedActorKey identifies that principal without wording it, for a client +// that writes its own message in the user's language. The words PrivilegedActor +// returns are English, and a translated sentence cannot borrow them. +func PrivilegedActorKey() string { + if runtime.GOOS == "windows" { + return ActorKeyAdministrator + } + return ActorKeyRoot +} + // ElevatedCommand renders a command so that running it grants the privileges the // operation needs. Windows has no in-line equivalent of sudo, so the command is // returned unchanged and the user is expected to run it from an elevated diff --git a/client/ui/build/linux/netbird-ui.desktop b/client/ui/build/linux/netbird-ui.desktop index 6b6ed42a5..a7c4ef3d4 100755 --- a/client/ui/build/linux/netbird-ui.desktop +++ b/client/ui/build/linux/netbird-ui.desktop @@ -1,10 +1,11 @@ [Desktop Entry] Type=Application -Name=netbird-ui +Name=NetBird +Comment=NetBird desktop client Exec=env WEBKIT_DISABLE_DMABUF_RENDERER=1 netbird-ui Icon=netbird-ui -Categories=Development; +Categories=Utility;Network; Terminal=false -Keywords=wails +Keywords=netbird;vpn;wireguard; Version=1.0 StartupNotify=false diff --git a/client/ui/build/linux/netbird.desktop b/client/ui/build/linux/netbird.desktop index a81f3698a..0d43b62a2 100644 --- a/client/ui/build/linux/netbird.desktop +++ b/client/ui/build/linux/netbird.desktop @@ -1,5 +1,6 @@ [Desktop Entry] -Name=Netbird +Name=NetBird +Comment=NetBird desktop client Exec=env WEBKIT_DISABLE_DMABUF_RENDERER=1 /usr/bin/netbird-ui Icon=netbird Type=Application diff --git a/client/ui/build/linux/nfpm/nfpm.yaml b/client/ui/build/linux/nfpm/nfpm.yaml index 764855a63..8f06a9886 100644 --- a/client/ui/build/linux/nfpm/nfpm.yaml +++ b/client/ui/build/linux/nfpm/nfpm.yaml @@ -23,6 +23,11 @@ contents: dst: "/usr/share/icons/hicolor/128x128/apps/netbird-ui.png" - src: "./build/linux/netbird-ui.desktop" dst: "/usr/share/applications/netbird-ui.desktop" + # Names the polkit action for the elevation prompt the app raises when an + # unprivileged user changes a privileged setting; without it the dialog shows a + # raw command line. + - src: "./build/linux/polkit/io.netbird.settings.policy" + dst: "/usr/share/polkit-1/actions/io.netbird.settings.policy" # Default dependencies for the GTK4 + WebKitGTK 6.0 stack (Ubuntu 24.04+ / Debian 13+) depends: diff --git a/client/ui/build/linux/polkit/io.netbird.settings.policy b/client/ui/build/linux/polkit/io.netbird.settings.policy new file mode 100644 index 000000000..f3e6920b8 --- /dev/null +++ b/client/ui/build/linux/polkit/io.netbird.settings.policy @@ -0,0 +1,56 @@ + + + + + + NetBird + https://netbird.io + + + Change privileged NetBird settings + Authentication is required to change NetBird settings that grant SSH access to this computer. + netbird-ui + + auth_admin + auth_admin + auth_admin + + /usr/bin/netbird-ui + --apply-privileged-settings + + + + Change privileged NetBird settings + Authentication is required to change NetBird settings that grant SSH access to this computer. + netbird-ui + + auth_admin + auth_admin + auth_admin + + /usr/local/bin/netbird-ui + --apply-privileged-settings + + diff --git a/client/ui/frontend/src/contexts/SettingsContext.tsx b/client/ui/frontend/src/contexts/SettingsContext.tsx index 3f4b2d0d2..df4ee3305 100644 --- a/client/ui/frontend/src/contexts/SettingsContext.tsx +++ b/client/ui/frontend/src/contexts/SettingsContext.tsx @@ -22,12 +22,18 @@ const logSaveError = (err: unknown) => console.error("[SettingsContext] save fai export type AutostartState = { supported: boolean; enabled: boolean }; +// GuardedField is a setting the daemon only accepts from root/administrator. +// Turning one on goes through saveGuardedField, which asks the operating system +// for the privileges rather than sending a request that would be refused. +export type GuardedField = "serverSshAllowed" | "enableSshRoot" | "disableSshAuth"; + type SettingsContextValue = { config: Config; guiVersion: string; setField: (k: K, v: Config[K]) => void; saveField: (k: K, v: Config[K]) => Promise; saveFields: (partial: Partial, opts?: { preSharedKey?: string }) => Promise; + saveGuardedField: (k: GuardedField, v: boolean) => Promise; saveNow: () => Promise; }; @@ -141,12 +147,17 @@ const useSettingsState = () => { async (profileName: string, next: Config, preSharedKey?: string) => { const preSharedKeyWrite = preSharedKey === undefined ? {} : { preSharedKey }; try { - await SettingsSvc.SetConfig({ + const { declined } = await SettingsSvc.SetConfig({ ...next, ...preSharedKeyWrite, profileName, username, }); + // The change needed authorization and the user said no, so the + // optimistic update is wrong. Nothing to report: they know. + if (declined) { + await reload(profileName); + } } catch (e) { // The optimistic update is wrong now: the daemon refused it // (a change that needs elevated privileges, an MDM-managed @@ -206,6 +217,59 @@ const useSettingsState = () => { [loaded, save], ); + // saveGuardedField applies a setting the daemon restricts to + // root/administrator by having the Go side run the app again under the + // platform's elevation prompt (UAC, the macOS authentication dialog, polkit). + // The prompt is the user's, so the call is made straight from their gesture + // and never from the debounce. + const saveGuardedField = useCallback( + async (k: GuardedField, v: boolean) => { + const cur = loadedRef.current; + if (!cur) return; + + // Flush what the debounce still owes, before the optimistic update + // below joins it: a later save carrying the guarded value would be + // refused, and its error dialog would be the second one for a change + // the user already authorized. + if (saveTimer.current) { + clearTimeout(saveTimer.current); + saveTimer.current = null; + await save(cur.profileName, cur.data); + } + + const next: LoadedConfig = { + profileName: cur.profileName, + data: { ...cur.data, [k]: v }, + }; + loadedRef.current = next; + setLoaded(next); + + try { + await SettingsSvc.SetGuardedSettings({ + profileName: cur.profileName, + username, + [k]: v, + }); + } catch (e) { + // The daemon is authoritative either way, so re-read before + // reporting. A declined prompt is not an error and does not come + // through here at all; this is a prompt that could not be raised, + // which carries the command that would have done it. + await reload(cur.profileName); + await errorDialog({ + Title: i18next.t("settings.error.saveTitle"), + Message: errorMessage(e), + Command: errorCommand(e), + }); + return; + } + // Either the change went through or the user declined it. The daemon + // says which. + await reload(cur.profileName); + }, + [username, save, reload], + ); + const saveFields = useCallback( async (partial: Partial, opts?: { preSharedKey?: string }) => { if (!loaded) return; @@ -225,15 +289,27 @@ const useSettingsState = () => { [loaded, save], ); - return { config: loaded?.data ?? null, guiVersion, setField, saveField, saveFields, saveNow }; + return { + config: loaded?.data ?? null, + guiVersion, + setField, + saveField, + saveFields, + saveGuardedField, + saveNow, + }; }; export const SettingsProvider = ({ children }: { children: ReactNode }) => { - const { config, guiVersion, setField, saveField, saveFields, saveNow } = useSettingsState(); + const { config, guiVersion, setField, saveField, saveFields, saveGuardedField, saveNow } = + useSettingsState(); const value = useMemo( - () => (config ? { config, guiVersion, setField, saveField, saveFields, saveNow } : null), - [config, guiVersion, setField, saveField, saveFields, saveNow], + () => + config + ? { config, guiVersion, setField, saveField, saveFields, saveGuardedField, saveNow } + : null, + [config, guiVersion, setField, saveField, saveFields, saveGuardedField, saveNow], ); if (!value) { diff --git a/client/ui/frontend/src/hooks/usePrivilege.ts b/client/ui/frontend/src/hooks/usePrivilege.ts index 05e9a7ce0..d67fcc4b1 100644 --- a/client/ui/frontend/src/hooks/usePrivilege.ts +++ b/client/ui/frontend/src/hooks/usePrivilege.ts @@ -1,6 +1,6 @@ import { useEffect, useState } from "react"; import { Settings as SettingsSvc } from "@bindings/services"; -import { Privilege } from "@bindings/services/models.js"; +import { type Privilege } from "@bindings/services/models.js"; // usePrivilege reports whether this UI process may perform the changes the daemon // restricts to root/administrator. It is answered in-process from our own token diff --git a/client/ui/frontend/src/modules/settings/SettingsSSH.tsx b/client/ui/frontend/src/modules/settings/SettingsSSH.tsx index bd91e520c..d74afae73 100644 --- a/client/ui/frontend/src/modules/settings/SettingsSSH.tsx +++ b/client/ui/frontend/src/modules/settings/SettingsSSH.tsx @@ -1,3 +1,4 @@ +import { type TFunction } from "i18next"; import { useTranslation } from "react-i18next"; import { CopyToClipboard } from "@/components/CopyToClipboard"; import FancyToggleSwitch from "@/components/switches/FancyToggleSwitch"; @@ -6,51 +7,91 @@ import { Input } from "@/components/inputs/Input"; import { Label } from "@/components/typography/Label"; import { cn } from "@/lib/cn"; import { SectionGroup } from "@/modules/settings/SettingsSection.tsx"; -import { useSettings } from "@/contexts/SettingsContext.tsx"; +import { type GuardedField, useSettings } from "@/contexts/SettingsContext.tsx"; import { usePrivilege } from "@/hooks/usePrivilege.ts"; -import { Privilege } from "@bindings/services/models.js"; +import type { Privilege } from "@bindings/services/models.js"; import { type ChangeEvent, type ReactNode, useEffect, useId, useState } from "react"; export function SettingsSSH() { const { t } = useTranslation(); - const { config, setField } = useSettings(); + const { config, setField, saveGuardedField } = useSettings(); const privilege = usePrivilege(); + // The field whose elevation prompt is currently up, if any. The prompt is + // modal to the operating system, not to us, so the guarded controls are held + // still meanwhile rather than allowed to stack a second one behind it. + const [authorizing, setAuthorizing] = useState(null); const isSSHServerEnabled = config.serverSshAllowed; + const authorize = async (field: GuardedField, value: boolean) => { + setAuthorizing(field); + try { + await saveGuardedField(field, value); + } finally { + setAuthorizing(null); + } + }; + // The daemon restricts only the direction that hands out shells from a process - // running as root. So for an unprivileged user a guarded control is either - // unavailable (it is off and only they could turn it on) or a one-way switch - // (it is on, they may turn it off, but not back on) — say which, either way. + // running as root: for all three settings that is switching the field on. + // + // An unprivileged user gets that direction routed through the platform's + // elevation prompt where there is one to raise, and otherwise the old + // arrangement, where the control is either unavailable (it is off and only a + // privileged caller could turn it on) or a one-way switch (it is on, they may + // turn it off but not back on) with the command that does it. // // A null privilege means we could not determine it: leave the control alone // rather than greying it out with nothing to explain why. The daemon enforces // this regardless, and a rejected save reports its own guidance. const guarded = ( - guardedDirectionActive: boolean, + field: GuardedField, command: (p: Privilege) => string, // inverted marks a control whose guarded direction is switching it off, so // the one-way warning has to read the other way round. inverted = false, ) => { + const plain = (value: boolean) => setField(field, value); if (!privilege || privilege.privileged) { - return { disabled: false, hint: undefined }; + return { apply: plain, disabled: false, hint: undefined }; } - const hint = ( - ( + ); - return { disabled: !guardedDirectionActive, hint }; + + if (privilege.canElevate) { + return { + // Switching off is ours to do; only switching on is authorized. + apply: (value: boolean) => { + if (!value) { + plain(value); + return; + } + void authorize(field, value); + }, + disabled: authorizing !== null, + hint: hint(authorizing === field), + }; + } + return { + apply: plain, + disabled: !guardedDirectionActive, + hint: hint(false, command(privilege)), + }; }; - const sshServer = guarded(config.serverSshAllowed, (p) => p.allowSshServer); - const sshRoot = guarded(config.enableSshRoot, (p) => p.enableSshRoot); + const sshServer = guarded("serverSshAllowed", (p) => p.allowSshServer); + const sshRoot = guarded("enableSshRoot", (p) => p.enableSshRoot); // Inverted control: the guarded direction is switching authentication off, so // it is the already-disabled state that is the one-way one. - const sshAuth = guarded(config.disableSshAuth, (p) => p.disableSshAuth, true); + const sshAuth = guarded("disableSshAuth", (p) => p.disableSshAuth, true); const jwtTtlId = useId(); const [jwtTtlInput, setJwtTtlInput] = useState(String(config.sshJwtCacheTtl)); @@ -84,7 +125,7 @@ export function SettingsSSH() { setField("serverSshAllowed", v)} + onChange={sshServer.apply} disabled={sshServer.disabled} label={t("settings.ssh.server.label")} helpText={t("settings.ssh.server.help")} @@ -98,7 +139,7 @@ export function SettingsSSH() { > setField("enableSshRoot", v)} + onChange={sshRoot.apply} disabled={sshRoot.disabled} label={t("settings.ssh.root.label")} helpText={t("settings.ssh.root.help")} @@ -130,7 +171,7 @@ export function SettingsSSH() { > setField("disableSshAuth", !v)} + onChange={(v) => sshAuth.apply(!v)} disabled={sshAuth.disabled} label={t("settings.ssh.jwt.label")} helpText={t("settings.ssh.jwt.help")} @@ -163,41 +204,81 @@ export function SettingsSSH() { ); } -// PrivilegeHint explains what an unprivileged user can and cannot do with a -// guarded control, and offers the command that does it with the privileges the -// daemon requires. oneWay covers the control being in the guarded state already: -// switching it back is the part that needs privileges. -function PrivilegeHint({ +// actorLabel names the principal the daemon requires, in the user's language. The +// Go side reports which one it is rather than wording it, because "administrator +// privileges" is English and a translated sentence cannot borrow it. +function actorLabel(privilege: Privilege, t: TFunction): string { + return privilege.actorKey === "administrator" + ? t("settings.ssh.privilege.actorAdministrator") + : t("settings.ssh.privilege.actorRoot"); +} + +// GuardedHint is what a control the daemon guards says to an unprivileged user. +// There are three things worth saying, and it says at most one: +// +// - A prompt is open. Worth a line because it can take a few seconds to appear, +// long enough that a control which merely went inert would read as a hang. +// - The setting is in its guarded state already (oneWay), so the user may switch +// it back as they please and it is switching it away again that will ask. No +// command either way: the direction they can take is theirs to take. +// - Only a privileged caller can move it at all, and there is no prompt to +// raise: the command that does it belongs here, and nothing else will do. +// +// Which leaves the case of a control whose guarded direction is still ahead of the +// user and a prompt that can be raised for it: nothing to say, because clicking it +// raises the prompt and the prompt explains itself. +function GuardedHint({ actor, - command, oneWay, inverted, + pending, + command, }: { actor: string; - command: string; oneWay: boolean; inverted: boolean; + pending: boolean; + command?: string; }): ReactNode { const { t } = useTranslation(); + + if (pending) { + return {t("settings.ssh.privilege.authorizePending")}; + } + if (oneWay) { + return ( + + + {inverted + ? t("settings.ssh.privilege.oneWayInverted", { actor }) + : t("settings.ssh.privilege.oneWay", { actor })} + + + ); + } if (!command) return null; + return ( + + {t("settings.ssh.privilege.hint", { actor })} + + + {command} + + + + ); +} + +// HintBox is the box a guarded control puts its explanation in, directly under the +// control it belongs to. +function HintBox({ children }: { children: ReactNode }): ReactNode { return (
- - {!oneWay - ? t("settings.ssh.privilege.hint", { actor }) - : inverted - ? t("settings.ssh.privilege.oneWayInverted", { actor }) - : t("settings.ssh.privilege.oneWay", { actor })} - - - - {command} - - + {children}
); } diff --git a/client/ui/i18n/locales/de/common.json b/client/ui/i18n/locales/de/common.json index 5e91e8d88..489cf9197 100644 --- a/client/ui/i18n/locales/de/common.json +++ b/client/ui/i18n/locales/de/common.json @@ -401,9 +401,6 @@ "networks.bulk.label": { "message": "Alle sichtbaren Ressourcen umschalten" }, - "settings.nav.label": { - "message": "Einstellungsbereiche" - }, "profile.switch.title": { "message": "Zu Profil \"{name}\" wechseln?" }, @@ -497,6 +494,9 @@ "settings.error.debugBundleTitle": { "message": "Debug-Paket fehlgeschlagen" }, + "settings.nav.label": { + "message": "Einstellungsbereiche" + }, "settings.tabs.general": { "message": "Allgemein" }, @@ -1330,5 +1330,29 @@ }, "error.unknown": { "message": "Vorgang fehlgeschlagen." + }, + "error.elevation_unavailable": { + "message": "NetBird konnte auf diesem System nicht die nötigen Rechte anfordern. Führen Sie stattdessen dies aus:" + }, + "error.elevation_failed": { + "message": "Die Änderung konnte mit erhöhten Rechten nicht angewendet werden. Führen Sie stattdessen dies aus:" + }, + "settings.ssh.privilege.actorRoot": { + "message": "root-Rechte" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "Administratorrechte" + }, + "settings.ssh.privilege.hint": { + "message": "Erfordert {actor}. Führen Sie stattdessen dies aus:" + }, + "settings.ssh.privilege.oneWay": { + "message": "Sie können dies deaktivieren, zum erneuten Aktivieren sind {actor} erforderlich." + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "Sie können dies aktivieren, zum erneuten Deaktivieren sind {actor} erforderlich." + }, + "settings.ssh.privilege.authorizePending": { + "message": "Warten auf Autorisierung…" } } diff --git a/client/ui/i18n/locales/en/common.json b/client/ui/i18n/locales/en/common.json index b668146e8..ae4f170a4 100644 --- a/client/ui/i18n/locales/en/common.json +++ b/client/ui/i18n/locales/en/common.json @@ -1775,16 +1775,36 @@ "message": "Operation failed.", "description": "Generic fallback error message used when no specific error applies." }, + "error.elevation_unavailable": { + "message": "NetBird could not ask this system for the privileges the change needs. Run this instead:", + "description": "Error: this computer has no way to prompt for elevated privileges. Followed by a copyable command that applies the setting from a terminal." + }, + "error.elevation_failed": { + "message": "The change could not be applied with elevated privileges. Run this instead:", + "description": "Error: the authorization succeeded but applying the setting afterwards failed. Followed by a copyable command that applies the setting from a terminal." + }, + "settings.ssh.privilege.actorRoot": { + "message": "root", + "description": "Fills {actor} in the settings.ssh.privilege.* messages on Linux, macOS and BSD, where the daemon requires the root account. 'root' is an account name and stays as it is; add the word for privileges or rights around it if the sentence needs one to read naturally." + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "administrator privileges", + "description": "Fills {actor} in the settings.ssh.privilege.* messages on Windows, where the daemon requires an elevated administrator. The Windows term for the rights an account is asked to elevate to." + }, "settings.ssh.privilege.hint": { "message": "Requires {actor}. Run this instead:", "description": "Help text under an SSH setting the user cannot change: it needs elevated privileges. {actor} is 'root' on Linux/macOS or 'administrator privileges' on Windows. Followed by a copyable command." }, "settings.ssh.privilege.oneWay": { - "message": "You can switch this off, but switching it back on needs {actor}:", - "description": "Warning under an SSH setting an unprivileged user may disable but not re-enable. {actor} is 'root' on Linux/macOS or 'administrator privileges' on Windows. Followed by a copyable command." + "message": "You can switch this off, but switching it back on needs {actor}.", + "description": "Help text under an SSH setting that is already on: an unprivileged user may switch it off freely, and switching it on again is what needs the privileges. No command follows, since the direction they can take is theirs to take. {actor} is 'root' on Linux/macOS or 'administrator privileges' on Windows." }, "settings.ssh.privilege.oneWayInverted": { - "message": "You can switch this on, but switching it back off needs {actor}:", - "description": "Warning under the SSH authentication setting, which an unprivileged user may re-enable but not disable again. {actor} is 'root' on Linux/macOS or 'administrator privileges' on Windows. Followed by a copyable command." + "message": "You can switch this on, but switching it back off needs {actor}.", + "description": "Same as settings.ssh.privilege.oneWay, for the SSH authentication setting once it has been switched off: switching it off again is what needs the privileges." + }, + "settings.ssh.privilege.authorizePending": { + "message": "Waiting for authorization…", + "description": "Replaces the help text under a guarded SSH setting while the authorization prompt is open, which can take a few seconds to appear. Keep the trailing ellipsis." } } diff --git a/client/ui/i18n/locales/es/common.json b/client/ui/i18n/locales/es/common.json index c036e4f75..2f42ab8e8 100644 --- a/client/ui/i18n/locales/es/common.json +++ b/client/ui/i18n/locales/es/common.json @@ -401,9 +401,6 @@ "networks.bulk.label": { "message": "Conmutar todos los recursos visibles" }, - "settings.nav.label": { - "message": "Secciones de configuración" - }, "profile.switch.title": { "message": "¿Cambiar el perfil a «{name}»?" }, @@ -497,6 +494,9 @@ "settings.error.debugBundleTitle": { "message": "Error en el paquete de diagnóstico" }, + "settings.nav.label": { + "message": "Secciones de configuración" + }, "settings.tabs.general": { "message": "General" }, @@ -1330,5 +1330,29 @@ }, "error.unknown": { "message": "La operación falló." + }, + "error.elevation_unavailable": { + "message": "NetBird no pudo solicitar a este sistema los privilegios necesarios. Ejecute esto en su lugar:" + }, + "error.elevation_failed": { + "message": "No se pudo aplicar el cambio con privilegios elevados. Ejecute esto en su lugar:" + }, + "settings.ssh.privilege.actorRoot": { + "message": "privilegios de root" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "privilegios de administrador" + }, + "settings.ssh.privilege.hint": { + "message": "Requiere {actor}. Ejecute esto en su lugar:" + }, + "settings.ssh.privilege.oneWay": { + "message": "Puede desactivarlo, pero volver a activarlo requiere {actor}." + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "Puede activarlo, pero volver a desactivarlo requiere {actor}." + }, + "settings.ssh.privilege.authorizePending": { + "message": "Esperando la autorización…" } } diff --git a/client/ui/i18n/locales/fr/common.json b/client/ui/i18n/locales/fr/common.json index c6b91fb25..97db6b039 100644 --- a/client/ui/i18n/locales/fr/common.json +++ b/client/ui/i18n/locales/fr/common.json @@ -401,9 +401,6 @@ "networks.bulk.label": { "message": "Activer/désactiver toutes les ressources visibles" }, - "settings.nav.label": { - "message": "Sections des paramètres" - }, "profile.switch.title": { "message": "Basculer vers le profil « {name} » ?" }, @@ -497,6 +494,9 @@ "settings.error.debugBundleTitle": { "message": "Échec du lot de diagnostic" }, + "settings.nav.label": { + "message": "Sections des paramètres" + }, "settings.tabs.general": { "message": "Général" }, @@ -1330,5 +1330,29 @@ }, "error.unknown": { "message": "L’opération a échoué." + }, + "error.elevation_unavailable": { + "message": "NetBird n’a pas pu demander à ce système les privilèges nécessaires. Exécutez plutôt ceci :" + }, + "error.elevation_failed": { + "message": "La modification n’a pas pu être appliquée avec des privilèges élevés. Exécutez plutôt ceci :" + }, + "settings.ssh.privilege.actorRoot": { + "message": "les privilèges root" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "les privilèges administrateur" + }, + "settings.ssh.privilege.hint": { + "message": "Nécessite {actor}. Exécutez plutôt ceci :" + }, + "settings.ssh.privilege.oneWay": { + "message": "Vous pouvez le désactiver, mais le réactiver nécessite {actor}." + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "Vous pouvez l’activer, mais le désactiver de nouveau nécessite {actor}." + }, + "settings.ssh.privilege.authorizePending": { + "message": "En attente de l’autorisation…" } } diff --git a/client/ui/i18n/locales/hu/common.json b/client/ui/i18n/locales/hu/common.json index dd5a1af6c..9604ce34f 100644 --- a/client/ui/i18n/locales/hu/common.json +++ b/client/ui/i18n/locales/hu/common.json @@ -401,9 +401,6 @@ "networks.bulk.label": { "message": "Összes látható erőforrás be/ki" }, - "settings.nav.label": { - "message": "Beállítások szakaszai" - }, "profile.switch.title": { "message": "Váltás a(z) \"{name}\" profilra?" }, @@ -497,6 +494,9 @@ "settings.error.debugBundleTitle": { "message": "Hibakeresési csomag sikertelen" }, + "settings.nav.label": { + "message": "Beállítások szakaszai" + }, "settings.tabs.general": { "message": "Általános" }, @@ -1330,5 +1330,29 @@ }, "error.unknown": { "message": "A művelet meghiúsult." + }, + "error.elevation_unavailable": { + "message": "A NetBird nem tudta bekérni a rendszertől a szükséges jogosultságokat. Futtassa inkább ezt:" + }, + "error.elevation_failed": { + "message": "A módosítást emelt szintű jogosultságokkal sem sikerült alkalmazni. Futtassa inkább ezt:" + }, + "settings.ssh.privilege.actorRoot": { + "message": "root jogosultság" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "rendszergazdai jogosultság" + }, + "settings.ssh.privilege.hint": { + "message": "{actor} szükséges hozzá. Futtassa inkább ezt:" + }, + "settings.ssh.privilege.oneWay": { + "message": "Kikapcsolhatja, de a visszakapcsolásához {actor} szükséges." + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "Bekapcsolhatja, de az ismételt kikapcsolásához {actor} szükséges." + }, + "settings.ssh.privilege.authorizePending": { + "message": "Várakozás az engedélyezésre…" } } diff --git a/client/ui/i18n/locales/it/common.json b/client/ui/i18n/locales/it/common.json index 7a2eb610c..fe6f2297d 100644 --- a/client/ui/i18n/locales/it/common.json +++ b/client/ui/i18n/locales/it/common.json @@ -401,9 +401,6 @@ "networks.bulk.label": { "message": "Attiva/disattiva tutte le risorse visibili" }, - "settings.nav.label": { - "message": "Sezioni delle impostazioni" - }, "profile.switch.title": { "message": "Passare al profilo «{name}»?" }, @@ -497,6 +494,9 @@ "settings.error.debugBundleTitle": { "message": "Pacchetto di debug non riuscito" }, + "settings.nav.label": { + "message": "Sezioni delle impostazioni" + }, "settings.tabs.general": { "message": "Generale" }, @@ -1330,5 +1330,29 @@ }, "error.unknown": { "message": "Operazione non riuscita." + }, + "error.elevation_unavailable": { + "message": "NetBird non ha potuto richiedere a questo sistema i privilegi necessari. Esegua invece questo:" + }, + "error.elevation_failed": { + "message": "Non è stato possibile applicare la modifica con privilegi elevati. Esegua invece questo:" + }, + "settings.ssh.privilege.actorRoot": { + "message": "i privilegi di root" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "i privilegi di amministratore" + }, + "settings.ssh.privilege.hint": { + "message": "Richiede {actor}. Esegua invece questo:" + }, + "settings.ssh.privilege.oneWay": { + "message": "Può disabilitarlo, ma riabilitarlo richiede {actor}." + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "Può abilitarlo, ma disabilitarlo di nuovo richiede {actor}." + }, + "settings.ssh.privilege.authorizePending": { + "message": "In attesa dell'autorizzazione…" } } diff --git a/client/ui/i18n/locales/ja/common.json b/client/ui/i18n/locales/ja/common.json index 326c825bf..4f9875680 100644 --- a/client/ui/i18n/locales/ja/common.json +++ b/client/ui/i18n/locales/ja/common.json @@ -1304,6 +1304,9 @@ "daemon.outdated.description": { "message": "このアプリを使用するには NetBird サービスを更新してください。" }, + "daemon.outdated.download": { + "message": "最新版をダウンロード" + }, "error.jwt_clock_skew": { "message": "サインインに失敗しました: このデバイスの時計がサーバーと同期していません。システムの時計を同期してからもう一度お試しください。" }, @@ -1327,5 +1330,29 @@ }, "error.unknown": { "message": "操作に失敗しました。" + }, + "error.elevation_unavailable": { + "message": "NetBird はこのシステムに必要な権限を要求できませんでした。代わりに次のコマンドを実行してください:" + }, + "error.elevation_failed": { + "message": "昇格した権限でも変更を適用できませんでした。代わりに次のコマンドを実行してください:" + }, + "settings.ssh.privilege.actorRoot": { + "message": "root 権限" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "管理者権限" + }, + "settings.ssh.privilege.hint": { + "message": "{actor}が必要です。代わりに次のコマンドを実行してください:" + }, + "settings.ssh.privilege.oneWay": { + "message": "無効にはできますが、再度有効にするには{actor}が必要です。" + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "有効にはできますが、再度無効にするには{actor}が必要です。" + }, + "settings.ssh.privilege.authorizePending": { + "message": "承認を待っています…" } } diff --git a/client/ui/i18n/locales/pt/common.json b/client/ui/i18n/locales/pt/common.json index 37b02d5a8..d08e8f230 100644 --- a/client/ui/i18n/locales/pt/common.json +++ b/client/ui/i18n/locales/pt/common.json @@ -401,9 +401,6 @@ "networks.bulk.label": { "message": "Alternar todos os recursos visíveis" }, - "settings.nav.label": { - "message": "Seções das configurações" - }, "profile.switch.title": { "message": "Alternar perfil para \"{name}\"?" }, @@ -497,6 +494,9 @@ "settings.error.debugBundleTitle": { "message": "Falha no pacote de depuração" }, + "settings.nav.label": { + "message": "Seções das configurações" + }, "settings.tabs.general": { "message": "Geral" }, @@ -1330,5 +1330,29 @@ }, "error.unknown": { "message": "A operação falhou." + }, + "error.elevation_unavailable": { + "message": "O NetBird não conseguiu solicitar a este sistema os privilégios necessários. Execute isto em vez disso:" + }, + "error.elevation_failed": { + "message": "Não foi possível aplicar a alteração com privilégios elevados. Execute isto em vez disso:" + }, + "settings.ssh.privilege.actorRoot": { + "message": "privilégios de root" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "privilégios de administrador" + }, + "settings.ssh.privilege.hint": { + "message": "Requer {actor}. Execute isto em vez disso:" + }, + "settings.ssh.privilege.oneWay": { + "message": "Você pode desativar isto, mas ativar novamente requer {actor}." + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "Você pode ativar isto, mas desativar novamente requer {actor}." + }, + "settings.ssh.privilege.authorizePending": { + "message": "Aguardando a autorização…" } } diff --git a/client/ui/i18n/locales/ru/common.json b/client/ui/i18n/locales/ru/common.json index b9ae59df2..75139496f 100644 --- a/client/ui/i18n/locales/ru/common.json +++ b/client/ui/i18n/locales/ru/common.json @@ -401,9 +401,6 @@ "networks.bulk.label": { "message": "Переключить все видимые ресурсы" }, - "settings.nav.label": { - "message": "Разделы настроек" - }, "profile.switch.title": { "message": "Переключиться на профиль «{name}»?" }, @@ -497,6 +494,9 @@ "settings.error.debugBundleTitle": { "message": "Не удалось создать отладочный пакет" }, + "settings.nav.label": { + "message": "Разделы настроек" + }, "settings.tabs.general": { "message": "Общие" }, @@ -1330,5 +1330,29 @@ }, "error.unknown": { "message": "Не удалось выполнить операцию." + }, + "error.elevation_unavailable": { + "message": "NetBird не смог запросить у этой системы нужные права. Выполните вместо этого:" + }, + "error.elevation_failed": { + "message": "Не удалось применить изменение с повышенными правами. Выполните вместо этого:" + }, + "settings.ssh.privilege.actorRoot": { + "message": "права root" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "права администратора" + }, + "settings.ssh.privilege.hint": { + "message": "Требуются {actor}. Выполните вместо этого:" + }, + "settings.ssh.privilege.oneWay": { + "message": "Отключить можно, но чтобы включить снова, нужны {actor}." + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "Включить можно, но чтобы отключить снова, нужны {actor}." + }, + "settings.ssh.privilege.authorizePending": { + "message": "Ожидание авторизации…" } } diff --git a/client/ui/i18n/locales/zh-CN/common.json b/client/ui/i18n/locales/zh-CN/common.json index 2141a770d..a7fb2294c 100644 --- a/client/ui/i18n/locales/zh-CN/common.json +++ b/client/ui/i18n/locales/zh-CN/common.json @@ -401,9 +401,6 @@ "networks.bulk.label": { "message": "切换所有可见资源" }, - "settings.nav.label": { - "message": "设置部分" - }, "profile.switch.title": { "message": "切换到配置文件“{name}”？" }, @@ -497,6 +494,9 @@ "settings.error.debugBundleTitle": { "message": "创建调试包失败" }, + "settings.nav.label": { + "message": "设置部分" + }, "settings.tabs.general": { "message": "常规" }, @@ -1330,5 +1330,29 @@ }, "error.unknown": { "message": "操作失败。" + }, + "error.elevation_unavailable": { + "message": "NetBird 无法向此系统请求所需的权限。请改为运行：" + }, + "error.elevation_failed": { + "message": "即使使用提升的权限也无法应用此更改。请改为运行：" + }, + "settings.ssh.privilege.actorRoot": { + "message": "root 权限" + }, + "settings.ssh.privilege.actorAdministrator": { + "message": "管理员权限" + }, + "settings.ssh.privilege.hint": { + "message": "需要{actor}。请改为运行：" + }, + "settings.ssh.privilege.oneWay": { + "message": "您可以关闭此项，但重新开启需要{actor}。" + }, + "settings.ssh.privilege.oneWayInverted": { + "message": "您可以开启此项，但再次关闭需要{actor}。" + }, + "settings.ssh.privilege.authorizePending": { + "message": "正在等待授权…" } } diff --git a/client/ui/main.go b/client/ui/main.go index e2d172e5b..1b827034c 100644 --- a/client/ui/main.go +++ b/client/ui/main.go @@ -8,6 +8,7 @@ import ( "flag" "io/fs" "log" + "os" "runtime" "strings" @@ -79,6 +80,14 @@ func init() { } func main() { + // The one-shot that applies the settings the daemon restricts to + // root/administrator, which this binary runs itself as under the platform's + // elevation prompt. Handled before anything GUI so no window, tray or + // single-instance lock is involved. + if services.IsPrivilegedSettingsRun(os.Args[1:]) { + os.Exit(runPrivilegedSettings(os.Args[1:])) + } + daemonAddr, userSetLogFile := parseFlagsAndInitLog() conn := NewConn(daemonAddr) diff --git a/client/ui/privileged_settings.go b/client/ui/privileged_settings.go new file mode 100644 index 000000000..1e8b4bbf6 --- /dev/null +++ b/client/ui/privileged_settings.go @@ -0,0 +1,27 @@ +//go:build !android && !ios && !freebsd && !js + +package main + +import ( + "github.com/netbirdio/netbird/client/proto" + "github.com/netbirdio/netbird/client/ui/services" +) + +// The one-shot mode this binary runs itself in, elevated, to apply the settings the +// daemon restricts to root/administrator. It is handled before anything GUI, so no +// window, tray or single-instance lock is involved. +// +// Only the wiring is here: what the mode accepts and does lives beside the code +// that asks for it, in services.RunPrivilegedSettings, so the settings it will +// apply are declared once. There is nothing privileged about the mode itself; it +// sends the same request the frontend would have sent, and the daemon authorizes it +// from the identity the kernel reports on the control channel exactly as it does +// for `sudo netbird up`. +func runPrivilegedSettings(args []string) int { + return services.RunPrivilegedSettings(args, func(addr string) (proto.DaemonServiceClient, error) { + if addr == "" { + addr = DaemonAddr() + } + return NewConn(addr).Client() + }) +} diff --git a/client/ui/services/guarded.go b/client/ui/services/guarded.go new file mode 100644 index 000000000..13a47ccbd --- /dev/null +++ b/client/ui/services/guarded.go @@ -0,0 +1,233 @@ +//go:build !android && !ios && !freebsd && !js + +package services + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/elevate" + "github.com/netbirdio/netbird/client/internal/ipcauth" +) + +// The command line of the one-shot mode this binary runs itself in, elevated, to +// apply a setting the daemon restricts to root/administrator. Named here, next to +// the code that builds the arguments; parsed by runPrivilegedSettings in the main +// package. The setting flags deliberately spell the same words as `netbird up`, +// so what the user is shown as a command and what runs behind the prompt read +// alike. +const ( + FlagApplyPrivilegedSettings = "apply-privileged-settings" + FlagDaemonAddr = "daemon-addr" + FlagProfile = "profile" + FlagUser = "user" + FlagLogLevel = "log-level" + FlagManagementURL = "management-url" + FlagAllowServerSSH = "allow-server-ssh" + FlagEnableSSHRoot = "enable-ssh-root" + FlagDisableSSHAuth = "disable-ssh-auth" +) + +// Error codes for the ways asking for privileges can fail. +const ( + CodeElevationUnavailable = "elevation_unavailable" + CodeElevationFailed = "elevation_failed" +) + +// elevationTimeout bounds the wait for a prompt and the change behind it, so an +// authentication dialog nobody ever answers does not leave the control it belongs +// to disabled for the rest of the session. Long enough to find a password manager, +// and no shorter than the platforms' own prompt timeouts (Windows gives up on its +// consent dialog after two minutes by itself). +// +// How much it can actually interrupt differs. On Linux the prompt is a child +// process and is killed with the context; on Windows the wait for it is +// interruptible. On macOS the dialog belongs to Security.framework, which offers no +// way to withdraw the request, so there the timeout only stops us waiting — the +// system's own dialog timeout is what ends it. +const elevationTimeout = 5 * time.Minute + +// elevator raises the platform's privilege prompt and runs the change behind it. +// An interface so tests can answer without a prompt. +type elevator interface { + // Run runs this binary again, elevated, with the given arguments. + Run(ctx context.Context, args ...string) error + // Available reports whether there is a prompt to raise on this host at all. + Available() bool +} + +// osElevator is the real thing: see the elevate package. +type osElevator struct{} + +func (osElevator) Run(ctx context.Context, args ...string) error { + return elevate.Run(ctx, args...) +} + +func (osElevator) Available() bool { + return elevate.Available() +} + +// SaveOutcome reports what became of a change that needed authorization. +// +// A declined prompt is a result, not an error: the user was asked and said no, so +// nothing was applied and nothing went wrong. Reporting it as an error would have +// every cancelled prompt logged as one. +type SaveOutcome struct { + // Declined is set when the user dismissed the authorization prompt, or was + // refused by policy. Nothing was changed. + Declined bool `json:"declined"` +} + +// GuardedSettings is the subset of the config the daemon restricts to +// root/administrator. Only the fields that are set are changed: a nil pointer, or +// an empty management URL, leaves that setting alone. +// +// The management URL is in here because pointing a host with the SSH server +// running at another management identity hands the decision of who may open a +// shell on it to whoever runs that server, which is the same power as enabling +// the SSH server in the first place. +type GuardedSettings struct { + ProfileName string `json:"profileName"` + Username string `json:"username"` + ManagementURL string `json:"managementUrl,omitempty"` + ServerSSHAllowed *bool `json:"serverSshAllowed,omitempty"` + EnableSSHRoot *bool `json:"enableSshRoot,omitempty"` + DisableSSHAuth *bool `json:"disableSshAuth,omitempty"` +} + +// guardedSetting is one setting to change, in the two spellings this needs: the +// one-shot's own flag, and the `netbird up` flag that does the same thing from a +// terminal, for when there is no prompt to raise. +type guardedSetting struct { + arg string + flag string +} + +// SetGuardedSettings applies settings the daemon refuses from an unprivileged +// caller, by having the operating system run this binary again, elevated, to send +// the same request the frontend would have sent itself. +// +// The user authorizes it at the platform's own prompt: the UAC consent dialog, +// the macOS authentication dialog, or the polkit agent's. Any credentials are the +// operating system's business; NetBird neither sees nor asks for them. Nothing +// about the daemon's rules changes, and the elevated process is authorized like +// any other privileged caller, from the identity the kernel reports for it. +// +// A declined prompt comes back as SaveOutcome.Declined with no error. When there is +// no prompt to raise, or the elevated run failed, the error carries the command +// that does the same thing from a terminal. +func (s *Settings) SetGuardedSettings(ctx context.Context, p GuardedSettings) (SaveOutcome, error) { + settings := guardedSettings(p) + if len(settings) == 0 { + return SaveOutcome{}, &ClientError{ + Code: CodeElevationFailed, + Short: "no setting to apply", + Long: "no setting to apply", + } + } + + args := append([]string{ + "--" + FlagApplyPrivilegedSettings, + "--" + FlagDaemonAddr, s.daemonAddr, + "--" + FlagProfile, p.ProfileName, + "--" + FlagUser, p.Username, + }, oneShotArgs(settings)...) + + ctx, cancel := context.WithTimeout(ctx, elevationTimeout) + defer cancel() + + // Both ends of it: when the prompt went up, and what came of it. These are + // changes that hand out shells on this host, so the log should say who was + // asked and when, and it is also the only account of a prompt that was slow to + // appear or never answered. The daemon records the change itself, against the + // identity it authorized. + log.Infof("asking for privileges to apply %s", guardedSummary(p)) + + if err := s.elevator.Run(ctx, args...); err != nil { + return s.elevationOutcome(err, p) + } + + log.Infof("applied %s with the privileges the user authorized", guardedSummary(p)) + return SaveOutcome{}, nil +} + +// elevationOutcome sorts what came back into the one normal ending and the two +// that need reporting, with the command that does the same thing by hand. +func (s *Settings) elevationOutcome(err error, p GuardedSettings) (SaveOutcome, error) { + switch { + case errors.Is(err, elevate.ErrDeclined): + // With the reason: an account that may not elevate at all lands here too, + // and the log is the only place that says which it was. + log.Infof("the elevation prompt for %s was declined: %v", guardedSummary(p), err) + return SaveOutcome{Declined: true}, nil + case errors.Is(err, elevate.ErrUnavailable): + log.Warnf("cannot ask for privileges to apply %s: %v", guardedSummary(p), err) + return SaveOutcome{}, &ClientError{ + Code: CodeElevationUnavailable, + Short: s.classifier.translateShort(CodeElevationUnavailable), + Long: err.Error(), + Command: guardedCommand(p), + } + default: + log.Errorf("applying %s with elevated privileges failed: %v", guardedSummary(p), err) + return SaveOutcome{}, &ClientError{ + Code: CodeElevationFailed, + Short: s.classifier.translateShort(CodeElevationFailed), + Long: err.Error(), + Command: guardedCommand(p), + } + } +} + +// guardedSettings renders the settings that are actually being changed, from the +// same table the one-shot parses them with: see oneshot.go. +func guardedSettings(p GuardedSettings) []guardedSetting { + var settings []guardedSetting + for _, field := range guardedFields { + value, ok := field.read(p) + if !ok { + continue + } + settings = append(settings, guardedSetting{ + arg: "--" + field.flag + "=" + value, + flag: field.up(value), + }) + } + return settings +} + +func oneShotArgs(settings []guardedSetting) []string { + args := make([]string, 0, len(settings)) + for _, setting := range settings { + args = append(args, setting.arg) + } + return args +} + +func upFlags(settings []guardedSetting) []string { + flags := make([]string, 0, len(settings)) + for _, setting := range settings { + flags = append(flags, setting.flag) + } + return flags +} + +// guardedCommand is the elevated command line equivalent to the requested +// change, the same shape the daemon names in its own refusals. +func guardedCommand(p GuardedSettings) string { + settings := guardedSettings(p) + if len(settings) == 0 { + return "" + } + return ipcauth.UpCommand(strings.Join(upFlags(settings), " ")) +} + +// guardedSummary names the change for the log. +func guardedSummary(p GuardedSettings) string { + return fmt.Sprintf("%v for profile %q", oneShotArgs(guardedSettings(p)), p.ProfileName) +} diff --git a/client/ui/services/guarded_test.go b/client/ui/services/guarded_test.go new file mode 100644 index 000000000..237468ea3 --- /dev/null +++ b/client/ui/services/guarded_test.go @@ -0,0 +1,290 @@ +//go:build !android && !ios && !freebsd && !js + +package services + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/genproto/googleapis/rpc/errdetails" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/internal/elevate" + "github.com/netbirdio/netbird/client/internal/ipcauth" + "github.com/netbirdio/netbird/client/proto" +) + +// A Unix socket, so the daemon address is one that carries a caller's identity and +// elevation is worth offering at all: see Settings.canElevate. +const testDaemonAddr = "unix:///var/run/netbird.sock" + +// stubElevator stands in for the platform's prompt: it records what would have run +// and answers with a fixed outcome. +type stubElevator struct { + outcome error + available bool + calls [][]string +} + +func (e *stubElevator) Run(_ context.Context, args ...string) error { + e.calls = append(e.calls, args) + return e.outcome +} + +func (e *stubElevator) Available() bool { return e.available } + +// stubDaemon implements only the RPC under test. The embedded interface is nil, so +// any other call panics rather than passing quietly. +type stubDaemon struct { + proto.DaemonServiceClient + setConfig func(*proto.SetConfigRequest) error + requests []*proto.SetConfigRequest +} + +func (d *stubDaemon) SetConfig(_ context.Context, in *proto.SetConfigRequest, _ ...grpc.CallOption) (*proto.SetConfigResponse, error) { + d.requests = append(d.requests, in) + if err := d.setConfig(in); err != nil { + return nil, err + } + return &proto.SetConfigResponse{}, nil +} + +type stubConn struct{ client proto.DaemonServiceClient } + +func (c stubConn) Client() (proto.DaemonServiceClient, error) { return c.client, nil } + +// privilegeRefusal is the error the daemon raises for a change it restricts to +// root, detail and all: see server.privilegeError. +func privilegeRefusal(t *testing.T) error { + t.Helper() + + st, err := gstatus.New(codes.PermissionDenied, "Changing the management URL requires root."). + WithDetails(&errdetails.ErrorInfo{ + Reason: ipcauth.ErrorReasonPrivilegeRequired, + Domain: ipcauth.ErrorDomain, + Metadata: map[string]string{ + ipcauth.ErrorMetaSummary: "Changing the management URL requires root.", + ipcauth.ErrorMetaCommand: "sudo netbird down; sudo netbird up -m https://mgmt.example.com", + }, + }) + require.NoError(t, err, "build the refusal detail") + return st.Err() +} + +func settingsWithElevation(t *testing.T, outcome error) (*Settings, *stubElevator) { + t.Helper() + + elev := &stubElevator{outcome: outcome, available: true} + return &Settings{daemonAddr: testDaemonAddr, elevator: elev}, elev +} + +// settingsRefusingOnce returns a Settings whose daemon refuses the first SetConfig +// for want of privileges and accepts anything after it. +func settingsRefusingOnce(t *testing.T, elev *stubElevator) (*Settings, *stubDaemon) { + t.Helper() + + refusal := privilegeRefusal(t) + daemon := &stubDaemon{} + daemon.setConfig = func(*proto.SetConfigRequest) error { + if len(daemon.requests) == 1 { + return refusal + } + return nil + } + return &Settings{conn: stubConn{client: daemon}, daemonAddr: testDaemonAddr, elevator: elev}, daemon +} + +func TestSetGuardedSettingsPassesOnlyTheChangedSettings(t *testing.T) { + s, elev := settingsWithElevation(t, nil) + + root := true + outcome, err := s.SetGuardedSettings(context.Background(), GuardedSettings{ + ProfileName: "work", + Username: "vma", + EnableSSHRoot: &root, + }) + require.NoError(t, err) + assert.False(t, outcome.Declined, "the prompt was answered") + + want := []string{ + "--" + FlagApplyPrivilegedSettings, + "--" + FlagDaemonAddr, testDaemonAddr, + "--" + FlagProfile, "work", + "--" + FlagUser, "vma", + "--" + FlagEnableSSHRoot + "=true", + } + require.Len(t, elev.calls, 1, "one prompt for one change") + assert.Equal(t, want, elev.calls[0], "elevated arguments") +} + +// Turning a setting off has to be as explicit as turning it on: a bare flag would +// read as "on" to the one-shot's parser. +func TestSetGuardedSettingsSpellsOutFalse(t *testing.T) { + s, elev := settingsWithElevation(t, nil) + + off := false + _, err := s.SetGuardedSettings(context.Background(), GuardedSettings{ + ProfileName: "default", + ServerSSHAllowed: &off, + DisableSSHAuth: &off, + }) + require.NoError(t, err) + + args := elev.calls[0] + assert.Contains(t, args, "--"+FlagAllowServerSSH+"=false", "the setting being switched off") + assert.Contains(t, args, "--"+FlagDisableSSHAuth+"=false", "the setting being switched off") + assert.NotContains(t, args, "--"+FlagEnableSSHRoot+"=false", "no flag for a setting nobody touched") +} + +func TestSetGuardedSettingsPassesTheManagementURL(t *testing.T) { + s, elev := settingsWithElevation(t, nil) + + _, err := s.SetGuardedSettings(context.Background(), GuardedSettings{ + ProfileName: "default", + ManagementURL: "https://mgmt.example.com:33073", + }) + require.NoError(t, err) + + assert.Contains(t, elev.calls[0], "--"+FlagManagementURL+"=https://mgmt.example.com:33073", + "the management URL to point the profile at") +} + +func TestSetGuardedSettingsWithoutASettingDoesNotElevate(t *testing.T) { + s, elev := settingsWithElevation(t, nil) + + _, err := s.SetGuardedSettings(context.Background(), GuardedSettings{ProfileName: "default"}) + + require.Error(t, err, "nothing to apply is not something to prompt for") + assert.Empty(t, elev.calls, "no prompt at all") +} + +// A declined prompt is the one ending that is not an error: reporting it as one +// would have every cancelled prompt logged as a failure. +func TestSetGuardedSettingsReportsADeclinedPromptAsAnOutcome(t *testing.T) { + s, _ := settingsWithElevation(t, elevate.ErrDeclined) + + root := true + outcome, err := s.SetGuardedSettings(context.Background(), GuardedSettings{ + ProfileName: "default", + EnableSSHRoot: &root, + }) + + require.NoError(t, err, "the user was asked and answered; nothing went wrong") + assert.True(t, outcome.Declined, "nothing was applied") +} + +func TestSetGuardedSettingsMapsFailures(t *testing.T) { + tests := []struct { + name string + outcome error + wantCode string + }{ + { + // Nothing to raise a prompt with: the user needs the command. + name: "no mechanism falls back to the command", + outcome: elevate.ErrUnavailable, + wantCode: CodeElevationUnavailable, + }, + { + name: "a failed run falls back to the command", + outcome: errors.New("elevated netbird exited with 1"), + wantCode: CodeElevationFailed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s, _ := settingsWithElevation(t, tt.outcome) + + root := true + _, err := s.SetGuardedSettings(context.Background(), GuardedSettings{ + ProfileName: "default", + EnableSSHRoot: &root, + }) + + var clientErr *ClientError + require.ErrorAs(t, err, &clientErr, "the frontend needs a code to act on") + assert.Equal(t, tt.wantCode, clientErr.Code, "error code") + assert.Contains(t, clientErr.Command, "--"+FlagEnableSSHRoot+"=true", + "the setting in the fallback command") + assert.Contains(t, clientErr.Command, "netbird up", "the fallback command") + }) + } +} + +// Changing the management URL is only privileged while the host runs the SSH +// server, which no control can know up front, so the refusal is what triggers the +// prompt. The original request goes again afterwards, so the fields the one-shot +// does not understand are applied too. +func TestSetConfigElevatesAfterARefusalAndRetries(t *testing.T) { + elev := &stubElevator{available: true} + s, daemon := settingsRefusingOnce(t, elev) + + mtu := int64(1280) + outcome, err := s.SetConfig(context.Background(), SetConfigParams{ + ProfileName: "default", + ManagementURL: "https://mgmt.example.com", + MTU: &mtu, + }) + require.NoError(t, err) + assert.False(t, outcome.Declined, "the prompt was answered") + + require.Len(t, elev.calls, 1, "one prompt") + assert.Contains(t, elev.calls[0], "--"+FlagManagementURL+"=https://mgmt.example.com", + "the guarded part of the request") + require.Len(t, daemon.requests, 2, "the refused request and the retry") + assert.Equal(t, mtu, daemon.requests[1].GetMtu(), + "the retry carries the rest of the request, which the one-shot does not understand") +} + +func TestSetConfigDoesNotRetryWhenTheUserDeclines(t *testing.T) { + elev := &stubElevator{outcome: elevate.ErrDeclined, available: true} + s, daemon := settingsRefusingOnce(t, elev) + + outcome, err := s.SetConfig(context.Background(), SetConfigParams{ + ProfileName: "default", + ManagementURL: "https://mgmt.example.com", + }) + + require.NoError(t, err, "a declined prompt is not an error") + assert.True(t, outcome.Declined, "nothing was applied") + assert.Len(t, daemon.requests, 1, "only the refused request") +} + +// With no prompt to raise, the refusal is reported as the daemon wrote it, which is +// the guidance that was there before elevation existed. +func TestSetConfigReportsTheRefusalWhenItCannotElevate(t *testing.T) { + elev := &stubElevator{available: false} + s, _ := settingsRefusingOnce(t, elev) + + _, err := s.SetConfig(context.Background(), SetConfigParams{ + ProfileName: "default", + ManagementURL: "https://mgmt.example.com", + }) + + var clientErr *ClientError + require.ErrorAs(t, err, &clientErr) + assert.Equal(t, "privilege_required", clientErr.Code, "error code") + assert.Contains(t, clientErr.Command, "netbird up -m https://mgmt.example.com", + "the daemon's own command") + assert.Empty(t, elev.calls, "no prompt where there is none to raise") +} + +// A refusal with nothing in the request the one-shot could apply: the daemon +// cannot see who is calling, and being root would not help either. +func TestSetConfigReportsARefusalWithNothingToElevate(t *testing.T) { + elev := &stubElevator{available: true} + s, _ := settingsRefusingOnce(t, elev) + + _, err := s.SetConfig(context.Background(), SetConfigParams{ProfileName: "default"}) + + var clientErr *ClientError + require.ErrorAs(t, err, &clientErr) + assert.Equal(t, "privilege_required", clientErr.Code, "error code") + assert.Empty(t, elev.calls, "no prompt") +} diff --git a/client/ui/services/oneshot.go b/client/ui/services/oneshot.go new file mode 100644 index 000000000..676b08005 --- /dev/null +++ b/client/ui/services/oneshot.go @@ -0,0 +1,233 @@ +//go:build !android && !ios && !freebsd && !js + +package services + +import ( + "context" + "errors" + "flag" + "fmt" + "os" + "strconv" + "time" + + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/internal/elevate" + "github.com/netbirdio/netbird/client/proto" + "github.com/netbirdio/netbird/util" +) + +// The other end of SetGuardedSettings: the mode this binary runs itself in, +// elevated, to apply the settings the daemon restricts to root/administrator. +// +// Both ends are here on purpose. What may be changed this way is an allowlist, and +// an allowlist declared twice is one that will eventually disagree with itself, so +// the arguments are rendered and parsed from a single table: guardedFields. Adding +// a setting is one row; nothing generic passes through, and no field outside the +// table can be reached with an elevated request no matter what lands on the command +// line. + +// oneShotTimeout bounds the whole one-shot: connect, one RPC, exit. Generous +// because the user has just waited for an authentication dialog, and a failure here +// costs them the entire round trip. +const oneShotTimeout = 30 * time.Second + +// Exit codes the parent reads where the platform gives it one. +const ( + exitOK = 0 + exitFailure = 1 + exitUsage = 2 +) + +// guardedField is one setting the one-shot understands, in the two spellings it +// needs and with the two halves of its plumbing. +type guardedField struct { + // flag names it on the one-shot's command line. + flag string + usage string + // read returns the value to send and whether the caller asked for this setting + // at all. + read func(GuardedSettings) (string, bool) + // write parses a value from the command line onto the request. It is the only + // thing that validates the value, so it fails on anything it does not + // recognise rather than guessing. + write func(*proto.SetConfigRequest, string) error + // up renders the equivalent `netbird up` flag, for the fallback command shown + // when there is no prompt to raise. + up func(value string) string +} + +var guardedFields = []guardedField{ + { + flag: FlagManagementURL, + usage: "Management server the profile registers with.", + read: func(p GuardedSettings) (string, bool) { return p.ManagementURL, p.ManagementURL != "" }, + write: func(req *proto.SetConfigRequest, value string) error { + req.ManagementUrl = value + return nil + }, + // The daemon names this one as `-m ` in its own refusals. + up: func(value string) string { return "-m " + value }, + }, + boolField(FlagAllowServerSSH, "Run the NetBird SSH server.", + func(p GuardedSettings) *bool { return p.ServerSSHAllowed }, + func(req *proto.SetConfigRequest, v *bool) { req.ServerSSHAllowed = v }), + boolField(FlagEnableSSHRoot, "Allow SSH sessions to privileged accounts.", + func(p GuardedSettings) *bool { return p.EnableSSHRoot }, + func(req *proto.SetConfigRequest, v *bool) { req.EnableSSHRoot = v }), + boolField(FlagDisableSSHAuth, "Accept SSH sessions without authentication.", + func(p GuardedSettings) *bool { return p.DisableSSHAuth }, + func(req *proto.SetConfigRequest, v *bool) { req.DisableSSHAuth = v }), +} + +// fieldValue is a flag that remembers whether it was given, and requires a value: +// the renderer always writes one, so a bare flag is a caller that got it wrong. +type fieldValue struct { + set bool + value string +} + +func (v *fieldValue) String() string { + if v == nil { + return "" + } + return v.value +} + +func (v *fieldValue) Set(value string) error { + v.set, v.value = true, value + return nil +} + +// boolField describes a setting that is on or off. The value is always spelled out, +// so that turning a setting off is as unambiguous as turning it on and a flag with +// no value is a mistake rather than an "on". +func boolField( + name, usage string, + read func(GuardedSettings) *bool, + write func(*proto.SetConfigRequest, *bool), +) guardedField { + return guardedField{ + flag: name, + usage: usage, + read: func(p GuardedSettings) (string, bool) { + value := read(p) + if value == nil { + return "", false + } + return strconv.FormatBool(*value), true + }, + write: func(req *proto.SetConfigRequest, value string) error { + parsed, err := strconv.ParseBool(value) + if err != nil { + return fmt.Errorf("parse %q as a boolean: %w", value, err) + } + write(req, &parsed) + return nil + }, + up: func(value string) string { return "--" + name + "=" + value }, + } +} + +// IsPrivilegedSettingsRun reports whether this process was started as the one-shot. +// The flag is a marker rather than a value, so only the bare forms count: reading a +// value would mean "--flag=false" started it too. +func IsPrivilegedSettingsRun(args []string) bool { + for _, arg := range args { + if arg == "--"+FlagApplyPrivilegedSettings || arg == "-"+FlagApplyPrivilegedSettings { + return true + } + } + return false +} + +// RunPrivilegedSettings applies the requested settings and returns the process exit +// code. connect dials the daemon, which is the caller's business because only it +// knows how this build talks to it. +// +// Everything it reports goes to stderr, which is what the parent captures where the +// platform lets it. On success it says so on standard output, because macOS gives +// the parent no exit status to read: see elevate.AppliedMarker. +func RunPrivilegedSettings(args []string, connect func(addr string) (proto.DaemonServiceClient, error)) int { + fs := flag.NewFlagSet("netbird-ui --"+FlagApplyPrivilegedSettings, flag.ContinueOnError) + fs.Bool(FlagApplyPrivilegedSettings, false, "Apply the settings the daemon restricts to root/administrator and exit.") + daemonAddr := fs.String(FlagDaemonAddr, "", "Daemon gRPC address: unix:///path, npipe://name or tcp://host:port") + logLevel := fs.String(FlagLogLevel, "info", "Log level: trace|debug|info|warn|error.") + profile := fs.String(FlagProfile, "", "Profile to change.") + username := fs.String(FlagUser, "", "Owner of the profile.") + + values := make([]fieldValue, len(guardedFields)) + for i, field := range guardedFields { + fs.Var(&values[i], field.flag, field.usage) + } + + if err := fs.Parse(args); err != nil { + return exitUsage + } + + if err := util.InitLog(*logLevel, "console"); err != nil { + fmt.Fprintf(os.Stderr, "init log: %v\n", err) + return exitFailure + } + + req, err := privilegedRequest(*profile, *username, values) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + return exitUsage + } + + ctx, cancel := context.WithTimeout(context.Background(), oneShotTimeout) + defer cancel() + + if err := applyPrivilegedSettings(ctx, *daemonAddr, req, connect); err != nil { + fmt.Fprintf(os.Stderr, "apply settings: %v\n", err) + return exitFailure + } + + fmt.Fprintln(os.Stdout, elevate.AppliedMarker) + return exitOK +} + +// privilegedRequest builds the request from the flags that were given, and refuses +// one that asks for nothing. +func privilegedRequest(profile, username string, values []fieldValue) (*proto.SetConfigRequest, error) { + req := &proto.SetConfigRequest{ProfileName: profile, Username: username} + + given := 0 + for i, field := range guardedFields { + if !values[i].set { + continue + } + if err := field.write(req, values[i].value); err != nil { + return nil, fmt.Errorf("--%s: %w", field.flag, err) + } + given++ + } + if given == 0 { + return nil, errors.New("no setting to apply") + } + return req, nil +} + +func applyPrivilegedSettings( + ctx context.Context, + daemonAddr string, + req *proto.SetConfigRequest, + connect func(addr string) (proto.DaemonServiceClient, error), +) error { + client, err := connect(daemonAddr) + if err != nil { + return err + } + if _, err := client.SetConfig(ctx, req); err != nil { + // Unwrapped: the daemon's message is written for a person, and a refusal + // elevation cannot fix has to say so where the parent can read it off + // stderr. + return errors.New(gstatus.Convert(err).Message()) + } + return nil +} + +// interface guard: the one-shot's flags are flag.Value. +var _ flag.Value = (*fieldValue)(nil) diff --git a/client/ui/services/oneshot_test.go b/client/ui/services/oneshot_test.go new file mode 100644 index 000000000..2c66a94f4 --- /dev/null +++ b/client/ui/services/oneshot_test.go @@ -0,0 +1,166 @@ +//go:build !android && !ios && !freebsd && !js + +package services + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/proto" +) + +func TestIsPrivilegedSettingsRun(t *testing.T) { + tests := []struct { + name string + args []string + want bool + }{ + {name: "no arguments"}, + {name: "double dash", args: []string{"--" + FlagApplyPrivilegedSettings}, want: true}, + {name: "single dash", args: []string{"-" + FlagApplyPrivilegedSettings}, want: true}, + { + name: "among other flags", + args: []string{"--daemon-addr", "unix:///tmp/x.sock", "--" + FlagApplyPrivilegedSettings}, + want: true, + }, + // A marker, not a value: the caller never passes one, and reading a value + // would mean "--flag=false" started the one-shot too. + {name: "with a value", args: []string{"--" + FlagApplyPrivilegedSettings + "=true"}}, + {name: "unrelated flags", args: []string{"--log-level", "debug"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, IsPrivilegedSettingsRun(tt.args), "args %v", tt.args) + }) + } +} + +// What SetGuardedSettings renders has to be what the one-shot reads back, for every +// setting in the table. This is the property that keeps the two ends of an allowlist +// from drifting, so it is checked field by field rather than by example. +func TestGuardedFieldsRoundTrip(t *testing.T) { + on, off := true, false + tests := []struct { + name string + settings GuardedSettings + want func(*testing.T, *proto.SetConfigRequest) + }{ + { + name: "management url", + settings: GuardedSettings{ManagementURL: "https://mgmt.example.com:33073"}, + want: func(t *testing.T, req *proto.SetConfigRequest) { + assert.Equal(t, "https://mgmt.example.com:33073", req.GetManagementUrl()) + }, + }, + { + name: "ssh server on", + settings: GuardedSettings{ServerSSHAllowed: &on}, + want: func(t *testing.T, req *proto.SetConfigRequest) { + require.NotNil(t, req.ServerSSHAllowed) + assert.True(t, *req.ServerSSHAllowed) + }, + }, + { + name: "ssh root off", + settings: GuardedSettings{EnableSSHRoot: &off}, + want: func(t *testing.T, req *proto.SetConfigRequest) { + require.NotNil(t, req.EnableSSHRoot, "an explicit false must survive, not read as absent") + assert.False(t, *req.EnableSSHRoot) + }, + }, + { + name: "ssh auth off", + settings: GuardedSettings{DisableSSHAuth: &on}, + want: func(t *testing.T, req *proto.SetConfigRequest) { + require.NotNil(t, req.DisableSSHAuth) + assert.True(t, *req.DisableSSHAuth) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := parseRendered(t, tt.settings) + tt.want(t, req) + }) + } +} + +// A setting nobody asked about must not arrive at the daemon at all: sending its +// zero value would change it. +func TestGuardedFieldsCarryOnlyWhatWasAsked(t *testing.T) { + on := true + req := parseRendered(t, GuardedSettings{ProfileName: "work", EnableSSHRoot: &on}) + + assert.Equal(t, "work", req.GetProfileName(), "profile") + require.NotNil(t, req.EnableSSHRoot) + assert.Nil(t, req.ServerSSHAllowed, "untouched setting") + assert.Nil(t, req.DisableSSHAuth, "untouched setting") + assert.Empty(t, req.GetManagementUrl(), "untouched setting") +} + +func TestPrivilegedRequestRejectsAnEmptyChange(t *testing.T) { + _, err := privilegedRequest("default", "vma", make([]fieldValue, len(guardedFields))) + require.Error(t, err, "nothing to apply is not a request worth sending as root") +} + +// A value the table cannot parse is refused rather than guessed at. +func TestPrivilegedRequestRejectsAnUnparseableValue(t *testing.T) { + values := make([]fieldValue, len(guardedFields)) + for i, field := range guardedFields { + if field.flag != FlagEnableSSHRoot { + continue + } + require.NoError(t, values[i].Set("perhaps")) + } + + _, err := privilegedRequest("default", "vma", values) + require.Error(t, err) + assert.Contains(t, err.Error(), FlagEnableSSHRoot, "which flag was wrong") +} + +// parseRendered puts the settings through both ends: rendered as the arguments the +// elevated process is given, then parsed as that process parses them. +func parseRendered(t *testing.T, p GuardedSettings) *proto.SetConfigRequest { + t.Helper() + + rendered := guardedSettings(p) + require.NotEmpty(t, rendered, "nothing rendered for %+v", p) + + values := make([]fieldValue, len(guardedFields)) + for _, setting := range rendered { + flag, value, found := splitFlag(setting.arg) + require.True(t, found, "rendered %q without a value", setting.arg) + + matched := false + for i, field := range guardedFields { + if field.flag != flag { + continue + } + require.NoError(t, values[i].Set(value)) + matched = true + } + require.True(t, matched, "rendered %q, which no field claims", setting.arg) + } + + req, err := privilegedRequest(p.ProfileName, p.Username, values) + require.NoError(t, err) + return req +} + +// splitFlag takes "--name=value" apart the way the flag package does. +func splitFlag(arg string) (name, value string, found bool) { + trimmed := arg + for len(trimmed) > 0 && trimmed[0] == '-' { + trimmed = trimmed[1:] + } + for i := 0; i < len(trimmed); i++ { + if trimmed[i] == '=' { + return trimmed[:i], trimmed[i+1:], true + } + } + return trimmed, "", false +} diff --git a/client/ui/services/settings.go b/client/ui/services/settings.go index 74e6f913c..dd4dd9b8b 100644 --- a/client/ui/services/settings.go +++ b/client/ui/services/settings.go @@ -44,12 +44,19 @@ type Restrictions struct { } // Privilege tells the frontend whether this process may perform the changes the -// daemon restricts to root/administrator, and carries the command for each so a -// disabled control can show the way to do it. +// daemon restricts to root/administrator, whether it can ask the operating +// system for the privileges instead, and the command for each so a control that +// can do neither can still show the way. type Privilege struct { Privileged bool `json:"privileged"` - // Actor names what the operation requires ("root", "administrator privileges"). - Actor string `json:"actor"` + // ActorKey identifies the principal the operation requires without wording it, + // so the frontend can name it in the user's language: see + // ipcauth.PrivilegedActorKey. The words are not sent, because English ones + // cannot be dropped into a translated sentence. + ActorKey string `json:"actorKey"` + // CanElevate reports whether a guarded control can offer to authorize the + // change through the platform's own prompt: see SetGuardedSettings. + CanElevate bool `json:"canElevate"` // Commands equivalent to the settings the daemon guards, ready to copy. AllowSSHServer string `json:"allowSshServer"` EnableSSHRoot string `json:"enableSshRoot"` @@ -128,6 +135,9 @@ type Settings struct { // daemonAddr is where the daemon listens, used to tell whether it runs as // this user and would therefore authorize us: see Privilege. daemonAddr string + // elevator raises the platform's privilege prompt when a change needs more + // rights than this process has. + elevator elevator } func NewSettings(conn DaemonConn, translator ErrorTranslator, prefs LanguagePreference, daemonAddr string) *Settings { @@ -135,6 +145,7 @@ func NewSettings(conn DaemonConn, translator ErrorTranslator, prefs LanguagePref conn: conn, classifier: errorClassifier{translator: translator, prefs: prefs}, daemonAddr: daemonAddr, + elevator: osElevator{}, } } @@ -180,10 +191,10 @@ func (s *Settings) GetConfig(ctx context.Context, p ConfigParams) (Config, error }, nil } -func (s *Settings) SetConfig(ctx context.Context, p SetConfigParams) error { +func (s *Settings) SetConfig(ctx context.Context, p SetConfigParams) (SaveOutcome, error) { cli, err := s.conn.Client() if err != nil { - return err + return SaveOutcome{}, err } req := &proto.SetConfigRequest{ ProfileName: p.ProfileName, @@ -215,19 +226,68 @@ func (s *Settings) SetConfig(ctx context.Context, p SetConfigParams) error { SshJWTCacheTTL: p.SSHJWTCacheTTL, } if _, err := cli.SetConfig(ctx, req); err != nil { + if _, refused := privilegeErrorInfo(err); refused { + return s.setConfigElevated(ctx, p, req, err) + } // Classified so the frontend gets the daemon's guidance instead of the - // gRPC envelope, which is what a refused privileged change looks like. - return s.classifier.classify(err) + // gRPC envelope. + return SaveOutcome{}, s.classifier.classify(err) } - return nil + return SaveOutcome{}, nil +} + +// setConfigElevated answers a request the daemon refused for want of privileges by +// asking the user to authorize it, and sending it again if they do. It is the same +// offer the SSH settings make up front, for the changes a control cannot know are +// guarded until it is told: repointing a profile at another management server is +// only privileged while that host runs the SSH server. +// +// Two steps, because the elevated one-shot deliberately understands only the +// settings the daemon guards: it applies those, and the original request then goes +// through as this user, its privileged parts now asking for nothing that is not +// already stored. Nothing was applied by the refused attempt — the daemon decides +// before it writes — so there is no half-applied state to undo either way. +func (s *Settings) setConfigElevated(ctx context.Context, p SetConfigParams, req *proto.SetConfigRequest, refusal error) (SaveOutcome, error) { + if !s.canElevate() { + return SaveOutcome{}, s.classifier.classify(refusal) + } + + guarded := GuardedSettings{ + ProfileName: p.ProfileName, + Username: p.Username, + ManagementURL: p.ManagementURL, + ServerSSHAllowed: p.ServerSSHAllowed, + EnableSSHRoot: p.EnableSSHRoot, + DisableSSHAuth: p.DisableSSHAuth, + } + if len(guardedSettings(guarded)) == 0 { + // Refused over something no prompt can settle, such as a control channel + // that carries no caller identity. Report the daemon's own guidance. + return SaveOutcome{}, s.classifier.classify(refusal) + } + + outcome, err := s.SetGuardedSettings(ctx, guarded) + if err != nil || outcome.Declined { + return outcome, err + } + + cli, err := s.conn.Client() + if err != nil { + return SaveOutcome{}, err + } + if _, err := cli.SetConfig(ctx, req); err != nil { + return SaveOutcome{}, s.classifier.classify(err) + } + return SaveOutcome{}, nil } // Privilege reports whether this UI process could carry out the changes the -// daemon restricts to root/administrator, and the command that performs the one -// users hit in the SSH settings. It applies the daemon's own rule to what it can -// see locally, so the frontend can present those controls as unavailable up front -// instead of letting a save fail. No daemon round-trip, so it also works while the -// daemon is down. +// daemon restricts to root/administrator, whether it can instead ask the +// operating system for the privileges when the user wants one of them, and the +// command that performs the ones users hit in the SSH settings. It applies the +// daemon's own rule to what it can see locally, so the frontend can decide up +// front how to present those controls instead of letting a save fail. No daemon +// round-trip, so it also works while the daemon is down. // // Being root or an elevated administrator is one way. The other is running as the // daemon's own user while the daemon is unprivileged, which the daemon accepts @@ -237,26 +297,40 @@ func (s *Settings) SetConfig(ctx context.Context, p SetConfigParams) error { func (s *Settings) Privilege() Privilege { id, err := ipcauth.CurrentProcessIdentity() if err != nil { - // Fail closed: report unprivileged, which only ever disables controls. + // Fail closed: report unprivileged, which only ever asks for more. log.Warnf("cannot read this process's identity, treating it as unprivileged: %v", err) - return newPrivilege(false) + return s.newPrivilege(false) } if id.IsPrivileged() { - return newPrivilege(true) + return s.newPrivilege(true) } - return newPrivilege(daemonaddr.DaemonRunsAsSelf(s.daemonAddr)) + return s.newPrivilege(daemonaddr.DaemonRunsAsSelf(s.daemonAddr)) } -func newPrivilege(privileged bool) Privilege { +func (s *Settings) newPrivilege(privileged bool) Privilege { return Privilege{ Privileged: privileged, - Actor: ipcauth.PrivilegedActor(), + ActorKey: ipcauth.PrivilegedActorKey(), + CanElevate: s.canElevate(), AllowSSHServer: ipcauth.UpCommand("--allow-server-ssh"), EnableSSHRoot: ipcauth.UpCommand("--enable-ssh-root"), DisableSSHAuth: ipcauth.UpCommand("--disable-ssh-auth"), } } +// canElevate reports whether offering the platform's elevation prompt would get +// the user anywhere. It needs a mechanism to raise the prompt with and a control +// channel that tells the daemon who is calling: on loopback TCP the daemon +// refuses these changes to everybody, root included, so a prompt there would +// only waste the user's password. +func (s *Settings) canElevate() bool { + if !daemonaddr.CarriesIdentity(s.daemonAddr) { + log.Debugf("not offering elevation: the daemon address %s carries no caller identity", s.daemonAddr) + return false + } + return s.elevator.Available() +} + func (s *Settings) GetRestrictions(ctx context.Context) (Restrictions, error) { cli, err := s.conn.Client() if err != nil {