Merge branch 'main' into fix/android-login-hint

This commit is contained in:
Zoltan Papp
2026-08-03 18:00:03 +02:00
committed by GitHub
123 changed files with 6440 additions and 1848 deletions
+1 -1
View File
@@ -314,7 +314,7 @@ func (c *Client) DebugBundle(platformFiles PlatformFiles, anonymize bool) (strin
uploadCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
key, err := debug.UploadDebugBundle(uploadCtx, types.DefaultBundleURL, cfg.ManagementURL.String(), path)
key, err := debug.UploadDebugBundle(uploadCtx, types.DefaultBundleURL, cfg.ManagementURL.String(), path, false)
if err != nil {
return "", fmt.Errorf("upload debug bundle: %w", err)
}
+9 -4
View File
@@ -29,8 +29,9 @@ const errCloseConnection = "Failed to close connection: %v"
var (
logFileCount uint32
systemInfoFlag bool
uploadBundleFlag bool
uploadBundleURLFlag string
uploadBundleFlag bool
uploadBundleURLFlag string
uploadBundleInsecureFlag bool
)
var debugCmd = &cobra.Command{
@@ -174,10 +175,11 @@ func debugBundle(cmd *cobra.Command, _ []string) error {
}
if uploadBundleFlag {
request.UploadURL = uploadBundleURLFlag
request.UploadInsecure = uploadBundleInsecureFlag
}
resp, err := client.DebugBundle(cmd.Context(), request)
if err != nil {
return fmt.Errorf("failed to bundle debug: %v", status.Convert(err).Message())
return daemonCallError("bundle debug", err)
}
cmd.Printf("Local file:\n%s\n", resp.GetPath())
@@ -373,10 +375,11 @@ func runForDuration(cmd *cobra.Command, args []string) error {
}
if uploadBundleFlag {
request.UploadURL = uploadBundleURLFlag
request.UploadInsecure = uploadBundleInsecureFlag
}
resp, err := client.DebugBundle(cmd.Context(), request)
if err != nil {
return fmt.Errorf("failed to bundle debug: %v", status.Convert(err).Message())
return daemonCallError("bundle debug", err)
}
if needsRestoreUp {
@@ -524,10 +527,12 @@ func init() {
debugBundleCmd.Flags().BoolVarP(&systemInfoFlag, "system-info", "S", true, "Adds system information to the debug bundle")
debugBundleCmd.Flags().BoolVarP(&uploadBundleFlag, "upload-bundle", "U", false, "Uploads the debug bundle to a server")
debugBundleCmd.Flags().StringVar(&uploadBundleURLFlag, "upload-bundle-url", types.DefaultBundleURL, "Service URL to get an URL to upload the debug bundle")
debugBundleCmd.Flags().BoolVar(&uploadBundleInsecureFlag, "upload-bundle-insecure", false, "Allow uploading to an http or untrusted-TLS upload server (self-hosted); requires root")
forCmd.Flags().Uint32VarP(&logFileCount, "log-file-count", "C", 1, "Number of rotated log files to include in debug bundle")
forCmd.Flags().BoolVarP(&systemInfoFlag, "system-info", "S", true, "Adds system information to the debug bundle")
forCmd.Flags().BoolVarP(&uploadBundleFlag, "upload-bundle", "U", false, "Uploads the debug bundle to a server")
forCmd.Flags().StringVar(&uploadBundleURLFlag, "upload-bundle-url", types.DefaultBundleURL, "Service URL to get an URL to upload the debug bundle")
forCmd.Flags().BoolVar(&uploadBundleInsecureFlag, "upload-bundle-insecure", false, "Allow uploading to an http or untrusted-TLS upload server (self-hosted); requires root")
forCmd.Flags().Bool("capture", false, "Capture packets during the debug duration and include in bundle")
}
+5
View File
@@ -6,6 +6,11 @@ import (
"runtime"
)
// UILogFile is the file name the desktop UI writes its log to. It is defined
// here so the UI (writer), the daemon's RegisterUILog validation, and the debug
// bundle collector all share one definition.
const UILogFile = "gui-client.log"
var StateDir string
func init() {
@@ -0,0 +1,60 @@
package nftables
import (
"testing"
"github.com/google/nftables/expr"
"github.com/stretchr/testify/require"
)
func TestBuildLegacyRouteRuleExpressions(t *testing.T) {
sourcePayload := &expr.Payload{}
sourceCmp := &expr.Cmp{}
destinationPayload := &expr.Payload{}
destinationCmp := &expr.Cmp{}
nilSourceDestination := &expr.Payload{}
nilDestinationSource := &expr.Cmp{}
tests := []struct {
name string
source []expr.Any
destination []expr.Any
matches []expr.Any
}{
{
name: "both non-empty",
source: []expr.Any{sourcePayload, sourceCmp},
destination: []expr.Any{destinationPayload, destinationCmp},
matches: []expr.Any{sourcePayload, sourceCmp, destinationPayload, destinationCmp},
},
{
name: "nil source",
destination: []expr.Any{nilSourceDestination},
matches: []expr.Any{nilSourceDestination},
},
{
name: "nil destination",
source: []expr.Any{nilDestinationSource},
matches: []expr.Any{nilDestinationSource},
},
{
name: "both nil",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := buildLegacyRouteRuleExpressions(tt.source, tt.destination)
require.Len(t, got, len(tt.matches)+2)
for i, match := range tt.matches {
require.Same(t, match, got[i])
}
require.IsType(t, &expr.Counter{}, got[len(tt.matches)])
verdict, ok := got[len(tt.matches)+1].(*expr.Verdict)
require.True(t, ok)
require.Equal(t, expr.VerdictAccept, verdict.Kind)
})
}
}
+12 -9
View File
@@ -953,6 +953,17 @@ func (r *router) addMSSClampingRules() error {
return r.conn.Flush()
}
func buildLegacyRouteRuleExpressions(sourceExp, destExp []expr.Any) []expr.Any {
exprs := make([]expr.Any, 0, len(sourceExp)+len(destExp)+2)
exprs = append(exprs, sourceExp...)
exprs = append(exprs, destExp...)
exprs = append(exprs,
&expr.Counter{},
&expr.Verdict{Kind: expr.VerdictAccept},
)
return exprs
}
// addLegacyRouteRule adds a legacy routing rule for mgmt servers pre route acls
func (r *router) addLegacyRouteRule(pair firewall.RouterPair) error {
sourceExp, err := r.applyNetwork(pair.Source, nil, true)
@@ -965,15 +976,7 @@ func (r *router) addLegacyRouteRule(pair firewall.RouterPair) error {
return fmt.Errorf("apply destination: %w", err)
}
exprs := []expr.Any{
&expr.Counter{},
&expr.Verdict{
Kind: expr.VerdictAccept,
},
}
exprs = append(exprs, sourceExp...)
exprs = append(exprs, destExp...)
exprs := buildLegacyRouteRuleExpressions(sourceExp, destExp)
ruleKey := firewall.GenKey(firewall.ForwardingFormat, pair)
+4 -1
View File
@@ -113,11 +113,14 @@ func (c *ConnectClient) RunOnAndroid(
stateFilePath string,
cacheDir string,
) error {
notifier := tunnelnotifier.New(networkChangeListener, nil)
defer notifier.Close()
// in case of non Android os these variables will be nil
mobileDependency := MobileDependency{
TunAdapter: tunAdapter,
IFaceDiscover: iFaceDiscover,
NetworkChangeListener: networkChangeListener,
NetworkChangeListener: notifier,
HostDNSAddresses: dnsAddresses,
DnsReadyListener: dnsReadyListener,
StateFilePath: stateFilePath,
+48 -22
View File
@@ -229,7 +229,6 @@ scutil_dns.txt (macOS only):
const (
clientLogFile = "client.log"
uiLogFile = "gui-client.log"
errorLogFile = "netbird.err"
stdoutLogFile = "netbird.out"
@@ -248,6 +247,20 @@ type MetricsExporter interface {
Export(w io.Writer) error
}
// LogOpener opens a log file for inclusion in the bundle. It exists so that log
// files whose path was supplied by an IPC caller can be opened under a check
// the daemon defines, instead of being opened with the daemon's privileges
// unconditionally.
type LogOpener func(path string) (*os.File, error)
func openLogFile(path string) (*os.File, error) {
f, err := os.Open(path)
if err != nil {
return nil, fmt.Errorf("open %s: %w", path, err)
}
return f, nil
}
type BundleGenerator struct {
anonymizer *anonymize.Anonymizer
@@ -257,6 +270,7 @@ type BundleGenerator struct {
syncResponse *mgmProto.SyncResponse
logPath string
uiLogPath string
uiLogOpener LogOpener
tempDir string
statePath string
cpuProfile []byte
@@ -285,14 +299,20 @@ type GeneratorDependencies struct {
SyncResponse *mgmProto.SyncResponse
LogPath string
UILogPath string // Absolute path to the desktop UI's gui-client.log, reported via RegisterUILog. Empty if no UI registered one.
TempDir string // Directory for temporary bundle zip files. If empty, os.TempDir() is used.
StatePath string // Path to the state file. If empty, the ServiceManager default path is used.
CPUProfile []byte
CapturePath string
RefreshStatus func()
ClientMetrics MetricsExporter
DaemonVersion string
CliVersion string
// UILogOpener opens the UI log and its rotated siblings. The path comes from
// a local IPC caller, so the daemon must not open it with plain os.Open: the
// opener is where the caller's right to that file is enforced. Defaults to
// os.Open, which is only correct where the path is not caller-supplied
// (mobile).
UILogOpener LogOpener
TempDir string // Directory for temporary bundle zip files. If empty, os.TempDir() is used.
StatePath string // Path to the state file. If empty, the ServiceManager default path is used.
CPUProfile []byte
CapturePath string
RefreshStatus func()
ClientMetrics MetricsExporter
DaemonVersion string
CliVersion string
}
func NewBundleGenerator(deps GeneratorDependencies, cfg BundleConfig) *BundleGenerator {
@@ -302,6 +322,11 @@ func NewBundleGenerator(deps GeneratorDependencies, cfg BundleConfig) *BundleGen
logFileCount = 1
}
uiLogOpener := deps.UILogOpener
if uiLogOpener == nil {
uiLogOpener = openLogFile
}
return &BundleGenerator{
anonymizer: anonymize.NewAnonymizer(anonymize.DefaultAddresses()),
@@ -310,6 +335,7 @@ func NewBundleGenerator(deps GeneratorDependencies, cfg BundleConfig) *BundleGen
syncResponse: deps.SyncResponse,
logPath: deps.LogPath,
uiLogPath: deps.UILogPath,
uiLogOpener: uiLogOpener,
tempDir: deps.TempDir,
statePath: deps.StatePath,
cpuProfile: deps.CPUProfile,
@@ -996,11 +1022,11 @@ func (g *BundleGenerator) addLogfile() error {
logDir := filepath.Dir(g.logPath)
if err := g.addSingleLogfile(g.logPath, clientLogFile); err != nil {
if err := g.addSingleLogfile(openLogFile, g.logPath, clientLogFile); err != nil {
return fmt.Errorf("add client log file to zip: %w", err)
}
g.addRotatedLogFiles(logDir, clientLogPrefix)
g.addRotatedLogFiles(openLogFile, logDir, clientLogPrefix)
stdErrLogPath := filepath.Join(logDir, errorLogFile)
stdoutLogPath := filepath.Join(logDir, stdoutLogFile)
@@ -1009,11 +1035,11 @@ func (g *BundleGenerator) addLogfile() error {
stdoutLogPath = darwinStdoutLogPath
}
if err := g.addSingleLogfile(stdErrLogPath, errorLogFile); err != nil {
if err := g.addSingleLogfile(openLogFile, stdErrLogPath, errorLogFile); err != nil {
log.Warnf("Failed to add %s to zip: %v", errorLogFile, err)
}
if err := g.addSingleLogfile(stdoutLogPath, stdoutLogFile); err != nil {
if err := g.addSingleLogfile(openLogFile, stdoutLogPath, stdoutLogFile); err != nil {
log.Warnf("Failed to add %s to zip: %v", stdoutLogFile, err)
}
@@ -1030,18 +1056,18 @@ func (g *BundleGenerator) addUILog() error {
return nil
}
if err := g.addSingleLogfile(g.uiLogPath, uiLogFile); err != nil {
if err := g.addSingleLogfile(g.uiLogOpener, g.uiLogPath, configs.UILogFile); err != nil {
return fmt.Errorf("add UI log file to zip: %w", err)
}
g.addRotatedLogFiles(filepath.Dir(g.uiLogPath), uiLogPrefix)
g.addRotatedLogFiles(g.uiLogOpener, filepath.Dir(g.uiLogPath), uiLogPrefix)
return nil
}
// addSingleLogfile adds a single log file to the archive
func (g *BundleGenerator) addSingleLogfile(logPath, targetName string) error {
logFile, err := os.Open(logPath)
func (g *BundleGenerator) addSingleLogfile(open LogOpener, logPath, targetName string) error {
logFile, err := open(logPath)
if err != nil {
return fmt.Errorf("open log file %s: %w", targetName, err)
}
@@ -1066,8 +1092,8 @@ func (g *BundleGenerator) addSingleLogfile(logPath, targetName string) error {
}
// addSingleLogFileGz adds a single gzipped log file to the archive
func (g *BundleGenerator) addSingleLogFileGz(logPath, targetName string) error {
f, err := os.Open(logPath)
func (g *BundleGenerator) addSingleLogFileGz(open LogOpener, logPath, targetName string) error {
f, err := open(logPath)
if err != nil {
return fmt.Errorf("open gz log file %s: %w", targetName, err)
}
@@ -1114,7 +1140,7 @@ func (g *BundleGenerator) addSingleLogFileGz(logPath, targetName string) error {
// addRotatedLogFiles adds rotated log files to the bundle based on logFileCount.
// prefix is the base log name without extension (e.g. "client", "gui-client");
// the glob matches both files rotated by us and by logrotate on linux.
func (g *BundleGenerator) addRotatedLogFiles(logDir, prefix string) {
func (g *BundleGenerator) addRotatedLogFiles(open LogOpener, logDir, prefix string) {
if g.logFileCount == 0 {
return
}
@@ -1154,9 +1180,9 @@ func (g *BundleGenerator) addRotatedLogFiles(logDir, prefix string) {
for i := 0; i < maxFiles; i++ {
name := filepath.Base(files[i])
if strings.HasSuffix(name, ".gz") {
err = g.addSingleLogFileGz(files[i], name)
err = g.addSingleLogFileGz(open, files[i], name)
} else {
err = g.addSingleLogfile(files[i], name)
err = g.addSingleLogfile(open, files[i], name)
}
if err != nil {
log.Warnf("failed to add rotated log %s: %v", name, err)
+1 -1
View File
@@ -27,7 +27,7 @@ func (g *BundleGenerator) addPlatformLog() error {
}
swiftLogPath := filepath.Join(filepath.Dir(g.logPath), swiftLogFile)
if err := g.addSingleLogfile(swiftLogPath, swiftLogFile); err != nil {
if err := g.addSingleLogfile(openLogFile, swiftLogPath, swiftLogFile); err != nil {
// The Swift log is best-effort: the app may not have written it yet.
log.Warnf("failed to add %s to debug bundle: %v", swiftLogFile, err)
}
+1 -1
View File
@@ -97,7 +97,7 @@ func runAddRotatedLogFilesPrefix(t *testing.T, dir, prefix string, logFileCount
archive: zip.NewWriter(&buf),
logFileCount: logFileCount,
}
g.addRotatedLogFiles(dir, prefix)
g.addRotatedLogFiles(openLogFile, dir, prefix)
require.NoError(t, g.archive.Close())
zr, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
+64
View File
@@ -0,0 +1,64 @@
package debug
import (
"archive/zip"
"bytes"
"fmt"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/configs"
)
// bundleEntries generates a bundle with the given generator and returns the
// set of entry names in the resulting archive.
func bundleEntries(t *testing.T, g *BundleGenerator) map[string]struct{} {
t.Helper()
path, err := g.Generate()
require.NoError(t, err)
t.Cleanup(func() { _ = os.Remove(path) })
data, err := os.ReadFile(path)
require.NoError(t, err)
zr, err := zip.NewReader(bytes.NewReader(data), int64(len(data)))
require.NoError(t, err)
names := make(map[string]struct{}, len(zr.File))
for _, f := range zr.File {
names[f.Name] = struct{}{}
}
return names
}
func TestBundleIncludesUILogWhenOpenerAllows(t *testing.T) {
path := filepath.Join(t.TempDir(), configs.UILogFile)
require.NoError(t, os.WriteFile(path, []byte("gui log"), 0600))
g := NewBundleGenerator(GeneratorDependencies{
UILogPath: path,
UILogOpener: openLogFile,
}, BundleConfig{})
require.Contains(t, bundleEntries(t, g), configs.UILogFile)
}
// A UILogOpener that refuses (as the ownership check does for a foreign file)
// keeps the UI log out of the bundle without failing bundle generation.
func TestBundleExcludesUILogWhenOpenerRefuses(t *testing.T) {
path := filepath.Join(t.TempDir(), configs.UILogFile)
require.NoError(t, os.WriteFile(path, []byte("secret"), 0600))
g := NewBundleGenerator(GeneratorDependencies{
UILogPath: path,
UILogOpener: func(string) (*os.File, error) {
return nil, fmt.Errorf("not owned by the caller")
},
}, BundleConfig{})
require.NotContains(t, bundleEntries(t, g), configs.UILogFile)
}
+78 -9
View File
@@ -3,10 +3,12 @@ package debug
import (
"context"
"crypto/sha256"
"crypto/tls"
"encoding/json"
"fmt"
"io"
"net/http"
neturl "net/url"
"os"
"github.com/netbirdio/netbird/upload-server/types"
@@ -14,20 +16,80 @@ import (
const maxBundleUploadSize = 50 * 1024 * 1024
func UploadDebugBundle(ctx context.Context, url, managementURL, filePath string) (key string, err error) {
response, err := getUploadURL(ctx, url, managementURL)
// requireHTTPS refuses any URL the daemon would fetch or upload to that is not
// https. The daemon runs as root and the bundle carries its logs and state, so a
// plaintext hop is a place to intercept the bundle or the presigned redirect.
// The server-side gate already enforces this for the desktop path; this also
// covers the mobile and job-runner callers that reach this package directly.
// Skipped when the caller opted into an insecure upload (self-hosted server).
func requireHTTPS(what, rawURL string) error {
parsed, err := neturl.Parse(rawURL)
if err != nil {
return fmt.Errorf("parse %s: %w", what, err)
}
if parsed.Scheme != "https" {
return fmt.Errorf("%s must use https, got scheme %q", what, parsed.Scheme)
}
return nil
}
// uploadClient returns the HTTP client for the upload requests. The default
// client verifies TLS and refuses a redirect that would downgrade to a non-https
// hop, so a bundle can never leave over http after an https start. The insecure
// variant accepts http and untrusted certificates, and is only reachable for a
// privileged caller that passed --upload-bundle-insecure (see
// requirePrivilegeForUploadURL).
func uploadClient(insecure bool) *http.Client {
if !insecure {
return &http.Client{CheckRedirect: rejectInsecureRedirect}
}
return &http.Client{
Transport: &http.Transport{
//nolint:gosec // opt-in, privileged, self-hosted upload servers
TLSClientConfig: &tls.Config{InsecureSkipVerify: true, MinVersion: tls.VersionTLS12},
},
}
}
// rejectInsecureRedirect refuses a redirect to a non-https target and keeps the
// standard library's 10-hop limit that a custom CheckRedirect would otherwise
// disable.
func rejectInsecureRedirect(req *http.Request, via []*http.Request) error {
if req.URL.Scheme != "https" {
return fmt.Errorf("refusing redirect to non-https URL %s", req.URL.Redacted())
}
if len(via) >= 10 {
return fmt.Errorf("stopped after 10 redirects")
}
return nil
}
func UploadDebugBundle(ctx context.Context, url, managementURL, filePath string, insecure bool) (key string, err error) {
if !insecure {
if err := requireHTTPS("upload service URL", url); err != nil {
return "", err
}
}
response, err := getUploadURL(ctx, url, managementURL, insecure)
if err != nil {
return "", err
}
err = upload(ctx, filePath, response)
if !insecure {
if err := requireHTTPS("upload URL from service", response.URL); err != nil {
return "", err
}
}
err = upload(ctx, filePath, response, insecure)
if err != nil {
return "", err
}
return response.Key, nil
}
func upload(ctx context.Context, filePath string, response *types.GetURLResponse) error {
func upload(ctx context.Context, filePath string, response *types.GetURLResponse, insecure bool) error {
fileData, err := os.Open(filePath)
if err != nil {
return fmt.Errorf("open file: %w", err)
@@ -52,7 +114,7 @@ func upload(ctx context.Context, filePath string, response *types.GetURLResponse
req.ContentLength = stat.Size()
req.Header.Set("Content-Type", "application/octet-stream")
putResp, err := http.DefaultClient.Do(req)
putResp, err := uploadClient(insecure).Do(req)
if err != nil {
return fmt.Errorf("upload failed: %v", err)
}
@@ -65,16 +127,23 @@ func upload(ctx context.Context, filePath string, response *types.GetURLResponse
return nil
}
func getUploadURL(ctx context.Context, url string, managementURL string) (*types.GetURLResponse, error) {
id := getURLHash(managementURL)
getReq, err := http.NewRequestWithContext(ctx, "GET", url+"?id="+id, nil)
func getUploadURL(ctx context.Context, serviceURL string, managementURL string, insecure bool) (*types.GetURLResponse, error) {
parsed, err := neturl.Parse(serviceURL)
if err != nil {
return nil, fmt.Errorf("parse upload service URL: %w", err)
}
q := parsed.Query()
q.Set("id", getURLHash(managementURL))
parsed.RawQuery = q.Encode()
getReq, err := http.NewRequestWithContext(ctx, "GET", parsed.String(), nil)
if err != nil {
return nil, fmt.Errorf("create GET request: %w", err)
}
getReq.Header.Set(types.ClientHeader, types.ClientHeaderValue)
resp, err := http.DefaultClient.Do(getReq)
resp, err := uploadClient(insecure).Do(getReq)
if err != nil {
return nil, fmt.Errorf("get presigned URL: %w", err)
}
+46 -1
View File
@@ -5,6 +5,7 @@ import (
"errors"
"net"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
@@ -43,7 +44,7 @@ func TestUpload(t *testing.T) {
fileContent := []byte("test file content")
err := os.WriteFile(file, fileContent, 0640)
require.NoError(t, err)
key, err := UploadDebugBundle(context.Background(), testURL+types.GetURLPath, testURL, file)
key, err := UploadDebugBundle(context.Background(), testURL+types.GetURLPath, testURL, file, true)
require.NoError(t, err)
id := getURLHash(testURL)
require.Contains(t, key, id+"/")
@@ -79,3 +80,47 @@ func waitForServer(t *testing.T, addr string) {
}
t.Fatalf("server did not start listening on %s in time", addr)
}
func TestRequireHTTPS(t *testing.T) {
require.NoError(t, requireHTTPS("upload URL", "https://upload.example/path"))
require.Error(t, requireHTTPS("upload URL", "http://upload.example/path"))
require.Error(t, requireHTTPS("upload URL", "ftp://upload.example/path"))
require.Error(t, requireHTTPS("upload URL", "://malformed"))
}
func TestRejectInsecureRedirect(t *testing.T) {
httpsReq, err := http.NewRequest(http.MethodGet, "https://a.example/", nil)
require.NoError(t, err)
require.NoError(t, rejectInsecureRedirect(httpsReq, nil), "https redirect target must be allowed")
httpReq, err := http.NewRequest(http.MethodGet, "http://a.example/", nil)
require.NoError(t, err)
require.Error(t, rejectInsecureRedirect(httpReq, nil), "http redirect target must be refused")
require.Error(t, rejectInsecureRedirect(httpsReq, make([]*http.Request, 10)), "the 10-redirect limit must be enforced")
}
// The secure client refuses to follow an https response that redirects to http,
// so a bundle can't be downgraded onto plaintext mid-flight.
func TestUploadClientRefusesHTTPSToHTTPRedirect(t *testing.T) {
plain := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
t.Cleanup(plain.Close)
secure := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, plain.URL, http.StatusFound)
}))
t.Cleanup(secure.Close)
client := uploadClient(false)
// Trust the test server's cert without disabling verification globally.
client.Transport = secure.Client().Transport
resp, err := client.Get(secure.URL)
if resp != nil {
_ = resp.Body.Close()
}
require.Error(t, err, "redirect from https to http must be refused")
require.Contains(t, err.Error(), "non-https")
}
+15
View File
@@ -0,0 +1,15 @@
package dns
import (
"fmt"
"net"
)
func getInterfaceIndex(interfaceName string) (int, error) {
iface, err := net.InterfaceByName(interfaceName)
if err != nil {
return 0, fmt.Errorf("lookup interface %q: %w", interfaceName, err)
}
return iface.Index, nil
}
@@ -0,0 +1,35 @@
package dns
import (
"net"
"testing"
)
func TestGetInterfaceIndexExisting(t *testing.T) {
interfaces, err := net.Interfaces()
if err != nil {
t.Fatalf("list network interfaces: %v", err)
}
if len(interfaces) == 0 {
t.Fatal("expected at least one network interface")
}
iface := interfaces[0]
index, err := getInterfaceIndex(iface.Name)
if err != nil {
t.Fatalf("look up existing interface %q: %v", iface.Name, err)
}
if index != iface.Index {
t.Fatalf("expected interface index %d, got %d", iface.Index, index)
}
}
func TestGetInterfaceIndexMissing(t *testing.T) {
index, err := getInterfaceIndex("netbird-interface-that-does-not-exist")
if index != 0 {
t.Fatalf("expected missing interface index to be 0, got %d", index)
}
if err == nil {
t.Fatal("expected missing interface lookup to return an error")
}
}
+1 -3
View File
@@ -51,7 +51,5 @@ func (n *notifier) notify() {
return
}
go func(l listener.NetworkChangeListener) {
l.OnNetworkChanged("")
}(n.listener)
n.listener.OnNetworkChanged("")
}
-5
View File
@@ -130,8 +130,3 @@ func GetClientPrivate(iface privateClientIface, upstreamIP netip.Addr, dialTimeo
}
return client, nil
}
func getInterfaceIndex(interfaceName string) (int, error) {
iface, err := net.InterfaceByName(interfaceName)
return iface.Index, err
}
+63
View File
@@ -0,0 +1,63 @@
package ipcauth
import (
"fmt"
"os"
)
// OpenOwnedFile opens path for reading on behalf of the IPC caller identified by
// id, and fails unless the opened file is a regular file that id owns.
//
// It exists for the paths a local caller hands to the daemon over the IPC. The
// daemon runs as root, so opening such a path unchecked lets any local user read
// any file through it. Ownership is the invariant that keeps the daemon from
// reading, with its own privileges, a file the caller could not read itself: a
// symlink or hard link planted at the path resolves to a file someone else owns
// and is refused.
//
// The check is made against the open descriptor rather than the path, so
// swapping the path between the check and the read cannot change the answer.
//
// A privileged caller is exempt: it can read the file directly, so refusing it
// here would protect nothing. The regular-file requirement still applies to
// everyone, since a fifo or device planted at the path is never a log file.
func OpenOwnedFile(id Identity, path string) (*os.File, error) {
f, err := openForRead(path)
if err != nil {
return nil, err
}
if err := checkOwnership(id, f); err != nil {
if cerr := f.Close(); cerr != nil {
return nil, fmt.Errorf("%w (close: %v)", err, cerr)
}
return nil, err
}
return f, nil
}
func checkOwnership(id Identity, f *os.File) error {
info, err := f.Stat()
if err != nil {
return fmt.Errorf("stat %s: %w", f.Name(), err)
}
if !info.Mode().IsRegular() {
return fmt.Errorf("%s is not a regular file", f.Name())
}
if IsPrivilegedCaller(id) {
return nil
}
owned, err := fileOwnedBy(id, f)
if err != nil {
return fmt.Errorf("read owner of %s: %w", f.Name(), err)
}
if !owned {
return fmt.Errorf("%s is not owned by the caller (%s)", f.Name(), id)
}
return nil
}
+64
View File
@@ -0,0 +1,64 @@
package ipcauth
import (
"io"
"os"
"path/filepath"
"runtime"
"testing"
"github.com/stretchr/testify/require"
)
// otherIdentity is an unprivileged caller that owns nothing the test creates.
func otherIdentity(t *testing.T) Identity {
t.Helper()
if runtime.GOOS == "windows" {
return Identity{SID: "S-1-5-21-1-2-3-1001"}
}
return Identity{UID: uint32(os.Geteuid() + 1), GID: uint32(os.Getegid() + 1)}
}
func TestOpenOwnedFileReadsFileOwnedByCaller(t *testing.T) {
path := filepath.Join(t.TempDir(), "gui-client.log")
require.NoError(t, os.WriteFile(path, []byte("hello"), 0600))
id, err := CurrentProcessIdentity()
require.NoError(t, err)
f, err := OpenOwnedFile(id, path)
require.NoError(t, err)
t.Cleanup(func() { _ = f.Close() })
content, err := io.ReadAll(f)
require.NoError(t, err)
require.Equal(t, "hello", string(content))
}
func TestOpenOwnedFileRefusesFileOwnedByAnother(t *testing.T) {
path := filepath.Join(t.TempDir(), "gui-client.log")
require.NoError(t, os.WriteFile(path, []byte("secret"), 0600))
_, err := OpenOwnedFile(otherIdentity(t), path)
require.ErrorContains(t, err, "not owned by the caller")
}
func TestOpenOwnedFileRefusesNonRegularFile(t *testing.T) {
dir := t.TempDir()
// The caller owns the directory, so this is the regular-file requirement
// talking, not the ownership check.
id, err := CurrentProcessIdentity()
require.NoError(t, err)
_, err = OpenOwnedFile(id, dir)
require.ErrorContains(t, err, "not a regular file")
}
func TestOpenOwnedFileRefusesMissingFile(t *testing.T) {
id, err := CurrentProcessIdentity()
require.NoError(t, err)
_, err = OpenOwnedFile(id, filepath.Join(t.TempDir(), "absent.log"))
require.Error(t, err)
}
+35
View File
@@ -0,0 +1,35 @@
//go:build !windows
package ipcauth
import (
"fmt"
"os"
"syscall"
)
// openForRead opens a caller-supplied path without following a symlink at its
// final component and without blocking: a fifo planted at the path would
// otherwise stall the open until a writer appears, and the daemon holds a lock
// while it collects the file.
func openForRead(path string) (*os.File, error) {
f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
if err != nil {
return nil, fmt.Errorf("open %s: %w", path, err)
}
return f, nil
}
func fileOwnedBy(id Identity, f *os.File) (bool, error) {
info, err := f.Stat()
if err != nil {
return false, err
}
stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return false, fmt.Errorf("no owner information in %T", info.Sys())
}
return stat.Uid == id.UID, nil
}
@@ -0,0 +1,57 @@
//go:build !windows
package ipcauth
import (
"errors"
"os"
"path/filepath"
"syscall"
"testing"
"time"
"github.com/stretchr/testify/require"
)
// A symlink is the shape the arbitrary-read attempt takes: the caller owns the
// link, the file it points at belongs to someone else.
func TestOpenOwnedFileRefusesSymlink(t *testing.T) {
dir := t.TempDir()
target := filepath.Join(dir, "target.log")
require.NoError(t, os.WriteFile(target, []byte("secret"), 0600))
link := filepath.Join(dir, "gui-client.log")
require.NoError(t, os.Symlink(target, link))
id, err := CurrentProcessIdentity()
require.NoError(t, err)
_, err = OpenOwnedFile(id, link)
// O_NOFOLLOW on a symlink reports ELOOP on Linux/Darwin and EMLINK on FreeBSD.
if !errors.Is(err, syscall.ELOOP) && !errors.Is(err, syscall.EMLINK) {
t.Fatalf("symlink open: got %v, want ELOOP or EMLINK", err)
}
}
// A fifo would block the open until a writer showed up, stalling the daemon
// while it holds its lock.
func TestOpenOwnedFileRefusesFifoWithoutBlocking(t *testing.T) {
path := filepath.Join(t.TempDir(), "gui-client.log")
require.NoError(t, syscall.Mkfifo(path, 0600))
id, err := CurrentProcessIdentity()
require.NoError(t, err)
done := make(chan error, 1)
go func() {
_, err := OpenOwnedFile(id, path)
done <- err
}()
select {
case err := <-done:
require.ErrorContains(t, err, "not a regular file")
case <-time.After(5 * time.Second):
t.Fatal("opening a fifo blocked")
}
}
@@ -0,0 +1,59 @@
//go:build windows
package ipcauth
import (
"fmt"
"os"
"golang.org/x/sys/windows"
)
// openForRead opens a caller-supplied path without following a reparse point at
// it. FILE_FLAG_OPEN_REPARSE_POINT is the Windows analogue of O_NOFOLLOW: it
// opens a symlink/junction itself rather than its target, so the regular-file
// check in checkOwnership refuses a link the caller planted to redirect the
// read. FILE_FLAG_BACKUP_SEMANTICS lets a directory open too (as os.Open does),
// so a directory planted at the path is refused as non-regular rather than
// erroring here. The share mode matches os.Open so a log being written stays
// openable.
func openForRead(path string) (*os.File, error) {
p, err := windows.UTF16PtrFromString(path)
if err != nil {
return nil, fmt.Errorf("convert path %s: %w", path, err)
}
handle, err := windows.CreateFile(
p,
windows.GENERIC_READ,
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
nil,
windows.OPEN_EXISTING,
windows.FILE_FLAG_OPEN_REPARSE_POINT|windows.FILE_FLAG_BACKUP_SEMANTICS,
0,
)
if err != nil {
return nil, fmt.Errorf("open %s: %w", path, err)
}
return os.NewFile(uintptr(handle), path), nil
}
// fileOwnedBy compares the file's owner SID with the caller's. Files an elevated
// process creates are owned by BUILTIN\Administrators rather than by the user,
// but such a caller is privileged and never reaches this check.
func fileOwnedBy(id Identity, f *os.File) (bool, error) {
// x/sys/windows GetSecurityInfo frees the OS buffer itself and returns a
// Go-heap copy, so there is nothing to LocalFree here.
sd, err := windows.GetSecurityInfo(windows.Handle(f.Fd()), windows.SE_FILE_OBJECT, windows.OWNER_SECURITY_INFORMATION)
if err != nil {
return false, fmt.Errorf("read security info: %w", err)
}
owner, _, err := sd.Owner()
if err != nil {
return false, fmt.Errorf("read owner: %w", err)
}
return id.SID != "" && owner.String() == id.SID, nil
}
@@ -0,0 +1,78 @@
//go:build windows
package ipcauth
import (
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
"golang.org/x/sys/windows"
)
// fileOwnerSID reads the owner SID of path the same way OpenOwnedFile does, so
// the test can construct an Identity that matches (or deliberately does not).
func fileOwnerSID(t *testing.T, path string) string {
t.Helper()
f, err := os.Open(path)
require.NoError(t, err)
t.Cleanup(func() { _ = f.Close() })
sd, err := windows.GetSecurityInfo(windows.Handle(f.Fd()), windows.SE_FILE_OBJECT, windows.OWNER_SECURITY_INFORMATION)
require.NoError(t, err)
owner, _, err := sd.Owner()
require.NoError(t, err)
return owner.String()
}
// The allow branch of fileOwnedBy is the SID-equality path the legitimate GUI
// flow depends on. Running elevated, a created file is owned by
// BUILTIN\Administrators; an Identity carrying that SID with Elevated=false and
// no groups is unprivileged by IsPrivileged (which reads the token, not the
// SID's RID), so this exercises the real GetSecurityInfo equality rather than
// the privileged-caller shortcut.
func TestOpenOwnedFileWindowsOwnerMatchAllows(t *testing.T) {
path := filepath.Join(t.TempDir(), "gui-client.log")
require.NoError(t, os.WriteFile(path, []byte("hello"), 0600))
ownerSID := fileOwnerSID(t, path)
id := Identity{SID: ownerSID}
require.False(t, id.IsPrivileged(), "identity built from the owner SID must be unprivileged for this to test the match path")
f, err := OpenOwnedFile(id, path)
require.NoError(t, err)
_ = f.Close()
}
func TestOpenOwnedFileWindowsOwnerMismatchRefuses(t *testing.T) {
path := filepath.Join(t.TempDir(), "gui-client.log")
require.NoError(t, os.WriteFile(path, []byte("secret"), 0600))
other := Identity{SID: "S-1-5-21-9-9-9-9999"}
require.False(t, other.IsPrivileged())
_, err := OpenOwnedFile(other, path)
require.ErrorContains(t, err, "not owned by the caller")
}
// FILE_FLAG_OPEN_REPARSE_POINT must make OpenOwnedFile refuse a symlink the same
// way O_NOFOLLOW does on Unix, so a planted link can't redirect the read to
// another file. Creating a symlink needs a privilege the runner may lack, so the
// test skips rather than fails when it can't.
func TestOpenOwnedFileWindowsRefusesSymlink(t *testing.T) {
dir := t.TempDir()
target := filepath.Join(dir, "target.log")
require.NoError(t, os.WriteFile(target, []byte("secret"), 0600))
link := filepath.Join(dir, "gui-client.log")
if err := os.Symlink(target, link); err != nil {
t.Skipf("cannot create symlink (privilege not held?): %v", err)
}
id, err := CurrentProcessIdentity()
require.NoError(t, err)
_, err = OpenOwnedFile(id, link)
require.Error(t, err, "a symlink must be refused")
}
+25 -13
View File
@@ -45,12 +45,35 @@ func (pm *ProfileManager) GetProfileState(id ID) (*ProfileState, error) {
return &state, nil
}
func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error {
// SetProfileState writes the state file of the profile identified by id. Prefer
// it over SetActiveProfileState whenever the caller knows which profile the data
// belongs to: an SSO login spans seconds of user interaction, and the active
// profile can change during it, which would file the account email under
// whichever profile happened to be active when the flow returned.
func (pm *ProfileManager) SetProfileState(id ID, state *ProfileState) error {
configDir, err := getConfigDir()
if err != nil {
return fmt.Errorf("get config directory: %w", err)
}
if id == "" {
return fmt.Errorf("empty profile ID")
}
if id != defaultProfileName && !IsValidProfileFilenameStem(id) {
return fmt.Errorf("invalid profile ID: %q", id)
}
stateFile := filepath.Join(configDir, id.String()+".state.json")
if err := util.WriteJsonWithRestrictedPermission(context.Background(), stateFile, state); err != nil {
return fmt.Errorf("write profile state: %w", err)
}
return nil
}
// SetActiveProfileState writes the state file of whichever profile is active at
// call time. Use SetProfileState when the target profile is known.
func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error {
activeProf, err := pm.GetActiveProfile()
if err != nil {
if errors.Is(err, ErrNoActiveProfile) {
@@ -59,18 +82,7 @@ func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error {
return fmt.Errorf("get active profile: %w", err)
}
id := activeProf.ID
if id != defaultProfileName && !IsValidProfileFilenameStem(id) {
return fmt.Errorf("invalid active profile ID: %q", id)
}
stateFile := filepath.Join(configDir, id.String()+".state.json")
err = util.WriteJsonWithRestrictedPermission(context.Background(), stateFile, state)
if err != nil {
return fmt.Errorf("write profile state: %w", err)
}
return nil
return pm.SetProfileState(activeProf.ID, state)
}
// RemoveProfileState deletes the per-profile state file (which holds the
@@ -479,7 +479,7 @@ func (d *DnsInterceptor) removeDNATMappings(realPrefixes []netip.Prefix, logger
// internalDnatFw checks if the firewall supports internal DNAT
func (d *DnsInterceptor) internalDnatFw() (internalDNATer, bool) {
if d.firewall == nil || runtime.GOOS != "android" {
if d.firewall == nil || d.fakeIPManager == nil || runtime.GOOS != "android" {
return nil, false
}
fw, ok := d.firewall.(internalDNATer)
+28 -20
View File
@@ -165,31 +165,36 @@ func (m *DefaultManager) setupAndroidRoutes(config ManagerConfig) {
routesForComparison := slices.Clone(cr)
if config.DNSFeatureFlag {
m.fakeIPManager = fakeip.NewManager()
v4ID := uuid.NewString()
fakeIPRoute := &route.Route{
ID: route.ID(v4ID),
Network: m.fakeIPManager.GetFakeIPBlock(),
NetID: route.NetID(v4ID),
Peer: m.pubKey,
NetworkType: route.IPv4Network,
}
v6ID := uuid.NewString()
fakeIPv6Route := &route.Route{
ID: route.ID(v6ID),
Network: m.fakeIPManager.GetFakeIPv6Block(),
NetID: route.NetID(v6ID),
Peer: m.pubKey,
NetworkType: route.IPv6Network,
}
cr = append(cr, fakeIPRoute, fakeIPv6Route)
m.notifier.SetFakeIPRoutes([]*route.Route{fakeIPRoute, fakeIPv6Route})
cr = append(cr, m.enableFakeIPRoutes()...)
}
m.notifier.SetInitialClientRoutes(cr, routesForComparison)
}
func (m *DefaultManager) enableFakeIPRoutes() []*route.Route {
m.fakeIPManager = fakeip.NewManager()
v4ID := uuid.NewString()
fakeIPRoute := &route.Route{
ID: route.ID(v4ID),
Network: m.fakeIPManager.GetFakeIPBlock(),
NetID: route.NetID(v4ID),
Peer: m.pubKey,
NetworkType: route.IPv4Network,
}
v6ID := uuid.NewString()
fakeIPv6Route := &route.Route{
ID: route.ID(v6ID),
Network: m.fakeIPManager.GetFakeIPv6Block(),
NetID: route.NetID(v6ID),
Peer: m.pubKey,
NetworkType: route.IPv6Network,
}
fakeRoutes := []*route.Route{fakeIPRoute, fakeIPv6Route}
m.notifier.SetFakeIPRoutes(fakeRoutes)
return fakeRoutes
}
func (m *DefaultManager) setupRefCounters(useNoop bool) {
var once sync.Once
var wgIface *net.Interface
@@ -464,6 +469,9 @@ func (m *DefaultManager) UpdateRoutes(
var merr *multierror.Error
if !m.disableClientRoutes {
if runtime.GOOS == "android" && useNewDNSRoute && m.fakeIPManager == nil {
m.enableFakeIPRoutes()
}
// Update route selector based on management server's isSelected status
m.updateRouteSelectorFromManagement(clientRoutes)
@@ -41,6 +41,7 @@ func (n *Notifier) SetInitialClientRoutes(initialRoutes []*route.Route, routesFo
// SetFakeIPRoutes stores the fake IP routes to be included in every TUN rebuild.
func (n *Notifier) SetFakeIPRoutes(routes []*route.Route) {
n.fakeIPRoutes = routes
n.notify()
}
func (n *Notifier) OnNewRoutes(idMap route.HAMap) {
@@ -78,9 +79,7 @@ func (n *Notifier) notify() {
routeStrings := n.routesToStrings(allRoutes)
sort.Strings(routeStrings)
go func(l listener.NetworkChangeListener) {
l.OnNetworkChanged(strings.Join(routeStrings, ","))
}(n.listener)
n.listener.OnNetworkChanged(strings.Join(routeStrings, ","))
}
func filterStatic(routes []*route.Route) []*route.Route {
@@ -102,16 +101,11 @@ func (n *Notifier) routesToStrings(routes []*route.Route) []string {
}
func (n *Notifier) hasRouteDiff(a []*route.Route, b []*route.Route) bool {
slices.SortFunc(a, func(x, y *route.Route) int {
return strings.Compare(x.NetString(), y.NetString())
})
slices.SortFunc(b, func(x, y *route.Route) int {
return strings.Compare(x.NetString(), y.NetString())
})
return !slices.EqualFunc(a, b, func(x, y *route.Route) bool {
return x.NetString() == y.NetString()
})
as := n.routesToStrings(a)
bs := n.routesToStrings(b)
sort.Strings(as)
sort.Strings(bs)
return !slices.Equal(as, bs)
}
func (n *Notifier) GetInitialRouteRanges() []string {
@@ -98,47 +98,44 @@ func (u *Installer) startDaemon(daemonFolder string) error {
func (u *Installer) startUIAsUser() error {
log.Infof("starting netbird-ui: %s", uiBinary)
// Get the current console user
cmd := exec.Command("stat", "-f", "%Su", "/dev/console")
output, err := cmd.Output()
username, err := consoleUser()
if err != nil {
return fmt.Errorf("failed to get console user: %w", err)
return err
}
username := strings.TrimSpace(string(output))
if username == "" || username == "root" {
return fmt.Errorf("no active user session found")
}
log.Infof("starting UI for user: %s", username)
// Get user's UID
userInfo, err := user.Lookup(username)
if err != nil {
return fmt.Errorf("failed to lookup user %s: %w", username, err)
return fmt.Errorf("lookup user %s: %w", username, err)
}
// Start the UI process as the console user using launchctl
// This ensures the app runs in the user's context with proper GUI access
launchCmd := exec.Command("launchctl", "asuser", userInfo.Uid, "open", "-a", uiBinary)
log.Infof("starting UI for user: %s (uid %s)", username, userInfo.Uid)
launchCmd := exec.Command("launchctl", "asuser", userInfo.Uid, "sudo", "-u", username, "-H", "open", "-a", uiBinary)
log.Infof("launchCmd: %s", launchCmd.String())
// Set the user's home directory for proper macOS app behavior
launchCmd.Env = append(os.Environ(), "HOME="+userInfo.HomeDir)
log.Infof("set HOME environment variable: %s", userInfo.HomeDir)
if err := launchCmd.Start(); err != nil {
return fmt.Errorf("failed to start UI process: %w", err)
}
// Release the process so it can run independently
if err := launchCmd.Process.Release(); err != nil {
log.Warnf("failed to release UI process: %v", err)
if err := launchCmd.Run(); err != nil {
return fmt.Errorf("run UI launch: %w", err)
}
log.Infof("netbird-ui started successfully for user %s", username)
return nil
}
func consoleUser() (string, error) {
output, err := exec.Command("stat", "-f", "%Su", "/dev/console").Output()
if err != nil {
return "", fmt.Errorf("get console user: %w", err)
}
username := strings.TrimSpace(string(output))
switch username {
case "", "root", "loginwindow", "_mbsetupuser":
return "", fmt.Errorf("no active GUI user session, console user: %q", username)
}
return username, nil
}
func (u *Installer) installPkgFile(ctx context.Context, path string) error {
log.Infof("installing pkg file: %s", path)
+14 -8
View File
@@ -158,13 +158,19 @@ func (c *Client) Run(fd int32, interfaceName string, envList *EnvList) error {
defer c.ctxCancel()
c.ctxCancelLock.Unlock()
auth := NewAuthWithConfig(ctx, cfg)
err = auth.LoginSync()
if err != nil {
return err
}
log.Infof("Auth successful")
// No login pre-flight here. The engine's own loginToManagement (connect.go) performs
// the authoritative Login immediately before the first Sync, so a LoginSync() call at
// this point only duplicated it — costing two extra Login RPCs (IsLoginRequired +
// Login) on every engine start, since IsLoginRequired is itself a full Login RPC.
//
// Auth failures still reach the caller through the engine path: loginToManagement
// returns PermissionDenied, which marks the shared status recorder
// (MarkManagementDisconnected) and fires ClientStop → onDisconnected, where
// IsLoginRequiredCached() reports login-required. The error is also returned out of Run().
//
// A pre-flight was also actively harmful when the server is unreachable: its 2-minute
// backoff blocked the start and then reported "login required" for what was really a
// timeout. The engine instead keeps retrying and recovers when the server returns.
// todo do not throw error in case of cancelled context
ctx = internal.CtxInitState(ctx)
c.onHostDnsFn = func([]string) {}
@@ -262,7 +268,7 @@ func (c *Client) DebugBundle(anonymize bool) (string, error) {
uploadCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
key, err := debug.UploadDebugBundle(uploadCtx, types.DefaultBundleURL, cfg.ManagementURL.String(), path)
key, err := debug.UploadDebugBundle(uploadCtx, types.DefaultBundleURL, cfg.ManagementURL.String(), path, false)
if err != nil {
return "", fmt.Errorf("upload debug bundle: %w", err)
}
+30 -8
View File
@@ -222,17 +222,36 @@ func (a *Auth) Login(resultListener ErrListener, urlOpener URLOpener, forceDevic
// LoginWithDeviceName performs interactive login with device authentication support
// The deviceName parameter allows specifying a custom device name (required for tvOS)
func (a *Auth) LoginWithDeviceName(resultListener ErrListener, urlOpener URLOpener, forceDeviceAuth bool, deviceName string) {
a.startLogin(resultListener, urlOpener, forceDeviceAuth, deviceName, false)
}
// LoginInteractive performs the same interactive login as LoginWithDeviceName but skips the
// IsLoginRequired() pre-flight and goes straight to the browser / device-code flow.
//
// IsLoginRequired() is itself a full Login RPC against the management server, so when the
// caller has ALREADY established that login is required it is a pure duplicate. On iOS the
// main app decides to show the browser based on its own isLoginRequired() check and then
// calls straight into this method, so re-asking the server would add another Login RPC to
// every interactive login.
//
// Use LoginWithDeviceName when the auth state is unknown and a silent (browser-less) login
// must still be possible; use this when the browser is going to be shown regardless.
func (a *Auth) LoginInteractive(resultListener ErrListener, urlOpener URLOpener, forceDeviceAuth bool, deviceName string) {
a.startLogin(resultListener, urlOpener, forceDeviceAuth, deviceName, true)
}
func (a *Auth) startLogin(resultListener ErrListener, urlOpener URLOpener, forceDeviceAuth bool, deviceName string, skipLoginCheck bool) {
if resultListener == nil {
log.Errorf("LoginWithDeviceName: resultListener is nil")
log.Errorf("startLogin: resultListener is nil")
return
}
if urlOpener == nil {
log.Errorf("LoginWithDeviceName: urlOpener is nil")
log.Errorf("startLogin: urlOpener is nil")
resultListener.OnError(fmt.Errorf("urlOpener is nil"))
return
}
go func() {
err := a.login(urlOpener, forceDeviceAuth, deviceName)
err := a.login(urlOpener, forceDeviceAuth, deviceName, skipLoginCheck)
if err != nil {
resultListener.OnError(err)
} else {
@@ -241,7 +260,7 @@ func (a *Auth) LoginWithDeviceName(resultListener ErrListener, urlOpener URLOpen
}()
}
func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName string) error {
func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName string, skipLoginCheck bool) error {
// Create context with device name if provided
ctx := a.ctx
if deviceName != "" {
@@ -255,10 +274,13 @@ func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName strin
}
defer authClient.Close()
// check if we need to generate JWT token
needsLogin, err := authClient.IsLoginRequired(ctx)
if err != nil {
return fmt.Errorf("failed to check login requirement: %v", err)
// check if we need to generate JWT token (skipped when the caller already knows)
needsLogin := true
if !skipLoginCheck {
needsLogin, err = authClient.IsLoginRequired(ctx)
if err != nil {
return fmt.Errorf("failed to check login requirement: %v", err)
}
}
jwtToken := ""
+1 -1
View File
@@ -54,7 +54,7 @@ func (e *Executor) BundleJob(ctx context.Context, debugBundleDependencies debug.
}
}()
key, err := debug.UploadDebugBundle(ctx, types.DefaultBundleURL, mgmURL, path)
key, err := debug.UploadDebugBundle(ctx, types.DefaultBundleURL, mgmURL, path, false)
if err != nil {
log.Errorf("failed to upload debug bundle: %v", err)
return "", fmt.Errorf("upload debug bundle: %w", err)
+22 -10
View File
@@ -2771,14 +2771,18 @@ func (x *ForwardingRulesResponse) GetRules() []*ForwardingRule {
// DebugBundler
type DebugBundleRequest struct {
state protoimpl.MessageState `protogen:"open.v1"`
Anonymize bool `protobuf:"varint,1,opt,name=anonymize,proto3" json:"anonymize,omitempty"`
SystemInfo bool `protobuf:"varint,3,opt,name=systemInfo,proto3" json:"systemInfo,omitempty"`
UploadURL string `protobuf:"bytes,4,opt,name=uploadURL,proto3" json:"uploadURL,omitempty"`
LogFileCount uint32 `protobuf:"varint,5,opt,name=logFileCount,proto3" json:"logFileCount,omitempty"`
CliVersion string `protobuf:"bytes,6,opt,name=cliVersion,proto3" json:"cliVersion,omitempty"`
unknownFields protoimpl.UnknownFields
sizeCache protoimpl.SizeCache
state protoimpl.MessageState `protogen:"open.v1"`
Anonymize bool `protobuf:"varint,1,opt,name=anonymize,proto3" json:"anonymize,omitempty"`
SystemInfo bool `protobuf:"varint,3,opt,name=systemInfo,proto3" json:"systemInfo,omitempty"`
UploadURL string `protobuf:"bytes,4,opt,name=uploadURL,proto3" json:"uploadURL,omitempty"`
LogFileCount uint32 `protobuf:"varint,5,opt,name=logFileCount,proto3" json:"logFileCount,omitempty"`
CliVersion string `protobuf:"bytes,6,opt,name=cliVersion,proto3" json:"cliVersion,omitempty"`
// uploadInsecure allows uploading to an http endpoint or one with an
// untrusted TLS certificate. Restricted to privileged callers; for
// self-hosted upload servers.
UploadInsecure bool `protobuf:"varint,7,opt,name=uploadInsecure,proto3" json:"uploadInsecure,omitempty"`
unknownFields protoimpl.UnknownFields
sizeCache protoimpl.SizeCache
}
func (x *DebugBundleRequest) Reset() {
@@ -2846,6 +2850,13 @@ func (x *DebugBundleRequest) GetCliVersion() string {
return ""
}
func (x *DebugBundleRequest) GetUploadInsecure() bool {
if x != nil {
return x.UploadInsecure
}
return false
}
type DebugBundleResponse struct {
state protoimpl.MessageState `protogen:"open.v1"`
Path string `protobuf:"bytes,1,opt,name=path,proto3" json:"path,omitempty"`
@@ -7242,7 +7253,7 @@ const file_daemon_proto_rawDesc = "" +
"\x12translatedHostname\x18\x04 \x01(\tR\x12translatedHostname\x128\n" +
"\x0etranslatedPort\x18\x05 \x01(\v2\x10.daemon.PortInfoR\x0etranslatedPort\"G\n" +
"\x17ForwardingRulesResponse\x12,\n" +
"\x05rules\x18\x01 \x03(\v2\x16.daemon.ForwardingRuleR\x05rules\"\xb4\x01\n" +
"\x05rules\x18\x01 \x03(\v2\x16.daemon.ForwardingRuleR\x05rules\"\xdc\x01\n" +
"\x12DebugBundleRequest\x12\x1c\n" +
"\tanonymize\x18\x01 \x01(\bR\tanonymize\x12\x1e\n" +
"\n" +
@@ -7252,7 +7263,8 @@ const file_daemon_proto_rawDesc = "" +
"\flogFileCount\x18\x05 \x01(\rR\flogFileCount\x12\x1e\n" +
"\n" +
"cliVersion\x18\x06 \x01(\tR\n" +
"cliVersion\"}\n" +
"cliVersion\x12&\n" +
"\x0euploadInsecure\x18\a \x01(\bR\x0euploadInsecure\"}\n" +
"\x13DebugBundleResponse\x12\x12\n" +
"\x04path\x18\x01 \x01(\tR\x04path\x12 \n" +
"\vuploadedKey\x18\x02 \x01(\tR\vuploadedKey\x120\n" +
+4
View File
@@ -536,6 +536,10 @@ message DebugBundleRequest {
string uploadURL = 4;
uint32 logFileCount = 5;
string cliVersion = 6;
// uploadInsecure allows uploading to an http endpoint or one with an
// untrusted TLS certificate. Restricted to privileged callers; for
// self-hosted upload servers.
bool uploadInsecure = 7;
}
message DebugBundleResponse {
+76 -16
View File
@@ -7,18 +7,62 @@ import (
"context"
"errors"
"fmt"
"path/filepath"
"runtime/pprof"
"strings"
"time"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
gstatus "google.golang.org/grpc/status"
"github.com/netbirdio/netbird/client/internal/debug"
"github.com/netbirdio/netbird/client/internal/ipcauth"
"github.com/netbirdio/netbird/client/proto"
mgmProto "github.com/netbirdio/netbird/shared/management/proto"
"github.com/netbirdio/netbird/version"
)
// DebugBundle creates a debug bundle and returns the location.
func (s *Server) DebugBundle(_ context.Context, req *proto.DebugBundleRequest) (resp *proto.DebugBundleResponse, err error) {
func (s *Server) DebugBundle(callerCtx context.Context, req *proto.DebugBundleRequest) (resp *proto.DebugBundleResponse, err error) {
if err := requirePrivilegeForUploadURL(callerCtx, req.GetUploadURL(), req.GetUploadInsecure()); err != nil {
return nil, err
}
// The UI log is opened as whoever asked for this bundle, so a caller only
// collects a log it owns (privileged callers excepted). ok is false on a
// socket that carries no identity, which skips the UI log.
callerID, callerIdentified := ipcauth.CallerIdentity(callerCtx)
path, managementURL, err := s.generateDebugBundle(req, uiLogOpener(callerID, callerIdentified))
if err != nil {
return nil, err
}
if req.GetUploadURL() == "" {
return &proto.DebugBundleResponse{Path: path}, nil
}
// The upload runs without s.mutex held: it does network I/O to a possibly
// slow destination and must not block the other RPCs that take the lock. The
// bounded context is a backstop against a hung connection.
uploadCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
key, err := debug.UploadDebugBundle(uploadCtx, req.GetUploadURL(), managementURL, path, req.GetUploadInsecure())
if err != nil {
log.Errorf("failed to upload debug bundle to %s: %v", req.GetUploadURL(), err)
return &proto.DebugBundleResponse{Path: path, UploadFailureReason: err.Error()}, nil
}
log.Infof("debug bundle uploaded to %s with key %s", req.GetUploadURL(), key)
return &proto.DebugBundleResponse{Path: path, UploadedKey: key}, nil
}
// generateDebugBundle builds the bundle under s.mutex and returns its path plus
// the management URL captured under the lock, so the caller can run the upload
// without holding the lock.
func (s *Server) generateDebugBundle(req *proto.DebugBundleRequest, uiOpener debug.LogOpener) (path string, managementURL string, err error) {
s.mutex.Lock()
defer s.mutex.Unlock()
@@ -68,6 +112,7 @@ func (s *Server) DebugBundle(_ context.Context, req *proto.DebugBundleRequest) (
SyncResponse: syncResponse,
LogPath: s.logFile,
UILogPath: s.uiLogPath,
UILogOpener: uiOpener,
CPUProfile: cpuProfileData,
CapturePath: capturePath,
RefreshStatus: refreshStatus,
@@ -82,23 +127,16 @@ func (s *Server) DebugBundle(_ context.Context, req *proto.DebugBundleRequest) (
},
)
path, err := bundleGenerator.Generate()
path, err = bundleGenerator.Generate()
if err != nil {
return nil, fmt.Errorf("generate debug bundle: %w", err)
return "", "", fmt.Errorf("generate debug bundle: %w", err)
}
if req.GetUploadURL() == "" {
return &proto.DebugBundleResponse{Path: path}, nil
}
key, err := debug.UploadDebugBundle(context.Background(), req.GetUploadURL(), s.config.ManagementURL.String(), path)
if err != nil {
log.Errorf("failed to upload debug bundle to %s: %v", req.GetUploadURL(), err)
return &proto.DebugBundleResponse{Path: path, UploadFailureReason: err.Error()}, nil
if s.config != nil && s.config.ManagementURL != nil {
managementURL = s.config.ManagementURL.String()
}
log.Infof("debug bundle uploaded to %s with key %s", req.GetUploadURL(), key)
return &proto.DebugBundleResponse{Path: path, UploadedKey: key}, nil
return path, managementURL, nil
}
// GetLogLevel gets the current logging level for the server.
@@ -138,12 +176,34 @@ func (s *Server) SetLogLevel(_ context.Context, req *proto.SetLogLevelRequest) (
// RegisterUILog records the desktop UI's absolute log path so DebugBundle can
// collect the GUI log. The daemon runs as root and can't resolve the user's
// config dir, so the UI reports it. Last-writer-wins (one UI per socket).
func (s *Server) RegisterUILog(_ context.Context, req *proto.RegisterUILogRequest) (*proto.RegisterUILogResponse, error) {
//
// The path arrives over an IPC any local user can reach and is later opened by
// a root daemon, so it is constrained to the file name the UI writes and to a
// local absolute path. Authorization happens when DebugBundle opens it: the
// bundle refuses a file its requester does not own. A caller the daemon cannot
// identify cannot register a path at all.
func (s *Server) RegisterUILog(callerCtx context.Context, req *proto.RegisterUILogRequest) (*proto.RegisterUILogResponse, error) {
if _, ok := ipcauth.CallerIdentity(callerCtx); !ok {
return nil, gstatus.Error(codes.PermissionDenied,
"registering a UI log path requires a control channel that carries the caller's identity")
}
path := filepath.Clean(req.GetPath())
if !filepath.IsAbs(path) || filepath.Base(path) != uiLogFileName {
return nil, gstatus.Errorf(codes.InvalidArgument, "UI log path must be an absolute path ending in %s", uiLogFileName)
}
// filepath.IsAbs accepts a Windows UNC path (\\host\share\...) and a device
// path (\\.\, \\?\); opening one would make the root daemon reach a remote
// or device namespace. Require a plain local path.
if strings.HasPrefix(path, `\\`) {
return nil, gstatus.Error(codes.InvalidArgument, "UI log path must be a local path, not a UNC or device path")
}
s.mutex.Lock()
defer s.mutex.Unlock()
s.uiLogPath = req.GetPath()
log.Infof("registered UI log path: %s", s.uiLogPath)
s.uiLogPath = path
log.Infof("registered UI log path %s", s.uiLogPath)
return &proto.RegisterUILogResponse{}, nil
}
+99
View File
@@ -0,0 +1,99 @@
//go:build !android && !ios
package server
import (
"context"
"fmt"
"net/url"
"os"
"strings"
"google.golang.org/grpc/codes"
gstatus "google.golang.org/grpc/status"
"github.com/netbirdio/netbird/client/configs"
"github.com/netbirdio/netbird/client/internal/debug"
"github.com/netbirdio/netbird/client/internal/ipcauth"
"github.com/netbirdio/netbird/upload-server/types"
)
// uiLogFileName is the only file name the daemon accepts as a UI log path. The
// UI (writer), this validation, and the bundle collector all read it from
// configs so they cannot drift.
const uiLogFileName = configs.UILogFile
// uiLogOpener opens the registered UI log, and its rotated siblings, on behalf
// of the caller requesting the bundle: OpenOwnedFile then collects the log only
// when that caller owns it (or is privileged). identified is false on a socket
// that carries no caller identity, in which case nothing is opened.
func uiLogOpener(id ipcauth.Identity, identified bool) debug.LogOpener {
return func(path string) (*os.File, error) {
if !identified {
return nil, fmt.Errorf("bundle requester has no verified identity")
}
return ipcauth.OpenOwnedFile(id, path)
}
}
// requirePrivilegeForUploadURL restricts where the daemon may send a debug
// bundle. The bundle holds the daemon's own logs and state, and the daemon
// fetches the upload URL itself, so an unrestricted endpoint turns the daemon
// into both an exfiltration channel and a request forwarder that reaches
// services only it can talk to.
//
// The upload service NetBird publishes is open to any caller, since that is what
// the CLI and the desktop UI use. Any other endpoint, self-hosted upload servers
// included, requires a privileged caller. Plaintext is refused for everyone: the
// daemon fetches the URL and then PUTs the bundle to whatever that fetch returns,
// so an http hop is a place to intercept the bundle or the redirect.
//
// insecure relaxes transport security (http, or an untrusted TLS certificate)
// for a self-hosted server. It weakens a root-privileged upload, so it is
// refused for an unprivileged caller regardless of the host.
func requirePrivilegeForUploadURL(ctx context.Context, rawURL string, insecure bool) error {
if rawURL == "" {
return nil
}
parsed, err := url.Parse(rawURL)
if err != nil {
return gstatus.Errorf(codes.InvalidArgument, "parse upload URL: %v", err)
}
// --insecure relaxes https to http or an untrusted certificate; it does not
// widen the URL to arbitrary schemes, so a host and http/https are required
// before the insecure branch takes over.
if parsed.Host == "" || (parsed.Scheme != "https" && parsed.Scheme != "http") {
return gstatus.Errorf(codes.InvalidArgument, "upload URL must be http or https with a host")
}
if insecure {
return denyPrivileged(ctx,
"uploading a debug bundle without transport security (--upload-bundle-insecure)",
ipcauth.ElevatedCommand("netbird debug bundle -U --upload-bundle-insecure --upload-bundle-url <url>"))
}
if parsed.Scheme != "https" {
return gstatus.Errorf(codes.InvalidArgument, "upload URL must use https, got scheme %q", parsed.Scheme)
}
if isDefaultUploadService(parsed) {
return nil
}
return denyPrivileged(ctx,
"uploading a debug bundle to an upload service other than the default one",
ipcauth.ElevatedCommand("netbird debug bundle -U --upload-bundle-url <url>"))
}
// isDefaultUploadService reports whether the URL points at the upload service
// NetBird runs. Only the host is compared: the service's path may differ between
// releases, and the host is what decides who receives the bundle.
func isDefaultUploadService(parsed *url.URL) bool {
defaultURL, err := url.Parse(types.DefaultBundleURL)
if err != nil {
return false
}
return parsed.Scheme == defaultURL.Scheme && strings.EqualFold(parsed.Host, defaultURL.Host)
}
+157
View File
@@ -0,0 +1,157 @@
//go:build !android && !ios
package server
import (
"os"
"path/filepath"
"runtime"
"testing"
"google.golang.org/grpc/codes"
gstatus "google.golang.org/grpc/status"
"github.com/netbirdio/netbird/client/internal/ipcauth"
"github.com/netbirdio/netbird/client/proto"
"github.com/netbirdio/netbird/upload-server/types"
)
func TestRegisterUILogRefusesUnidentifiedCaller(t *testing.T) {
s := &Server{}
_, err := s.RegisterUILog(noIdentityCtx(), &proto.RegisterUILogRequest{
Path: filepath.Join(t.TempDir(), uiLogFileName),
})
if gstatus.Code(err) != codes.PermissionDenied {
t.Fatalf("code = %v, want PermissionDenied", gstatus.Code(err))
}
}
func TestRegisterUILogRefusesForeignPath(t *testing.T) {
secret := "/etc/shadow"
if runtime.GOOS == "windows" {
secret = `C:\Windows\System32\config\SAM`
}
tests := []struct {
name string
path string
}{
{"empty", ""},
{"relative", filepath.Join("netbird", uiLogFileName)},
{"another file", secret},
{"directory of the log", t.TempDir()},
{"unc path", `\\attacker\share\` + uiLogFileName},
{"device path", `\\.\C:\` + uiLogFileName},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
s := &Server{}
_, err := s.RegisterUILog(userCtx(), &proto.RegisterUILogRequest{Path: tc.path})
if gstatus.Code(err) != codes.InvalidArgument {
t.Fatalf("code = %v, want InvalidArgument", gstatus.Code(err))
}
if s.uiLogPath != "" {
t.Fatalf("path %q was recorded despite the refusal", s.uiLogPath)
}
})
}
}
func TestRegisterUILogRecordsPath(t *testing.T) {
s := &Server{}
path := filepath.Join(t.TempDir(), uiLogFileName)
if _, err := s.RegisterUILog(userCtx(), &proto.RegisterUILogRequest{Path: path}); err != nil {
t.Fatalf("register: %v", err)
}
if s.uiLogPath != path {
t.Fatalf("path = %q, want %q", s.uiLogPath, path)
}
}
// The UI log is opened as the bundle requester, so a second local user cannot
// collect a log they do not own, and an unidentified requester collects nothing.
func TestUILogOpenerBindsToRequester(t *testing.T) {
path := filepath.Join(t.TempDir(), uiLogFileName)
if err := os.WriteFile(path, []byte("log line"), 0600); err != nil {
t.Fatalf("write log: %v", err)
}
// A different unprivileged user than the file's owner: refused.
if _, err := uiLogOpener(unprivilegedIdentity(), true)(path); err == nil {
t.Fatal("expected a file the requester does not own to be refused")
}
// No verified identity: refused.
if _, err := uiLogOpener(ipcauth.Identity{}, false)(path); err == nil {
t.Fatal("expected an unidentified requester to be refused")
}
// The requester that owns the file: allowed. The test process created it, so
// its own identity is the owner (and a privileged runner is exempt anyway).
owner, err := ipcauth.CurrentProcessIdentity()
if err != nil {
t.Fatalf("current identity: %v", err)
}
f, err := uiLogOpener(owner, true)(path)
if err != nil {
t.Fatalf("expected the owning requester to be allowed, got %v", err)
}
_ = f.Close()
}
func TestRequirePrivilegeForUploadURL(t *testing.T) {
tests := []struct {
name string
url string
insecure bool
unprivOK bool
invalid bool
rootAlso bool
}{
{name: "no upload", url: "", unprivOK: true},
{name: "default service", url: types.DefaultBundleURL, unprivOK: true},
{name: "default service, other path", url: "https://upload.debug.netbird.io/other", unprivOK: true},
{name: "loopback exfiltration endpoint", url: "https://127.0.0.1:8080/upload-url", rootAlso: true},
{name: "custom upload service", url: "https://attacker.example/upload-url", rootAlso: true},
{name: "plaintext default host", url: "http://upload.debug.netbird.io/upload-url", invalid: true},
{name: "plaintext custom host", url: "http://attacker.example/upload-url", invalid: true},
{name: "unsupported scheme", url: "file:///etc/shadow", invalid: true},
// insecure relaxes transport security; privileged only, whatever the host.
{name: "insecure http custom", url: "http://selfhosted.local/upload-url", insecure: true, rootAlso: true},
{name: "insecure https custom", url: "https://selfhosted.local/upload-url", insecure: true, rootAlso: true},
{name: "insecure default host", url: types.DefaultBundleURL, insecure: true, rootAlso: true},
// --insecure must not widen the URL to non-http(s) schemes or a hostless URL.
{name: "insecure file scheme", url: "file:///etc/shadow", insecure: true, invalid: true},
{name: "insecure hostless", url: "https:///upload-url", insecure: true, invalid: true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := requirePrivilegeForUploadURL(userCtx(), tc.url, tc.insecure)
switch {
case tc.invalid:
if gstatus.Code(err) != codes.InvalidArgument {
t.Fatalf("code = %v, want InvalidArgument", gstatus.Code(err))
}
return
case tc.unprivOK:
assertAllowed(t, err)
return
default:
assertDenied(t, err)
}
if tc.rootAlso {
assertAllowed(t, requirePrivilegeForUploadURL(rootCtx(), tc.url, tc.insecure))
}
})
}
}
+51
View File
@@ -0,0 +1,51 @@
package server
import (
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/proto"
)
// The daemon takes guardedConfigMu before s.mutex. authorizeAndPrepareLogin
// takes s.mutex while holding guardedConfigMu, so a SetConfig that grabbed
// s.mutex first and then waited for guardedConfigMu would deadlock the daemon
// against a concurrent login: two unprivileged IPC calls are enough.
//
// The held guardedConfigMu below stands in for that login. While SetConfig waits
// for it, s.mutex must stay free, otherwise the login waiting for s.mutex could
// never release guardedConfigMu.
func TestSetConfig_TakesGuardedConfigMuBeforeServerMutex(t *testing.T) {
s, ctx, profName, username, _ := setupServerWithProfile(t)
s.guardedConfigMu.Lock()
done := make(chan error, 1)
go func() {
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
ProfileName: profName,
Username: username,
})
done <- err
}()
require.Never(t, func() bool {
if !s.mutex.TryLock() {
return true
}
s.mutex.Unlock()
return false
}, 500*time.Millisecond, 10*time.Millisecond,
"SetConfig held s.mutex while waiting for guardedConfigMu, which deadlocks against a concurrent login")
s.guardedConfigMu.Unlock()
select {
case err := <-done:
require.NoError(t, err)
case <-time.After(5 * time.Second):
t.Fatal("SetConfig did not finish after guardedConfigMu was released")
}
}
+89
View File
@@ -0,0 +1,89 @@
package server
import (
"context"
"encoding/json"
"errors"
"os"
"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/proto"
)
// A login that never reached Management is not a decision about the peer's
// credentials, so it must come back as a retryable error rather than an SSO
// prompt: the user cannot finish a browser login while Management is down, and
// the CLI's own backoff resolves the outage on its own once the daemon reports
// the failure. Reproduces `netbird down; netbird up` printing a device-code URL
// because Management happened to be restarting when the daemon dialed it.
func TestLogin_ManagementUnreachableIsReturnedInsteadOfDemandingSSO(t *testing.T) {
s, _, _, username, _ := setupServerWithProfile(t)
s.rootCtx = internal.CtxInitState(context.Background())
unreachable := errors.New("create connection: dial context: context deadline exceeded")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return internal.StatusLoginFailed, unreachable
}
resp, err := s.Login(userCtx(), &proto.LoginRequest{Username: &username})
require.Error(t, err)
require.ErrorIs(t, err, unreachable, "the transport failure was replaced by something else")
require.Nil(t, resp, "a failed login must not answer with a login response")
require.Equal(t, 1, attempts)
require.Nil(t, s.oauthAuthFlow.flow, "the daemon started an SSO flow for a peer whose login was never decided")
status, err := internal.CtxGetState(s.rootCtx).Status()
require.NoError(t, err)
require.Equal(t, internal.StatusLoginFailed, status,
"a peer that could not reach Management is not waiting on a login")
}
// The counterpart: Management refusing the peer's credentials is a decision, and
// the SSO flow still has to start for it. The profile carries an unusable
// private key so the flow setup fails immediately instead of dialing, which is
// enough to show the branch was entered — the refusal itself is never what comes
// back out.
func TestLogin_AuthRefusalStartsSSOFlow(t *testing.T) {
s, _, _, username, cfgPath := setupServerWithProfile(t)
s.rootCtx = internal.CtxInitState(context.Background())
breakProfilePrivateKey(t, cfgPath)
refused := gstatus.Error(codes.PermissionDenied, "peer is not registered")
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
return internal.StatusNeedsLogin, refused
}
_, err := s.Login(userCtx(), &proto.LoginRequest{Username: &username})
require.Error(t, err)
require.NotErrorIs(t, err, refused,
"the refusal was handed back to the caller instead of starting the SSO flow")
status, stateErr := internal.CtxGetState(s.rootCtx).Status()
require.NoError(t, stateErr)
require.Equal(t, internal.StatusLoginFailed, status,
"the SSO flow setup was never reached with the broken key")
}
// breakProfilePrivateKey replaces the profile's private key with an unparseable
// one, which makes any attempt to build a Management client fail on the spot.
func breakProfilePrivateKey(t *testing.T, cfgPath string) {
t.Helper()
raw, err := os.ReadFile(cfgPath)
require.NoError(t, err)
var cfg map[string]any
require.NoError(t, json.Unmarshal(raw, &cfg))
cfg["PrivateKey"] = "not-a-key"
patched, err := json.Marshal(cfg)
require.NoError(t, err)
require.NoError(t, os.WriteFile(cfgPath, patched, 0o600))
}
+46 -10
View File
@@ -72,6 +72,9 @@ type Server struct {
// RegisterUILog. Guarded by mutex. Consumed by DebugBundle so the bundle
// can collect the GUI log even though the daemon runs as root and can't
// resolve the user's config dir. Last-writer-wins (one UI per socket).
// DebugBundle opens it on behalf of the bundle requester and refuses a file
// that caller does not own, so a local user cannot read another user's log
// or a root-only file through it.
uiLogPath string
oauthAuthFlow oauthAuthFlow
@@ -132,6 +135,11 @@ type Server struct {
updateManager *updater.Manager
jwtCache *jwtCache
// loginAttemptFn stands in for the Management login round trip. Tests set
// it to drive the login outcomes that need a server on the other end;
// production leaves it nil, and every login goes through loginAttempt.
loginAttemptFn func(ctx context.Context, setupKey, jwtToken string) (internal.StatusType, error)
}
type oauthAuthFlow struct {
@@ -367,7 +375,19 @@ func (s *Server) connectionGoroutineRunning() bool {
}
}
// loginAttempt attempts to login using the provided information. it returns a status in case something fails
// attemptLogin runs a login round trip against Management, or the stand-in a
// test installed in place of it.
func (s *Server) attemptLogin(ctx context.Context, setupKey, jwtToken string) (internal.StatusType, error) {
if s.loginAttemptFn != nil {
return s.loginAttemptFn(ctx, setupKey, jwtToken)
}
return s.loginAttempt(ctx, setupKey, jwtToken)
}
// loginAttempt attempts to login using the provided information. It returns
// StatusNeedsLogin when Management refused the peer's credentials and
// StatusLoginFailed for every other failure, so callers can tell an
// authentication decision apart from a login that never got made.
func (s *Server) loginAttempt(ctx context.Context, setupKey, jwtToken string) (internal.StatusType, error) {
authClient, err := auth.NewAuth(ctx, s.config.PrivateKey, s.config.ManagementURL, s.config)
if err != nil {
@@ -393,6 +413,16 @@ func (s *Server) loginAttempt(ctx context.Context, setupKey, jwtToken string) (i
// Login uses setup key to prepare configuration for the daemon.
func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigRequest) (*proto.SetConfigResponse, error) {
// 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.
//
// Taken before s.mutex: authorizeAndPrepareLogin takes s.mutex while holding
// guardedConfigMu, so acquiring the two in the other order here would let a
// concurrent login deadlock the daemon.
s.guardedConfigMu.Lock()
defer s.guardedConfigMu.Unlock()
s.mutex.Lock()
defer s.mutex.Unlock()
@@ -417,12 +447,6 @@ 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
@@ -616,11 +640,23 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
s.config = config
s.mutex.Unlock()
if _, err := s.loginAttempt(ctx, "", ""); err == nil {
loginStatus, err := s.attemptLogin(ctx, "", "")
if err == nil {
state.Set(internal.StatusIdle)
return &proto.LoginResponse{}, nil
}
// Only an authentication refusal means the peer has to (re-)authenticate.
// Any other failure leaves the login undecided: Management unreachable, a
// restart mid-request, an internal error. Those are returned for the caller
// to retry, because turning them into an SSO prompt asks the user to solve
// something that is not theirs to solve, and a browser login cannot succeed
// while Management is unreachable anyway.
if loginStatus != internal.StatusNeedsLogin {
state.Set(loginStatus)
return nil, err
}
if msg.SetupKey == "" {
hint := ""
if msg.Hint != nil {
@@ -677,7 +713,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
// which returns NeedsLogin and parks on the browser leg.
state.Set(internal.StatusConnecting)
if loginStatus, err := s.loginAttempt(ctx, msg.SetupKey, ""); err != nil {
if loginStatus, err := s.attemptLogin(ctx, msg.SetupKey, ""); err != nil {
state.Set(loginStatus)
return nil, err
}
@@ -832,7 +868,7 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
s.oauthAuthFlow.expiresAt = time.Now()
s.mutex.Unlock()
if loginStatus, err := s.loginAttempt(ctx, "", tokenInfo.GetTokenToUse()); err != nil {
if loginStatus, err := s.attemptLogin(ctx, "", tokenInfo.GetTokenToUse()); err != nil {
state.Set(loginStatus)
return nil, err
}
+4 -4
View File
@@ -26,17 +26,17 @@ contents:
# Default dependencies for the GTK4 + WebKitGTK 6.0 stack (Ubuntu 24.04+ / Debian 13+)
depends:
- libgtk-4-1
- libgtk-4-1 (>= 4.14)
- libwebkitgtk-6.0-4
- xdg-utils
# Distribution-specific overrides for different package formats
overrides:
# RPM packages for Fedora / RHEL / AlmaLinux / Rocky Linux
# RPM packages for Fedora / RHEL / AlmaLinux / Rocky Linux / openSUSE
rpm:
depends:
- gtk4
- webkitgtk6.0
- (gtk4 >= 4.14 or libgtk-4-1 >= 4.14)
- (webkitgtk6.0 or libwebkitgtk-6_0-4)
- xdg-utils
# Arch Linux packages
+7 -2
View File
@@ -43,7 +43,12 @@ function buildSsoCancelPromise(state: SsoState, signal?: AbortSignal): Promise<v
}
async function runSsoLogin(
result: { verificationUri: string; verificationUriComplete: string; userCode: string },
result: {
verificationUri: string;
verificationUriComplete: string;
userCode: string;
profileId: string;
},
state: SsoState,
signal?: AbortSignal,
): Promise<void> {
@@ -56,7 +61,7 @@ async function runSsoLogin(
// suspended, so a frontend-driven Up (a promise continuation) would not
// fire until the user woke the window (e.g. hovering the tray icon).
const waitPromise = Connection.WaitSSOLoginAndUp(
{ userCode: result.userCode, hostname: "" },
{ userCode: result.userCode, hostname: "", profileId: result.profileId },
{ profileName: "", username: "" },
);
+3 -4
View File
@@ -14,7 +14,6 @@ import (
"github.com/sirupsen/logrus"
"github.com/wailsapp/wails/v3/pkg/application"
"github.com/wailsapp/wails/v3/pkg/events"
"github.com/wailsapp/wails/v3/pkg/services/notifications"
"github.com/netbirdio/netbird/client/ui/authsession"
"github.com/netbirdio/netbird/client/ui/i18n"
@@ -63,7 +62,7 @@ type registeredServices struct {
profiles *services.Profiles
update *services.Update
daemonFeed *services.DaemonFeed
notifier *notifications.NotificationService
notifier *Notifier
compat *services.Compat
profileSwitcher *services.ProfileSwitcher
bundle *i18n.Bundle
@@ -102,7 +101,7 @@ func main() {
updaterHolder := updater.NewHolder(app.Event)
update := services.NewUpdate(conn, updaterHolder)
daemonFeed := services.NewDaemonFeed(conn, app.Event, updaterHolder, debugLog)
notifier := notifications.New()
notifier := newNotifier()
compat := services.NewCompat(conn)
// macOS shows no toast until permission is requested. Run it after
// ApplicationStarted so the notifier's Startup has initialised the
@@ -210,7 +209,7 @@ func main() {
// requestNotificationAuthorization prompts for macOS notification permission.
// The request blocks until the user responds (up to 3 minutes), so callers run
// it in a goroutine. No-op on Linux/Windows.
func requestNotificationAuthorization(notifier *notifications.NotificationService) {
func requestNotificationAuthorization(notifier *Notifier) {
authorized, err := notifier.CheckNotificationAuthorization()
if err != nil {
logrus.Debugf("check notification authorization: %v", err)
+7 -2
View File
@@ -29,8 +29,13 @@ cask "{{ $projectName }}" do
end
uninstall_preflight do
system_command "#{appdir}/Netbird UI.app/uninstaller.sh",
sudo: false
system_command "/bin/sh",
args: ["-c", <<~CMD],
launchctl bootout system/netbird 2>/dev/null || \
launchctl unload /Library/LaunchDaemons/netbird.plist 2>/dev/null || true
rm -f /Library/LaunchDaemons/netbird.plist
CMD
sudo: true
end
name "Netbird UI"
+101
View File
@@ -0,0 +1,101 @@
//go:build !android && !ios && !freebsd && !js
package main
import (
"context"
"errors"
"sync/atomic"
log "github.com/sirupsen/logrus"
"github.com/wailsapp/wails/v3/pkg/application"
"github.com/wailsapp/wails/v3/pkg/services/notifications"
)
var errNotificationsUnavailable = errors.New("notifications unavailable")
// Notifier wraps the Wails notification service so an unavailable backend
// disables notifications instead of aborting the app. Startup fails for
// environment reasons (a bare unbundled binary on macOS has no bundle
// identifier, a headless Linux session has no D-Bus session bus), and Wails
// treats a service startup error as fatal. After a failed startup every call
// is a no-op: on macOS, touching UNUserNotificationCenter without a bundle
// identifier raises an Objective-C exception that recover() cannot catch.
type Notifier struct {
inner *notifications.NotificationService
available atomic.Bool
}
func newNotifier() *Notifier {
return &Notifier{inner: notifications.New()}
}
// ServiceName implements the Wails service-name hook for startup logs.
func (n *Notifier) ServiceName() string {
return n.inner.ServiceName()
}
// ServiceStartup starts the platform notifier, downgrading failure to a
// warning so the app keeps running without notifications.
func (n *Notifier) ServiceStartup(ctx context.Context, options application.ServiceOptions) error {
if err := n.inner.ServiceStartup(ctx, options); err != nil {
log.Warnf("notifications disabled: %v", err)
return nil
}
n.available.Store(true)
return nil
}
func (n *Notifier) ServiceShutdown() error {
if !n.available.Load() {
return nil
}
return n.inner.ServiceShutdown()
}
func (n *Notifier) CheckNotificationAuthorization() (bool, error) {
if !n.available.Load() {
return false, errNotificationsUnavailable
}
return n.inner.CheckNotificationAuthorization()
}
func (n *Notifier) RequestNotificationAuthorization() (bool, error) {
if !n.available.Load() {
return false, errNotificationsUnavailable
}
return n.inner.RequestNotificationAuthorization()
}
// SendNotification delivers a notification, silently dropping it when the
// backend never started (notifications are best-effort everywhere).
func (n *Notifier) SendNotification(options notifications.NotificationOptions) error {
if !n.available.Load() {
log.Debugf("notifications disabled, dropping %q", options.ID)
return nil
}
return n.inner.SendNotification(options)
}
func (n *Notifier) SendNotificationWithActions(options notifications.NotificationOptions) error {
if !n.available.Load() {
log.Debugf("notifications disabled, dropping %q", options.ID)
return nil
}
return n.inner.SendNotificationWithActions(options)
}
func (n *Notifier) RegisterNotificationCategory(category notifications.NotificationCategory) error {
if !n.available.Load() {
return nil
}
return n.inner.RegisterNotificationCategory(category)
}
// OnNotificationResponse registers the response callback. Pure Go state, so
// it is safe (and simply inert) when the backend never started.
//
//wails:ignore
func (n *Notifier) OnNotificationResponse(callback func(result notifications.NotificationResult)) {
n.inner.OnNotificationResponse(callback)
}
+1
View File
@@ -246,6 +246,7 @@ func (s *Store) ExistedAtLoad() bool {
func (s *Store) load() error {
if _, err := os.Stat(s.path); err != nil {
if errors.Is(err, os.ErrNotExist) {
log.Infof("no ui preferences file at %s; using defaults", s.path)
return nil
}
return fmt.Errorf("stat preferences: %w", err)
+40
View File
@@ -33,12 +33,21 @@ type LoginResult struct {
UserCode string `json:"userCode"`
VerificationURI string `json:"verificationUri"`
VerificationURIComplete string `json:"verificationUriComplete"`
// ProfileID is the ID of the profile this login ran against, or "" when the
// caller named the profile itself and no ID was resolved. Pass it back in
// WaitSSOParams so the account email lands on this profile even if the
// active one changes during SSO.
ProfileID string `json:"profileId"`
}
// WaitSSOParams are the inputs to waitSSOLogin.
type WaitSSOParams struct {
UserCode string `json:"userCode"`
Hostname string `json:"hostname"`
// ProfileID is the profile the login was started for, used to file the
// account email against it rather than against whichever profile is active
// when the flow returns. Optional: empty falls back to the active profile.
ProfileID string `json:"profileId"`
}
// UpParams selects the profile to bring up.
@@ -77,11 +86,16 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
// Fall back to the daemon's active profile and the current OS user.
profileName := p.ProfileName
username := p.Username
// Only set when the daemon told us the ID. A caller-supplied ProfileName is
// a handle — a display name or an ID prefix resolve too — and the state file
// is named after the ID, so passing a handle on would name the wrong file.
profileID := ""
if profileName == "" {
if active, aerr := cli.GetActiveProfile(ctx, &proto.GetActiveProfileRequest{}); aerr == nil {
// Address the active profile by ID (the daemon resolves it as a
// handle); names can collide, the ID cannot.
profileName = active.GetId()
profileID = profileName
if username == "" {
username = active.GetUsername()
}
@@ -122,6 +136,7 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
UserCode: resp.GetUserCode(),
VerificationURI: resp.GetVerificationURI(),
VerificationURIComplete: resp.GetVerificationURIComplete(),
ProfileID: profileID,
}, nil
}
@@ -242,6 +257,31 @@ func (s *Connection) waitSSOLogin(ctx context.Context, p WaitSSOParams) (string,
return "", s.classifyDaemonError(err)
}
log.Infof("SSO login completed, daemon reported success")
// Persist the account email the same way the CLI does after its own
// WaitSSOLogin: the daemon returns it but cannot store it, since it runs as
// root and the per-profile state file is user-owned (see Logout below).
// Without this the profile has no email, so Profiles.List shows no account
// and later logins and session extends go out without a login_hint —
// leaving the IdP to guess which account was meant.
if email := resp.GetEmail(); email != "" {
state := &profilemanager.ProfileState{Email: email}
pm := profilemanager.NewProfileManager()
// Against the profile the login was started for: SSO spans seconds of
// user interaction, and a profile switch in that window would otherwise
// file the email under the wrong profile.
if p.ProfileID != "" {
err = pm.SetProfileState(profilemanager.ID(p.ProfileID), state)
} else {
err = pm.SetActiveProfileState(state)
}
if err != nil {
// Non-fatal: the login itself succeeded.
log.Warnf("failed to store account email: %v", err)
}
}
return resp.GetEmail(), nil
}
+24 -2
View File
@@ -6,6 +6,8 @@ import (
"context"
"os/user"
log "github.com/sirupsen/logrus"
"github.com/netbirdio/netbird/client/internal/profilemanager"
"github.com/netbirdio/netbird/client/proto"
)
@@ -151,11 +153,31 @@ func (s *Profiles) Remove(ctx context.Context, p ProfileRef) error {
if err != nil {
return err
}
_, err = cli.RemoveProfile(ctx, &proto.RemoveProfileRequest{
resp, err := cli.RemoveProfile(ctx, &proto.RemoveProfileRequest{
ProfileName: p.ProfileName,
Username: p.Username,
})
return err
if err != nil {
return err
}
// The daemon deletes what it owns but runs as root, so it leaves the
// user-owned state file holding the account email behind (same split as
// Connection.Logout). Legacy profiles are keyed by name rather than by a
// generated ID, so a recreated profile of the same name would inherit the
// deleted one's email and offer it as the login_hint.
//
// Keyed on the ID the daemon resolved, not on the request handle: that may
// have been a display name or an ID prefix, which would name a different
// file (or none).
if id := resp.GetId(); id != "" {
if err := profilemanager.NewProfileManager().RemoveProfileState(id); err != nil {
// Non-fatal: the profile itself is gone.
log.Warnf("failed to remove profile state for %s: %v", id, err)
}
}
return nil
}
// Rename changes a profile's display name. The on-disk ID is unaffected, so
+1 -1
View File
@@ -44,7 +44,7 @@ type TrayServices struct {
Profiles *services.Profiles
Networks *services.Networks
DaemonFeed *services.DaemonFeed
Notifier *notifications.NotificationService
Notifier *Notifier
Update *services.Update
ProfileSwitcher *services.ProfileSwitcher
WindowManager *services.WindowManager
+1 -1
View File
@@ -44,7 +44,7 @@ func safeSendNotification(send sendFn, what string, opts notifications.Notificat
// notifyIfDaemonOutdated probes the daemon once and fires an OS toast when it
// is reachable but too old for this UI. A probe error means the daemon isn't
// reachable (not outdated), so it is left to the normal connection flow.
func notifyIfDaemonOutdated(compat *services.Compat, notifier *notifications.NotificationService, loc *Localizer) {
func notifyIfDaemonOutdated(compat *services.Compat, notifier *Notifier, loc *Localizer) {
ready, err := compat.DaemonReady(context.Background())
if err != nil {
log.Debugf("daemon compatibility probe: %v", err)
+2 -2
View File
@@ -21,7 +21,7 @@ type trayUpdater struct {
app *application.App
window *application.WebviewWindow
update *services.Update
notifier *notifications.NotificationService
notifier *Notifier
loc *Localizer
onIconChange func()
// onMenuChange drives a full tray relayout: the update row lives in the
@@ -36,7 +36,7 @@ type trayUpdater struct {
progressWindowOpen bool
}
func newTrayUpdater(app *application.App, window *application.WebviewWindow, update *services.Update, notifier *notifications.NotificationService, loc *Localizer, onIconChange func(), onMenuChange func()) *trayUpdater {
func newTrayUpdater(app *application.App, window *application.WebviewWindow, update *services.Update, notifier *Notifier, loc *Localizer, onIconChange func(), onMenuChange func()) *trayUpdater {
u := &trayUpdater{
app: app,
window: window,
+4 -6
View File
@@ -8,21 +8,19 @@ import (
log "github.com/sirupsen/logrus"
"github.com/netbirdio/netbird/client/configs"
"github.com/netbirdio/netbird/client/ui/guilog"
)
// uiLogFileName must stay in sync with the daemon's "gui-client*.log.*" glob
// for rotated siblings (addUILog in client/internal/debug).
const uiLogFileName = "gui-client.log"
// uiLogPath returns the GUI log path with native separators, since the daemon
// opens it directly for debug-bundle collection.
// opens it directly for debug-bundle collection. The file name comes from
// configs.UILogFile so the daemon validates and collects the same name.
func uiLogPath() (string, error) {
dir, err := os.UserConfigDir()
if err != nil {
return "", err
}
return filepath.Join(dir, "netbird", uiLogFileName), nil
return filepath.Join(dir, "netbird", configs.UILogFile), nil
}
// newDebugLog builds the GUI debug log, disabled when userSetLogFile is set