diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 261083783..d9c0b416e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -234,12 +234,22 @@ cd client/ui task dev ``` -Pass daemon flags after `--`: +Pass daemon flags after `--`, pointing the UI at the socket the daemon serves: ``` -task dev -- --daemon-addr=tcp://127.0.0.1:41731 +task dev -- --daemon-addr=unix:///var/run/netbird.sock # Linux, macOS +task dev -- --daemon-addr=npipe://netbird # Windows ``` +On Windows the daemon serves a named pipe (`npipe://netbird`). Which path that +ends up being depends on what the daemon may create: as a service or elevated it +serves `\\.\pipe\ProtectedPrefix\Administrators\netbird`, which no unprivileged +process can take from it, and otherwise it falls back to `\\.\pipe\netbird`. +Clients try both and check who owns the pipe before using the plain one. Avoid +`tcp://127.0.0.1:41731`: loopback TCP carries no caller identity, so the daemon +refuses the operations that require an administrator and you will not exercise +those paths. + Production build (frontend assets embedded into the binary, output in `client/ui/bin/`): ``` diff --git a/client/cmd/daemon_error.go b/client/cmd/daemon_error.go new file mode 100644 index 000000000..0d5b1307e --- /dev/null +++ b/client/cmd/daemon_error.go @@ -0,0 +1,66 @@ +package cmd + +import ( + "errors" + "fmt" + "strings" + + "google.golang.org/genproto/googleapis/rpc/errdetails" + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/internal/ipcauth" +) + +// daemonCallError prepares a daemon error for display. A refusal the daemon +// raised because the operation needs root/administrator is already guidance +// written for the user, so it is surfaced on its own instead of buried under the +// gRPC envelope and the name of the RPC that hit it. Anything else is wrapped +// with context as usual. +func daemonCallError(context string, err error) error { + if guidance, ok := privilegeGuidance(err); ok { + return errors.New(guidance) + } + return fmt.Errorf("%s: %w", context, err) +} + +// privilegeGuidance renders the daemon's privilege refusal as a summary and the +// command that performs the operation with the privileges it needs. It reports +// false for any other error. +func privilegeGuidance(err error) (string, bool) { + info, ok := privilegeErrorInfo(err) + if !ok { + return "", false + } + + summary := info.GetMetadata()[ipcauth.ErrorMetaSummary] + command := info.GetMetadata()[ipcauth.ErrorMetaCommand] + if summary == "" { + // Detail without a summary: fall back to the status message, which + // carries the same text. + summary = strings.TrimSpace(gstatus.Convert(err).Message()) + } + if command == "" { + return summary, true + } + + return fmt.Sprintf("%s\n\n %s\n", summary, command), true +} + +// privilegeErrorInfo returns the daemon's privilege-refusal detail, if the error +// carries one. +func privilegeErrorInfo(err error) (*errdetails.ErrorInfo, bool) { + if err == nil { + return nil, false + } + + for _, detail := range gstatus.Convert(err).Details() { + info, ok := detail.(*errdetails.ErrorInfo) + if !ok { + continue + } + if info.GetReason() == ipcauth.ErrorReasonPrivilegeRequired && info.GetDomain() == ipcauth.ErrorDomain { + return info, true + } + } + return nil, false +} diff --git a/client/cmd/logout.go b/client/cmd/logout.go index 1a5281acb..dcd7b5075 100644 --- a/client/cmd/logout.go +++ b/client/cmd/logout.go @@ -46,7 +46,7 @@ var logoutCmd = &cobra.Command{ } if _, err := daemonClient.Logout(ctx, req); err != nil { - return fmt.Errorf("deregister: %v", err) + return daemonCallError("deregister", err) } cmd.Println("Deregistered successfully") diff --git a/client/cmd/root.go b/client/cmd/root.go index f1ef32717..ebaae7e3e 100644 --- a/client/cmd/root.go +++ b/client/cmd/root.go @@ -20,7 +20,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" daddr "github.com/netbirdio/netbird/client/internal/daemonaddr" "github.com/netbirdio/netbird/client/internal/profilemanager" @@ -91,6 +90,7 @@ var ( // Don't resolve for service commands — they create the socket, not connect to it. if !isServiceCmd(cmd) { daemonAddr = daddr.ResolveUnixDaemonAddr(daemonAddr) + daemonAddr = daddr.ResolveDaemonAddr(daemonAddr) } return nil }, @@ -143,10 +143,10 @@ func init() { defaultDaemonAddr := "unix:///var/run/netbird.sock" if runtime.GOOS == "windows" { - defaultDaemonAddr = "tcp://127.0.0.1:41731" + defaultDaemonAddr = daddr.WindowsPipeAddr } - rootCmd.PersistentFlags().StringVar(&daemonAddr, "daemon-addr", defaultDaemonAddr, "Daemon service address to serve CLI requests [unix|tcp]://[path|host:port]") + rootCmd.PersistentFlags().StringVar(&daemonAddr, "daemon-addr", defaultDaemonAddr, "Daemon service address to serve CLI requests [unix|tcp|npipe]://[path|host:port|name]") rootCmd.PersistentFlags().StringVarP(&managementURL, "management-url", "m", "", fmt.Sprintf("Management Service URL [http|https]://[host]:[port] (default \"%s\")", profilemanager.DefaultManagementURL)) rootCmd.PersistentFlags().StringVar(&adminURL, "admin-url", "", fmt.Sprintf("Admin Panel URL [http|https]://[host]:[port] (default \"%s\")", profilemanager.DefaultAdminURL)) rootCmd.PersistentFlags().StringVarP(&logLevel, "log-level", "l", "info", "sets NetBird log level") @@ -269,12 +269,10 @@ func DialClientGRPCServer(ctx context.Context, addr string) (*grpc.ClientConn, e ctx, cancel := context.WithTimeout(ctx, time.Second*10) defer cancel() - return grpc.DialContext( - ctx, - strings.TrimPrefix(addr, "tcp://"), - grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithBlock(), - ) + target, opts := daddr.DialTarget(addr) + opts = append(opts, grpc.WithBlock()) + + return grpc.DialContext(ctx, target, opts...) } // WithBackOff execute function in backoff cycle. diff --git a/client/cmd/service.go b/client/cmd/service.go index b0a56c71a..7410d60ea 100644 --- a/client/cmd/service.go +++ b/client/cmd/service.go @@ -33,10 +33,15 @@ var ( ) type program struct { - ctx context.Context - cancel context.CancelFunc - serv *grpc.Server - jsonServ *http.Server + ctx context.Context + cancel context.CancelFunc + serv *grpc.Server + jsonServ *http.Server + // jsonClient is the gateway's own connection to the daemon. It is held so + // shutting the gateway down also closes it: nothing else references it once + // the handlers are registered, so its transport goroutines would otherwise + // outlive the server. + jsonClient *grpc.ClientConn jsonServMu sync.Mutex serverInstance *server.Server serverInstanceMu sync.Mutex diff --git a/client/cmd/service_controller.go b/client/cmd/service_controller.go index 5ef13a0a6..9ba3bce25 100644 --- a/client/cmd/service_controller.go +++ b/client/cmd/service_controller.go @@ -5,6 +5,7 @@ package cmd import ( "context" "fmt" + "runtime" "time" "github.com/kardianos/service" @@ -13,6 +14,8 @@ import ( "github.com/spf13/cobra" "google.golang.org/grpc" + "github.com/netbirdio/netbird/client/internal/daemonaddr" + "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/client/proto" "github.com/netbirdio/netbird/client/server" "github.com/netbirdio/netbird/client/system" @@ -26,6 +29,31 @@ func validateJSONSocketFlags() error { return nil } +// daemonServerOptions installs the transport credentials that expose each +// caller's kernel-authenticated identity to the handlers, which is what lets +// the daemon require root/administrator for privileged operations. +// +// The handshake exchanges no bytes, so older CLI and UI binaries still +// interoperate. Callers on a TCP socket carry no identity at all: the daemon +// keeps serving them, and the privileged operations deny them, so a warning is +// logged to make the loss of functionality visible. +func daemonServerOptions(network string) []grpc.ServerOption { + if network == "tcp" { + log.Warnf("daemon is listening on TCP (%s): callers carry no verifiable identity over TCP, "+ + "so privileged operations (SSH root login, SSH auth, enabling the SSH server, management URL changes, "+ + "deregistration) will be denied. Use a unix socket, or npipe:// on Windows", daemonAddr) + return nil + } + + creds := ipcauth.NewTransportCredentials() + if creds == nil { + log.Warnf("daemon IPC has no peer-identity primitive on %s: privileged operations will be denied", runtime.GOOS) + return nil + } + + return []grpc.ServerOption{grpc.Creds(creds)} +} + func (p *program) Start(svc service.Service) error { // Start should not block. Do the actual work async. log.Info("starting NetBird service") //nolint @@ -37,68 +65,106 @@ func (p *program) Start(svc service.Service) error { // Collect static system and platform information system.UpdateStaticInfoAsync() - // in any case, even if configuration does not exists we run daemon to serve CLI gRPC API. - p.serv = grpc.NewServer() - - daemonListener, err := listenOnAddress(daemonAddr) - if err != nil { - return fmt.Errorf("listen daemon interface: %w", err) + // A daemon installed before named-pipe support has the loopback TCP address + // persisted. Move it to the named pipe so an upgraded daemon can identify + // its callers instead of silently serving an unauthenticated socket. + if migrated, ok := daemonaddr.MigrateLegacy(daemonAddr); ok { + log.Infof("daemon address %q predates named-pipe support, listening on %q so callers can be identified", daemonAddr, migrated) + daemonAddr = migrated } - var jsonListener *socketListener - if enableJSONSocket { - jsonListener, err = listenOnAddress(jsonSocket) - if err != nil { - _ = daemonListener.Close() - return fmt.Errorf("listen daemon JSON interface: %w", err) - } - } else { - removeStaleUnixSocketForAddress(jsonSocket) + network, _, err := parseListenAddress(daemonAddr) + if err != nil { + return fmt.Errorf("parse daemon address: %w", err) + } + + // in any case, even if configuration does not exists we run daemon to serve CLI gRPC API. + p.serv = grpc.NewServer(daemonServerOptions(network)...) + + daemonListener, jsonListener, err := listenDaemonSockets() + if err != nil { + return err } go func() { - defer daemonListener.Close() - if jsonListener != nil { - defer jsonListener.Close() - } - - if err := daemonListener.chmodUnixSocket("daemon"); err != nil { - log.Error(err) - return - } - if jsonListener != nil { - if err := jsonListener.chmodUnixSocket("daemon JSON"); err != nil { - log.Error(err) - return - } - } - - serverInstance := server.New(p.ctx, util.FindFirstLogPath(logFiles), configPath, profilesDisabled, updateSettingsDisabled, captureEnabled, networksDisabled) - if err := serverInstance.Start(); err != nil { - log.Fatalf("failed to start daemon: %v", err) - } - proto.RegisterDaemonServiceServer(p.serv, serverInstance) - - p.serverInstanceMu.Lock() - p.serverInstance = serverInstance - p.serverInstanceMu.Unlock() - - if jsonListener != nil { - if err := p.startJSONGateway(jsonListener, daemonAddr); err != nil { - log.Fatalf("failed to start daemon JSON server: %v", err) - } - } else { - log.Debug("daemon JSON socket disabled") - } - - log.Printf("started daemon server: %v", daemonListener.address) - if err := p.serv.Serve(daemonListener.Listener); err != nil { - log.Errorf("failed to serve daemon requests: %v", err) + // Fatal here rather than inside serve, so serve's deferred listener + // closes run before the process exits. + if err := p.serve(daemonListener, jsonListener); err != nil { + log.Fatalf("failed to %v", err) } }() return nil } +// listenDaemonSockets opens the daemon control socket and, when it is enabled, the +// JSON gateway socket. The control socket is closed again if the second one fails, +// so a failed start leaves nothing listening. The returned JSON listener is nil +// when the socket is disabled. +func listenDaemonSockets() (*socketListener, *socketListener, error) { + daemonListener, err := listenOnAddress(daemonAddr) + if err != nil { + return nil, nil, fmt.Errorf("listen daemon interface: %w", err) + } + + if !enableJSONSocket { + removeStaleUnixSocketForAddress(jsonSocket) + return daemonListener, nil, nil + } + + jsonListener, err := listenOnAddress(jsonSocket) + if err != nil { + if cerr := daemonListener.Close(); cerr != nil { + log.Debugf("close daemon listener: %v", cerr) + } + return nil, nil, fmt.Errorf("listen daemon JSON interface: %w", err) + } + + return daemonListener, jsonListener, nil +} + +// serve brings up the daemon server on an already-open control socket and blocks +// until it stops. jsonListener is nil when the JSON socket is disabled. A returned +// error means the daemon cannot run at all and the caller is expected to exit; the +// failures it recovers from on its own are logged here. +func (p *program) serve(daemonListener, jsonListener *socketListener) error { + defer daemonListener.Close() + if jsonListener != nil { + defer jsonListener.Close() + } + + // chmodUnixSocket is a no-op for a nil listener and for a non-unix one. + if err := daemonListener.chmodUnixSocket("daemon"); err != nil { + log.Error(err) + return nil + } + if err := jsonListener.chmodUnixSocket("daemon JSON"); err != nil { + log.Error(err) + return nil + } + + serverInstance := server.New(p.ctx, util.FindFirstLogPath(logFiles), configPath, profilesDisabled, updateSettingsDisabled, captureEnabled, networksDisabled) + if err := serverInstance.Start(); err != nil { + return fmt.Errorf("start daemon: %w", err) + } + proto.RegisterDaemonServiceServer(p.serv, serverInstance) + + p.serverInstanceMu.Lock() + p.serverInstance = serverInstance + p.serverInstanceMu.Unlock() + + if jsonListener == nil { + log.Debug("daemon JSON socket disabled") + } else if err := p.startJSONGateway(jsonListener, daemonAddr); err != nil { + return fmt.Errorf("start daemon JSON server: %w", err) + } + + log.Printf("started daemon server: %v", daemonListener.address) + if err := p.serv.Serve(daemonListener.Listener); err != nil { + log.Errorf("failed to serve daemon requests: %v", err) + } + return nil +} + func (p *program) Stop(srv service.Service) error { p.serverInstanceMu.Lock() if p.serverInstance != nil { @@ -113,8 +179,13 @@ func (p *program) Stop(srv service.Service) error { p.cancel() p.jsonServMu.Lock() - jsonServ := p.jsonServ + jsonServ, jsonClient := p.jsonServ, p.jsonClient p.jsonServMu.Unlock() + if jsonClient != nil { + if err := jsonClient.Close(); err != nil { + log.Debugf("close daemon JSON gateway client: %v", err) + } + } if jsonServ != nil { shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 2*time.Second) if err := jsonServ.Shutdown(shutdownCtx); err != nil { diff --git a/client/cmd/service_json_gateway.go b/client/cmd/service_json_gateway.go index 29c1a6456..b6864f338 100644 --- a/client/cmd/service_json_gateway.go +++ b/client/cmd/service_json_gateway.go @@ -5,27 +5,123 @@ package cmd import ( "context" "errors" + "fmt" "net" "net/http" - "strings" + "sync" "time" "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" log "github.com/sirupsen/logrus" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" + "github.com/netbirdio/netbird/client/internal/daemonaddr" + "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/client/proto" ) -func grpcGatewayEndpoint(addr string) string { - return strings.TrimPrefix(addr, "tcp://") +// jsonPeerIdentity is the context key under which the connecting HTTP client's +// identity is stashed for the lifetime of its connection. +type jsonPeerIdentity struct{} + +// jsonPeerIdentityValue pairs the identity with whether it could be read at +// all, so an unreadable identity is forwarded as "unknown" rather than omitted. +type jsonPeerIdentityValue struct { + id ipcauth.Identity + known bool +} + +// jsonConnContext reads the identity of the client connecting to the JSON +// socket and stashes it on the connection's context. The gateway re-dials the +// daemon in-process, so the daemon would otherwise see every JSON request as +// coming from the daemon itself. +func jsonConnContext(ctx context.Context, c net.Conn) context.Context { + value := jsonPeerIdentityValue{} + id, err := ipcauth.ConnIdentity(c) + if err != nil { + log.Warnf("json gateway: cannot read HTTP client identity, privileged operations will be denied for this connection: %v", err) + } else { + value.id = id + value.known = true + } + return context.WithValue(ctx, jsonPeerIdentity{}, value) +} + +// forwardIdentity stamps the HTTP client's identity onto every call the gateway +// makes to the daemon. +// +// It is an interceptor on the gateway's client connection rather than a +// runtime.WithMetadata annotator because grpc-gateway skips annotators when no +// request header maps to metadata, which an HTTP/1.0 request with no Host header +// over a unix socket achieves. The daemon would then receive no marker, see its own +// identity as the transport peer, and authorize the request as the daemon itself. +// An interceptor runs for every RPC whatever the request looked like. +func forwardIdentity(ctx context.Context) context.Context { + value, ok := ctx.Value(jsonPeerIdentity{}).(jsonPeerIdentityValue) + if !ok { + // No ConnContext ran for this request, so forward an unknown identity: + // the daemon must not mistake its own identity for the client's. + return ipcauth.WithForwardedIdentity(ctx, ipcauth.Identity{}, false) + } + return ipcauth.WithForwardedIdentity(ctx, value.id, value.known) +} + +func forwardIdentityUnary(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + return invoker(forwardIdentity(ctx), method, req, reply, cc, opts...) +} + +func forwardIdentityStream(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { + return streamer(forwardIdentity(ctx), desc, cc, method, opts...) +} + +// reservedHeaderWarning limits the dropped-header warning to the first occurrence. +var reservedHeaderWarning sync.Once + +// jsonIncomingHeaderMatcher keeps an HTTP client from supplying the metadata the +// gateway uses to forward its identity. grpc-gateway turns "Grpc-Metadata-" +// headers into gRPC metadata and joins them ahead of what its annotators add, so +// without this filter a JSON client could send its own x-netbird-fwd-uid and the +// daemon would authorize that instead of the client's real identity. +func jsonIncomingHeaderMatcher(key string) (string, bool) { + mapped, ok := runtime.DefaultHeaderMatcher(key) + if !ok { + return "", false + } + if ipcauth.IsReservedForwardKey(mapped) { + // Warn once: any client can send these on every request, so warning each + // time hands it a way to fill the log. The rest are debug-level. + reservedHeaderWarning.Do(func() { + log.Warnf("json gateway: dropping reserved header %q from a request: only the gateway may set the caller's identity", key) + }) + log.Debugf("json gateway: dropping reserved header %q", key) + return "", false + } + return mapped, true } func (p *program) startJSONGateway(jsonListener *socketListener, daemonEndpoint string) error { - mux := runtime.NewServeMux() - opts := []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())} - if err := proto.RegisterDaemonServiceHandlerFromEndpoint(p.ctx, mux, grpcGatewayEndpoint(daemonEndpoint), opts); err != nil { + if jsonListener.network == "tcp" { + log.Warnf("daemon JSON socket is listening on TCP (%s): callers carry no verifiable identity over TCP, "+ + "so privileged operations will be denied for JSON clients", jsonListener.address) + } + + mux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher(jsonIncomingHeaderMatcher)) + + // grpc.NewClient does not connect until the first request, so registering + // the handler here cannot block daemon startup. + target, opts := daemonaddr.DialTarget(daemonEndpoint) + opts = append(opts, + grpc.WithChainUnaryInterceptor(forwardIdentityUnary), + grpc.WithChainStreamInterceptor(forwardIdentityStream), + ) + conn, err := grpc.NewClient(target, opts...) + if err != nil { + return fmt.Errorf("create daemon client for JSON gateway: %w", err) + } + if err := proto.RegisterDaemonServiceHandler(p.ctx, mux, conn); err != nil { + if cerr := conn.Close(); cerr != nil { + log.Debugf("close daemon client after failed JSON gateway registration: %v", cerr) + } return err } @@ -35,10 +131,12 @@ func (p *program) startJSONGateway(jsonListener *socketListener, daemonEndpoint BaseContext: func(net.Listener) context.Context { return p.ctx }, + ConnContext: jsonConnContext, } p.jsonServMu.Lock() p.jsonServ = jsonServer + p.jsonClient = conn p.jsonServMu.Unlock() go func() { diff --git a/client/cmd/service_json_gateway_test.go b/client/cmd/service_json_gateway_test.go new file mode 100644 index 000000000..dfeef1c46 --- /dev/null +++ b/client/cmd/service_json_gateway_test.go @@ -0,0 +1,261 @@ +//go:build !windows && !ios && !android + +package cmd + +import ( + "context" + "net" + "net/http" + "path/filepath" + "testing" + "time" + + "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" + + "github.com/netbirdio/netbird/client/internal/ipcauth" +) + +// The JSON gateway runs inside the daemon and re-dials it locally, so every JSON +// request reaches a handler with the daemon's own identity as the transport peer. +// The gateway therefore forwards its HTTP client's identity as metadata, and the +// daemon authorizes that instead of itself. These tests drive the real wiring +// (jsonConnContext, forwardIdentity, jsonIncomingHeaderMatcher) and check the +// identity a handler would end up authorizing. + +// daemonSideCtx is what a handler sees for a gateway-relayed call. The transport +// peer must be this process's own identity: the gateway is the daemon, so the two +// cannot differ, and hardcoding root here instead would describe a state that +// never occurs. +func daemonSideCtx(t *testing.T, md metadata.MD) context.Context { + t.Helper() + self, err := ipcauth.CurrentProcessIdentity() + if err != nil { + t.Skipf("cannot read this process's identity: %v", err) + } + ctx := peer.NewContext(context.Background(), &peer.Peer{ + AuthInfo: ipcauth.AuthInfo{ + CommonAuthInfo: credentials.CommonAuthInfo{SecurityLevel: credentials.NoSecurity}, + Identity: self, + }, + }) + return metadata.NewIncomingContext(ctx, md) +} + +// gatewayMetadata reproduces what the daemon receives for a JSON request: the +// mux annotates the context from the request's headers, then the interceptor on the +// gateway's client connection stamps the caller's identity. The order matters, +// since the interceptor must win over anything a header put there. +func gatewayMetadata(t *testing.T, req *http.Request, ctx context.Context) metadata.MD { + t.Helper() + mux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher(jsonIncomingHeaderMatcher)) + annotated, err := runtime.AnnotateContext(ctx, mux, req, + "/daemon.DaemonService/SetConfig", + runtime.WithHTTPPathPattern("/daemon.DaemonService/SetConfig")) + if err != nil { + t.Fatalf("annotate: %v", err) + } + + md, ok := metadata.FromOutgoingContext(forwardIdentity(annotated)) + if !ok { + t.Fatal("the interceptor produced no metadata") + } + return md +} + +// clientCtx is the connection context jsonConnContext would have produced for an +// HTTP client whose identity the gateway could read. +func clientCtx(id ipcauth.Identity, known bool) context.Context { + return context.WithValue(context.Background(), jsonPeerIdentity{}, + jsonPeerIdentityValue{id: id, known: known}) +} + +// An HTTP client must not be able to name its own identity. grpc-gateway turns +// Grpc-Metadata- headers into gRPC metadata, so without the header filter and +// the interceptor overwriting the reserved keys, this request would authorize as +// uid 0. +func TestJSONGateway_ForgedIdentityHeaderIsDropped(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, "http://localhost/daemon.DaemonService/SetConfig", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Grpc-Metadata-X-Netbird-Fwd-Uid", "0") + req.Header.Set("Grpc-Metadata-X-Netbird-Fwd-Gid", "0") + req.Header.Set("Grpc-Metadata-X-Netbird-Fwd", "1") + req.Header.Set("Grpc-Metadata-X-Netbird-Fwd-Sid", "S-1-5-18") + + caller := ipcauth.Identity{UID: 31000, GID: 31000} + md := gatewayMetadata(t, req, clientCtx(caller, true)) + + id, ok := ipcauth.CallerIdentity(daemonSideCtx(t, md)) + if !ok { + t.Fatal("the forwarded identity should be usable") + } + if id.IsPrivileged() { + t.Errorf("forged header was believed: authorized as %v", id) + } + if id.UID != caller.UID { + t.Errorf("authorized as uid %d, want the real client %d", id.UID, caller.UID) + } +} + +// A request with no headers at all (HTTP/1.0 needs no Host, and a unix socket +// yields no host:port) makes grpc-gateway produce no metadata whatsoever and skip +// its annotators: "if len(pairs) == 0 { return ctx, nil, nil }" in +// runtime/context.go. That is why the identity is stamped by an interceptor +// instead. This is the case that previously reached the gate as the daemon itself. +func TestJSONGateway_HeaderlessRequestIsStillMarkedForwarded(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, "http://localhost/daemon.DaemonService/SetConfig", nil) + if err != nil { + t.Fatal(err) + } + req.Header = http.Header{} + req.Host = "" + + caller := ipcauth.Identity{UID: 31000, GID: 31000} + ctx := clientCtx(caller, true) + + // Pin the skip path itself: if grpc-gateway ever produced a pair here, this + // test would still pass below while no longer covering what it was written for. + mux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher(jsonIncomingHeaderMatcher)) + annotated, err := runtime.AnnotateContext(ctx, mux, req, + "/daemon.DaemonService/SetConfig", + runtime.WithHTTPPathPattern("/daemon.DaemonService/SetConfig")) + if err != nil { + t.Fatalf("annotate: %v", err) + } + if md, ok := metadata.FromOutgoingContext(annotated); ok { + t.Fatalf("grpc-gateway produced metadata %v for a headerless request; "+ + "this test no longer covers the annotator-skip path", md) + } + + md := gatewayMetadata(t, req, ctx) + + id, ok := ipcauth.CallerIdentity(daemonSideCtx(t, md)) + if !ok { + t.Fatal("the forwarded identity should be usable") + } + if id.UID != caller.UID || id.IsPrivileged() { + t.Errorf("authorized as %v, want the real client uid %d", id, caller.UID) + } +} + +// When the gateway cannot read its client's identity (a TCP JSON socket, say) it +// forwards the marker alone. The daemon must then report "unidentified" so the +// privileged operations refuse, rather than falling back to the gateway's own +// identity. +func TestJSONGateway_UnreadableClientIdentityIsUnidentified(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, "http://localhost/daemon.DaemonService/SetConfig", nil) + if err != nil { + t.Fatal(err) + } + + md := gatewayMetadata(t, req, clientCtx(ipcauth.Identity{}, false)) + + if id, ok := ipcauth.CallerIdentity(daemonSideCtx(t, md)); ok { + t.Errorf("a request with no client identity was authorized as %v", id) + } +} + +// A request that never passed through jsonConnContext (no stashed identity) must +// also come out unidentified rather than as the daemon. +func TestJSONGateway_MissingConnContextIsUnidentified(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, "http://localhost/daemon.DaemonService/SetConfig", nil) + if err != nil { + t.Fatal(err) + } + + md := gatewayMetadata(t, req, context.Background()) + + if id, ok := ipcauth.CallerIdentity(daemonSideCtx(t, md)); ok { + t.Errorf("a request with no connection context was authorized as %v", id) + } +} + +// End to end over a real unix socket: the gateway reads the connecting client's +// identity from the socket itself, so a client cannot present anything else. +func TestJSONGateway_IdentityComesFromTheSocket(t *testing.T) { + mux := runtime.NewServeMux(runtime.WithIncomingHeaderMatcher(jsonIncomingHeaderMatcher)) + + type observed struct { + md metadata.MD + } + seen := make(chan observed, 1) + + srv := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx, err := runtime.AnnotateContext(r.Context(), mux, r, + "/daemon.DaemonService/SetConfig", + runtime.WithHTTPPathPattern("/daemon.DaemonService/SetConfig")) + if err != nil { + t.Errorf("annotate: %v", err) + return + } + md, _ := metadata.FromOutgoingContext(forwardIdentity(ctx)) + seen <- observed{md: md} + w.WriteHeader(http.StatusOK) + }), + ReadHeaderTimeout: 5 * time.Second, + ConnContext: jsonConnContext, + } + + sock := filepath.Join(t.TempDir(), "http.sock") + ln, err := net.Listen("unix", sock) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := srv.Close(); err != nil { + t.Logf("close server: %v", err) + } + }) + go func() { + if err := srv.Serve(ln); err != nil && err != http.ErrServerClosed { + t.Logf("serve: %v", err) + } + }() + + conn, err := net.Dial("unix", sock) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := conn.Close(); err != nil { + t.Logf("close conn: %v", err) + } + }) + + // Forge the identity headers on the wire as well. + request := "POST /daemon.DaemonService/SetConfig HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Grpc-Metadata-X-Netbird-Fwd: 1\r\n" + + "Grpc-Metadata-X-Netbird-Fwd-Uid: 0\r\n" + + "Content-Length: 0\r\n\r\n" + if _, err := conn.Write([]byte(request)); err != nil { + t.Fatal(err) + } + + select { + case got := <-seen: + self, err := ipcauth.CurrentProcessIdentity() + if err != nil { + t.Skipf("cannot read this process's identity: %v", err) + } + // The socket peer is this test process, so that is the identity the + // gateway must forward, not the uid 0 the request asked for. + if uids := got.md.Get("x-netbird-fwd-uid"); len(uids) != 1 { + t.Fatalf("x-netbird-fwd-uid = %v, want exactly the gateway's own value", uids) + } + id, ok := ipcauth.CallerIdentity(daemonSideCtx(t, got.md)) + if !ok { + t.Fatal("the forwarded identity should be usable") + } + if id.UID != self.UID { + t.Errorf("authorized as uid %d, want the socket peer %d", id.UID, self.UID) + } + case <-time.After(5 * time.Second): + t.Fatal("the gateway never handled the request") + } +} diff --git a/client/cmd/service_params.go b/client/cmd/service_params.go index f25087a69..750b22ae6 100644 --- a/client/cmd/service_params.go +++ b/client/cmd/service_params.go @@ -13,6 +13,7 @@ import ( "github.com/spf13/cobra" "github.com/netbirdio/netbird/client/configs" + "github.com/netbirdio/netbird/client/internal/daemonaddr" "github.com/netbirdio/netbird/util" ) @@ -125,6 +126,13 @@ func applyServiceParams(cmd *cobra.Command, params *serviceParams) { if !rootCmd.PersistentFlags().Changed("daemon-addr") && params.DaemonAddr != "" { daemonAddr = params.DaemonAddr + // An install that predates named-pipe support has the loopback TCP + // address saved. Callers carry no identity over TCP, so move it to the + // pipe instead of restoring a socket the daemon cannot authorize on. + if migrated, ok := daemonaddr.MigrateLegacy(daemonAddr); ok { + cmd.Printf("Moving the saved daemon address from %s to %s so the daemon can identify its callers\n", daemonAddr, migrated) + daemonAddr = migrated + } } if !serviceCmd.PersistentFlags().Changed("json-socket") && params.JSONSocket != "" { diff --git a/client/cmd/service_pipe_other.go b/client/cmd/service_pipe_other.go new file mode 100644 index 000000000..c7cc72469 --- /dev/null +++ b/client/cmd/service_pipe_other.go @@ -0,0 +1,14 @@ +//go:build !windows + +package cmd + +import ( + "fmt" + "net" +) + +// listenNamedPipe is Windows-only: no other platform serves the daemon on a +// named pipe. +func listenNamedPipe(string) (net.Listener, string, error) { + return nil, "", fmt.Errorf("named pipes are only supported on Windows") +} diff --git a/client/cmd/service_pipe_windows.go b/client/cmd/service_pipe_windows.go new file mode 100644 index 000000000..b6e860f51 --- /dev/null +++ b/client/cmd/service_pipe_windows.go @@ -0,0 +1,41 @@ +//go:build windows + +package cmd + +import ( + "errors" + "fmt" + "net" + + "github.com/Microsoft/go-winio" + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/daemonaddr" + "github.com/netbirdio/netbird/client/internal/ipcauth" +) + +// listenNamedPipe creates the daemon control pipe and reports the path it ended +// up on. The security descriptor lets any local caller connect, as a Unix socket +// at 0666 does, and the privileged operations are authorized separately from the +// caller's token. +// +// The protected name comes first so that an unprivileged process cannot take the +// name before the service does. Creating it requires being an administrator or +// LocalSystem, so a daemon an ordinary user runs themselves, as in netstack mode, +// falls back to the plain name; clients try both and check who serves them. +func listenNamedPipe(name string) (net.Listener, string, error) { + var errs []error + for _, path := range daemonaddr.PipePaths(name) { + listener, err := winio.ListenPipe(path, &winio.PipeConfig{ + SecurityDescriptor: ipcauth.DefaultPipeSDDL(), + }) + if err != nil { + log.Debugf("not serving the daemon on %s: %v", path, err) + errs = append(errs, fmt.Errorf("%s: %w", path, err)) + continue + } + return listener, path, nil + } + + return nil, "", errors.Join(errs...) +} diff --git a/client/cmd/service_socket.go b/client/cmd/service_socket.go index f825a4062..ed1f001a7 100644 --- a/client/cmd/service_socket.go +++ b/client/cmd/service_socket.go @@ -26,6 +26,14 @@ func listenOnAddress(addr string) (*socketListener, error) { return nil, err } + if network == "npipe" { + listener, path, err := listenNamedPipe(address) + if err != nil { + return nil, err + } + return &socketListener{Listener: listener, network: network, address: path}, nil + } + if network == "unix" { removeStaleUnixSocket(address) } @@ -41,11 +49,11 @@ func listenOnAddress(addr string) (*socketListener, error) { func parseListenAddress(addr string) (string, string, error) { network, address, ok := strings.Cut(addr, "://") if !ok || network == "" || address == "" { - return "", "", fmt.Errorf("address must be in [unix|tcp]://[path|host:port] format: %q", addr) + return "", "", fmt.Errorf("address must be in [unix|tcp|npipe]://[path|host:port|name] format: %q", addr) } switch network { - case "unix", "tcp": + case "unix", "tcp", "npipe": return network, address, nil default: return "", "", fmt.Errorf("unsupported daemon address protocol: %v", network) diff --git a/client/cmd/up.go b/client/cmd/up.go index 2d9731f26..142bcf6bd 100644 --- a/client/cmd/up.go +++ b/client/cmd/up.go @@ -325,7 +325,7 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command, pm *profilemanager if st, ok := gstatus.FromError(err); ok && st.Code() == codes.Unavailable { log.Warnf("setConfig method is not available in the daemon: %s", st.Message()) } else { - return fmt.Errorf("call service setConfig method: %v", err) + return daemonCallError("call service setConfig method", err) } } @@ -379,7 +379,7 @@ func doDaemonUp(ctx context.Context, cmd *cobra.Command, client proto.DaemonServ } if loginErr != nil { - return fmt.Errorf("login failed: %v", loginErr) + return daemonCallError("login failed", loginErr) } if loginResp.NeedsSSOLogin { @@ -392,7 +392,7 @@ func doDaemonUp(ctx context.Context, cmd *cobra.Command, client proto.DaemonServ ProfileName: &profileID, Username: &username, }); err != nil { - return fmt.Errorf("call service up method: %v", err) + return daemonCallError("call service up method", err) } return nil diff --git a/client/internal/daemonaddr/owner.go b/client/internal/daemonaddr/owner.go new file mode 100644 index 000000000..c476f9ae6 --- /dev/null +++ b/client/internal/daemonaddr/owner.go @@ -0,0 +1,15 @@ +package daemonaddr + +// DaemonRunsAsSelf reports whether the daemon listening at addr runs as this very +// user. That is what makes an unprivileged daemon authorize this process for the +// changes it otherwise restricts to root or an administrator, so a client can tell +// up front whether those controls are usable instead of letting a save fail. +// +// It is answered from the ownership of the socket or pipe the daemon created, so it +// costs no round trip and needs no cooperation from the daemon. Ownership that +// cannot be read is reported as false, including for a TCP address, so a caller +// reading this as "the daemon would allow it" fails closed. The daemon remains the +// only thing that authorizes anything: this only decides what a client offers. +func DaemonRunsAsSelf(addr string) bool { + return daemonRunsAsSelf(addr) +} diff --git a/client/internal/daemonaddr/owner_unix.go b/client/internal/daemonaddr/owner_unix.go new file mode 100644 index 000000000..493e6528d --- /dev/null +++ b/client/internal/daemonaddr/owner_unix.go @@ -0,0 +1,40 @@ +//go:build !windows + +package daemonaddr + +import ( + "os" + "strings" + "syscall" + + log "github.com/sirupsen/logrus" +) + +// daemonRunsAsSelf compares the owner of the daemon's Unix socket with this +// process's uid. Root is not treated specially here: a root caller is privileged +// on its own merits, and a root-owned socket says nothing about the caller. +func daemonRunsAsSelf(addr string) bool { + path, ok := strings.CutPrefix(addr, "unix://") + if !ok { + return false + } + + info, err := os.Stat(path) + if err != nil { + log.Debugf("stat daemon socket %s: %v", path, err) + return false + } + + // Only a socket says anything about a daemon. A directory or a leftover + // regular file at that path is not one, and reading it as "the daemon runs as + // us" would offer controls the daemon then refuses. + if info.Mode()&os.ModeSocket == 0 { + return false + } + + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return false + } + return stat.Uid == uint32(os.Getuid()) +} diff --git a/client/internal/daemonaddr/owner_unix_test.go b/client/internal/daemonaddr/owner_unix_test.go new file mode 100644 index 000000000..363c7d95d --- /dev/null +++ b/client/internal/daemonaddr/owner_unix_test.go @@ -0,0 +1,62 @@ +//go:build !windows + +package daemonaddr + +import ( + "net" + "os" + "path/filepath" + "testing" +) + +// A socket this user created means the daemon runs as this user, which is the +// rootless case where the daemon delegates its authority to its own identity. +func TestDaemonRunsAsSelf_OwnSocket(t *testing.T) { + path := filepath.Join(t.TempDir(), "netbird.sock") + ln, err := net.Listen("unix", path) + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { + if err := ln.Close(); err != nil { + t.Logf("close listener: %v", err) + } + }) + + if !DaemonRunsAsSelf("unix://" + path) { + t.Error("a socket owned by this user must count as the daemon running as us") + } +} + +// Everything that is not a readable socket of ours has to answer false, because +// the caller reads a true as "the daemon would authorize me". +func TestDaemonRunsAsSelf_FailsClosed(t *testing.T) { + dir := t.TempDir() + + // A socket owned by another user, which is what a root-run daemon looks like + // to an unprivileged client. Only assertable when we are not root ourselves. + rootOwned := "unix:///var/run/netbird.sock" + if _, err := os.Stat("/var/run/netbird.sock"); err == nil && os.Getuid() != 0 { + if DaemonRunsAsSelf(rootOwned) { + t.Error("a socket owned by another user must not count as ours") + } + } + + for name, addr := range map[string]string{ + "missing socket": "unix://" + filepath.Join(dir, "absent.sock"), + "tcp address": "tcp://127.0.0.1:41731", + "named pipe": "npipe://netbird", + "empty": "", + "no scheme": filepath.Join(dir, "absent.sock"), + "directory": "unix://" + dir, + "unknown scheme": "http://localhost:8080", + "scheme only": "unix://", + "relative socket": "unix://netbird.sock", + } { + t.Run(name, func(t *testing.T) { + if DaemonRunsAsSelf(addr) { + t.Errorf("%q must not count as a daemon running as us", addr) + } + }) + } +} diff --git a/client/internal/daemonaddr/owner_windows.go b/client/internal/daemonaddr/owner_windows.go new file mode 100644 index 000000000..1cd2bba15 --- /dev/null +++ b/client/internal/daemonaddr/owner_windows.go @@ -0,0 +1,42 @@ +//go:build windows + +package daemonaddr + +import ( + "context" + "strings" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/ipcauth" +) + +// daemonRunsAsSelf reads the owner of the daemon's pipe. A daemon running as the +// service account owns its pipe as LocalSystem, and an elevated one as +// BUILTIN\Administrators, so only a daemon the user started themselves matches. +func daemonRunsAsSelf(addr string) bool { + name, ok := strings.CutPrefix(addr, pipeScheme) + if !ok { + return false + } + + for _, path := range PipePaths(name) { + // Bounded: this runs on the UI's path for deciding which controls to + // offer, so a pipe that does not answer promptly must not stall it. A + // timeout leaves the caller unprivileged, which only disables controls. + ctx, cancel := context.WithTimeout(context.Background(), probeTimeout) + conn, err := dialPipe(ctx, path) + cancel() + if err != nil { + continue + } + + owned := ipcauth.PipeOwnedBySelf(conn) + if cerr := conn.Close(); cerr != nil { + log.Debugf("close daemon pipe %s after ownership check: %v", path, cerr) + } + return owned + } + + return false +} diff --git a/client/internal/daemonaddr/pipe.go b/client/internal/daemonaddr/pipe.go new file mode 100644 index 000000000..51815ef5e --- /dev/null +++ b/client/internal/daemonaddr/pipe.go @@ -0,0 +1,103 @@ +package daemonaddr + +import ( + "context" + "net" + "runtime" + "strings" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" +) + +const ( + // WindowsPipeAddr is the default daemon address on Windows. A named pipe + // carries the connecting process's token, which loopback TCP does not, so + // it is the only Windows transport on which the daemon can tell who is + // calling it. + WindowsPipeAddr = "npipe://netbird" + + // legacyWindowsAddr is the loopback-TCP address the Windows daemon used + // before named-pipe support. + legacyWindowsAddr = "tcp://127.0.0.1:41731" + + pipeScheme = "npipe://" + + // protectedPrefix is the NPFS namespace in which only LocalSystem and + // members of BUILTIN\Administrators may create a pipe. A daemon running as + // the service account creates its pipe there so that an unprivileged process + // cannot pre-create the name, which would keep the daemon from starting and + // leave callers talking to the squatter. Opening such a pipe needs no + // privilege, so unprivileged clients still reach the daemon. + protectedPrefix = `ProtectedPrefix\Administrators\` +) + +// DialTarget returns the gRPC dial target and transport options for a daemon +// address. The npipe scheme needs a context dialer because gRPC has no +// named-pipe resolver; unix and tcp are handled by gRPC itself. +func DialTarget(addr string) (string, []grpc.DialOption) { + opts := []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())} + + if name, ok := strings.CutPrefix(addr, pipeScheme); ok { + paths := PipePaths(name) + opts = append(opts, grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return dialPipePaths(ctx, paths) + })) + return "passthrough:///netbird-daemon-pipe", opts + } + + return strings.TrimPrefix(addr, "tcp://"), opts +} + +// PipePath maps an npipe address name ("netbird", from "npipe://netbird") to a +// Windows named-pipe path (\\.\pipe\netbird). A fully qualified path is left as +// is. +func PipePath(name string) string { + if strings.HasPrefix(name, `\\`) { + return name + } + return `\\.\pipe\` + name +} + +// PipePaths returns the paths a daemon control pipe may live at for an npipe +// address name, in the order both sides must try them: the protected name first, +// then the plain one. +// +// The daemon serves the first it can create, which is the protected name when it +// runs as the service account and the plain one when it runs as an ordinary user, +// as it does in netstack mode. Clients therefore have to try both, and because a +// client cannot tell from the name alone who created the pipe, the plain name is +// only usable once the server's identity has been checked: see +// verifyPipeServer. +// +// A fully qualified path is what the operator asked for and is used as is. +func PipePaths(name string) []string { + if strings.HasPrefix(name, `\\`) { + return []string{name} + } + return []string{PipePath(protectedPrefix + name), PipePath(name)} +} + +// IsProtectedPipePath reports whether a pipe path is in the namespace only an +// administrator or LocalSystem can create in, which is what lets a client trust +// such a pipe from its name alone. +func IsProtectedPipePath(path string) bool { + return strings.HasPrefix(path, `\\.\pipe\`+protectedPrefix) +} + +// MigrateLegacy upgrades the pre-named-pipe Windows daemon address to the named +// pipe, reporting whether it rewrote the address. Existing installs persist the +// daemon address, so without this an upgraded daemon would keep listening on +// loopback TCP, where callers carry no identity and privileged operations would +// have to be refused for everyone. Only the exact legacy default is rewritten: +// a deliberately chosen custom address is left alone. +func MigrateLegacy(addr string) (string, bool) { + return migrateLegacyForOS(runtime.GOOS, addr) +} + +func migrateLegacyForOS(goos, addr string) (string, bool) { + if goos == "windows" && addr == legacyWindowsAddr { + return WindowsPipeAddr, true + } + return addr, false +} diff --git a/client/internal/daemonaddr/pipe_other.go b/client/internal/daemonaddr/pipe_other.go new file mode 100644 index 000000000..04e8e7331 --- /dev/null +++ b/client/internal/daemonaddr/pipe_other.go @@ -0,0 +1,15 @@ +//go:build !windows + +package daemonaddr + +import ( + "context" + "fmt" + "net" +) + +// dialPipePaths is Windows-only: no other platform serves the daemon on a named +// pipe. +func dialPipePaths(context.Context, []string) (net.Conn, error) { + return nil, fmt.Errorf("named pipes are only supported on Windows") +} diff --git a/client/internal/daemonaddr/pipe_test.go b/client/internal/daemonaddr/pipe_test.go new file mode 100644 index 000000000..b9dfd90f1 --- /dev/null +++ b/client/internal/daemonaddr/pipe_test.go @@ -0,0 +1,30 @@ +package daemonaddr + +import ( + "slices" + "testing" +) + +// The protected name must be tried before the plain one on both sides: it is the +// one an unprivileged process cannot create, so preferring it is what keeps a +// squatter from owning the name the service daemon would otherwise use. +func TestPipePaths_PrefersTheProtectedName(t *testing.T) { + got := PipePaths("netbird") + want := []string{ + `\\.\pipe\ProtectedPrefix\Administrators\netbird`, + `\\.\pipe\netbird`, + } + if !slices.Equal(got, want) { + t.Errorf("PipePaths = %q, want %q", got, want) + } +} + +// An operator who passes a full path chose exactly one pipe, so neither side may +// look anywhere else. +func TestPipePaths_QualifiedPathIsUsedAsIs(t *testing.T) { + path := `\\.\pipe\custom-netbird` + got := PipePaths(path) + if !slices.Equal(got, []string{path}) { + t.Errorf("PipePaths = %q, want just %q", got, path) + } +} diff --git a/client/internal/daemonaddr/pipe_windows.go b/client/internal/daemonaddr/pipe_windows.go new file mode 100644 index 000000000..3cd10a6c3 --- /dev/null +++ b/client/internal/daemonaddr/pipe_windows.go @@ -0,0 +1,59 @@ +//go:build windows + +package daemonaddr + +import ( + "context" + "errors" + "fmt" + "net" + + "github.com/Microsoft/go-winio" + log "github.com/sirupsen/logrus" + "golang.org/x/sys/windows" + + "github.com/netbirdio/netbird/client/internal/ipcauth" +) + +// dialPipePaths connects to the first path that answers with a pipe server this +// client may trust, and returns the last error when none does. +func dialPipePaths(ctx context.Context, paths []string) (net.Conn, error) { + var lastErr error + for _, path := range paths { + conn, err := dialPipe(ctx, path) + if err != nil { + log.Debugf("dial daemon pipe %s: %v", path, err) + lastErr = err + continue + } + + // A pipe in the protected namespace could only have been created by an + // administrator or LocalSystem, so its name is the guarantee. Any other + // name has to be checked, because any local user can create one. + if !IsProtectedPipePath(path) { + if err := ipcauth.PipeServerTrusted(conn); err != nil { + if closeErr := conn.Close(); closeErr != nil { + log.Debugf("close untrusted pipe %s: %v", path, closeErr) + } + lastErr = fmt.Errorf("%s: %w", path, err) + continue + } + } + + return conn, nil + } + + if lastErr == nil { + lastErr = errors.New("no daemon pipe to connect to") + } + return nil, lastErr +} + +// dialPipe connects to the daemon control pipe at SECURITY_IDENTIFICATION. +// winio's plain DialPipe connects at SECURITY_ANONYMOUS, under which the daemon +// cannot read the caller's token at all. Identification lets the daemon read the +// caller's SID and groups without granting it the ability to act as the caller. +func dialPipe(ctx context.Context, path string) (net.Conn, error) { + access := uint32(windows.GENERIC_READ | windows.GENERIC_WRITE) + return winio.DialPipeAccessImpLevel(ctx, path, access, winio.PipeImpLevelIdentification) +} diff --git a/client/internal/daemonaddr/resolve_pipe_other.go b/client/internal/daemonaddr/resolve_pipe_other.go new file mode 100644 index 000000000..1aede8453 --- /dev/null +++ b/client/internal/daemonaddr/resolve_pipe_other.go @@ -0,0 +1,9 @@ +//go:build !windows + +package daemonaddr + +// ResolveDaemonAddr is a no-op off Windows, where there is no named-pipe +// default to fall back from. +func ResolveDaemonAddr(addr string) string { + return addr +} diff --git a/client/internal/daemonaddr/resolve_pipe_windows.go b/client/internal/daemonaddr/resolve_pipe_windows.go new file mode 100644 index 000000000..d12ddb15d --- /dev/null +++ b/client/internal/daemonaddr/resolve_pipe_windows.go @@ -0,0 +1,82 @@ +//go:build windows + +package daemonaddr + +import ( + "net" + "strings" + "time" + + "github.com/Microsoft/go-winio" + log "github.com/sirupsen/logrus" +) + +// probeTimeout bounds each transport probe. Both are local, so a daemon that is +// listening answers immediately and one that is not fails immediately. +const probeTimeout = 300 * time.Millisecond + +// ResolveDaemonAddr keeps a client on the named pipe and never silently moves it +// off. When the pipe does not answer it checks the legacy loopback TCP address, so +// a client meeting a daemon that has not restarted since the upgrade can say what +// is wrong, but it does not connect there. +// +// Using that address automatically would be a downgrade the user never asked for: +// any local process can bind 127.0.0.1 while the daemon is not listening, and the +// transport carries no caller identity, so a client that accepted whatever answered +// would hand a setup key, a pre-shared key or an SSO prompt to a local impostor. An +// operator who needs the legacy address during the upgrade window can still pass +// --daemon-addr explicitly, which is a deliberate choice and still refuses the +// privileged operations. +// +// Only the pipe address is resolved. A custom address is left alone, though passing +// --daemon-addr npipe://netbird explicitly is indistinguishable from the default +// here, so it is treated the same way. +func ResolveDaemonAddr(addr string) string { + if addr != WindowsPipeAddr { + return addr + } + + for _, path := range PipePaths("netbird") { + if pipeAvailable(path) { + return addr + } + } + + if tcpAvailable(legacyWindowsAddr) { + log.Warnf("the daemon is not serving %s, but something is listening on the legacy %s. "+ + "Restart the NetBird service so it serves the pipe. That address is not used automatically: "+ + "any local user can bind it and it carries no caller identity, so pass --daemon-addr %s "+ + "explicitly if you accept that", + WindowsPipeAddr, legacyWindowsAddr, legacyWindowsAddr) + } + + return addr +} + +func pipeAvailable(path string) bool { + timeout := probeTimeout + conn, err := winio.DialPipe(path, &timeout) + if err != nil { + return false + } + if err := conn.Close(); err != nil { + log.Debugf("close daemon pipe probe: %v", err) + } + return true +} + +func tcpAvailable(addr string) bool { + host := addr + if _, after, ok := strings.Cut(addr, "://"); ok { + host = after + } + + conn, err := net.DialTimeout("tcp", host, probeTimeout) + if err != nil { + return false + } + if err := conn.Close(); err != nil { + log.Debugf("close daemon TCP probe: %v", err) + } + return true +} diff --git a/client/internal/ipcauth/creds_stub.go b/client/internal/ipcauth/creds_stub.go new file mode 100644 index 000000000..154948716 --- /dev/null +++ b/client/internal/ipcauth/creds_stub.go @@ -0,0 +1,31 @@ +//go:build !linux && !darwin && !freebsd && !windows + +package ipcauth + +import ( + "errors" + "net" + + "google.golang.org/grpc/credentials" +) + +// errUnsupported is returned on platforms with no local peer-identity +// primitive, so consumers fail closed instead of guessing an identity. +var errUnsupported = errors.New("peer identity is not available on this platform") + +// NewTransportCredentials returns nil: without a peer-identity primitive the +// daemon cannot authenticate local callers, and the caller must treat that as +// "authorization cannot be enforced". +func NewTransportCredentials() credentials.TransportCredentials { + return nil +} + +// PeerIdentity always fails on this platform. +func PeerIdentity(net.Conn) (Identity, error) { + return Identity{}, errUnsupported +} + +// ConnIdentity always fails on this platform. +func ConnIdentity(net.Conn) (Identity, error) { + return Identity{}, errUnsupported +} diff --git a/client/internal/ipcauth/creds_unix.go b/client/internal/ipcauth/creds_unix.go new file mode 100644 index 000000000..688fe4623 --- /dev/null +++ b/client/internal/ipcauth/creds_unix.go @@ -0,0 +1,56 @@ +//go:build linux || darwin || freebsd + +package ipcauth + +import ( + "context" + "net" + + "google.golang.org/grpc/credentials" +) + +// NewTransportCredentials returns gRPC transport credentials that expose the +// caller's kernel-authenticated identity via IdentityFromContext. It returns +// nil on platforms that have no peer-identity primitive, which the caller must +// treat as "authorization cannot be enforced". +// +// The handshake exchanges no bytes on the wire, so a client dialing with +// insecure credentials interoperates with a server using these. That keeps +// older CLI and UI binaries working against an upgraded daemon. +func NewTransportCredentials() credentials.TransportCredentials { + return unixCreds{} +} + +// ConnIdentity extracts the caller's identity from an accepted local IPC +// connection. It is shared by the gRPC transport credentials and by the JSON +// gateway, which reads the identity of its own HTTP clients. +func ConnIdentity(conn net.Conn) (Identity, error) { + return PeerIdentity(conn) +} + +type unixCreds struct{} + +func (unixCreds) ClientHandshake(_ context.Context, _ string, conn net.Conn) (net.Conn, credentials.AuthInfo, error) { + return conn, AuthInfo{}, nil +} + +// ServerHandshake extracts the peer identity and fails closed when it cannot +// be read, so a connection whose caller is unknown never reaches a handler. +func (unixCreds) ServerHandshake(conn net.Conn) (net.Conn, credentials.AuthInfo, error) { + id, err := ConnIdentity(conn) + if err != nil { + return nil, nil, err + } + return conn, AuthInfo{ + CommonAuthInfo: credentials.CommonAuthInfo{SecurityLevel: credentials.NoSecurity}, + Identity: id, + }, nil +} + +func (unixCreds) Info() credentials.ProtocolInfo { + return credentials.ProtocolInfo{SecurityProtocol: AuthInfo{}.AuthType()} +} + +func (unixCreds) Clone() credentials.TransportCredentials { return unixCreds{} } + +func (unixCreds) OverrideServerName(string) error { return nil } diff --git a/client/internal/ipcauth/creds_windows.go b/client/internal/ipcauth/creds_windows.go new file mode 100644 index 000000000..37f902c52 --- /dev/null +++ b/client/internal/ipcauth/creds_windows.go @@ -0,0 +1,194 @@ +//go:build windows + +package ipcauth + +import ( + "context" + "fmt" + "net" + "runtime" + + log "github.com/sirupsen/logrus" + "golang.org/x/sys/windows" + "google.golang.org/grpc/credentials" +) + +var ( + modadvapi32 = windows.NewLazySystemDLL("advapi32.dll") + procImpersonateNamedPipeClient = modadvapi32.NewProc("ImpersonateNamedPipeClient") +) + +// DefaultPipeSDDL is the security descriptor for the daemon control pipe. +// +// D:P protected DACL, no inheritance +// (A;;GA;;;SY) allow GENERIC_ALL to LocalSystem (the daemon's service account) +// (A;;GA;;;WD) allow GENERIC_ALL to Everyone +// +// Any local caller may connect, as with a Unix socket at 0666; what a caller may +// actually do is decided from its token, not from the DACL. Remote callers are not +// a concern here: winio.ListenPipe creates the pipe with +// FILE_PIPE_REJECT_REMOTE_CLIENTS, so NPFS rejects connections from other machines +// before the descriptor is consulted. +// +// A deny ACE on the NETWORK SID would not add anything and would break callers: +// that SID is present in any network-logon token, which includes OpenSSH and WinRM +// sessions, so it denies administrators driving the CLI over SSH and denies the +// daemon itself when started from such a session. +func DefaultPipeSDDL() string { + return "D:P(A;;GA;;;SY)(A;;GA;;;WD)" +} + +// NewTransportCredentials returns gRPC transport credentials that derive the +// caller's identity from the named-pipe client token. +// +// The client must connect at SECURITY_IDENTIFICATION for the daemon to be able +// to read its token, which is what DialNamedPipe does. +func NewTransportCredentials() credentials.TransportCredentials { + return winpipeCreds{} +} + +// ConnIdentity extracts the caller's identity from an accepted named-pipe +// connection by impersonating the pipe client and reading its token. It is +// shared by the gRPC transport credentials and by the JSON gateway, which +// reads the identity of its own HTTP clients. +func ConnIdentity(conn net.Conn) (Identity, error) { + // go-winio's pipe connection embeds *win32File, which exposes Fd(). + fdConn, ok := conn.(interface{ Fd() uintptr }) + if !ok { + return Identity{}, fmt.Errorf("connection %T does not expose a pipe handle", conn) + } + return pipeClientIdentity(windows.Handle(fdConn.Fd())) +} + +type winpipeCreds struct{} + +func (winpipeCreds) ClientHandshake(_ context.Context, _ string, conn net.Conn) (net.Conn, credentials.AuthInfo, error) { + return conn, AuthInfo{}, nil +} + +// ServerHandshake extracts the connecting client's identity and fails closed +// when the handle or token cannot be read, so a connection whose caller is +// unknown never reaches a handler. +func (winpipeCreds) ServerHandshake(conn net.Conn) (net.Conn, credentials.AuthInfo, error) { + id, err := ConnIdentity(conn) + if err != nil { + return nil, nil, err + } + return conn, AuthInfo{ + CommonAuthInfo: credentials.CommonAuthInfo{SecurityLevel: credentials.NoSecurity}, + Identity: id, + }, nil +} + +func (winpipeCreds) Info() credentials.ProtocolInfo { + return credentials.ProtocolInfo{SecurityProtocol: AuthInfo{}.AuthType()} +} + +func (winpipeCreds) Clone() credentials.TransportCredentials { return winpipeCreds{} } + +func (winpipeCreds) OverrideServerName(string) error { return nil } + +// pipeClientIdentity reads the connecting client's user SID, usable group +// SIDs, and elevation state by impersonating the pipe client on this thread +// and reading the resulting impersonation token. +func pipeClientIdentity(handle windows.Handle) (id Identity, err error) { + // Impersonation is per-thread, so the goroutine must stay on this thread + // until RevertToSelf, otherwise an unrelated goroutine could inherit the + // impersonated context. + runtime.LockOSThread() + + // The thread only goes back to the runtime's pool once it is provably no + // longer impersonating the client. If the revert fails, leaving it locked + // makes Go terminate it when this goroutine exits, which costs one thread + // and keeps a thread running as the client from ever being reused. + clean := false + defer func() { + if clean { + runtime.UnlockOSThread() + } + }() + + if err = impersonateNamedPipeClient(handle); err != nil { + clean = true + return Identity{}, fmt.Errorf("impersonate named pipe client: %w", err) + } + defer func() { + // Surface the revert failure only when nothing else failed: leaving + // the thread impersonated is worse than the original error. + revErr := windows.RevertToSelf() + if revErr != nil { + if err == nil { + err = fmt.Errorf("revert impersonation: %w", revErr) + } + return + } + clean = true + }() + + // openAsSelf=true opens the token with the daemon's own process context + // rather than the impersonated client's, so the open cannot fail because + // the client lacks access to its own token. + var token windows.Token + if err = windows.OpenThreadToken(windows.CurrentThread(), windows.TOKEN_QUERY, true, &token); err != nil { + return Identity{}, fmt.Errorf("open thread token: %w", err) + } + defer func() { + if cerr := token.Close(); cerr != nil { + log.Debugf("close client token: %v", cerr) + } + }() + + return identityFromToken(token) +} + +// identityFromToken reads the user SID, usable group SIDs and elevation state +// out of a Windows token. +func identityFromToken(token windows.Token) (Identity, error) { + user, err := token.GetTokenUser() + if err != nil { + return Identity{}, fmt.Errorf("read token user: %w", err) + } + + groups, err := tokenGroupSIDs(token) + if err != nil { + return Identity{}, err + } + + return Identity{ + SID: user.User.Sid.String(), + Groups: groups, + Elevated: token.IsElevated(), + }, nil +} + +// tokenGroupSIDs returns the SIDs of the groups the token can actually +// exercise. Groups that are disabled or marked deny-only are skipped: a +// UAC-filtered administrator carries BUILTIN\Administrators as deny-only, and +// treating that as membership would hand every admin account privilege it +// cannot currently use. +func tokenGroupSIDs(token windows.Token) ([]string, error) { + tg, err := token.GetTokenGroups() + if err != nil { + return nil, fmt.Errorf("read token groups: %w", err) + } + + var sids []string + for _, g := range tg.AllGroups() { + if g.Attributes&windows.SE_GROUP_ENABLED == 0 { + continue + } + if g.Attributes&windows.SE_GROUP_USE_FOR_DENY_ONLY != 0 { + continue + } + sids = append(sids, g.Sid.String()) + } + return sids, nil +} + +func impersonateNamedPipeClient(h windows.Handle) error { + r, _, e := procImpersonateNamedPipeClient.Call(uintptr(h)) + if r == 0 { + return e + } + return nil +} diff --git a/client/internal/ipcauth/forward.go b/client/internal/ipcauth/forward.go new file mode 100644 index 000000000..57749528c --- /dev/null +++ b/client/internal/ipcauth/forward.go @@ -0,0 +1,272 @@ +package ipcauth + +import ( + "context" + "crypto/rand" + "crypto/subtle" + "encoding/hex" + "fmt" + "slices" + "strconv" + "strings" + + "google.golang.org/grpc/metadata" +) + +// Metadata keys the local JSON gateway uses to forward the identity of its own +// HTTP client to the daemon. The gateway runs inside the daemon process and +// re-dials the daemon over the control socket, so without forwarding every +// JSON request would appear to come from the daemon itself. +const ( + // mdFwd marks a request as forwarded by the JSON gateway. It is always + // set, even when the gateway could not read its client's identity, so the + // daemon can tell "no identity available" apart from "not forwarded". + mdFwd = "x-netbird-fwd" + mdFwdUID = "x-netbird-fwd-uid" // Unix user ID + mdFwdGID = "x-netbird-fwd-gid" // Unix primary group ID + mdFwdSID = "x-netbird-fwd-sid" // Windows user SID + mdFwdGroup = "x-netbird-fwd-group" // Windows group SID, repeated + mdFwdElevated = "x-netbird-fwd-elevated" // Windows, "1" when elevated + + // mdFwdProof proves the forwarded identity was stamped by this process. The + // gateway runs inside the daemon, so a secret held in memory is available to + // the only legitimate producer and to nothing else. + mdFwdProof = "x-netbird-fwd-proof" +) + +// forwardKeys is every metadata key the gateway sets. An HTTP client must never +// be able to supply one itself: see IsReservedForwardKey. +var forwardKeys = []string{mdFwd, mdFwdUID, mdFwdGID, mdFwdSID, mdFwdGroup, mdFwdElevated, mdFwdProof} + +// forwardProof authenticates the gateway's forwarding metadata. It is generated +// once per daemon process and never leaves it: it is not written to disk, not +// logged, and not sent anywhere except over the daemon's own control socket to +// itself. +// +// Without it, trusting a forwarded identity rests on every layer in front of it +// stripping incoming forwarding keys, and on each key's value shape being +// distinguishable from an injected one. A single injected group SID or an +// injected "elevated" flag has the same shape as a legitimate one, so no +// cardinality rule can catch it. Requiring the proof means metadata that did not +// come from this process is refused whatever it contains. +var forwardProof = mustForwardProof() + +func mustForwardProof() string { + var buf [32]byte + if _, err := rand.Read(buf[:]); err != nil { + // Continuing would leave the forwarded path authenticated by a + // predictable value, which is worse than not starting. + panic(fmt.Sprintf("generate identity forwarding proof: %v", err)) + } + return hex.EncodeToString(buf[:]) +} + +// IsReservedForwardKey reports whether a gRPC metadata key belongs to the +// gateway's identity forwarding, and therefore must be dropped when it arrives +// from outside. +// +// grpc-gateway maps "Grpc-Metadata-" request headers into gRPC metadata and +// joins them ahead of the values its own annotators add. Without dropping these, +// an HTTP client could hand the daemon "x-netbird-fwd-uid: 0" and be believed, +// because the daemon trusts forwarded metadata when the transport peer is the +// (privileged) gateway. +func IsReservedForwardKey(key string) bool { + key = strings.ToLower(key) + return slices.Contains(forwardKeys, key) +} + +// ForwardIdentityMetadata encodes an HTTP client's identity for the JSON +// gateway to forward to the daemon. When known is false only the marker is +// set, which makes the daemon treat the caller as unidentified rather than as +// the daemon itself. +func ForwardIdentityMetadata(id Identity, known bool) metadata.MD { + md := metadata.MD{} + md.Set(mdFwd, "1") + md.Set(mdFwdProof, forwardProof) + if !known { + return md + } + + if id.IsWindows() { + md.Set(mdFwdSID, id.SID) + if len(id.Groups) > 0 { + md.Set(mdFwdGroup, id.Groups...) + } + if id.Elevated { + md.Set(mdFwdElevated, "1") + } + return md + } + + md.Set(mdFwdUID, strconv.FormatUint(uint64(id.UID), 10)) + md.Set(mdFwdGID, strconv.FormatUint(uint64(id.GID), 10)) + return md +} + +// CallerIdentity returns the identity to authorize a request against. For a +// direct connection that is the transport peer's kernel identity. For a +// request relayed by the local JSON gateway it is the identity the gateway +// forwarded, since the transport peer is then the daemon itself. +// +// A forwarded identity is only honoured when the transport peer is the daemon's +// own identity and the metadata carries this process's forwarding proof, so +// forged forwarding metadata gains a caller nothing. A forwarded request that +// carries no identity is reported as unidentified, never as the daemon. +// +// The second return value is false when no identity could be established, and +// callers MUST fail closed in that case. +func CallerIdentity(ctx context.Context) (Identity, bool) { + id, ok := IdentityFromContext(ctx) + if !ok { + return Identity{}, false + } + + // A forwarding key that arrives more than once did not come from the gateway + // alone, so nothing about the request can be trusted to describe its caller. + // Refusing outright matters because the alternative reading, "not forwarded", + // would authorize the request as the transport peer, which on the gateway's + // connection is the daemon itself. + if duplicatedForwardKey(ctx) { + return Identity{}, false + } + + forwarded := isForwarded(ctx) + + // Our own process on the other end of the socket is the JSON gateway, the only + // thing that dials the daemon from inside it. Such a call must carry a + // forwarded identity; without one there is no caller to authorize, and + // treating it as the daemon would authorize whatever reached the JSON socket. + // Only Linux reports the peer PID, so this is a belt on top of the gateway's + // interceptor rather than the sole guarantee. + if id.PID != 0 && int(id.PID) == selfPID && !forwarded { + return Identity{}, false + } + + // Only the gateway's own connection may speak for someone else. Being + // privileged is not enough and not the point: the gateway runs inside the + // daemon, so it dials as the daemon's identity whatever user that is, which + // also covers a rootless container. + if !forwarded || !IsDaemonSelf(id) { + return id, true + } + + // Speaking for someone else additionally requires the proof only this process + // holds. Refusing is the only safe reading: the transport peer here is the + // daemon itself, so falling back to it would authorize the request as the + // daemon. This is also what makes the forwarded values trustworthy once + // accepted, so they need no shape checks of their own. + if !authenticForward(ctx) { + return Identity{}, false + } + + return forwardedIdentity(ctx) +} + +// duplicatedForwardKey reports whether any forwarding key carries more than one +// value. The gateway's interceptor sets each key exactly once and replaces what +// was already there, so a repeat means a second source supplied it. +func duplicatedForwardKey(ctx context.Context) bool { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return false + } + for _, key := range forwardKeys { + // Group SIDs are legitimately repeated; the rest identify the caller. + if key == mdFwdGroup { + continue + } + if len(md.Get(key)) > 1 { + return true + } + } + return false +} + +// authenticForward reports whether the request carries this process's forwarding +// proof, which only the in-process JSON gateway can supply. +func authenticForward(ctx context.Context) bool { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return false + } + got := mdSingle(md, mdFwdProof) + return subtle.ConstantTimeCompare([]byte(got), []byte(forwardProof)) == 1 +} + +// isForwarded reports whether the request carries the JSON gateway marker. +func isForwarded(ctx context.Context) bool { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return false + } + return mdSingle(md, mdFwd) != "" +} + +// forwardedIdentity decodes the identity the JSON gateway attached. +func forwardedIdentity(ctx context.Context) (Identity, bool) { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return Identity{}, false + } + + if sid := mdSingle(md, mdFwdSID); sid != "" { + return Identity{ + SID: sid, + // Repeated by design, one value per group, and only reachable once + // the forwarding proof has been verified. + Groups: md.Get(mdFwdGroup), + Elevated: mdSingle(md, mdFwdElevated) == "1", + }, true + } + + uid, err := strconv.ParseUint(mdSingle(md, mdFwdUID), 10, 32) + if err != nil { + return Identity{}, false + } + + id := Identity{UID: uint32(uid)} + if gid, err := strconv.ParseUint(mdSingle(md, mdFwdGID), 10, 32); err == nil { + id.GID = uint32(gid) + } + return id, true +} + +// mdSingle returns the value of a forwarded key only when exactly one was +// supplied. The gateway's interceptor sets each key exactly once, so more than one +// value means something else also supplied it, and the whole identity is treated as +// unknown rather than picking a winner. Defence in depth behind the gateway's +// header filter. +func mdSingle(md metadata.MD, key string) string { + if v := md.Get(key); len(v) == 1 { + return v[0] + } + return "" +} + +// WithForwardedIdentity stamps id onto a context's outgoing metadata for the JSON +// gateway's call to the daemon, replacing any forwarding keys already present so +// values supplied from outside cannot survive alongside it. +// +// This is deliberately not done with runtime.WithMetadata: grpc-gateway skips its +// annotators entirely when no request header maps to metadata ("if len(pairs) == 0 +// { return ctx, nil, nil }", runtime/context.go), which an HTTP/1.0 request with no +// Host header over a unix socket achieves. The daemon would then see an unmarked +// call whose transport peer is the daemon's own identity, and authorize it as the +// daemon. A client interceptor runs for every RPC regardless of headers. +func WithForwardedIdentity(ctx context.Context, id Identity, known bool) context.Context { + md, ok := metadata.FromOutgoingContext(ctx) + if !ok { + md = metadata.MD{} + } else { + md = md.Copy() + } + + for _, key := range forwardKeys { + delete(md, key) + } + for key, values := range ForwardIdentityMetadata(id, known) { + md[key] = values + } + + return metadata.NewOutgoingContext(ctx, md) +} diff --git a/client/internal/ipcauth/forward_test.go b/client/internal/ipcauth/forward_test.go new file mode 100644 index 000000000..d9adf05da --- /dev/null +++ b/client/internal/ipcauth/forward_test.go @@ -0,0 +1,214 @@ +package ipcauth + +import ( + "context" + "testing" + + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" +) + +// transportCtx builds a request context as the daemon's transport credentials +// would: the identity of whoever opened the socket, plus whatever metadata the +// request carried. +func transportCtx(id Identity, md metadata.MD) context.Context { + ctx := peer.NewContext(context.Background(), &peer.Peer{ + AuthInfo: AuthInfo{ + CommonAuthInfo: credentials.CommonAuthInfo{SecurityLevel: credentials.NoSecurity}, + Identity: id, + }, + }) + if md != nil { + ctx = metadata.NewIncomingContext(ctx, md) + } + return ctx +} + +var ( + root = Identity{UID: 0} + unprivUser = Identity{UID: 1000, GID: 1000} +) + +// asDaemon pins which identity counts as this process for the duration of a test. +// Without it the test binary's own uid decides, which silently changes what +// "the gateway" means. +func asDaemon(t *testing.T, id Identity) { + t.Helper() + prevID, prevKnown, prevDelegate := selfIdentity, selfKnown, selfMayDelegate + t.Cleanup(func() { selfIdentity, selfKnown, selfMayDelegate = prevID, prevKnown, prevDelegate }) + selfIdentity, selfKnown = id, true + selfMayDelegate = !id.IsPrivileged() +} + +func TestCallerIdentity_DirectConnections(t *testing.T) { + t.Run("no transport credentials is not an identity", func(t *testing.T) { + if _, ok := CallerIdentity(context.Background()); ok { + t.Fatal("a caller with no credentials must not be identified") + } + }) + + t.Run("a direct caller is its transport identity", func(t *testing.T) { + id, ok := CallerIdentity(transportCtx(unprivUser, nil)) + if !ok || id.UID != 1000 { + t.Fatalf("got %v ok=%t, want uid 1000", id, ok) + } + }) + + // The whole point of honouring forwarded metadata only from a privileged + // transport peer: an unprivileged caller can set any metadata it likes on its + // own connection to the daemon socket. + t.Run("an unprivileged caller cannot forge an identity", func(t *testing.T) { + asDaemon(t, root) + forged := metadata.Pairs(mdFwd, "1", mdFwdUID, "0", mdFwdGID, "0") + id, ok := CallerIdentity(transportCtx(unprivUser, forged)) + if !ok { + t.Fatal("caller should still be identified, as itself") + } + if id.IsPrivileged() || id.UID != 1000 { + t.Fatalf("forged metadata was believed: got %v", id) + } + }) +} + +func TestCallerIdentity_GatewayForwarding(t *testing.T) { + t.Run("the gateway's client identity is used, not the gateway's own", func(t *testing.T) { + asDaemon(t, root) + md := ForwardIdentityMetadata(unprivUser, true) + id, ok := CallerIdentity(transportCtx(root, md)) + if !ok { + t.Fatal("forwarded identity should be usable") + } + if id.IsPrivileged() || id.UID != 1000 { + t.Fatalf("got %v, want the forwarded uid 1000 and not privileged", id) + } + }) + + t.Run("a privileged gateway client stays privileged", func(t *testing.T) { + asDaemon(t, root) + md := ForwardIdentityMetadata(root, true) + id, ok := CallerIdentity(transportCtx(root, md)) + if !ok || !id.IsPrivileged() { + t.Fatalf("got %v ok=%t, want a privileged identity", id, ok) + } + }) + + // A JSON socket the gateway cannot read peer credentials from (a TCP socket, + // say) must not make every request look like the daemon itself. + t.Run("an unreadable client identity is unknown, not the daemon", func(t *testing.T) { + asDaemon(t, root) + md := ForwardIdentityMetadata(Identity{}, false) + if _, ok := CallerIdentity(transportCtx(root, md)); ok { + t.Fatal("a forwarded request with no identity must not be identified") + } + }) + + // grpc-gateway turns Grpc-Metadata- headers into gRPC metadata and joins + // them ahead of its annotators' values. If an HTTP client's header survived + // that, this is the shape the daemon would see: the attacker's uid 0 first, + // the real uid second. The gateway filters those headers out, and reading a + // duplicated key as unknown makes the daemon safe even if it did not. + t.Run("a duplicated key from an injected header is not believed", func(t *testing.T) { + asDaemon(t, root) + md := metadata.MD{} + md.Append(mdFwd, "1") + md.Append(mdFwdUID, "0") // injected by the HTTP client + md.Append(mdFwdUID, "1000") // appended by the gateway's annotator + if id, ok := CallerIdentity(transportCtx(root, md)); ok { + t.Fatalf("injected uid was accepted: got %v", id) + } + }) + + t.Run("a duplicated marker is not believed either", func(t *testing.T) { + asDaemon(t, root) + md := metadata.MD{} + md.Append(mdFwd, "1") + md.Append(mdFwd, "1") + md.Append(mdFwdUID, "1000") + // A repeated marker must not be read as "not forwarded": that would + // authorize the request as the transport peer, which on the gateway's + // connection is the daemon itself. + if id, ok := CallerIdentity(transportCtx(root, md)); ok { + t.Fatalf("a duplicated marker was believed: got %v", id) + } + }) + + // The layers in front of this (the gateway's header matcher, and its + // interceptor replacing every forwarding key) are what keep outside metadata + // from arriving at all. The proof is what the daemon can check for itself, and + // it is the only defence that works for a value whose legitimate shape is + // indistinguishable from an injected one: a lone group SID, or "elevated". + t.Run("forwarding metadata without this process's proof is refused", func(t *testing.T) { + asDaemon(t, root) + for name, md := range map[string]metadata.MD{ + "no proof": metadata.Pairs(mdFwd, "1", mdFwdUID, "0"), + "wrong proof": metadata.Pairs(mdFwd, "1", mdFwdUID, "0", mdFwdProof, "deadbeef"), + "windows identity without a proof": metadata.Pairs(mdFwd, "1", + mdFwdSID, "S-1-5-21-1-2-3-1001", mdFwdGroup, sidAdministrators, mdFwdElevated, "1"), + } { + t.Run(name, func(t *testing.T) { + if id, ok := CallerIdentity(transportCtx(root, md)); ok { + t.Fatalf("unstamped forwarding metadata was believed: got %v", id) + } + }) + } + }) + + // A caller that reaches the gateway cannot see the proof, so it cannot append + // a group of its own to a genuine forwarded identity: doing so would have to + // go through the interceptor, which replaces the whole set. + t.Run("a group appended to a stamped identity does not survive the interceptor", func(t *testing.T) { + asDaemon(t, root) + injected := metadata.MD{} + injected.Append(mdFwdGroup, sidAdministrators) + + ctx := WithForwardedIdentity(metadata.NewOutgoingContext(context.Background(), injected), + Identity{SID: "S-1-5-21-1-2-3-1001"}, true) + out, ok := metadata.FromOutgoingContext(ctx) + if !ok { + t.Fatal("no outgoing metadata") + } + if groups := out.Get(mdFwdGroup); len(groups) != 0 { + t.Fatalf("injected group survived: %v", groups) + } + }) +} + +func TestIsReservedForwardKey(t *testing.T) { + for _, key := range forwardKeys { + if !IsReservedForwardKey(key) { + t.Errorf("%q must be reserved", key) + } + } + + // grpc-gateway canonicalises header names, so the check has to be + // case-insensitive. + if !IsReservedForwardKey("X-Netbird-Fwd-Uid") { + t.Error("the check must be case-insensitive") + } + + for _, key := range []string{"authorization", "x-netbird", "x-netbird-fwd-uid-extra", ""} { + if IsReservedForwardKey(key) { + t.Errorf("%q must not be reserved", key) + } + } +} + +func TestForwardIdentityMetadata_AlwaysMarksForwarded(t *testing.T) { + for _, tc := range []struct { + name string + id Identity + known bool + }{ + {"known unix identity", unprivUser, true}, + {"unknown identity", Identity{}, false}, + {"windows identity", Identity{SID: "S-1-5-21-1-2-3-1001", Elevated: true}, true}, + } { + t.Run(tc.name, func(t *testing.T) { + md := ForwardIdentityMetadata(tc.id, tc.known) + if got := md.Get(mdFwd); len(got) != 1 || got[0] != "1" { + t.Fatalf("marker = %v, want exactly one \"1\"", got) + } + }) + } +} diff --git a/client/internal/ipcauth/identity.go b/client/internal/ipcauth/identity.go new file mode 100644 index 000000000..ff70c209a --- /dev/null +++ b/client/internal/ipcauth/identity.go @@ -0,0 +1,127 @@ +// Package ipcauth provides the kernel-authenticated identity of a local IPC +// (gRPC) caller and the transport credentials that surface it into the gRPC +// context, so the daemon can authorize individual RPCs by caller identity. +// +// On Unix the identity is read from the kernel via SO_PEERCRED (Linux) or +// LOCAL_PEERCRED (Darwin/FreeBSD). On Windows it is derived from the +// named-pipe client token. Platforms without a peer-identity primitive get no +// credentials, and every consumer must fail closed when no identity is +// available. +package ipcauth + +import ( + "context" + "fmt" + "slices" + + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/peer" +) + +// Well-known Windows SIDs that identify a fully privileged principal. +const ( + sidLocalSystem = "S-1-5-18" // NT AUTHORITY\SYSTEM + sidLocalService = "S-1-5-19" // NT AUTHORITY\LOCAL SERVICE + sidNetworkService = "S-1-5-20" // NT AUTHORITY\NETWORK SERVICE + sidAdministrators = "S-1-5-32-544" // BUILTIN\Administrators +) + +// Identity is the kernel-authenticated identity of a local IPC caller. The +// zero value is not a valid identity: consumers must only use one obtained +// with a true ok/nil error return. +type Identity struct { + // UID and GID are the caller's Unix user ID and primary group ID. Both are + // zero on Windows, where SID is authoritative instead. + UID uint32 + GID uint32 + + // SID is the caller's Windows security identifier, empty on Unix. + SID string + + // Groups holds the caller's Windows group SIDs, captured from the client + // token at handshake time. Only groups that are enabled and not + // deny-only are captured, so a group listed here is one the caller can + // actually exercise. Empty on Unix. + Groups []string + + // Elevated reports whether the Windows client token is elevated (running + // as administrator, or an administrator with UAC turned off). Always false + // on Unix, where privilege is uid 0. + Elevated bool + + // PID is the caller's process ID where the platform reports it (Linux's + // SO_PEERCRED), and 0 where it does not. It identifies the daemon's own + // process dialling itself, which is what the JSON gateway does, and is never + // used to grant anything. + PID int32 +} + +// IsWindows reports whether this identity is a Windows principal (SID-based) +// rather than a Unix uid/gid principal. +func (i Identity) IsWindows() bool { + return i.SID != "" +} + +// IsPrivileged reports whether the caller is the platform's administrative +// principal, which is what the daemon requires for changes that cross the +// user-to-root boundary. +// +// On Windows the decision comes from the caller's token rather than from +// account names or group RIDs: an elevated token, one of the service accounts +// the daemon itself may run as, or a token with BUILTIN\Administrators +// enabled. A UAC-filtered administrator has that group marked deny-only, and +// deny-only groups are dropped when the identity is captured, so such a +// caller is correctly reported as unprivileged. Domain group memberships +// (Domain Admins and friends) are deliberately not consulted: they say +// nothing about what this token may do on this machine. +func (i Identity) IsPrivileged() bool { + if !i.IsWindows() { + return i.UID == 0 + } + + if i.Elevated { + return true + } + + switch i.SID { + case sidLocalSystem, sidLocalService, sidNetworkService: + return true + } + + return slices.Contains(i.Groups, sidAdministrators) +} + +// String renders the identity for audit logs and denial messages. +func (i Identity) String() string { + if i.IsWindows() { + return fmt.Sprintf("sid=%s elevated=%t", i.SID, i.Elevated) + } + return fmt.Sprintf("uid=%d gid=%d", i.UID, i.GID) +} + +// AuthInfo carries the peer Identity as a gRPC credentials.AuthInfo so +// handlers can retrieve it from the request context via IdentityFromContext. +type AuthInfo struct { + credentials.CommonAuthInfo + Identity Identity +} + +// AuthType identifies the authentication scheme. +func (AuthInfo) AuthType() string { return "netbird-ipc-peercred" } + +// IdentityFromContext extracts the caller's kernel-authenticated identity from +// the gRPC peer context. The second return value is false when no IPC +// transport credentials were negotiated, which happens on a TCP daemon socket +// and on platforms without a peer-identity primitive. Callers MUST fail closed +// in that case. +func IdentityFromContext(ctx context.Context) (Identity, bool) { + p, ok := peer.FromContext(ctx) + if !ok { + return Identity{}, false + } + info, ok := p.AuthInfo.(AuthInfo) + if !ok { + return Identity{}, false + } + return info.Identity, true +} diff --git a/client/internal/ipcauth/peercred_bsd.go b/client/internal/ipcauth/peercred_bsd.go new file mode 100644 index 000000000..6d9c5247f --- /dev/null +++ b/client/internal/ipcauth/peercred_bsd.go @@ -0,0 +1,43 @@ +//go:build darwin || freebsd + +package ipcauth + +import ( + "fmt" + "net" + + "golang.org/x/sys/unix" +) + +// PeerIdentity reads the kernel-authenticated identity of the process on the +// other end of a Unix socket via LOCAL_PEERCRED. The xucred is recorded by the +// kernel at connect() time and carries the peer's uid and its group list, of +// which the first entry is the primary group. +func PeerIdentity(conn net.Conn) (Identity, error) { + uc, ok := conn.(*net.UnixConn) + if !ok { + return Identity{}, fmt.Errorf("connection is not a unix socket: %T", conn) + } + + raw, err := uc.SyscallConn() + if err != nil { + return Identity{}, fmt.Errorf("raw conn: %w", err) + } + + var cred *unix.Xucred + var credErr error + if err := raw.Control(func(fd uintptr) { + cred, credErr = unix.GetsockoptXucred(int(fd), unix.SOL_LOCAL, unix.LOCAL_PEERCRED) + }); err != nil { + return Identity{}, fmt.Errorf("control raw conn: %w", err) + } + if credErr != nil { + return Identity{}, fmt.Errorf("read LOCAL_PEERCRED: %w", credErr) + } + + id := Identity{UID: cred.Uid} + if cred.Ngroups > 0 { + id.GID = cred.Groups[0] + } + return id, nil +} diff --git a/client/internal/ipcauth/peercred_linux.go b/client/internal/ipcauth/peercred_linux.go new file mode 100644 index 000000000..417cc1e00 --- /dev/null +++ b/client/internal/ipcauth/peercred_linux.go @@ -0,0 +1,39 @@ +//go:build linux + +package ipcauth + +import ( + "fmt" + "net" + + "golang.org/x/sys/unix" +) + +// PeerIdentity reads the kernel-authenticated identity of the process on the +// other end of a Unix socket via SO_PEERCRED. The credentials are recorded by +// the kernel at connect() time and cannot be changed for the life of the +// connection, so they are not spoofable by the caller. +func PeerIdentity(conn net.Conn) (Identity, error) { + uc, ok := conn.(*net.UnixConn) + if !ok { + return Identity{}, fmt.Errorf("connection is not a unix socket: %T", conn) + } + + raw, err := uc.SyscallConn() + if err != nil { + return Identity{}, fmt.Errorf("raw conn: %w", err) + } + + var cred *unix.Ucred + var credErr error + if err := raw.Control(func(fd uintptr) { + cred, credErr = unix.GetsockoptUcred(int(fd), unix.SOL_SOCKET, unix.SO_PEERCRED) + }); err != nil { + return Identity{}, fmt.Errorf("control raw conn: %w", err) + } + if credErr != nil { + return Identity{}, fmt.Errorf("read SO_PEERCRED: %w", credErr) + } + + return Identity{UID: cred.Uid, GID: cred.Gid, PID: cred.Pid}, nil +} diff --git a/client/internal/ipcauth/pipeserver_windows.go b/client/internal/ipcauth/pipeserver_windows.go new file mode 100644 index 000000000..7ba59d574 --- /dev/null +++ b/client/internal/ipcauth/pipeserver_windows.go @@ -0,0 +1,87 @@ +//go:build windows + +package ipcauth + +import ( + "fmt" + "net" + + log "github.com/sirupsen/logrus" + "golang.org/x/sys/windows" +) + +// PipeServerTrusted reports an error unless the pipe behind conn was created by a +// principal this client may hand secrets to. Clients call it for a pipe whose name +// carries no guarantee of its own, which is any name outside the +// ProtectedPrefix\Administrators namespace: that namespace already restricts +// creation to administrators and LocalSystem, while a plain name can be created by +// any local user before the daemon gets there. +// +// The decision is made from the pipe object's owner, not from the serving process, +// because a client cannot open a process running as another user at all, and the +// legitimate case is precisely an unprivileged client talking to a privileged +// daemon. Trusted owners are the service accounts, BUILTIN\Administrators, and +// this client's own user, the last of which is the daemon a user runs themselves +// as in netstack mode. A pipe owned by anyone else gets no setup key, pre-shared +// key or SSO prompt out of this client. +func PipeServerTrusted(conn net.Conn) error { + // go-winio's pipe connection embeds *win32File, which exposes Fd(). + fdConn, ok := conn.(interface{ Fd() uintptr }) + if !ok { + return fmt.Errorf("connection %T does not expose a pipe handle", conn) + } + + owner, err := pipeOwnerSID(windows.Handle(fdConn.Fd())) + if err != nil { + return err + } + + if !trustedPipeOwner(owner) { + return fmt.Errorf("pipe owned by %s, which is neither an administrator nor this user", owner) + } + return nil +} + +// PipeOwnedBySelf reports whether the pipe behind conn was created by this very +// user, which is how a client recognises a daemon running as itself. Ownership it +// cannot read is reported as false. +func PipeOwnedBySelf(conn net.Conn) bool { + fdConn, ok := conn.(interface{ Fd() uintptr }) + if !ok { + return false + } + + owner, err := pipeOwnerSID(windows.Handle(fdConn.Fd())) + if err != nil { + log.Debugf("read daemon pipe owner: %v", err) + return false + } + return selfKnown && selfIdentity.SID != "" && owner == selfIdentity.SID +} + +// pipeOwnerSID reads the owner of the pipe object a client is connected to. The +// handle was opened with GENERIC_READ, which includes READ_CONTROL, so no extra +// access is needed. +func pipeOwnerSID(handle windows.Handle) (string, error) { + sd, err := windows.GetSecurityInfo(handle, windows.SE_KERNEL_OBJECT, windows.OWNER_SECURITY_INFORMATION) + if err != nil { + return "", fmt.Errorf("read pipe security info: %w", err) + } + + owner, _, err := sd.Owner() + if err != nil { + return "", fmt.Errorf("read pipe owner: %w", err) + } + return owner.String(), nil +} + +// trustedPipeOwner reports whether a pipe's owner is a principal a client may +// speak to. An elevated process's objects are owned by BUILTIN\Administrators by +// default, an unelevated one's by the user, which is why both forms appear here. +func trustedPipeOwner(owner string) bool { + switch owner { + case sidLocalSystem, sidLocalService, sidNetworkService, sidAdministrators: + return true + } + return selfKnown && selfIdentity.SID != "" && owner == selfIdentity.SID +} diff --git a/client/internal/ipcauth/privileged.go b/client/internal/ipcauth/privileged.go new file mode 100644 index 000000000..95f2a50e9 --- /dev/null +++ b/client/internal/ipcauth/privileged.go @@ -0,0 +1,125 @@ +package ipcauth + +import ( + "os" + "runtime" +) + +// Fields of the ErrorInfo detail the daemon attaches to a PermissionDenied it +// raises for an operation that requires root/administrator. Clients match on +// Reason and Domain rather than on the message text, and render the summary and +// command themselves so the user gets guidance instead of a gRPC error dump. +const ( + // ErrorReasonPrivilegeRequired identifies the detail. + ErrorReasonPrivilegeRequired = "PRIVILEGE_REQUIRED" + // ErrorDomain scopes the reason to the NetBird daemon. + ErrorDomain = "daemon.netbird.io" + // ErrorMetaSummary is the one-sentence explanation of what was refused. + ErrorMetaSummary = "summary" + // ErrorMetaCommand is the command that performs the same operation with the + // privileges it needs, ready to copy and run. + ErrorMetaCommand = "command" +) + +// The identity of the process evaluating callers, captured once because it cannot +// change. selfKnown is false when it could not be read, in which case nothing is +// ever treated as this process. selfMayDelegate additionally requires this +// process to be unprivileged: see IsPrivilegedCaller. +var ( + selfIdentity Identity + selfKnown bool + selfMayDelegate bool + // selfPID is this process's PID, used to recognise the daemon dialling itself. + selfPID = os.Getpid() +) + +func init() { + id, err := CurrentProcessIdentity() + if err != nil { + return + } + selfIdentity, selfKnown = id, true + // Only an unprivileged daemon delegates its authority to its own identity. + // When it is root or LocalSystem, sharing its identity does not mean sharing + // its power: on Windows a filtered and a full token carry the same SID, so + // matching there would let a non-elevated shell of an administrator account + // act as an administrator, which is the boundary the token check exists to + // keep. + selfMayDelegate = !id.IsPrivileged() +} + +// IsDaemonSelf reports whether an identity is this very process. The JSON gateway +// runs inside the daemon and re-dials it locally, so this is what distinguishes +// the gateway from any other caller, whatever user the daemon runs as. +func IsDaemonSelf(id Identity) bool { + if !selfKnown || id.IsWindows() != selfIdentity.IsWindows() { + return false + } + if id.IsWindows() { + return id.SID != "" && id.SID == selfIdentity.SID + } + return id.UID == selfIdentity.UID +} + +// IsPrivilegedCaller reports whether an identity may make the changes the daemon +// restricts to the platform administrator. This is the daemon's own rule and +// cannot be evaluated by a client, which does not know what the daemon runs as. +// +// Beyond root/administrator it accepts a caller running as the daemon's own +// identity when the daemon is itself unprivileged. That keeps a rootless container +// working, where there is no uid 0 at all, and a Windows daemon in netstack mode, +// which needs no administrator rights. In those setups a caller sharing the +// daemon's identity can already rewrite the config files it reads and replace the +// binary it runs, so refusing it a config change would protect nothing; and an +// unprivileged daemon cannot hand out a root shell in the first place. +func IsPrivilegedCaller(id Identity) bool { + if id.IsPrivileged() { + return true + } + return selfMayDelegate && IsDaemonSelf(id) +} + +// SelfDelegatesTo returns the identity this process delegates its authority to, +// and whether it delegates at all. Only an unprivileged daemon does: see +// IsPrivilegedCaller. It exists so a refusal can name who may actually perform the +// operation, because on such a host root is neither required nor necessarily +// available. +func SelfDelegatesTo() (Identity, bool) { + if !selfKnown || !selfMayDelegate { + return Identity{}, false + } + return selfIdentity, true +} + +// PrivilegedActor names the principal a privileged operation requires, for use +// in messages shown to the user. +func PrivilegedActor() string { + if runtime.GOOS == "windows" { + return "administrator privileges" + } + return "root" +} + +// 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 +// terminal. +func ElevatedCommand(command string) string { + if runtime.GOOS == "windows" { + return command + } + return "sudo " + command +} + +// UpCommand renders an elevated `netbird up` with the given flags, preceded by a +// `down`. The down is what makes the command work on a connected client: `netbird +// up` prints "Already connected" and returns without applying any config flag, so +// on its own the command would appear to do nothing. It is a no-op, exit 0, when +// the client is not connected. +// +// ";" rather than "&&" so the line can be pasted into any of the shells a user +// might have: PowerShell 5.1, still the default on Windows Server, rejects "&&" +// as a syntax error. +func UpCommand(flags string) string { + return ElevatedCommand("netbird down") + "; " + ElevatedCommand("netbird up "+flags) +} diff --git a/client/internal/ipcauth/privileged_test.go b/client/internal/ipcauth/privileged_test.go new file mode 100644 index 000000000..c1c7c1543 --- /dev/null +++ b/client/internal/ipcauth/privileged_test.go @@ -0,0 +1,134 @@ +package ipcauth + +import "testing" + +// The self rule is the one place privilege is granted to something other than the +// platform administrator, so its two guards matter: it must apply only when the +// daemon is itself unprivileged, and only to a caller with the daemon's identity. +func TestIsPrivilegedCaller_SelfRule(t *testing.T) { + tests := []struct { + name string + // self stands in for the process the daemon runs as. + self Identity + selfKnown bool + caller Identity + want bool + }{ + { + name: "root is privileged whatever the daemon runs as", + self: Identity{UID: 1000}, + selfKnown: true, + caller: Identity{UID: 0}, + want: true, + }, + { + name: "an unprivileged daemon delegates to its own user (rootless container)", + self: Identity{UID: 1000}, + selfKnown: true, + caller: Identity{UID: 1000}, + want: true, + }, + { + name: "an unprivileged daemon delegates to nobody else", + self: Identity{UID: 1000}, + selfKnown: true, + caller: Identity{UID: 1001}, + want: false, + }, + { + // The daemon is root on a normal install, so sharing its identity is + // already covered by being root; nothing else may match. + name: "a root daemon delegates to nobody", + self: Identity{UID: 0}, + selfKnown: true, + caller: Identity{UID: 1000}, + want: false, + }, + { + // Windows netstack mode: the daemon needs no administrator rights. + name: "an unprivileged windows daemon delegates to its own SID", + self: Identity{SID: "S-1-5-21-1-2-3-1001"}, + selfKnown: true, + caller: Identity{SID: "S-1-5-21-1-2-3-1001"}, + want: true, + }, + { + name: "an unprivileged windows daemon delegates to no other SID", + self: Identity{SID: "S-1-5-21-1-2-3-1001"}, + selfKnown: true, + caller: Identity{SID: "S-1-5-21-1-2-3-1002"}, + want: false, + }, + { + // The UAC boundary: a filtered and a full token of the same account + // carry the same SID but not the same power, so an elevated daemon must + // never delegate to its own SID. + name: "an elevated windows daemon does not delegate to its own SID", + self: Identity{SID: "S-1-5-21-1-2-3-500", Elevated: true}, + selfKnown: true, + caller: Identity{SID: "S-1-5-21-1-2-3-500"}, + want: false, + }, + { + name: "LocalSystem is privileged on its own merits, not by delegation", + self: Identity{SID: sidLocalSystem}, + selfKnown: true, + caller: Identity{SID: sidLocalSystem}, + want: true, // LocalSystem is privileged on its own merits + }, + { + name: "identities of different kinds never match", + self: Identity{UID: 1000}, + selfKnown: true, + caller: Identity{SID: "S-1-5-21-1-2-3-1001"}, + want: false, + }, + { + name: "an unknown self identity delegates to nobody", + self: Identity{}, + selfKnown: false, + caller: Identity{UID: 1000}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prevID, prevKnown, prevDelegate := selfIdentity, selfKnown, selfMayDelegate + t.Cleanup(func() { selfIdentity, selfKnown, selfMayDelegate = prevID, prevKnown, prevDelegate }) + + selfIdentity, selfKnown = tt.self, tt.selfKnown + selfMayDelegate = tt.selfKnown && !tt.self.IsPrivileged() + + if got := IsPrivilegedCaller(tt.caller); got != tt.want { + t.Fatalf("IsPrivilegedCaller(%v) with daemon %v = %t, want %t", + tt.caller, tt.self, got, tt.want) + } + }) + } +} + +// The real process must never accidentally delegate: a test binary running as a +// normal user is unprivileged, so it may match itself, but nothing else. +func TestIsPrivilegedCaller_ThisProcess(t *testing.T) { + id, err := CurrentProcessIdentity() + if err != nil { + t.Skipf("cannot read this process's identity: %v", err) + } + + // This process is always allowed to act as itself: either it is privileged, or + // it is unprivileged and therefore delegates to its own identity. + if !IsPrivilegedCaller(id) { + t.Errorf("this process %v was refused its own identity", id) + } + + // A caller that is neither root nor this process must be refused, whatever + // this process happens to be. + other := Identity{UID: id.UID + 1} + if id.IsWindows() { + other = Identity{SID: id.SID + "9"} + } + if IsPrivilegedCaller(other) { + t.Errorf("an unrelated identity %v was treated as privileged", other) + } +} diff --git a/client/internal/ipcauth/self_unix.go b/client/internal/ipcauth/self_unix.go new file mode 100644 index 000000000..1b86c4fc0 --- /dev/null +++ b/client/internal/ipcauth/self_unix.go @@ -0,0 +1,17 @@ +//go:build !windows + +package ipcauth + +import "os" + +// CurrentProcessIdentity returns this process's identity as the daemon would +// see it if this process connected to the local IPC. It lets a client (the UI) +// decide up front whether a privileged operation can succeed, without a +// round-trip and without duplicating the rules: the answer comes from the same +// Identity.IsPrivileged the daemon applies. +func CurrentProcessIdentity() (Identity, error) { + return Identity{ + UID: uint32(os.Geteuid()), + GID: uint32(os.Getegid()), + }, nil +} diff --git a/client/internal/ipcauth/self_windows.go b/client/internal/ipcauth/self_windows.go new file mode 100644 index 000000000..5474cc101 --- /dev/null +++ b/client/internal/ipcauth/self_windows.go @@ -0,0 +1,35 @@ +//go:build windows + +package ipcauth + +import ( + "fmt" + + "golang.org/x/sys/windows" +) + +// CurrentProcessIdentity returns this process's identity as the daemon would see +// it if this process connected to the local IPC. It lets a client (the UI) +// decide up front whether a privileged operation can succeed, without a +// round-trip and without duplicating the rules: the answer comes from the same +// Identity.IsPrivileged the daemon applies to the token it reads off the pipe. +func CurrentProcessIdentity() (Identity, error) { + // A pseudo-token, so it must not be closed. + token := windows.GetCurrentProcessToken() + + user, err := token.GetTokenUser() + if err != nil { + return Identity{}, fmt.Errorf("read token user: %w", err) + } + + groups, err := tokenGroupSIDs(token) + if err != nil { + return Identity{}, err + } + + return Identity{ + SID: user.User.Sid.String(), + Groups: groups, + Elevated: token.IsElevated(), + }, nil +} diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index a110e4102..e1668238e 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -746,6 +746,13 @@ func (config *Config) applyMDMPolicy(policy *mdm.Policy) { // appended for https or ":80" for http. The serviceName parameter is // used to contextualise error messages. On success returns the parsed // *url.URL; on failure returns a non-nil error. +// ParseServiceURL normalises a service URL exactly as the config layer does when +// it stores one, so callers comparing a requested URL against a stored one do not +// have to reimplement the scheme validation and default-port handling. +func ParseServiceURL(serviceName, serviceURL string) (*url.URL, error) { + return parseURL(serviceName, serviceURL) +} + func parseURL(serviceName, serviceURL string) (*url.URL, error) { parsedMgmtURL, err := url.ParseRequestURI(serviceURL) if err != nil { diff --git a/client/server/login_gate_test.go b/client/server/login_gate_test.go new file mode 100644 index 000000000..de62a8180 --- /dev/null +++ b/client/server/login_gate_test.go @@ -0,0 +1,127 @@ +package server + +import ( + "context" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/internal" + "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/client/proto" +) + +// A refused login must not leave the profile switched. Login can both switch +// profiles and carry the guarded config fields, so the gate has to run before the +// switch: otherwise a caller whose change is refused still gets the side effect of +// activating whichever profile the request named. +func TestLogin_RefusedChangeLeavesTheProfileAlone(t *testing.T) { + s, _, activeProfile, username, _ := setupServerWithProfile(t) + + // Login reads process state off the daemon's root context. + s.rootCtx = internal.CtxInitState(context.Background()) + + // A second profile that runs the SSH server, which is what makes repointing + // its management binding a privileged change. + target := "ssh-enabled" + _, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ + ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, target+".json"), + ManagementURL: "https://api.netbird.io:443", + ServerSSHAllowed: boolPtr(true), + }) + require.NoError(t, err) + + _, err = s.Login(userCtx(), &proto.LoginRequest{ + ProfileName: &target, + Username: &username, + ManagementUrl: "https://mgmt.attacker.example:443", + }) + require.Error(t, err, "an unprivileged caller must not move the management URL of an SSH-enabled profile") + require.Equal(t, codes.PermissionDenied, gstatus.Code(err), "want a privilege refusal, got %v", err) + + active, err := s.profileManager.GetActiveProfileState() + require.NoError(t, err) + require.Equal(t, profilemanager.ID(activeProfile), active.ID, + "the refused login switched the active profile anyway") +} + +// A caller whose change becomes privileged only after its first check must be +// refused without having cancelled a login or switched profiles: the first check is +// unsynchronized, so the SSH server can be enabled by a concurrent privileged +// request in between, and the authoritative check happens before any side effect. +func TestLogin_ChangeThatBecomesPrivilegedMidRequestHasNoSideEffects(t *testing.T) { + s, _, activeProfile, username, _ := setupServerWithProfile(t) + s.rootCtx = internal.CtxInitState(context.Background()) + + // The target profile has SSH off, so the first check lets the request through. + target := "ssh-later" + targetPath := filepath.Join(profilemanager.DefaultConfigPathDir, target+".json") + _, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ + ConfigPath: targetPath, + ManagementURL: "https://api.netbird.io:443", + ServerSSHAllowed: boolPtr(false), + }) + require.NoError(t, err) + + cancelled := false + s.actCancel = func() { cancelled = true } + + // Stand in for a privileged SetConfig that enables the SSH server between the + // two checks, which is the interleaving the lock has to make safe. + afterLoginPreCheck = func() { + _, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ + ConfigPath: targetPath, + ServerSSHAllowed: boolPtr(true), + }) + require.NoError(t, err) + } + t.Cleanup(func() { afterLoginPreCheck = nil }) + + _, err = s.Login(userCtx(), &proto.LoginRequest{ + ProfileName: &target, + Username: &username, + ManagementUrl: "https://mgmt.attacker.example:443", + }) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, gstatus.Code(err), "want a privilege refusal, got %v", err) + require.False(t, cancelled, "the refused login cancelled the login already in progress") + + active, err := s.profileManager.GetActiveProfileState() + require.NoError(t, err) + require.Equal(t, profilemanager.ID(activeProfile), active.ID, "the refused login switched the active profile anyway") + + stored, err := profilemanager.ReadConfig(targetPath) + require.NoError(t, err) + require.Equal(t, "https://api.netbird.io:443", stored.ManagementURL.String(), "the refused login moved the management URL") +} + +// Login cancels whatever login is already in progress before starting its own. A +// refused caller must not get that far, otherwise anyone able to reach the socket +// can abort someone else's login by sending a request that is denied. +func TestLogin_RefusedChangeLeavesAnInProgressLoginAlone(t *testing.T) { + s, _, _, username, _ := setupServerWithProfile(t) + s.rootCtx = internal.CtxInitState(context.Background()) + + target := "ssh-enabled" + _, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ + ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, target+".json"), + ManagementURL: "https://api.netbird.io:443", + ServerSSHAllowed: boolPtr(true), + }) + require.NoError(t, err) + + cancelled := false + s.actCancel = func() { cancelled = true } + + _, err = s.Login(userCtx(), &proto.LoginRequest{ + ProfileName: &target, + Username: &username, + ManagementUrl: "https://mgmt.attacker.example:443", + }) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, gstatus.Code(err), "want a privilege refusal, got %v", err) + require.False(t, cancelled, "the refused login cancelled the login already in progress") +} diff --git a/client/server/server.go b/client/server/server.go index 8047006fe..db5909272 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -82,6 +82,12 @@ type Server struct { // extend flow or vice versa. extendAuthSessionFlow *auth.PendingFlow + // guardedConfigMu serializes a privilege check against the write it + // authorizes. Without it the two are separate steps over the same file, and a + // change that was allowed because the profile had the SSH server disabled + // could land after a concurrent privileged request enabled it. + guardedConfigMu sync.Mutex + mutex sync.Mutex config *profilemanager.Config proto.UnimplementedDaemonServiceServer @@ -411,6 +417,20 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques return nil, err } + // Privilege gate: refuse the parts of the request that would let a local + // user turn the root daemon into a root shell. Held across the write so the + // config cannot gain the SSH server between the decision and the update. + s.guardedConfigMu.Lock() + defer s.guardedConfigMu.Unlock() + + stored, err := s.storedProfileConfig(msg.ProfileName, msg.Username) + if err != nil { + return nil, err + } + if err := requirePrivilegeForConfigChange(callerCtx, stored, privilegedChangeFromSetConfig(msg)); err != nil { + return nil, err + } + config, err := s.setConfigInputFromRequest(msg) if err != nil { return nil, err @@ -537,22 +557,23 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro } } - s.mutex.Lock() - if s.actCancel != nil { - s.actCancel() - } - ctx, cancel := context.WithCancel(callerCtx) - - md, ok := metadata.FromIncomingContext(callerCtx) - if ok { - ctx = metadata.NewOutgoingContext(ctx, md) + activeProf, err := s.profileManager.GetActiveProfileState() + if err != nil { + log.Errorf("failed to get active profile state: %v", err) + return nil, fmt.Errorf("failed to get active profile state: %w", err) } - s.actCancel = cancel - s.mutex.Unlock() - - if err := RestoreResidualState(s.rootCtx, s.profileManager.GetStatePath()); err != nil { - log.Warnf(errRestoreResidualState, err) + // Privilege gate: same restrictions as SetConfig, since LoginRequest can carry + // the same fields. It runs before anything here changes daemon state, so a + // refused login neither switches the profile nor cancels a login already in + // progress, and it reads the profile the request targets, which is the one the + // switch below would activate. + stored, err := s.storedLoginConfig(activeProf, msg) + if err != nil { + return nil, err + } + if err := requirePrivilegeForConfigChange(callerCtx, stored, privilegedChangeFromLogin(msg)); err != nil { + return nil, err } state := internal.CtxGetState(s.rootCtx) @@ -563,23 +584,16 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro } }() - activeProf, err := s.profileManager.GetActiveProfileState() + ctx, activeProf, err := s.authorizeAndPrepareLogin(callerCtx, msg, activeProf) if err != nil { - log.Errorf("failed to get active profile state: %v", err) - return nil, fmt.Errorf("failed to get active profile state: %w", err) - } - - if msg.ProfileName != nil { - if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { - log.Errorf("failed to switch profile: %v", err) - return nil, err + // The RPC boundary is where this gets recorded: nothing logs handler + // errors for us, and a caller that retries would otherwise leave no + // trace in the daemon log. A refusal is skipped because the gate has + // already logged the decision, with the caller's identity. + if gstatus.Code(err) != codes.PermissionDenied { + log.Errorf("failed to prepare login: %v", err) } - } - - activeProf, err = s.profileManager.GetActiveProfileState() - if err != nil { - log.Errorf("failed to get active profile state: %v", err) - return nil, fmt.Errorf("failed to get active profile state: %w", err) + return nil, err } log.Infof("active profile: %s for %s", activeProf.ID, activeProf.Username) @@ -593,11 +607,6 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro s.mutex.Unlock() - if err := persistLoginOverrides(activeProf, msg.ManagementUrl, msg.OptionalPreSharedKey); err != nil { - log.Errorf("failed to persist login overrides: %v", err) - return nil, fmt.Errorf("persist login overrides: %w", err) - } - config, _, err := s.getConfig(activeProf) if err != nil { log.Errorf("failed to get active profile config: %v", err) @@ -980,6 +989,63 @@ func (s *Server) waitForUp(callerCtx context.Context) (*proto.UpResponse, error) } } +// storedProfileConfig loads the on-disk config of the profile a request +// targets, so a privileged-change decision can be made against the values the +// profile currently holds. A profile that has no config file yet yields nil, +// which every caller must read as "nothing enabled yet". +func (s *Server) storedProfileConfig(handle, username string) (*profilemanager.Config, error) { + resolved, err := s.resolveProfileHandle(handle, username) + if err != nil { + return nil, err + } + + path := resolved.Path + if path == "" { + path = profilemanager.DefaultConfigPath + } + + return s.storedConfigAtPath(path) +} + +// storedLoginConfig loads the on-disk config of the profile a login request +// targets: the one it names, or the active one when it names none. Used to decide +// a privileged change before the request is allowed to switch profiles. +func (s *Server) storedLoginConfig(activeProf *profilemanager.ActiveProfileState, msg *proto.LoginRequest) (*profilemanager.Config, error) { + if msg.ProfileName == nil { + cfgPath, err := activeProf.FilePath() + if err != nil { + return nil, fmt.Errorf("active profile file path: %w", err) + } + return s.storedConfigAtPath(cfgPath) + } + + // Mirrors switchProfileIfNeeded: the default profile resolves without a + // username, so this reads the same profile the switch would activate. + handle := *msg.ProfileName + username := "" + if handle != profilemanager.DefaultProfileName { + username = msg.GetUsername() + } + return s.storedProfileConfig(handle, username) +} + +// storedConfigAtPath reads a profile config file, yielding nil when it does not +// exist yet. +func (s *Server) storedConfigAtPath(path string) (*profilemanager.Config, error) { + if _, err := os.Stat(path); err != nil { + if os.IsNotExist(err) { + return nil, nil //nolint:nilnil + } + return nil, fmt.Errorf("stat profile config: %w", err) + } + + cfg, err := profilemanager.GetConfig(path) + if err != nil { + return nil, fmt.Errorf("read profile config: %w", err) + } + return cfg, nil +} + // resolveProfileHandle resolves a wire-level profile handle (display // name, ID, or unique ID prefix) to a concrete profile. Returns gRPC // status errors so handlers can return them directly. @@ -1197,6 +1263,12 @@ func (s *Server) handleProfileLogout(ctx context.Context, msg *proto.LogoutReque if err := s.logoutFromProfile(ctx, resolved); err != nil { log.Errorf("failed to logout from profile %s: %v", resolved.ID, err) + // A refused deregistration is already a status error carrying the reason + // and the command to run; rewrapping it as Internal would flatten both + // into a gRPC dump for the user. + if _, isStatus := gstatus.FromError(err); isStatus { + return nil, err + } return nil, gstatus.Errorf(codes.Internal, "logout: %v", err) } @@ -1318,6 +1390,13 @@ func (s *Server) sendLogoutRequest(ctx context.Context) error { } func (s *Server) sendLogoutRequestWithConfig(ctx context.Context, config *profilemanager.Config) error { + // Privilege gate: deregistering frees this machine's key to be registered + // against another management server, which is only restricted while the SSH + // server makes that a privilege handover. + if err := requirePrivilegeForDeregistration(ctx, config); err != nil { + return err + } + key, err := wgtypes.ParseKey(config.PrivateKey) if err != nil { return fmt.Errorf("parse private key: %w", err) @@ -2063,7 +2142,10 @@ func (s *Server) RemoveProfile(ctx context.Context, msg *proto.RemoveProfileRequ } if err := s.logoutFromProfile(ctx, resolved); err != nil { - log.Warnf("failed to logout from profile %s before removal: %v", resolved.ID, err) + // Deregistration is best-effort here: the local profile is removed + // either way, so an unprivileged caller leaves the peer registered on + // the management server rather than being blocked from removing it. + log.Warnf("removing profile %s locally without deregistering it: %v", resolved.ID, err) } if err := s.profileManager.RemoveProfile(resolved.ID, msg.Username); err != nil { @@ -2360,6 +2442,69 @@ func sendTerminalNotification() error { // persistLoginOverrides writes management URL and pre-shared key from a LoginRequest to the // active profile config so that subsequent reads pick them up. Empty/nil values are ignored. +// afterLoginPreCheck is a seam for tests to run a concurrent config change +// between Login's first privilege check and the authoritative one. +var afterLoginPreCheck func() + +// authorizeAndPrepareLogin makes the authoritative privilege decision for a login +// and, when it passes, carries out every state change that decision authorizes: +// cancelling an login already in progress, switching to the requested profile, and +// persisting the config overrides the request carries. +// +// All of it happens under guardedConfigMu, which SetConfig also holds across its +// own check and write. Login's earlier check refuses the ordinary case before any +// of this is reached; this one exists because that check is not synchronized +// against a concurrent privileged request that enables the SSH server, and a +// caller refused here must not have cancelled or switched anything either. +func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto.LoginRequest, activeProf *profilemanager.ActiveProfileState) (context.Context, *profilemanager.ActiveProfileState, error) { + if afterLoginPreCheck != nil { + afterLoginPreCheck() + } + + s.guardedConfigMu.Lock() + defer s.guardedConfigMu.Unlock() + + stored, err := s.storedLoginConfig(activeProf, msg) + if err != nil { + return nil, nil, err + } + if err := requirePrivilegeForConfigChange(callerCtx, stored, privilegedChangeFromLogin(msg)); err != nil { + return nil, nil, err + } + + s.mutex.Lock() + if s.actCancel != nil { + s.actCancel() + } + ctx, cancel := context.WithCancel(callerCtx) + if md, ok := metadata.FromIncomingContext(callerCtx); ok { + ctx = metadata.NewOutgoingContext(ctx, md) + } + s.actCancel = cancel + s.mutex.Unlock() + + if err := RestoreResidualState(s.rootCtx, s.profileManager.GetStatePath()); err != nil { + log.Warnf(errRestoreResidualState, err) + } + + if msg.ProfileName != nil { + if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { + return nil, nil, fmt.Errorf("switch profile: %w", err) + } + } + + activeProf, err = s.profileManager.GetActiveProfileState() + if err != nil { + return nil, nil, fmt.Errorf("active profile state: %w", err) + } + + if err := persistLoginOverrides(activeProf, msg.ManagementUrl, msg.OptionalPreSharedKey); err != nil { + return nil, nil, fmt.Errorf("persist login overrides: %w", err) + } + + return ctx, activeProf, nil +} + func persistLoginOverrides(activeProf *profilemanager.ActiveProfileState, managementURL string, preSharedKey *string) error { if preSharedKey != nil && *preSharedKey == "" { preSharedKey = nil diff --git a/client/server/setconfig_mdm_test.go b/client/server/setconfig_mdm_test.go index 9baf16136..ae323ea8c 100644 --- a/client/server/setconfig_mdm_test.go +++ b/client/server/setconfig_mdm_test.go @@ -66,7 +66,11 @@ func setupServerWithProfile(t *testing.T) (s *Server, ctx context.Context, profN Username: currUser.Username, })) - ctx = context.Background() + // The privileged-change gate reads the caller's kernel identity from the + // context, which a real caller gets from the daemon's transport credentials. + // This test drives the handler directly, so it stands in for a root caller; + // without an identity the gate would (correctly) refuse the SSH fields. + ctx = privilegedTestCtx() s = New(ctx, "console", "", false, false, false, false) return s, ctx, profName, currUser.Username, cfgPath } diff --git a/client/server/setconfig_test.go b/client/server/setconfig_test.go index 0e55257a9..db7a26f03 100644 --- a/client/server/setconfig_test.go +++ b/client/server/setconfig_test.go @@ -1,7 +1,6 @@ package server import ( - "context" "os/user" "path/filepath" "reflect" @@ -52,7 +51,11 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) { }) require.NoError(t, err) - ctx := context.Background() + // The privileged-change gate reads the caller's kernel identity from the + // context, which a real caller gets from the daemon's transport credentials. + // This test drives the handler directly, so it stands in for a root caller; + // without an identity the gate would (correctly) refuse the SSH fields. + ctx := privilegedTestCtx() s := New(ctx, "console", "", false, false, false, false) rosenpassEnabled := true diff --git a/client/server/ssh_gate.go b/client/server/ssh_gate.go new file mode 100644 index 000000000..ca1b4c4ee --- /dev/null +++ b/client/server/ssh_gate.go @@ -0,0 +1,282 @@ +package server + +import ( + "context" + "fmt" + "net/url" + "runtime" + "strings" + + log "github.com/sirupsen/logrus" + "google.golang.org/genproto/googleapis/rpc/errdetails" + "google.golang.org/grpc/codes" + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/internal/daemonaddr" + "github.com/netbirdio/netbird/client/internal/ipcauth" + "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/client/proto" + "github.com/netbirdio/netbird/util" +) + +// The daemon runs as root/LocalSystem, so a handful of config changes cross the +// user-to-root boundary and are restricted to privileged callers: +// +// - Enabling SSH root login, or disabling SSH authentication, turns the +// daemon's SSH server into a root (or unauthenticated) shell. +// - Enabling the SSH server at all is what makes the above reachable, and a +// profile the caller owns is not a privilege they hold. +// - While the SSH server is enabled, repointing the profile at another +// management identity hands SSH authorization decisions, including which +// keys and users are accepted, to whoever controls that identity. Changing +// the management URL and deregistering the peer are both ways to do that. +// +// Everything else stays unauthenticated, so this is not an authorization model: +// it only refuses the changes that would let a local user become root. A caller +// whose identity cannot be established is refused as well. + +// privilegedConfigChange is the subset of a config request that crosses the +// user-to-root boundary. Fields are nil or empty when the request leaves them +// untouched. +type privilegedConfigChange struct { + managementURL string + serverSSHAllowed *bool + enableSSHRoot *bool + disableSSHAuth *bool +} + +func privilegedChangeFromSetConfig(msg *proto.SetConfigRequest) privilegedConfigChange { + return privilegedConfigChange{ + managementURL: msg.GetManagementUrl(), + serverSSHAllowed: msg.ServerSSHAllowed, + enableSSHRoot: msg.EnableSSHRoot, + disableSSHAuth: msg.DisableSSHAuth, + } +} + +func privilegedChangeFromLogin(msg *proto.LoginRequest) privilegedConfigChange { + return privilegedConfigChange{ + managementURL: msg.GetManagementUrl(), + serverSSHAllowed: msg.ServerSSHAllowed, + enableSSHRoot: msg.EnableSSHRoot, + disableSSHAuth: msg.DisableSSHAuth, + } +} + +// requirePrivilegeForConfigChange refuses the privileged parts of a config +// change when the caller is not root/administrator. stored is the profile's +// current config, or nil when it has none yet. +// +// Each check compares against the stored value so that a request restating a +// value it does not change is never refused: a UI that submits the whole +// settings form must not start failing once an administrator has enabled SSH. +func requirePrivilegeForConfigChange(ctx context.Context, stored *profilemanager.Config, change privilegedConfigChange) error { + if enables(storedFlag(stored, func(c *profilemanager.Config) *bool { return c.EnableSSHRoot }), change.enableSSHRoot) { + return denyPrivileged(ctx, "enabling SSH root login", ipcauth.UpCommand("--enable-ssh-root")) + } + + if enables(storedFlag(stored, func(c *profilemanager.Config) *bool { return c.DisableSSHAuth }), change.disableSSHAuth) { + return denyPrivileged(ctx, "disabling SSH authentication", ipcauth.UpCommand("--disable-ssh-auth")) + } + + if enables(sshServerCurrentlyAllowed(stored), change.serverSSHAllowed) { + return denyPrivileged(ctx, "enabling the NetBird SSH server", ipcauth.UpCommand("--allow-server-ssh")) + } + + // Only guard the management binding while the SSH server is enabled: that is + // when the management identity decides who may open a shell here. + if !sshServerEnabled(stored) { + return nil + } + + if change.managementURL != "" && !sameManagementURL(stored.ManagementURL, change.managementURL) { + return denyPrivileged(ctx, + "changing the management URL while the NetBird SSH server is enabled", + ipcauth.UpCommand("-m "+change.managementURL)) + } + + return nil +} + +// requirePrivilegeForDeregistration refuses to deregister the peer from the +// management server when the caller is not privileged and the profile has the +// SSH server enabled. Deregistering frees the peer's key to be registered +// against another management identity, which is the same handover the +// management URL check refuses. +// +// Callers that treat deregistration as best-effort (profile removal) continue +// without it; callers that were asked to deregister surface the error. +func requirePrivilegeForDeregistration(ctx context.Context, cfg *profilemanager.Config) error { + if !sshServerEnabled(cfg) { + return nil + } + + return denyPrivileged(ctx, + "deregistering this peer while the NetBird SSH server is enabled", + ipcauth.ElevatedCommand("netbird logout")) +} + +// denyPrivileged returns nil when the caller is privileged, and otherwise a +// PermissionDenied whose message names the action and the command that performs +// it with the privileges it needs. The same summary and command ride along as an +// ErrorInfo detail so the CLI and the UI can present them without parsing text. +// +// action reads as the subject of a sentence ("enabling SSH root login"), and +// command is the equivalent command, already elevated for the platform. +func denyPrivileged(ctx context.Context, action, command string) error { + id, ok := ipcauth.CallerIdentity(ctx) + if !ok { + log.Warnf("denying %s: the caller's identity cannot be verified on this control channel", action) + return privilegeError(unidentifiedSummary(action), reinstallCommand()) + } + + if ipcauth.IsPrivilegedCaller(id) { + log.Infof("allowing %s for privileged caller %s", action, id) + return nil + } + + log.Warnf("denying %s for unprivileged caller %s", action, id) + actor, command := requiredActor(command) + return privilegeError(privilegeSummary(action, actor), command) +} + +// requiredActor names who may perform the operation and adjusts the command to +// match. A daemon that is not itself privileged delegates to its own identity, so +// telling that host's user to become root is wrong twice over: root is not what the +// daemon checks for, and a rootless container has neither root nor sudo. +func requiredActor(command string) (string, string) { + self, delegates := ipcauth.SelfDelegatesTo() + if !delegates { + return ipcauth.PrivilegedActor(), command + } + return fmt.Sprintf("the user the daemon runs as (%s)", self), strings.ReplaceAll(command, "sudo ", "") +} + +// privilegeError builds the PermissionDenied carrying summary and command. +func privilegeError(summary, command string) error { + st := gstatus.New(codes.PermissionDenied, fmt.Sprintf("%s\n\n%s", summary, command)) + + detailed, err := st.WithDetails(&errdetails.ErrorInfo{ + Reason: ipcauth.ErrorReasonPrivilegeRequired, + Domain: ipcauth.ErrorDomain, + Metadata: map[string]string{ + ipcauth.ErrorMetaSummary: summary, + ipcauth.ErrorMetaCommand: command, + }, + }) + if err != nil { + log.Debugf("attach privilege error detail: %v", err) + return st.Err() + } + return detailed.Err() +} + +// privilegeSummary states what is refused and what it needs, in one sentence +// that reads the same in a dialog and in a terminal. +func privilegeSummary(action, actor string) string { + return fmt.Sprintf("%s requires %s.", capitalize(action), actor) +} + +// unidentifiedSummary covers a control channel that carries no caller identity. +// Elevating does not help there, so it points at the daemon's socket instead. +func unidentifiedSummary(action string) string { + return fmt.Sprintf("%s requires %s, and the daemon cannot verify who is calling over its current socket. "+ + "Reinstall the service on a socket that carries the caller's identity.", capitalize(action), ipcauth.PrivilegedActor()) +} + +// reinstallCommand is the command that moves the daemon onto a socket whose +// callers can be identified. +func reinstallCommand() string { + if runtime.GOOS == "windows" { + return fmt.Sprintf("netbird service install --daemon-addr %s", daemonaddr.WindowsPipeAddr) + } + return "sudo netbird service install --daemon-addr unix:///var/run/netbird.sock" +} + +func capitalize(s string) string { + if s == "" { + return s + } + return strings.ToUpper(s[:1]) + s[1:] +} + +// enables reports whether requested turns a flag on that is currently off. A +// request that restates the stored value, or turns the flag off, is not a +// privileged change. +func enables(stored, requested *bool) bool { + if requested == nil || !*requested { + return false + } + return stored == nil || !*stored +} + +// storedFlag reads a flag from the stored config, tolerating a config that does +// not exist yet. +func storedFlag(cfg *profilemanager.Config, get func(*profilemanager.Config) *bool) *bool { + if cfg == nil { + return nil + } + return get(cfg) +} + +// sshServerEnabled reports whether the profile currently runs the SSH server. +// +// A nil flag means ON, matching what the engine does with the same config +// (util.ReturnBoolWithDefaultTrue in internal/connect.go, kept for configs written +// before the flag existed). Reading it as OFF here would open the management-URL +// and deregistration guards on exactly those legacy hosts, whose SSH server is +// running. Configs loaded through profilemanager have already been materialised by +// apply(), so this is the same answer by a route that does not depend on that. +func sshServerEnabled(cfg *profilemanager.Config) bool { + if cfg == nil { + return false + } + return util.ReturnBoolWithDefaultTrue(cfg.ServerSSHAllowed) +} + +// sshServerCurrentlyAllowed is the value an enable request is compared against. It +// shares sshServerEnabled's nil-means-on default, so restating "on" for a legacy +// config is correctly seen as no change. +func sshServerCurrentlyAllowed(cfg *profilemanager.Config) *bool { + enabled := sshServerEnabled(cfg) + if cfg == nil { + return nil + } + return &enabled +} + +// sameManagementURL reports whether requested addresses the same management +// server as stored, comparing scheme, host and effective port so that an +// equivalent spelling ("https://api.netbird.io" for a stored +// "https://api.netbird.io:443") is not treated as a change. It fails closed: +// anything unparseable counts as a change and therefore needs privilege. +func sameManagementURL(stored *url.URL, requested string) bool { + if stored == nil { + return false + } + + // Normalise the requested URL through the config layer's own parser, so the + // comparison cannot drift from how the value would actually be stored. + parsed, err := profilemanager.ParseServiceURL("Management URL", requested) + if err != nil { + return false + } + + return stored.Scheme == parsed.Scheme && + stored.Hostname() == parsed.Hostname() && + effectivePort(stored) == effectivePort(parsed) +} + +func effectivePort(u *url.URL) string { + if port := u.Port(); port != "" { + return port + } + switch u.Scheme { + case "https": + return "443" + case "http": + return "80" + default: + return "" + } +} diff --git a/client/server/ssh_gate_test.go b/client/server/ssh_gate_test.go new file mode 100644 index 000000000..cbd345f16 --- /dev/null +++ b/client/server/ssh_gate_test.go @@ -0,0 +1,348 @@ +package server + +import ( + "context" + "net/url" + "os" + "runtime" + "strings" + "testing" + + "google.golang.org/genproto/googleapis/rpc/errdetails" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/peer" + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/internal/ipcauth" + "github.com/netbirdio/netbird/client/internal/profilemanager" +) + +// ctxWithIdentity builds a request context carrying the identity the transport +// credentials would have attached. +func ctxWithIdentity(id ipcauth.Identity) context.Context { + return peer.NewContext(context.Background(), &peer.Peer{ + AuthInfo: ipcauth.AuthInfo{ + CommonAuthInfo: credentials.CommonAuthInfo{SecurityLevel: credentials.NoSecurity}, + Identity: id, + }, + }) +} + +// unprivUID is deliberately not this process's own uid. An unprivileged daemon +// treats a caller sharing its identity as privileged (rootless containers), and +// the test binary would otherwise stand in for both the daemon and the caller. +// os.Geteuid returns -1 on Windows, where identities are SIDs instead and this is +// unused. +var unprivUID = uint32(os.Geteuid() + 1) + +// The fabricated identities have to be shaped like the platform's: a uid says +// nothing on Windows, and a zero uid there would read as root and be privileged. +func rootCtx() context.Context { return ctxWithIdentity(privilegedIdentity()) } +func userCtx() context.Context { return ctxWithIdentity(unprivilegedIdentity()) } + +func privilegedIdentity() ipcauth.Identity { + if runtime.GOOS == "windows" { + // LocalSystem, which is what the Windows service account is. + return ipcauth.Identity{SID: "S-1-5-18"} + } + return ipcauth.Identity{UID: 0} +} + +func unprivilegedIdentity() ipcauth.Identity { + if runtime.GOOS == "windows" { + // A plain user SID: no groups, so no BUILTIN\Administrators, and not + // elevated. + return ipcauth.Identity{SID: "S-1-5-21-1-2-3-1001"} + } + return ipcauth.Identity{UID: unprivUID, GID: unprivUID} +} +func noIdentityCtx() context.Context { return context.Background() } + +func boolPtr(v bool) *bool { return &v } + +func mustURL(t *testing.T, raw string) *url.URL { + t.Helper() + u, err := url.Parse(raw) + if err != nil { + t.Fatalf("parse %q: %v", raw, err) + } + return u +} + +func assertDenied(t *testing.T, err error) { + t.Helper() + if err == nil { + t.Fatal("expected the change to be refused, got nil") + } + st := gstatus.Convert(err) + if st.Code() != codes.PermissionDenied { + t.Fatalf("code = %v, want PermissionDenied", st.Code()) + } + // The refusal must be machine-readable: the CLI and the UI render the + // summary and command from the detail rather than parsing the message. + var info *errdetails.ErrorInfo + for _, d := range st.Details() { + if got, ok := d.(*errdetails.ErrorInfo); ok { + info = got + } + } + if info == nil { + t.Fatal("refusal carries no ErrorInfo detail") + } + if info.GetReason() != ipcauth.ErrorReasonPrivilegeRequired || info.GetDomain() != ipcauth.ErrorDomain { + t.Fatalf("detail = %s/%s, want %s/%s", info.GetDomain(), info.GetReason(), ipcauth.ErrorDomain, ipcauth.ErrorReasonPrivilegeRequired) + } + if info.GetMetadata()[ipcauth.ErrorMetaSummary] == "" { + t.Error("detail carries no summary") + } + if info.GetMetadata()[ipcauth.ErrorMetaCommand] == "" { + t.Error("detail carries no command") + } +} + +func assertAllowed(t *testing.T, err error) { + t.Helper() + if err != nil { + t.Fatalf("expected the change to be allowed, got %v", err) + } +} + +func TestRequirePrivilegeForConfigChange_SSHFlags(t *testing.T) { + tests := []struct { + name string + stored *profilemanager.Config + change privilegedConfigChange + privileged bool + wantDeny bool + }{ + { + name: "enabling the ssh server unprivileged is refused", + stored: &profilemanager.Config{ServerSSHAllowed: boolPtr(false)}, + change: privilegedConfigChange{serverSSHAllowed: boolPtr(true)}, + wantDeny: true, + }, + { + name: "enabling the ssh server as root is allowed", + stored: &profilemanager.Config{ServerSSHAllowed: boolPtr(false)}, + change: privilegedConfigChange{serverSSHAllowed: boolPtr(true)}, + privileged: true, + }, + { + name: "restating an already enabled ssh server is not a change", + stored: &profilemanager.Config{ServerSSHAllowed: boolPtr(true)}, + change: privilegedConfigChange{serverSSHAllowed: boolPtr(true)}, + }, + { + name: "turning the ssh server off is not guarded", + stored: &profilemanager.Config{ServerSSHAllowed: boolPtr(true)}, + change: privilegedConfigChange{serverSSHAllowed: boolPtr(false)}, + }, + { + name: "a profile with no config yet counts as off, so enabling is refused", + stored: nil, + change: privilegedConfigChange{serverSSHAllowed: boolPtr(true)}, + wantDeny: true, + }, + { + name: "enabling ssh root login unprivileged is refused", + stored: &profilemanager.Config{EnableSSHRoot: boolPtr(false)}, + change: privilegedConfigChange{enableSSHRoot: boolPtr(true)}, + wantDeny: true, + }, + { + name: "restating ssh root login is not a change", + stored: &profilemanager.Config{EnableSSHRoot: boolPtr(true)}, + change: privilegedConfigChange{enableSSHRoot: boolPtr(true)}, + }, + { + name: "turning ssh root login off is not guarded", + stored: &profilemanager.Config{EnableSSHRoot: boolPtr(true)}, + change: privilegedConfigChange{enableSSHRoot: boolPtr(false)}, + }, + { + name: "disabling ssh authentication unprivileged is refused", + stored: &profilemanager.Config{DisableSSHAuth: boolPtr(false)}, + change: privilegedConfigChange{disableSSHAuth: boolPtr(true)}, + wantDeny: true, + }, + { + name: "re-enabling ssh authentication is not guarded", + stored: &profilemanager.Config{DisableSSHAuth: boolPtr(true)}, + change: privilegedConfigChange{disableSSHAuth: boolPtr(false)}, + }, + { + name: "a request that touches none of the guarded fields is allowed", + stored: &profilemanager.Config{ServerSSHAllowed: boolPtr(false)}, + change: privilegedConfigChange{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := userCtx() + if tt.privileged { + ctx = rootCtx() + } + err := requirePrivilegeForConfigChange(ctx, tt.stored, tt.change) + if tt.wantDeny { + assertDenied(t, err) + return + } + assertAllowed(t, err) + }) + } +} + +func TestRequirePrivilegeForConfigChange_ManagementURL(t *testing.T) { + sshOn := func(raw string) *profilemanager.Config { + return &profilemanager.Config{ServerSSHAllowed: boolPtr(true), ManagementURL: mustURL(t, raw)} + } + sshOff := func(raw string) *profilemanager.Config { + return &profilemanager.Config{ServerSSHAllowed: boolPtr(false), ManagementURL: mustURL(t, raw)} + } + + tests := []struct { + name string + stored *profilemanager.Config + requested string + privileged bool + wantDeny bool + }{ + { + name: "moving the binding while ssh is enabled is refused", + stored: sshOn("https://api.netbird.io:443"), + requested: "https://attacker.example.com:443", + wantDeny: true, + }, + { + name: "moving the binding as root is allowed", + stored: sshOn("https://api.netbird.io:443"), + requested: "https://selfhosted.example.com:443", + privileged: true, + }, + { + name: "the same url restated is not a change", + stored: sshOn("https://api.netbird.io:443"), + requested: "https://api.netbird.io:443", + }, + { + name: "an equivalent spelling of the same url is not a change", + stored: sshOn("https://api.netbird.io:443"), + requested: "https://api.netbird.io", + }, + { + name: "an equivalent spelling with an explicit http port is not a change", + stored: sshOn("http://mgmt.internal:80"), + requested: "http://mgmt.internal", + }, + { + name: "a different port on the same host is a change", + stored: sshOn("https://api.netbird.io:443"), + requested: "https://api.netbird.io:8443", + wantDeny: true, + }, + { + name: "a different scheme on the same host is a change", + stored: sshOn("https://mgmt.internal:443"), + requested: "http://mgmt.internal:443", + wantDeny: true, + }, + { + name: "with ssh disabled the binding is not guarded at all", + stored: sshOff("https://api.netbird.io:443"), + requested: "https://attacker.example.com:443", + }, + { + name: "an unparseable url fails closed", + stored: sshOn("https://api.netbird.io:443"), + requested: "ht tp://%zz", + wantDeny: true, + }, + { + name: "an empty url leaves the binding alone", + stored: sshOn("https://api.netbird.io:443"), + requested: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := userCtx() + if tt.privileged { + ctx = rootCtx() + } + err := requirePrivilegeForConfigChange(ctx, tt.stored, privilegedConfigChange{managementURL: tt.requested}) + if tt.wantDeny { + assertDenied(t, err) + return + } + assertAllowed(t, err) + }) + } +} + +// A caller the daemon cannot identify must be refused, not trusted: that is the +// state on a TCP daemon socket, where no peer credentials exist. +func TestRequirePrivilegeForConfigChange_UnidentifiedCallerIsRefused(t *testing.T) { + err := requirePrivilegeForConfigChange(noIdentityCtx(), + &profilemanager.Config{ServerSSHAllowed: boolPtr(false)}, + privilegedConfigChange{serverSSHAllowed: boolPtr(true)}) + assertDenied(t, err) + + // The guidance must point at the socket rather than at sudo, since elevating + // would not help. + st := gstatus.Convert(err) + if !strings.Contains(st.Message(), "service install") { + t.Errorf("message %q does not tell the operator how to fix the socket", st.Message()) + } +} + +func TestRequirePrivilegeForDeregistration(t *testing.T) { + tests := []struct { + name string + cfg *profilemanager.Config + privileged bool + wantDeny bool + }{ + { + name: "deregistering while ssh is enabled is refused", + cfg: &profilemanager.Config{ServerSSHAllowed: boolPtr(true)}, + wantDeny: true, + }, + { + name: "deregistering while ssh is enabled is allowed for root", + cfg: &profilemanager.Config{ServerSSHAllowed: boolPtr(true)}, + privileged: true, + }, + { + name: "deregistering with ssh disabled is not guarded", + cfg: &profilemanager.Config{ServerSSHAllowed: boolPtr(false)}, + }, + { + name: "deregistering a profile with no config is not guarded", + cfg: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := userCtx() + if tt.privileged { + ctx = rootCtx() + } + err := requirePrivilegeForDeregistration(ctx, tt.cfg) + if tt.wantDeny { + assertDenied(t, err) + return + } + assertAllowed(t, err) + }) + } +} + +// privilegedTestCtx is the context a handler-level test should use when it is +// standing in for a root/administrator caller. Tests that drive the handlers +// directly have no transport credentials, and the privileged-change gate refuses +// a caller it cannot identify. +func privilegedTestCtx() context.Context { return rootCtx() } diff --git a/client/ssh/client/client.go b/client/ssh/client/client.go index ebf8eb794..4180849cd 100644 --- a/client/ssh/client/client.go +++ b/client/ssh/client/client.go @@ -9,7 +9,6 @@ import ( "path/filepath" "runtime" "strconv" - "strings" "time" log "github.com/sirupsen/logrus" @@ -17,7 +16,6 @@ import ( "golang.org/x/crypto/ssh/knownhosts" "golang.org/x/term" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" "github.com/netbirdio/netbird/client/internal/daemonaddr" "github.com/netbirdio/netbird/client/internal/profilemanager" @@ -32,7 +30,7 @@ const ( // DefaultDaemonAddr is the default address for the NetBird daemon DefaultDaemonAddr = "unix:///var/run/netbird.sock" // DefaultDaemonAddrWindows is the default address for the NetBird daemon on Windows - DefaultDaemonAddrWindows = "tcp://127.0.0.1:41731" + DefaultDaemonAddrWindows = daemonaddr.WindowsPipeAddr ) // Client wraps crypto/ssh Client for simplified SSH operations @@ -268,7 +266,7 @@ func getDefaultDaemonAddr() string { return addr } if runtime.GOOS == "windows" { - return DefaultDaemonAddrWindows + return daemonaddr.ResolveDaemonAddr(DefaultDaemonAddrWindows) } return daemonaddr.ResolveUnixDaemonAddr(DefaultDaemonAddr) } @@ -410,12 +408,9 @@ func verifyHostKeyViaDaemon(hostname string, remote net.Addr, key ssh.PublicKey, } func connectToDaemon(daemonAddr string) (*grpc.ClientConn, error) { - addr := strings.TrimPrefix(daemonAddr, "tcp://") + target, opts := daemonaddr.DialTarget(daemonAddr) - conn, err := grpc.NewClient( - addr, - grpc.WithTransportCredentials(insecure.NewCredentials()), - ) + conn, err := grpc.NewClient(target, opts...) if err != nil { log.Debugf("failed to create gRPC client for NetBird daemon at %s: %v", daemonAddr, err) return nil, fmt.Errorf("failed to connect to NetBird daemon: %w", err) diff --git a/client/ssh/proxy/proxy.go b/client/ssh/proxy/proxy.go index 73b50122c..721810edb 100644 --- a/client/ssh/proxy/proxy.go +++ b/client/ssh/proxy/proxy.go @@ -9,7 +9,6 @@ import ( "net" "os" "strconv" - "strings" "sync" "time" @@ -17,8 +16,8 @@ import ( log "github.com/sirupsen/logrus" cryptossh "golang.org/x/crypto/ssh" "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" + "github.com/netbirdio/netbird/client/internal/daemonaddr" "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/proto" nbssh "github.com/netbirdio/netbird/client/ssh" @@ -55,8 +54,8 @@ type SSHProxy struct { } func New(daemonAddr, targetHost string, targetPort int, stderr io.Writer, browserOpener func(string) error) (*SSHProxy, error) { - grpcAddr := strings.TrimPrefix(daemonAddr, "tcp://") - grpcConn, err := grpc.NewClient(grpcAddr, grpc.WithTransportCredentials(insecure.NewCredentials())) + target, opts := daemonaddr.DialTarget(daemonAddr) + grpcConn, err := grpc.NewClient(target, opts...) if err != nil { return nil, fmt.Errorf("connect to daemon: %w", err) } diff --git a/client/ui/frontend/src/components/CopyToClipboard.tsx b/client/ui/frontend/src/components/CopyToClipboard.tsx index 1b2a87da4..3cf681a1c 100644 --- a/client/ui/frontend/src/components/CopyToClipboard.tsx +++ b/client/ui/frontend/src/components/CopyToClipboard.tsx @@ -18,6 +18,9 @@ type CopyToClipboardProps = { className?: string; iconClassName?: string; alwaysShowIcon?: boolean; + // wrap lets long content (a shell command, a path) break across lines + // instead of being truncated to one line. + wrap?: boolean; variant?: CopyToClipboardVariant; "aria-label"?: string; tabIndex?: number; @@ -32,6 +35,7 @@ export const CopyToClipboard = ({ className, iconClassName, alwaysShowIcon = false, + wrap = false, variant = "default", "aria-label": ariaLabel, tabIndex = 0, @@ -83,7 +87,8 @@ export const CopyToClipboard = ({ > { loadedRef.current = loaded; }, [loaded]); + // reload re-reads the daemon's config, which is authoritative. Used on + // mount, on the daemon's config_changed event, and to undo an optimistic + // update the daemon then rejected. + const reload = useCallback( + async (profileName: string) => { + try { + const data = await SettingsSvc.GetConfig({ profileName, username }); + setLoaded({ profileName, data }); + } catch (e) { + console.warn("[SettingsContext] reload after rejected save failed", e); + } + }, + [username], + ); + useEffect(() => { if (!profileLoaded || !activeProfileId) return; let cancelled = false; @@ -133,13 +148,20 @@ const useSettingsState = () => { username, }); } catch (e) { + // The optimistic update is wrong now: the daemon refused it + // (a change that needs elevated privileges, an MDM-managed + // field, ...). Snap the controls back to what it actually + // holds before reporting, so the UI never shows a value the + // daemon does not have. + await reload(profileName); await errorDialog({ Title: i18next.t("settings.error.saveTitle"), Message: errorMessage(e), + Command: errorCommand(e), }); } }, - [username], + [username, reload], ); const setField = useCallback( diff --git a/client/ui/frontend/src/hooks/usePrivilege.ts b/client/ui/frontend/src/hooks/usePrivilege.ts new file mode 100644 index 000000000..05e9a7ce0 --- /dev/null +++ b/client/ui/frontend/src/hooks/usePrivilege.ts @@ -0,0 +1,32 @@ +import { useEffect, useState } from "react"; +import { Settings as SettingsSvc } from "@bindings/services"; +import { 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 +// with the daemon's own rule, so there is no round-trip and it works while the +// daemon is down. +// +// null means "not known yet", which includes the read having failed. Callers must +// treat that as "do not restrict": the daemon enforces this regardless, so the +// only thing a wrong guess here costs is a control that looks unavailable when it +// is not, or a save that fails with the daemon's own guidance. +export const usePrivilege = (): Privilege | null => { + const [privilege, setPrivilege] = useState(null); + + useEffect(() => { + let cancelled = false; + SettingsSvc.Privilege() + .then((p) => { + if (!cancelled) setPrivilege(p); + }) + .catch((e: unknown) => { + console.warn("[usePrivilege] read failed, not restricting controls", e); + }); + return () => { + cancelled = true; + }; + }, []); + + return privilege; +}; diff --git a/client/ui/frontend/src/lib/errors.ts b/client/ui/frontend/src/lib/errors.ts index 34a90dace..b4dee2717 100644 --- a/client/ui/frontend/src/lib/errors.ts +++ b/client/ui/frontend/src/lib/errors.ts @@ -1,6 +1,6 @@ import { WindowManager } from "@bindings/services"; -type ClassifiedError = { short: string; long: string }; +type ClassifiedError = { short: string; long: string; command: string }; const asObject = (v: unknown): Record | null => v && typeof v === "object" ? (v as Record) : null; @@ -22,20 +22,24 @@ const toWailsEnvelope = (e: unknown): Record | null => { return asObject(obj.cause) ?? parseJsonObject(obj.message); }; -// Read { short, long } from wherever the classified error sits in the envelope +// Read { short, long, command } from wherever the classified error sits in the envelope const toClassifiedError = (v: unknown): ClassifiedError | null => { const o = asObject(v); if (!o) return null; const short = typeof o.short === "string" ? o.short : ""; const long = typeof o.long === "string" ? o.long : ""; - return short || long ? { short, long } : null; + const command = typeof o.command === "string" ? o.command : ""; + return short || long ? { short, long, command } : null; +}; + +const classify = (e: unknown): ClassifiedError | null => { + const envelope = toWailsEnvelope(e); + return toClassifiedError(envelope?.cause) ?? toClassifiedError(envelope); }; export const formatErrorMessage = (e: unknown): string => { - const envelope = toWailsEnvelope(e); - // Prefer the structured { short, long } the daemon classifier produced. - const classified = toClassifiedError(envelope?.cause) ?? toClassifiedError(envelope); + const classified = classify(e); if (classified) { const { short, long } = classified; if (short && long && long !== short) return `${short} Details: ${long}`; @@ -44,17 +48,26 @@ export const formatErrorMessage = (e: unknown): string => { } // Unclassified (a service returned the raw daemon error) + const envelope = toWailsEnvelope(e); const message = envelope?.message; if (typeof message === "string" && message) return message; if (e instanceof Error) return e.message; return String(e); }; +// errorCommand returns a command the user can run to complete an operation the +// daemon refused, when the error carries one (a change that needs elevated +// privileges). Empty for every other error. +export const errorCommand = (e: unknown): string => classify(e)?.command ?? ""; + export type ErrorDialogOptions = { Title: string; Message: string; + // Command is shown for copying below the message. Defaults to the one the + // error carries, so callers only pass it to override. + Command?: string; }; export function errorDialog(options: ErrorDialogOptions): Promise { - return WindowManager.OpenError(options.Title, options.Message); + return WindowManager.OpenError(options.Title, options.Message, options.Command ?? ""); } diff --git a/client/ui/frontend/src/modules/error/ErrorDialog.tsx b/client/ui/frontend/src/modules/error/ErrorDialog.tsx index f7929fad2..4fbb78052 100644 --- a/client/ui/frontend/src/modules/error/ErrorDialog.tsx +++ b/client/ui/frontend/src/modules/error/ErrorDialog.tsx @@ -3,6 +3,7 @@ import { useTranslation } from "react-i18next"; import { useSearchParams } from "react-router-dom"; import { AlertCircleIcon } from "lucide-react"; import { Button } from "@/components/buttons/Button"; +import { CopyToClipboard } from "@/components/CopyToClipboard"; import { ConfirmDialog } from "@/components/dialog/ConfirmDialog"; import { DialogActions } from "@/components/dialog/DialogActions"; import { DialogDescription } from "@/components/dialog/DialogDescription"; @@ -12,14 +13,22 @@ import { WindowManager } from "@bindings/services"; import { useAutoSizeWindow } from "@/hooks/useAutoSizeWindow"; const WINDOW_WIDTH = 380; +// A command needs the room to wrap at a sensible number of characters instead of +// breaking every few words. +const WINDOW_WIDTH_WITH_COMMAND = 460; export default function ErrorDialog() { const { t } = useTranslation(); - const contentRef = useAutoSizeWindow(WINDOW_WIDTH); const [params] = useSearchParams(); const title = params.get("title") || t("window.title.error"); const message = params.get("message") || ""; + // Set when the daemon refused an operation that needs elevated privileges: + // the command that performs it, offered for copying. + const command = params.get("command") || ""; + const contentRef = useAutoSizeWindow( + command ? WINDOW_WIDTH_WITH_COMMAND : WINDOW_WIDTH, + ); const close = useCallback(() => { WindowManager.CloseError().catch(console.error); @@ -37,15 +46,37 @@ export default function ErrorDialog() { -
+
{title} {message && ( - {message} + {/* select-text: the message often names a path, a flag or an + address the user needs to act on. */} + + {message} + )} + {command && ( + + + {command} + + + )}
diff --git a/client/ui/frontend/src/modules/settings/SettingsSSH.tsx b/client/ui/frontend/src/modules/settings/SettingsSSH.tsx index f18fd9493..bd91e520c 100644 --- a/client/ui/frontend/src/modules/settings/SettingsSSH.tsx +++ b/client/ui/frontend/src/modules/settings/SettingsSSH.tsx @@ -1,4 +1,5 @@ import { useTranslation } from "react-i18next"; +import { CopyToClipboard } from "@/components/CopyToClipboard"; import FancyToggleSwitch from "@/components/switches/FancyToggleSwitch"; import { HelpText } from "@/components/typography/HelpText"; import { Input } from "@/components/inputs/Input"; @@ -6,12 +7,50 @@ 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 ChangeEvent, useEffect, useId, useState } from "react"; +import { usePrivilege } from "@/hooks/usePrivilege.ts"; +import { 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 privilege = usePrivilege(); const isSSHServerEnabled = config.serverSshAllowed; + + // 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. + // + // 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, + 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, + ) => { + if (!privilege || privilege.privileged) { + return { disabled: false, hint: undefined }; + } + const hint = ( + + ); + return { disabled: !guardedDirectionActive, hint }; + }; + + const sshServer = guarded(config.serverSshAllowed, (p) => p.allowSshServer); + const sshRoot = guarded(config.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 jwtTtlId = useId(); const [jwtTtlInput, setJwtTtlInput] = useState(String(config.sshJwtCacheTtl)); @@ -46,9 +85,11 @@ export function SettingsSSH() { setField("serverSshAllowed", v)} + disabled={sshServer.disabled} label={t("settings.ssh.server.label")} helpText={t("settings.ssh.server.help")} /> + {sshServer.hint} setField("enableSshRoot", v)} + disabled={sshRoot.disabled} label={t("settings.ssh.root.label")} helpText={t("settings.ssh.root.help")} /> + {sshRoot.hint} setField("enableSshSftp", v)} @@ -88,9 +131,11 @@ export function SettingsSSH() { setField("disableSshAuth", !v)} + disabled={sshAuth.disabled} label={t("settings.ssh.jwt.label")} helpText={t("settings.ssh.jwt.help")} /> + {sshAuth.hint}
); } + +// 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({ + actor, + command, + oneWay, + inverted, +}: { + actor: string; + command: string; + oneWay: boolean; + inverted: boolean; +}): ReactNode { + const { t } = useTranslation(); + if (!command) return null; + return ( +
+ + {!oneWay + ? t("settings.ssh.privilege.hint", { actor }) + : inverted + ? t("settings.ssh.privilege.oneWayInverted", { actor }) + : t("settings.ssh.privilege.oneWay", { actor })} + + + + {command} + + +
+ ); +} diff --git a/client/ui/grpc.go b/client/ui/grpc.go index c8e3aed76..5450e136d 100644 --- a/client/ui/grpc.go +++ b/client/ui/grpc.go @@ -5,14 +5,13 @@ package main import ( "fmt" "runtime" - "strings" "sync" "time" "google.golang.org/grpc" "google.golang.org/grpc/backoff" - "google.golang.org/grpc/credentials/insecure" + "github.com/netbirdio/netbird/client/internal/daemonaddr" "github.com/netbirdio/netbird/client/proto" "github.com/netbirdio/netbird/client/ui/desktop" ) @@ -36,9 +35,10 @@ func (c *Conn) Client() (proto.DaemonServiceClient, error) { return c.client, nil } - cc, err := grpc.NewClient( - strings.TrimPrefix(c.addr, "tcp://"), - grpc.WithTransportCredentials(insecure.NewCredentials()), + // Lazy on purpose: grpc.NewClient does not connect here, so a daemon that + // is down surfaces as a per-RPC Unavailable instead of blocking the UI. + target, opts := daemonaddr.DialTarget(daemonaddr.ResolveDaemonAddr(c.addr)) + opts = append(opts, grpc.WithUserAgent(desktop.GetUIUserAgent()), // Cap reconnect backoff at 5s; gRPC's default 120s MaxDelay would // leave the UI waiting 30-60s to notice a freshly-started daemon. @@ -51,6 +51,8 @@ func (c *Conn) Client() (proto.DaemonServiceClient, error) { }, }), ) + + cc, err := grpc.NewClient(target, opts...) if err != nil { return nil, fmt.Errorf("dial daemon: %w", err) } @@ -58,10 +60,12 @@ func (c *Conn) Client() (proto.DaemonServiceClient, error) { return c.client, nil } -// DaemonAddr returns the default daemon gRPC address: a Unix socket on Linux/macOS, TCP loopback on Windows. +// DaemonAddr returns the default daemon gRPC address: a Unix socket on +// Linux/macOS, a named pipe on Windows. The pipe carries the caller's token, +// which loopback TCP does not, so the daemon can tell who is calling. func DaemonAddr() string { if runtime.GOOS == "windows" { - return "tcp://127.0.0.1:41731" + return daemonaddr.WindowsPipeAddr } return "unix:///var/run/netbird.sock" } diff --git a/client/ui/i18n/locales/en/common.json b/client/ui/i18n/locales/en/common.json index 24bbc67ce..b668146e8 100644 --- a/client/ui/i18n/locales/en/common.json +++ b/client/ui/i18n/locales/en/common.json @@ -1774,5 +1774,17 @@ "error.unknown": { "message": "Operation failed.", "description": "Generic fallback error message used when no specific error applies." + }, + "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." + }, + "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." } } diff --git a/client/ui/main.go b/client/ui/main.go index 9a3e17743..782e76d1c 100644 --- a/client/ui/main.go +++ b/client/ui/main.go @@ -96,7 +96,6 @@ func main() { } }) - settings := services.NewSettings(conn) profiles := services.NewProfiles(conn) // updater.Holder owns the typed update State; DaemonFeed feeds it and the // Update service is a thin Wails-bound facade over it plus the install RPCs. @@ -117,6 +116,7 @@ func main() { bundle, prefStore, localizer := buildI18n(app) // After bundle + prefStore: both are used to localise daemon errors. + settings := services.NewSettings(conn, bundle, prefStore, daemonAddr) connection := services.NewConnection(conn, bundle, prefStore) profileSwitcher := services.NewProfileSwitcher(profiles, connection, daemonFeed) // authsession.Session owns the full extend + dismiss surface the tray diff --git a/client/ui/services/errors.go b/client/ui/services/errors.go index f1679e764..0c6f2f20f 100644 --- a/client/ui/services/errors.go +++ b/client/ui/services/errors.go @@ -6,13 +6,30 @@ import ( "encoding/json" "strings" + "google.golang.org/genproto/googleapis/rpc/errdetails" gcodes "google.golang.org/grpc/codes" gstatus "google.golang.org/grpc/status" + "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/client/ui/i18n" "github.com/netbirdio/netbird/client/ui/preferences" ) +// privilegeErrorInfo returns the daemon's privilege-refusal detail, if the error +// carries one. +func privilegeErrorInfo(err error) (*errdetails.ErrorInfo, bool) { + for _, detail := range gstatus.Convert(err).Details() { + info, ok := detail.(*errdetails.ErrorInfo) + if !ok { + continue + } + if info.GetReason() == ipcauth.ErrorReasonPrivilegeRequired && info.GetDomain() == ipcauth.ErrorDomain { + return info, true + } + } + return nil, false +} + // ErrorTranslator localises daemon errors; runtime impl is *i18n.Bundle. type ErrorTranslator interface { Translate(lang i18n.LanguageCode, key string, args ...string) string @@ -30,6 +47,10 @@ type ClientError struct { Code string `json:"code"` Short string `json:"short"` Long string `json:"long"` + // Command is a command the user can run to complete the operation + // themselves, set when the daemon refused it for want of privileges. The + // frontend offers it for copying. + Command string `json:"command,omitempty"` } // Error returns the short message for plain Go callers. @@ -72,6 +93,24 @@ func (c errorClassifier) classify(err error) *ClientError { msg = st.Message() grpcCode = st.Code() } + + // A refusal for want of privileges carries its own summary and the command + // that performs the operation, both written for the user. Surface them + // verbatim: no substring guessing, and no localisation of a message the + // daemon composed. + if info, ok := privilegeErrorInfo(err); ok { + summary := info.GetMetadata()[ipcauth.ErrorMetaSummary] + if summary == "" { + summary = msg + } + return &ClientError{ + Code: "privilege_required", + Short: summary, + Long: summary, + Command: info.GetMetadata()[ipcauth.ErrorMetaCommand], + } + } + lower := strings.ToLower(msg) code := "unknown" diff --git a/client/ui/services/settings.go b/client/ui/services/settings.go index 3b6f6f81b..74e6f913c 100644 --- a/client/ui/services/settings.go +++ b/client/ui/services/settings.go @@ -7,6 +7,10 @@ import ( "fmt" "reflect" + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/daemonaddr" + "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/client/proto" ) @@ -39,6 +43,19 @@ type Restrictions struct { Features Features `json:"features"` } +// 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. +type Privilege struct { + Privileged bool `json:"privileged"` + // Actor names what the operation requires ("root", "administrator privileges"). + Actor string `json:"actor"` + // Commands equivalent to the settings the daemon guards, ready to copy. + AllowSSHServer string `json:"allowSshServer"` + EnableSSHRoot string `json:"enableSshRoot"` + DisableSSHAuth string `json:"disableSshAuth"` +} + type ConfigParams struct { ProfileName string `json:"profileName"` Username string `json:"username"` @@ -106,11 +123,19 @@ type SetConfigParams struct { } type Settings struct { - conn DaemonConn + conn DaemonConn + classifier errorClassifier + // daemonAddr is where the daemon listens, used to tell whether it runs as + // this user and would therefore authorize us: see Privilege. + daemonAddr string } -func NewSettings(conn DaemonConn) *Settings { - return &Settings{conn: conn} +func NewSettings(conn DaemonConn, translator ErrorTranslator, prefs LanguagePreference, daemonAddr string) *Settings { + return &Settings{ + conn: conn, + classifier: errorClassifier{translator: translator, prefs: prefs}, + daemonAddr: daemonAddr, + } } func (s *Settings) GetConfig(ctx context.Context, p ConfigParams) (Config, error) { @@ -189,8 +214,47 @@ func (s *Settings) SetConfig(ctx context.Context, p SetConfigParams) error { DisableSSHAuth: p.DisableSSHAuth, SshJWTCacheTTL: p.SSHJWTCacheTTL, } - _, err = cli.SetConfig(ctx, req) - return err + if _, err := cli.SetConfig(ctx, req); err != nil { + // 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) + } + return 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. +// +// 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 +// because such a caller can already rewrite the config it reads; that is the +// rootless-container and Windows netstack-mode case, and it is read from the +// ownership of the socket or pipe the daemon created. +func (s *Settings) Privilege() Privilege { + id, err := ipcauth.CurrentProcessIdentity() + if err != nil { + // Fail closed: report unprivileged, which only ever disables controls. + log.Warnf("cannot read this process's identity, treating it as unprivileged: %v", err) + return newPrivilege(false) + } + if id.IsPrivileged() { + return newPrivilege(true) + } + return newPrivilege(daemonaddr.DaemonRunsAsSelf(s.daemonAddr)) +} + +func newPrivilege(privileged bool) Privilege { + return Privilege{ + Privileged: privileged, + Actor: ipcauth.PrivilegedActor(), + AllowSSHServer: ipcauth.UpCommand("--allow-server-ssh"), + EnableSSHRoot: ipcauth.UpCommand("--enable-ssh-root"), + DisableSSHAuth: ipcauth.UpCommand("--disable-ssh-auth"), + } } func (s *Settings) GetRestrictions(ctx context.Context) (Restrictions, error) { diff --git a/client/ui/services/windowmanager.go b/client/ui/services/windowmanager.go index bac9790b4..5f7aaa7bd 100644 --- a/client/ui/services/windowmanager.go +++ b/client/ui/services/windowmanager.go @@ -393,15 +393,17 @@ func (s *WindowManager) CloseWelcome() { } } -// OpenError shows the custom error dialog; title/message are pre-localised and ride in the -// start URL. A second error replaces the open one via SetURL. Singleton, destroyed on close. -func (s *WindowManager) OpenError(title, message string) { +// OpenError shows the custom error dialog; title/message/command are pre-localised +// and ride in the start URL. command is optional and, when set, is offered for +// copying so the user can run the operation the daemon refused. A second error +// replaces the open one via SetURL. Singleton, destroyed on close. +func (s *WindowManager) OpenError(title, message, command string) { if ShuttingDown() { return } s.mu.Lock() defer s.mu.Unlock() - startURL := errorDialogURL(title, message) + startURL := errorDialogURL(title, message, command) if s.errorDialog == nil { s.errorDialog = s.app.Window.NewWithOptions( DialogWindowOptions("error", s.title("window.title.error"), startURL, s.linuxIcon), @@ -601,8 +603,8 @@ func (s *WindowManager) getScreenBasedOnCursorPosition() *application.Screen { return nil } -// errorDialogURL builds the error window's start URL with title/message as escaped query params. -func errorDialogURL(title, message string) string { +// errorDialogURL builds the error window's start URL with title/message/command as escaped query params. +func errorDialogURL(title, message, command string) string { q := url.Values{} if title != "" { q.Set("title", title) @@ -610,6 +612,9 @@ func errorDialogURL(title, message string) string { if message != "" { q.Set("message", message) } + if command != "" { + q.Set("command", command) + } startURL := "/#/dialog/error" if enc := q.Encode(); enc != "" { startURL += "?" + enc diff --git a/go.mod b/go.mod index ca798decc..26a194958 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,7 @@ require ( require ( github.com/DeRuina/timberjack v1.4.2 + github.com/Microsoft/go-winio v0.6.2 github.com/awnumar/memguard v0.23.0 github.com/aws/aws-sdk-go-v2 v1.38.3 github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.1 @@ -133,6 +134,7 @@ require ( golang.org/x/term v0.45.0 golang.org/x/time v0.15.0 google.golang.org/api v0.276.0 + google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 gopkg.in/yaml.v3 v3.0.1 gorm.io/driver/mysql v1.5.7 gorm.io/driver/postgres v1.5.7 @@ -156,7 +158,6 @@ require ( github.com/Masterminds/goutils v1.1.1 // indirect github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/Masterminds/sprig/v3 v3.3.0 // indirect - github.com/Microsoft/go-winio v0.6.2 // indirect github.com/adrg/xdg v0.5.3 // indirect github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be // indirect github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect @@ -317,7 +318,6 @@ require ( golang.org/x/tools v0.47.0 // indirect golang.zx2c4.com/wintun v0.0.0-20230126152724-0fa3db229ce2 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260319201613-d00831a3d3e7 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260401024825-9d38bb4040a9 // indirect gopkg.in/square/go-jose.v2 v2.6.0 // indirect gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect