From 66a28fd666834d11eac54a64cbb569e78f52eb6a Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Mon, 12 Oct 2026 13:54:15 +0200 Subject: [PATCH] [client] Add nblink, an unprivileged forwarder into the network (#7955) Reaching an internal service meant installing the agent, which needs root for a TUN device. nblink runs the embedded client in netstack mode and forwards local HTTP listeners to upstreams inside the tunnel, so it works on managed machines and in rootless containers. Upstream names resolve in-process. Forwards bind loopback unless --allow-public-bind is given. The listener refuses rebound and cross-site browser requests and bounds stalled request bodies. It ships as release archives and a distroless image that runs under an arbitrary UID. An embedded client given no StatePath now keeps its state in memory rather than falling back to the installed agent's state file, which every embedded client in a proxy process also shared. The state manager gets an explicit in-memory store for it. --- .github/workflows/agent-network-e2e.yml | 2 +- .goreleaser.yaml | 54 ++ client/Dockerfile-nblink | 34 + client/embed/embed.go | 16 +- client/grpc/dialer_generic.go | 24 +- client/internal/connect.go | 23 +- client/internal/statemanager/manager.go | 63 +- client/internal/statemanager/manager_test.go | 65 ++ client/internal/statemanager/store.go | 91 +++ client/link/README.md | 225 +++++++ client/link/auth.go | 101 +++ client/link/auth_test.go | 70 ++ client/link/cmd/nblink/main.go | 69 ++ client/link/cmd/nblink/main_test.go | 80 +++ client/link/config.go | 245 +++++++ client/link/config_test.go | 219 ++++++ client/link/forward.go | 217 ++++++ client/link/forward_http.go | 422 ++++++++++++ client/link/forward_http_test.go | 563 ++++++++++++++++ client/link/forward_http_upstream_test.go | 286 ++++++++ client/link/forward_test.go | 170 +++++ client/link/link.go | 253 +++++++ client/link/link_test.go | 440 ++++++++++++ e2e/agentnetwork/nblink_test.go | 670 +++++++++++++++++++ e2e/harness/Dockerfile.nblink | 37 + e2e/harness/nblink.go | 508 ++++++++++++++ e2e/harness/proxy.go | 7 + shared/management/client/grpc.go | 8 +- 28 files changed, 4891 insertions(+), 71 deletions(-) create mode 100644 client/Dockerfile-nblink create mode 100644 client/internal/statemanager/manager_test.go create mode 100644 client/internal/statemanager/store.go create mode 100644 client/link/README.md create mode 100644 client/link/auth.go create mode 100644 client/link/auth_test.go create mode 100644 client/link/cmd/nblink/main.go create mode 100644 client/link/cmd/nblink/main_test.go create mode 100644 client/link/config.go create mode 100644 client/link/config_test.go create mode 100644 client/link/forward.go create mode 100644 client/link/forward_http.go create mode 100644 client/link/forward_http_test.go create mode 100644 client/link/forward_http_upstream_test.go create mode 100644 client/link/forward_test.go create mode 100644 client/link/link.go create mode 100644 client/link/link_test.go create mode 100644 e2e/agentnetwork/nblink_test.go create mode 100644 e2e/harness/Dockerfile.nblink create mode 100644 e2e/harness/nblink.go diff --git a/.github/workflows/agent-network-e2e.yml b/.github/workflows/agent-network-e2e.yml index 9501c5fba..41bfe153d 100644 --- a/.github/workflows/agent-network-e2e.yml +++ b/.github/workflows/agent-network-e2e.yml @@ -52,7 +52,7 @@ jobs: uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 with: path: /tmp/.buildx-cache - key: ${{ runner.os }}-anet-e2e-buildx-${{ hashFiles('go.sum', 'combined/Dockerfile.multistage', 'proxy/Dockerfile.multistage', 'e2e/harness/Dockerfile.client') }} + key: ${{ runner.os }}-anet-e2e-buildx-${{ hashFiles('go.sum', 'combined/Dockerfile.multistage', 'proxy/Dockerfile.multistage', 'e2e/harness/Dockerfile.client', 'e2e/harness/Dockerfile.nblink') }} restore-keys: | ${{ runner.os }}-anet-e2e-buildx- diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 6bb4e9e8a..ebb4a8525 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -216,6 +216,29 @@ builds: - -s -w -X main.Version={{.Version}} -X main.Commit={{.Commit}} -X main.BuildDate={{.CommitDate}} mod_timestamp: "{{ .CommitTimestamp }}" + - id: nblink + dir: client/link/cmd/nblink + env: [CGO_ENABLED=0] + binary: nblink + goos: + - linux + - darwin + - windows + goarch: + - amd64 + - arm64 + - arm + goarm: + - 7 + ignore: + - goos: windows + goarch: arm + - goos: darwin + goarch: arm + ldflags: + - -s -w -X github.com/netbirdio/netbird/version.version={{.Version}} + mod_timestamp: "{{ .CommitTimestamp }}" + - id: netbird-idp-migrate dir: tools/idp-migrate env: @@ -280,6 +303,15 @@ archives: builds: - netbird-mysql-migrate name_template: "netbird-mysql-migrate_{{ .Version }}_{{ .Os }}_{{ .Arch }}" + - id: nblink + builds: + - nblink + name_template: "nblink_{{ .Version }}_{{ .Os }}_{{ .Arch }}" + # goreleaser archives as tar.gz everywhere unless told otherwise, and + # nblink is downloaded and extracted by hand on Windows too. + format_overrides: + - goos: windows + format: zip nfpms: - maintainer: Netbird @@ -619,6 +651,28 @@ dockers_v2: "org.opencontainers.image.revision": "{{.FullCommit}}" "org.opencontainers.image.source": "{{.GitURL}}" "maintainer": "dev@netbird.io" + - id: nblink + disable: "{{ .Env.SKIP_DOCKER_PUSH }}" + ids: + - nblink + images: + - netbirdio/nblink + - ghcr.io/netbirdio/nblink + tags: + - "{{ .Version }}" + - "{{ if eq .Env.SKIP_PUBLISH \"false\" }}latest{{ end }}" + dockerfile: client/Dockerfile-nblink + platforms: + - linux/amd64 + - linux/arm64 + - linux/arm + annotations: + "org.opencontainers.image.created": "{{.Date}}" + "org.opencontainers.image.title": "nblink" + "org.opencontainers.image.version": "{{.Version}}" + "org.opencontainers.image.revision": "{{.FullCommit}}" + "org.opencontainers.image.source": "{{.GitURL}}" + "maintainer": "dev@netbird.io" - id: proxy-ubi disable: "{{ .Env.SKIP_DOCKER_PUSH }}" ids: diff --git a/client/Dockerfile-nblink b/client/Dockerfile-nblink new file mode 100644 index 000000000..63177a6c0 --- /dev/null +++ b/client/Dockerfile-nblink @@ -0,0 +1,34 @@ +# nblink runs unprivileged: no TUN device, no capabilities, no host networking. +# +# The image is built for platforms that assign an arbitrary UID at run time, so +# it must not depend on a passwd entry and its state directory has to be +# writable by the root group rather than by one named user. +FROM alpine:3.22 AS build + +# Group 0 owns the state directory so a container started as any UID in that +# group can still persist its identity. Mode 770 rather than g=u, because the +# directory holds the peer's keys and nothing outside owner and group should +# read them, including when the path is bind-mounted from a host. +RUN mkdir -p /tmp/state/nblink \ + && chgrp -R 0 /tmp/state/nblink \ + && chmod -R 770 /tmp/state/nblink + +FROM gcr.io/distroless/static-debian13:nonroot + +ARG TARGETPLATFORM +COPY ${TARGETPLATFORM}/nblink /usr/local/bin/nblink +# COPY of a directory copies its contents and creates the target with mode +# 755, which group 0 cannot write. Copying the parent makes nblink/ itself the +# copied entry, so its mode survives. +COPY --from=build --chown=65532:0 /tmp/state/ /var/lib/ + +# Numeric so the runtime can substitute its own UID without a passwd lookup. +USER 65532:0 + +# No NB_STATE_DIR by default: state stays in memory, so the image runs on a +# read-only root filesystem without a volume. Set NB_STATE_DIR=/var/lib/nblink +# and mount a volume there to keep the peer identity; a volume mounted over +# the directory starts out with its group-0 ownership. +ENV NB_LOG_LEVEL=info + +ENTRYPOINT ["/usr/local/bin/nblink"] diff --git a/client/embed/embed.go b/client/embed/embed.go index 5a3d540ec..6fdddab48 100644 --- a/client/embed/embed.go +++ b/client/embed/embed.go @@ -54,6 +54,9 @@ type Client struct { jwtToken string connect *internal.ConnectClient recorder *peer.Status + // inMemoryState is set when no StatePath was given in netstack mode, + // where the client changes nothing on the host that needs restoring. + inMemoryState bool } // Options configures a new Client. @@ -78,7 +81,8 @@ type Options struct { NoUserspace bool // ConfigPath is the path to the netbird config file. If empty, the config will be stored in memory and not persisted. ConfigPath string - // StatePath is the path to the netbird state file + // StatePath is the path to the netbird state file. When empty in netstack + // mode, state is kept in memory; with NoUserspace the default path is used. StatePath string // DisableClientRoutes disables the client routes DisableClientRoutes bool @@ -193,7 +197,6 @@ func New(opts Options) (*Client, error) { } if opts.StatePath != "" { - // TODO: Disable state if path not provided if err := os.Setenv("NB_DNS_STATE_FILE", opts.StatePath); err != nil { return nil, fmt.Errorf("setenv: %w", err) } @@ -261,6 +264,9 @@ func New(opts Options) (*Client, error) { jwtToken: opts.JWTToken, config: config, recorder: peer.NewRecorder(config.ManagementURL.String()), + // Without a path the default would be the installed agent's state + // file, which this client must neither read nor overwrite. + inMemoryState: opts.StatePath == "" && !opts.NoUserspace, }, nil } @@ -292,7 +298,11 @@ func (c *Client) Start(startCtx context.Context) error { if err, _ := authClient.Login(ctx, c.setupKey, c.jwtToken); err != nil { return fmt.Errorf("login: %w", err) } - client := internal.NewConnectClient(ctx, c.config, c.recorder) + var connectOpts []internal.ConnectClientOption + if c.inMemoryState { + connectOpts = append(connectOpts, internal.WithInMemoryState()) + } + client := internal.NewConnectClient(ctx, c.config, c.recorder, connectOpts...) client.SetSyncResponsePersistence(true) // either startup error (permanent backoff err) or nil err (successful engine up) diff --git a/client/grpc/dialer_generic.go b/client/grpc/dialer_generic.go index 737787223..72b3538ba 100644 --- a/client/grpc/dialer_generic.go +++ b/client/grpc/dialer_generic.go @@ -6,12 +6,9 @@ import ( "context" "fmt" "net" - "os/user" + "os" "runtime" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - log "github.com/sirupsen/logrus" "google.golang.org/grpc" @@ -45,18 +42,13 @@ func WithSweeper(sweeper Sweeper) grpc.DialOption { } func dialContext(ctx context.Context, addr string) (net.Conn, error) { - if runtime.GOOS == "linux" { - currentUser, err := user.Current() - if err != nil { - return nil, status.Errorf(codes.FailedPrecondition, "failed to get current user: %v", err) - } - - // the custom dialer requires root permissions which are not required for use cases run as non-root - if currentUser.Uid != "0" { - log.Debug("Not running as root, using standard dialer") - dialer := &net.Dialer{} - return dialer.DialContext(ctx, "tcp", addr) - } + // The custom dialer requires root permissions which are not required for + // use cases run as non-root. The effective UID is read directly: a passwd + // lookup fails without cgo for a UID that has no entry, as on OpenShift. + if runtime.GOOS == "linux" && os.Geteuid() != 0 { + log.Debug("Not running as root, using standard dialer") + dialer := &net.Dialer{} + return dialer.DialContext(ctx, "tcp", addr) } conn, err := nbnet.NewDialer().DialContext(ctx, "tcp", addr) diff --git a/client/internal/connect.go b/client/internal/connect.go index de7f46b2e..7173f6875 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -72,6 +72,9 @@ type ConnectClient struct { updateManager *updater.Manager persistSyncResponse bool + // inMemoryState keeps the state manager off disk, for clients that change + // nothing on the host that a later run would have to restore. + inMemoryState bool // netMgr gates every reconnection loop on OS-reported network // availability and sweeps connections on network change. @@ -84,6 +87,12 @@ type ConnectClient struct { // ConnectClientOption configures optional ConnectClient behavior. type ConnectClientOption func(*ConnectClient) +// WithInMemoryState keeps client state in memory instead of the default state +// file, so the client neither reads nor writes another installation's state. +func WithInMemoryState() ConnectClientOption { + return func(c *ConnectClient) { c.inMemoryState = true } +} + // WithNetEvents injects the OS network event handling. func WithNetEvents(events *netevents.Manager) ConnectClientOption { return func(c *ConnectClient) { c.netMgr = events } @@ -272,8 +281,15 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan return err } + // path stays alongside the manager because the engine derives StateDir + // from it; an in-memory client leaves it empty. var path string - if runtime.GOOS == "ios" || runtime.GOOS == "android" { + var stateManager *statemanager.Manager + switch { + case c.inMemoryState: + log.Debugf("client state is kept in memory") + stateManager = statemanager.NewInMemory() + case runtime.GOOS == "ios" || runtime.GOOS == "android": // On mobile, use the provided state file path directly if !fileExists(mobileDependency.StateFilePath) { if err := createFile(mobileDependency.StateFilePath); err != nil { @@ -282,11 +298,12 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan } } path = mobileDependency.StateFilePath - } else { + stateManager = statemanager.New(path) + default: sm := profilemanager.NewServiceManager("") path = sm.GetStatePath() + stateManager = statemanager.New(path) } - stateManager := statemanager.New(path) stateManager.RegisterState(&sshconfig.ShutdownState{}) if c.updateManager != nil { diff --git a/client/internal/statemanager/manager.go b/client/internal/statemanager/manager.go index ca4194690..413c910f0 100644 --- a/client/internal/statemanager/manager.go +++ b/client/internal/statemanager/manager.go @@ -1,13 +1,9 @@ package statemanager import ( - "bytes" "context" "encoding/json" - "errors" "fmt" - "io/fs" - "os" "reflect" "sync" "time" @@ -17,7 +13,6 @@ import ( "golang.org/x/exp/maps" nberrors "github.com/netbirdio/netbird/client/errors" - "github.com/netbirdio/netbird/util" ) const ( @@ -56,7 +51,7 @@ type Manager struct { cancel context.CancelFunc done chan struct{} - filePath string + store store // holds the states that are registered with the manager and that are to be persisted states map[string]State // holds the state names that have been updated and need to be persisted with the next save @@ -65,10 +60,20 @@ type Manager struct { stateTypes map[string]reflect.Type } -// New creates a new Manager instance +// New creates a Manager that persists its states to filePath. func New(filePath string) *Manager { + return newManager(&fileStore{path: filePath}) +} + +// NewInMemory creates a Manager that persists nothing, for a client that +// changes nothing on the host and so has no state a later run must restore. +func NewInMemory() *Manager { + return newManager(memoryStore{}) +} + +func newManager(s store) *Manager { return &Manager{ - filePath: filePath, + store: s, states: make(map[string]State), dirty: make(map[string]struct{}), stateTypes: make(map[string]reflect.Type), @@ -274,7 +279,7 @@ func (m *Manager) PersistState(ctx context.Context) error { done := make(chan error, 1) start := time.Now() go func() { - done <- util.WriteBytesWithRestrictedPermission(ctx, m.filePath, bs) + done <- m.store.save(ctx, bs) }() select { @@ -293,45 +298,9 @@ func (m *Manager) PersistState(ctx context.Context) error { return nil } -// loadStateFile reads and unmarshals the state file into a map of raw JSON messages +// loadStateFile returns the states an earlier run persisted. func (m *Manager) loadStateFile(deleteCorrupt bool) (map[string]json.RawMessage, error) { - data, err := os.ReadFile(m.filePath) - if err != nil { - if errors.Is(err, fs.ErrNotExist) { - log.Debugf("state file %s does not exist", m.filePath) - return nil, nil // nolint:nilnil - } - return nil, fmt.Errorf("read state file: %w", err) - } - - var rawStates map[string]json.RawMessage - if err := json.Unmarshal(data, &rawStates); err != nil { - if len(bytes.TrimSpace(data)) == 0 { - log.Warnf("state file %s is empty (%d bytes)", m.filePath, len(data)) - } else { - log.Warnf("state file %s has malformed content (%d bytes)", m.filePath, len(data)) - } - m.handleCorruptedState(deleteCorrupt) - return nil, fmt.Errorf("unmarshal states: %w", err) - } - - return rawStates, nil -} - -// handleCorruptedState creates a backup of a corrupted state file by moving it -func (m *Manager) handleCorruptedState(deleteCorrupt bool) { - if !deleteCorrupt { - return - } - log.Warn("State file appears to be corrupted, attempting to back it up") - - backupPath := fmt.Sprintf("%s.corrupted.%d", m.filePath, time.Now().UnixNano()) - if err := os.Rename(m.filePath, backupPath); err != nil { - log.Errorf("Failed to backup corrupted state file: %v", err) - return - } - - log.Infof("Created backup of corrupted state file at: %s", backupPath) + return m.store.load(deleteCorrupt) } // loadSingleRawState unmarshals a raw state into a concrete state object diff --git a/client/internal/statemanager/manager_test.go b/client/internal/statemanager/manager_test.go new file mode 100644 index 000000000..52b9ac585 --- /dev/null +++ b/client/internal/statemanager/manager_test.go @@ -0,0 +1,65 @@ +package statemanager + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type testState struct { + Value string `json:"value"` +} + +func (s *testState) Name() string { return "test_state" } + +func TestInMemoryManagerNeverTouchesDisk(t *testing.T) { + // Run from an empty directory, so a write to a relative or empty path + // would leave a file behind here. + dir := t.TempDir() + t.Chdir(dir) + + m := NewInMemory() + m.RegisterState(&testState{}) + require.NoError(t, m.UpdateState(&testState{Value: "kept"})) + require.NoError(t, m.PersistState(context.Background()), "persisting in memory must succeed") + + entries, err := os.ReadDir(dir) + require.NoError(t, err) + assert.Empty(t, entries, "an in-memory manager must not write a state file") + + assert.Equal(t, &testState{Value: "kept"}, m.GetState(&testState{}), "the state must still be held in memory") + assert.NoError(t, m.LoadState(&testState{}), "loading with nothing on disk must succeed") + + names, err := m.GetSavedStateNames() + require.NoError(t, err) + assert.Empty(t, names, "an in-memory manager has no saved states") +} + +// An empty path is a misconfiguration rather than a request for memory: the +// in-memory manager has its own constructor, so a path that resolved to +// nothing must surface instead of silently discarding every write. +func TestManagerWithoutAPathFailsToPersist(t *testing.T) { + m := New("") + m.RegisterState(&testState{}) + require.NoError(t, m.UpdateState(&testState{Value: "kept"})) + + assert.Error(t, m.PersistState(context.Background()), + "an empty path must report that it cannot persist, not quietly drop the state") +} + +func TestManagerPersistsToItsFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + + m := New(path) + m.RegisterState(&testState{}) + require.NoError(t, m.UpdateState(&testState{Value: "kept"})) + require.NoError(t, m.PersistState(context.Background())) + + names, err := New(path).GetSavedStateNames() + require.NoError(t, err) + assert.Equal(t, []string{"test_state"}, names, "a manager with a path must write its state there") +} diff --git a/client/internal/statemanager/store.go b/client/internal/statemanager/store.go new file mode 100644 index 000000000..fffa38396 --- /dev/null +++ b/client/internal/statemanager/store.go @@ -0,0 +1,91 @@ +package statemanager + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io/fs" + "os" + "time" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/util" +) + +// store is where a Manager keeps its states between runs. +type store interface { + // load returns the states persisted by an earlier run, or an empty map + // when there are none. deleteCorrupt asks the store to move a state it + // cannot parse out of the way. + load(deleteCorrupt bool) (map[string]json.RawMessage, error) + // save replaces the persisted states with data. + save(ctx context.Context, data []byte) error +} + +// fileStore persists states to a file, which is what an installed agent needs: +// it changes routing, DNS and firewall rules on the host, and a later run has +// to know what to put back. +type fileStore struct { + path string +} + +// memoryStore keeps nothing. An embedded client in netstack mode changes +// nothing on the host, so there is no state a later run has to restore, and +// writing one would mean reading and overwriting another installation's file. +type memoryStore struct{} + +func (f *fileStore) load(deleteCorrupt bool) (map[string]json.RawMessage, error) { + data, err := os.ReadFile(f.path) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + log.Debugf("state file %s does not exist", f.path) + return nil, nil //nolint:nilnil // no file is not a failure, there is simply nothing to load + } + return nil, fmt.Errorf("read state file: %w", err) + } + + var rawStates map[string]json.RawMessage + if err := json.Unmarshal(data, &rawStates); err != nil { + if len(bytes.TrimSpace(data)) == 0 { + log.Warnf("state file %s is empty (%d bytes)", f.path, len(data)) + } else { + log.Warnf("state file %s has malformed content (%d bytes)", f.path, len(data)) + } + f.handleCorrupted(deleteCorrupt) + return nil, fmt.Errorf("unmarshal states: %w", err) + } + + return rawStates, nil +} + +func (f *fileStore) save(ctx context.Context, data []byte) error { + return util.WriteBytesWithRestrictedPermission(ctx, f.path, data) +} + +// handleCorrupted moves a state file that cannot be parsed aside, so the next +// run starts from nothing rather than failing on the same content forever. +func (f *fileStore) handleCorrupted(deleteCorrupt bool) { + if !deleteCorrupt { + return + } + log.Warn("State file appears to be corrupted, attempting to back it up") + + backupPath := fmt.Sprintf("%s.corrupted.%d", f.path, time.Now().UnixNano()) + if err := os.Rename(f.path, backupPath); err != nil { + log.Errorf("Failed to backup corrupted state file: %v", err) + return + } + + log.Infof("Created backup of corrupted state file at: %s", backupPath) +} + +func (memoryStore) load(bool) (map[string]json.RawMessage, error) { + return map[string]json.RawMessage{}, nil +} + +func (memoryStore) save(context.Context, []byte) error { + return nil +} diff --git a/client/link/README.md b/client/link/README.md new file mode 100644 index 000000000..ffd08c7ad --- /dev/null +++ b/client/link/README.md @@ -0,0 +1,225 @@ +# nblink + +`nblink` forwards local ports into a NetBird network from an unprivileged +process. It runs the client in userspace mode, so there is no TUN device to +create and no elevated privileges to grant. That lets it run on managed +machines and in rootless containers where the normal agent cannot. + +Overlay names on the upstream side are resolved inside the tunnel, so +`/etc/resolv.conf` is never touched. + +## Install + +Download the archive for your platform, extract it, and run the binary. There +is no installer and no service to register. + +## Use + +A forward is one self-describing argument, and the flag repeats: + +```sh +nblink --forward 'http://8080=https://grafana.internal' +``` + +The scheme on the left selects the listener type. The right side is the +upstream reached over the network. Giving only a port binds loopback, so the +example above listens on `127.0.0.1:8080`. + +Without a setup key, `nblink` opens a browser to log in and prints the URL so +it also works over SSH. + +```sh +# several forwards from one session +nblink --forward 'http://8080=https://grafana.internal' \ + --forward 'http://9090=https://prometheus.internal' + +# non-interactive +nblink --forward 'http://8080=https://grafana.internal' --setup-key "$NB_SETUP_KEY" + +# check the configuration without touching the network +nblink --forward 'http://8080=https://grafana.internal' --check +``` + +Port `0` asks the operating system for a free port. The address actually bound +is printed at startup. + +## Container + +Every flag has a matching `NB_`-prefixed environment variable, which is how the +image is configured. `NB_FORWARD` takes a comma separated list. + +```sh +docker run --rm -p 127.0.0.1:8080:8080 \ + --mount type=bind,src="$(pwd)/nb_setup_key",dst=/run/secrets/nb_setup_key,readonly \ + -e NB_SETUP_KEY=file:/run/secrets/nb_setup_key \ + -e NB_FORWARD='http://0.0.0.0:8080=https://grafana.internal' \ + -e NB_ALLOW_PUBLIC_BIND=true \ + netbirdio/nblink +``` + +The key file has to exist before the run and be readable by the container +user, which is UID 65532 in group 0. A bind mount keeps the host file's owner +and mode, so a root-owned key written with the usual `chmod 600` is not +readable and the run fails on startup with `permission denied`. Either give the +file to that UID (`chown 65532`) or keep it mode `640` in group 0 +(`chgrp 0`). If the source path does not exist, `--mount` refuses to start the +container; the older `-v` form creates a directory there instead, which the read +then reports as a directory. + +The image needs no `--privileged`, no `--cap-add NET_ADMIN`, no +`--device /dev/net/tun` and no host networking. + +Inside a container the listener has to bind `0.0.0.0`, because a published +port arrives on the container's own interface rather than its loopback. That +still requires `NB_ALLOW_PUBLIC_BIND`, so exposing a forward is always +something the operator wrote down. Publish it on the host's loopback as above: +a bare `-p 8080:8080` publishes on every host interface, which hands the +peer's identity to anyone on the network who can reach the port. + +A public listener accepts requests addressed to `localhost` or to an IP +address. Any name other machines use to reach it, such as a DNS name or a +Kubernetes Service name, has to be listed with `--allowed-host` +(`NB_ALLOWED_HOST`). See [Limits](#limits) for why. + +The image keeps its state in memory, so it runs on a read-only root filesystem +and each start registers a new peer. To keep one identity across restarts, set +`NB_STATE_DIR=/var/lib/nblink` and mount a volume there: + +```sh +docker run --rm --read-only -p 127.0.0.1:8080:8080 \ + -v nblink-state:/var/lib/nblink -e NB_STATE_DIR=/var/lib/nblink \ + ... +``` + +Without a state directory, use an ephemeral setup key so the peers left behind +by earlier starts are removed once they go offline, rather than piling up. + +A kept identity is not yet a kept login. Without a setup key every start runs +the browser flow, even when the peer's session is still valid, because the +token is resolved before the stored profile is opened. Give a restart that has +to be unattended a setup key. + +## Kubernetes and OpenShift + +The image fits OpenShift's `restricted-v2` SCC and the Kubernetes `restricted` +Pod Security Standard as it is: + +- It runs as any UID. OpenShift assigns one from the namespace range and adds + group 0, and everything the process writes is owned by group 0. +- It needs no capabilities, no privilege escalation, no TUN device and no host + networking, so `capabilities: {drop: [ALL]}` and + `allowPrivilegeEscalation: false` hold. +- With state in memory it needs no writable path, so + `readOnlyRootFilesystem: true` holds. + +The simplest shape is a sidecar. The application in the same pod reaches the +forward on `localhost`, so the listener stays on loopback and needs neither +`NB_ALLOW_PUBLIC_BIND` nor a Service: + +```yaml +containers: + - name: nblink + image: netbirdio/nblink + env: + - name: NB_FORWARD + value: http://8080=https://grafana.internal + - name: NB_SETUP_KEY + value: file:/run/secrets/nblink/setup-key + volumeMounts: + - name: nblink-key + mountPath: /run/secrets/nblink + readOnly: true + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + runAsNonRoot: true + capabilities: {drop: [ALL]} + seccompProfile: {type: RuntimeDefault} +volumes: + - name: nblink-key + secret: + secretName: nblink-setup-key + defaultMode: 0440 +``` + +A Secret volume is owned by the pod's `fsGroup`, which OpenShift sets for +every pod, so mode `0440` is readable by the container and by nothing else. + +To keep the identity across pod restarts, mount a persistent volume at +`/var/lib/nblink` and set `NB_STATE_DIR` to it. `fsGroup` makes the volume +writable by the arbitrary UID. A `Deployment` with more than one replica +cannot share one state directory: every replica is its own peer, so give +each one its own volume through a `StatefulSet`, or keep state in memory with +an ephemeral setup key. + +Serving a forward to other pods through a Service needs `NB_ALLOW_PUBLIC_BIND` +and every name callers use in `NB_ALLOWED_HOST`, for example +`grafana-link,grafana-link.monitoring.svc,grafana-link.monitoring.svc.cluster.local`. +Each caller acts under the forwarder peer's identity, so restrict who can +reach the Service with a `NetworkPolicy`. Do not expose it through a Route or +an Ingress. + +## Configuration + +| Flag | Environment | Default | Purpose | +| --- | --- | --- | --- | +| `--forward` | `NB_FORWARD` | | Forward spec, repeatable. Comma separated in the environment. | +| `--setup-key` | `NB_SETUP_KEY` | | Non-interactive login. Accepts a `file:` prefix. | +| `--management-url` | `NB_MANAGEMENT_URL` | NetBird cloud | Management server. | +| `--hostname` | `NB_HOSTNAME` | host name | Peer name in the network. | +| `--state-dir` | `NB_STATE_DIR` | memory | Where identity and state persist. | +| `--log-level` | `NB_LOG_LEVEL` | `info` | Log level. | +| `--allow-public-bind` | `NB_ALLOW_PUBLIC_BIND` | `false` | Required before binding a non-loopback address. | +| `--allowed-host` | `NB_ALLOWED_HOST` | | Extra name a listener accepts in `Host`, repeatable. Comma separated in the environment. | +| `--no-browser` | `NB_NO_BROWSER` | `false` | Print the login URL instead of opening a browser. | +| `--check` | `NB_CHECK` | `false` | Validate, print the effective forwards, exit. | + +A secret given as `file:/path` is read from that file, so a container can mount +it instead of exposing it in the process environment. + +## Limits + +`nblink` only dials out. It refuses inbound connections and does not reach the +host's own network, so the peer cannot become a route into the machine it runs +on. + +A loopback forward answers only requests whose `Host` names the loopback +interface or a name listed with `--allowed-host`, and refuses any request a +browser marks as belonging to another page, through either a cross-origin +`Origin` or a cross-site `Sec-Fetch-Site`. A page the user visits can point its +own hostname at `127.0.0.1` or embed the address directly, and without those +checks the browser could reach the upstream through this listener under the +peer's identity. Ordinary callers send neither header and are unaffected. + +A public forward applies the same checks, and also accepts any IP address in +`Host`, since a page cannot rebind an address. The `Host` check matters there +too: a container port published on the host's loopback is reached by a +rebound page with a matching `Origin`, and its unlisted name is the only thing +that gives it away. + +A name listed with `--allowed-host` is trusted rather than merely permitted, +and the list applies to every forward in the process, loopback ones included. +`Host` is the one signal a rebound page cannot forge, and listing a name gives +that up for it: a page served on that name can resolve it to the listener and +send a matching `Host` and `Origin`, which the browser then marks same-origin. +List only names you control, and prefer a fully qualified one, since a +single-label name can be answered by anything on the local network. + +A forward on `0.0.0.0` listens on IPv4 only, and one on `::` on IPv6 only. + +`nblink` does not ask the local router for a port mapping through UPnP, +NAT-PMP or PCP, since an unprivileged forwarder should not reconfigure the +network it runs on. Connections are still established through hole punching +or the relay. Set `NB_DISABLE_NAT_MAPPER=false` to allow it. + +A browser too old to send `Sec-Fetch-Site`, which browsers have sent since +2020, can still reach a loopback forward with an embedded no-cors GET. + +Userspace mode carries TCP, UDP and ping only. It cannot act as an exit node or +a routing peer and does not take over system DNS. Ports below 1024 need +privileges the process usually does not have, though some container runtimes +lower that boundary. + +This build forwards HTTP. The `tcp`, `udp` and `socks5` schemes are reserved by +the grammar and rejected with a message saying so, so adding them later needs +no change to how a forward is written. diff --git a/client/link/auth.go b/client/link/auth.go new file mode 100644 index 000000000..91aa0a426 --- /dev/null +++ b/client/link/auth.go @@ -0,0 +1,101 @@ +package link + +import ( + "context" + "fmt" + "io" + "os" + "strings" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/auth" + "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/util" +) + +// credentials carries exactly one proof of identity to the embedded client. +type credentials struct { + setupKey string + jwtToken string +} + +// resolveCredentials returns a setup key when one was supplied, and otherwise +// drives the interactive login flow. +// +// The embedded client accepts a setup key or an already-issued token, so the +// browser flow is run here and only its result is handed over. +func resolveCredentials(ctx context.Context, cfg *Config) (credentials, error) { + if cfg.SetupKey != "" { + log.Info("authenticating with the configured setup key") + return credentials{setupKey: cfg.SetupKey}, nil + } + + log.Info("no setup key configured, starting interactive login") + token, err := interactiveLogin(ctx, cfg) + if err != nil { + return credentials{}, err + } + return credentials{jwtToken: token}, nil +} + +// interactiveLogin runs the standard browser login and returns the token to +// authenticate the peer with. +func interactiveLogin(ctx context.Context, cfg *Config) (string, error) { + // The flow only needs somewhere to read the management URL from and a key + // to talk to it with. The peer identity the client ends up using is + // created separately, so this config is deliberately not persisted. + flowConfig, err := profilemanager.CreateInMemoryConfig(profilemanager.ConfigInput{ + ManagementURL: cfg.ManagementURL, + }) + if err != nil { + return "", fmt.Errorf("prepare login: %w", err) + } + + flow, err := auth.NewOAuthFlow(ctx, flowConfig, util.HasGraphicalSession(), false, "", false) + if err != nil { + return "", fmt.Errorf("start login: %w", err) + } + + info, err := flow.RequestAuthInfo(ctx) + if err != nil { + return "", fmt.Errorf("request login: %w", err) + } + + promptLogin(os.Stderr, util.OpenBrowser, info, cfg.NoBrowser) + + token, err := flow.WaitToken(ctx, info) + if err != nil { + return "", fmt.Errorf("wait for login: %w", err) + } + + return token.GetTokenToUse(), nil +} + +// promptLogin writes the verification URL to out, which the caller points at +// stderr so stdout stays clean for piping, and opens a browser unless asked not +// to. The URL is always printed so the flow still works over SSH where no +// browser can open. +func promptLogin(out io.Writer, openBrowser func(string) error, info auth.AuthFlowInfo, noBrowser bool) { + uri := info.VerificationURIComplete + if uri == "" { + uri = info.VerificationURI + } + + var b strings.Builder + fmt.Fprintf(&b, "\nOpen this URL to log in:\n\n %s\n", uri) + if info.UserCode != "" && !strings.Contains(uri, info.UserCode) { + fmt.Fprintf(&b, "\n and enter the code %s\n", info.UserCode) + } + b.WriteString("\n") + if _, err := io.WriteString(out, b.String()); err != nil { + log.Warnf("print login URL: %v", err) + } + + if noBrowser { + return + } + if err := openBrowser(uri); err != nil { + log.Debugf("could not open a browser: %v", err) + } +} diff --git a/client/link/auth_test.go b/client/link/auth_test.go new file mode 100644 index 000000000..69920a96c --- /dev/null +++ b/client/link/auth_test.go @@ -0,0 +1,70 @@ +package link + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/internal/auth" +) + +// browserSpy records the URLs a login prompt tried to open. +type browserSpy struct { + opened []string + err error +} + +func (b *browserSpy) open(uri string) error { + b.opened = append(b.opened, uri) + return b.err +} + +func TestPromptLoginPrintsURLAndOpensBrowser(t *testing.T) { + var out strings.Builder + browser := &browserSpy{} + info := auth.AuthFlowInfo{VerificationURIComplete: "https://login.example/device?code=ABCD-EFGH", UserCode: "ABCD-EFGH"} + + promptLogin(&out, browser.open, info, false) + + assert.Contains(t, out.String(), info.VerificationURIComplete, "the URL must be printed") + assert.NotContains(t, out.String(), "enter the code", "a code already in the URL must not be repeated") + assert.Equal(t, []string{info.VerificationURIComplete}, browser.opened, "the browser must be opened on the same URL") +} + +func TestPromptLoginNoBrowserStillPrintsURL(t *testing.T) { + var out strings.Builder + browser := &browserSpy{} + info := auth.AuthFlowInfo{VerificationURI: "https://login.example/device", UserCode: "WXYZ-1234"} + + promptLogin(&out, browser.open, info, true) + + assert.Contains(t, out.String(), "https://login.example/device", "the URL must be printed even without a browser") + assert.Contains(t, out.String(), "enter the code WXYZ-1234", "a code missing from the URL must be printed") + assert.Empty(t, browser.opened, "--no-browser must not open a browser") +} + +func TestPromptLoginSurvivesBrowserFailure(t *testing.T) { + var out strings.Builder + browser := &browserSpy{err: errors.New("no display")} + info := auth.AuthFlowInfo{VerificationURIComplete: "https://login.example/authorize"} + + promptLogin(&out, browser.open, info, false) + + assert.Contains(t, out.String(), "https://login.example/authorize", + "the URL must be printed so the login still works where no browser opens") +} + +func TestResolveCredentialsUsesSetupKeyWithoutLogin(t *testing.T) { + // An unroutable management URL: a login attempt would fail, so success + // proves the setup key path never starts one. + cfg := &Config{SetupKey: "11111111-2222-3333-4444-555555555555", ManagementURL: "https://management.invalid"} + + creds, err := resolveCredentials(context.Background(), cfg) + require.NoError(t, err) + assert.Equal(t, cfg.SetupKey, creds.setupKey, "the configured key must be used") + assert.Empty(t, creds.jwtToken, "exactly one proof of identity must be handed over") +} diff --git a/client/link/cmd/nblink/main.go b/client/link/cmd/nblink/main.go new file mode 100644 index 000000000..6ee580c7b --- /dev/null +++ b/client/link/cmd/nblink/main.go @@ -0,0 +1,69 @@ +// Command nblink forwards local listeners into a NetBird network from an +// unprivileged process. +package main + +import ( + "context" + "fmt" + "os" + "os/signal" + "syscall" + + "github.com/spf13/cobra" + + "github.com/netbirdio/netbird/client/link" + "github.com/netbirdio/netbird/util" + "github.com/netbirdio/netbird/version" +) + +func main() { + if err := rootCmd().Execute(); err != nil { + fmt.Fprintf(os.Stderr, "nblink: %v\n", err) + os.Exit(1) + } +} + +func rootCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "nblink", + Short: "Forward local ports into a NetBird network", + Long: "nblink runs a NetBird peer in userspace and forwards local listeners\n" + + "into the network. It needs no TUN device and no elevated privileges,\n" + + "so it runs on managed machines and in rootless containers.\n\n" + + "Every flag can also be set through an NB_-prefixed environment variable\n" + + "of the same name, which is how the container image is configured.", + Example: " nblink --forward 'http://8080=https://grafana.internal'\n" + + " nblink --forward 'http://127.0.0.1:8080=https://grafana.internal' --setup-key file:/run/secrets/key", + Version: version.NetbirdVersion(), + // A stray positional argument is a mistake, most often a forward spec + // that lost its --forward, and must not start the forwarder anyway. + Args: cobra.NoArgs, + // main prints the error, so cobra must not print it a second time. + SilenceUsage: true, + SilenceErrors: true, + } + + raw := link.BindFlags(cmd) + + cmd.RunE = func(cmd *cobra.Command, _ []string) error { + if err := link.SetFlagsFromEnvVars(cmd); err != nil { + return err + } + + cfg, err := raw.Resolve() + if err != nil { + return err + } + + if err := util.InitLog(cfg.LogLevel, util.LogConsole); err != nil { + return fmt.Errorf("init log: %w", err) + } + + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) + defer stop() + + return link.Run(ctx, cfg) + } + + return cmd +} diff --git a/client/link/cmd/nblink/main_test.go b/client/link/cmd/nblink/main_test.go new file mode 100644 index 000000000..46bd158e6 --- /dev/null +++ b/client/link/cmd/nblink/main_test.go @@ -0,0 +1,80 @@ +package main + +import ( + "fmt" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/version" +) + +// TestMain clears the ambient NB_ environment before any test runs. +// +// Every flag reads an NB_-prefixed variable, so a developer who has nblink +// configured in their own shell would otherwise hand these commands a +// configuration the test never asked for: an inherited NB_FORWARD alone makes +// the missing-forward case resolve successfully and the test fail. Clearing +// here rather than in execute keeps t.Setenv working for the tests that do +// want a variable set. +func TestMain(m *testing.M) { + for _, entry := range os.Environ() { + name, _, _ := strings.Cut(entry, "=") + if strings.HasPrefix(name, "NB_") { + if err := os.Unsetenv(name); err != nil { + fmt.Fprintf(os.Stderr, "unset %s: %v\n", name, err) + os.Exit(1) + } + } + } + os.Exit(m.Run()) +} + +func execute(t *testing.T, args ...string) (string, error) { + t.Helper() + cmd := rootCmd() + var out strings.Builder + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SetArgs(args) + err := cmd.Execute() + return out.String(), err +} + +// A forward spec that lost its --forward is the usual stray argument, and it +// must not start a forwarder with whatever else was configured. +func TestRejectsPositionalArguments(t *testing.T) { + _, err := execute(t, "--check", "--forward", "http://8080=https://grafana.internal", "http://9090=https://prometheus.internal") + require.Error(t, err, "a positional argument must be refused") + assert.Contains(t, err.Error(), "unknown command", "the error must point at the stray argument") +} + +func TestRejectsInvalidEnvironmentValue(t *testing.T) { + t.Setenv("NB_ALLOW_PUBLIC_BIND", "maybe") + _, err := execute(t, "--check", "--forward", "http://8080=https://grafana.internal") + require.Error(t, err, "an unparseable environment value must stop the start") + assert.Contains(t, err.Error(), "NB_ALLOW_PUBLIC_BIND", "the error must name the variable") +} + +func TestEnvironmentConfiguresAFullCheck(t *testing.T) { + t.Setenv("NB_FORWARD", "http://8080=https://grafana.internal,http://0.0.0.0:9090=https://prometheus.internal") + t.Setenv("NB_ALLOW_PUBLIC_BIND", "true") + t.Setenv("NB_CHECK", "true") + _, err := execute(t) + assert.NoError(t, err, "a complete environment configuration must pass the check without flags") +} + +func TestRequiresAForward(t *testing.T) { + _, err := execute(t, "--check") + require.Error(t, err, "a run with no forward must be refused") + assert.Contains(t, err.Error(), "NB_FORWARD", "the error must name the environment variable too") +} + +func TestPrintsVersion(t *testing.T) { + out, err := execute(t, "--version") + require.NoError(t, err) + assert.Contains(t, out, version.NetbirdVersion(), "--version must print the build version") +} diff --git a/client/link/config.go b/client/link/config.go new file mode 100644 index 000000000..1f86965f7 --- /dev/null +++ b/client/link/config.go @@ -0,0 +1,245 @@ +package link + +import ( + "fmt" + "net/netip" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + + "github.com/netbirdio/netbird/client/internal/profilemanager" +) + +// envPrefix matches the prefix the agent uses, so an operator who already sets +// NB_SETUP_KEY or NB_MANAGEMENT_URL for the agent can reuse them here. +const envPrefix = "NB_" + +// fileScheme marks a flag value that holds a path to read the real value from, +// so a container can pass a secret as a mounted file instead of an environment +// variable visible to anything that can read the process environment. +const fileScheme = "file:" + +// hostnamePattern matches a DNS name per RFC 1123 section 2.1: dot-separated +// labels of letters, digits and inner hyphens. +var hostnamePattern = regexp.MustCompile(`^([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$`) + +// Config is the resolved runtime configuration. +type Config struct { + Forwards []Forward + AllowedHosts []string + SetupKey string + ManagementURL string + Hostname string + StateDir string + LogLevel string + AllowPublicBind bool + NoBrowser bool + Check bool +} + +// rawConfig holds flag values before parsing and validation. +type rawConfig struct { + forwards []string + allowedHosts []string + setupKey string + managementURL string + hostname string + stateDir string + logLevel string + allowPublicBind bool + noBrowser bool + check bool +} + +// Resolve validates the raw flag values and produces a runtime configuration. +// It reports every problem it can rather than stopping at the first, so an +// operator fixing a container environment sees the whole list in one run. +func (r *rawConfig) Resolve() (*Config, error) { + cfg := &Config{ + ManagementURL: r.managementURL, + Hostname: r.hostname, + StateDir: r.stateDir, + LogLevel: r.logLevel, + AllowPublicBind: r.allowPublicBind, + NoBrowser: r.noBrowser, + Check: r.check, + } + + setupKey, err := resolveSecret(r.setupKey) + if err != nil { + return nil, fmt.Errorf("setup key: %w", err) + } + cfg.SetupKey = setupKey + + if len(r.forwards) == 0 { + return nil, fmt.Errorf("at least one --forward is required (env %s)", FlagNameToEnvVar("forward")) + } + + var problems []string + cfg.AllowedHosts, problems = parseAllowedHosts(r.allowedHosts) + + seen := make(map[string]string, len(r.forwards)) + for _, spec := range r.forwards { + fwd, err := ParseForward(spec) + if err != nil { + problems = append(problems, err.Error()) + continue + } + // Port 0 means the OS assigns one, so several such forwards do not + // collide even though they share a spec. + if prev, dup := seen[fwd.Listen]; dup && !strings.HasSuffix(fwd.Listen, ":0") { + problems = append(problems, fmt.Sprintf("forward %q: %s is already bound by %q", spec, fwd.Listen, prev)) + continue + } + if !cfg.AllowPublicBind && !isLoopback(fwd.Listen) { + problems = append(problems, fmt.Sprintf( + "forward %q: %s is not loopback, pass --allow-public-bind (env %s) to expose it", + spec, fwd.Listen, FlagNameToEnvVar("allow-public-bind"))) + continue + } + seen[fwd.Listen] = spec + fwd.AllowedHosts = cfg.AllowedHosts + cfg.Forwards = append(cfg.Forwards, fwd) + } + + if len(problems) > 0 { + return nil, fmt.Errorf("invalid configuration:\n %s", strings.Join(problems, "\n ")) + } + + return cfg, nil +} + +// ConfigPath returns where the peer identity is persisted, or an empty string +// when the client should keep it in memory only. +func (c *Config) ConfigPath() string { + if c.StateDir == "" { + return "" + } + return filepath.Join(c.StateDir, "config.json") +} + +// StatePath returns where runtime state is persisted, or an empty string when +// no state directory was configured. +func (c *Config) StatePath() string { + if c.StateDir == "" { + return "" + } + return filepath.Join(c.StateDir, "state.json") +} + +// BindFlags registers nblink's flags on cmd and returns the backing values. +// Every flag gains an NB_-prefixed environment variable of the same name, so +// the container configuration cannot drift from the command line. +func BindFlags(cmd *cobra.Command) *rawConfig { + raw := &rawConfig{} + f := cmd.PersistentFlags() + + f.StringSliceVar(&raw.forwards, "forward", nil, + "forward spec scheme://[host:]port=upstream, repeatable (env: comma separated)") + f.StringSliceVar(&raw.allowedHosts, "allowed-host", nil, + "extra name a listener may be reached under, repeatable (env: comma separated)") + f.StringVar(&raw.setupKey, "setup-key", "", + "setup key for non-interactive login, accepts a file: prefix") + f.StringVar(&raw.managementURL, "management-url", profilemanager.DefaultManagementURL, + "management server URL") + f.StringVar(&raw.hostname, "hostname", "", "peer name in the network") + f.StringVar(&raw.stateDir, "state-dir", "", + "directory for config and state, kept in memory when empty") + f.StringVar(&raw.logLevel, "log-level", "info", "log level") + f.BoolVar(&raw.allowPublicBind, "allow-public-bind", false, + "permit forwards that bind an address other than loopback") + f.BoolVar(&raw.noBrowser, "no-browser", false, + "print the login URL instead of opening a browser") + f.BoolVar(&raw.check, "check", false, + "validate configuration, print the effective forwards and exit") + + return raw +} + +// SetFlagsFromEnvVars fills unset flags from NB_-prefixed environment +// variables derived from each flag name, so --management-url reads +// NB_MANAGEMENT_URL. A flag given on the command line wins. +// +// A value that does not parse is an error rather than a warning, so a +// container cannot start with a configuration other than the one it was given. +func SetFlagsFromEnvVars(cmd *cobra.Command) error { + flags := cmd.PersistentFlags() + var err error + flags.VisitAll(func(f *pflag.Flag) { + if err != nil || f.Changed { + return + } + env := FlagNameToEnvVar(f.Name) + value, ok := os.LookupEnv(env) + if !ok { + return + } + if setErr := flags.Set(f.Name, value); setErr != nil { + err = fmt.Errorf("%s: %w", env, setErr) + } + }) + return err +} + +// FlagNameToEnvVar converts a flag name to its environment variable, so +// allow-public-bind becomes NB_ALLOW_PUBLIC_BIND. +func FlagNameToEnvVar(name string) string { + return envPrefix + strings.ToUpper(strings.ReplaceAll(name, "-", "_")) +} + +// parseAllowedHosts normalizes the names given with --allowed-host and reports +// every entry that is not a plain hostname. +func parseAllowedHosts(raw []string) ([]string, []string) { + var hosts, problems []string + for _, entry := range raw { + name := normalizeHostname(entry) + if _, err := netip.ParseAddr(name); err == nil { + problems = append(problems, fmt.Sprintf( + "allowed host %q: addresses are already accepted on a public listener, list names only", entry)) + continue + } + if !hostnamePattern.MatchString(name) { + problems = append(problems, fmt.Sprintf( + "allowed host %q: must be a hostname, without scheme, port or wildcard", entry)) + continue + } + hosts = append(hosts, name) + } + return hosts, problems +} + +// normalizeHostname lowercases a name and drops a trailing root dot, so the +// spellings a client may send compare equal. +func normalizeHostname(name string) string { + return strings.TrimSuffix(strings.ToLower(strings.TrimSpace(name)), ".") +} + +// resolveSecret reads a value that may carry a file: prefix naming the file +// that holds it. Whitespace around the file contents is trimmed so a trailing +// newline from the usual tooling does not become part of the secret. +func resolveSecret(value string) (string, error) { + path, ok := strings.CutPrefix(value, fileScheme) + if !ok { + return value, nil + } + if path == "" { + return "", fmt.Errorf("%s prefix with no path", fileScheme) + } + + content, err := os.ReadFile(path) + if err != nil { + return "", fmt.Errorf("read %s: %w", path, err) + } + + // An empty file would otherwise look like no secret at all, silently + // turning a non-interactive start into a login that waits for a browser. + secret := strings.TrimSpace(string(content)) + if secret == "" { + return "", fmt.Errorf("%s is empty", path) + } + return secret, nil +} diff --git a/client/link/config_test.go b/client/link/config_test.go new file mode 100644 index 000000000..72b238873 --- /dev/null +++ b/client/link/config_test.go @@ -0,0 +1,219 @@ +package link + +import ( + "os" + "path/filepath" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newTestCommand wires the flags the same way the binary does, so the tests +// exercise the real flag and environment plumbing. +func newTestCommand() (*cobra.Command, *rawConfig) { + cmd := &cobra.Command{Use: "nblink"} + return cmd, BindFlags(cmd) +} + +func TestFlagNameToEnvVar(t *testing.T) { + assert.Equal(t, "NB_FORWARD", FlagNameToEnvVar("forward")) + assert.Equal(t, "NB_ALLOW_PUBLIC_BIND", FlagNameToEnvVar("allow-public-bind")) + assert.Equal(t, "NB_MANAGEMENT_URL", FlagNameToEnvVar("management-url")) +} + +func TestSetFlagsFromEnvVars(t *testing.T) { + cmd, raw := newTestCommand() + t.Setenv("NB_FORWARD", "http://8080=https://a.internal,http://8081=https://b.internal") + t.Setenv("NB_MANAGEMENT_URL", "https://mgmt.example:443") + + require.NoError(t, SetFlagsFromEnvVars(cmd)) + + assert.Equal(t, []string{"http://8080=https://a.internal", "http://8081=https://b.internal"}, + raw.forwards, "NB_FORWARD should split on commas into repeated forwards") + assert.Equal(t, "https://mgmt.example:443", raw.managementURL) +} + +// A value given on the command line must win over the environment, otherwise a +// container's baked-in defaults could not be overridden at run time. +func TestFlagBeatsEnvVar(t *testing.T) { + cmd, raw := newTestCommand() + require.NoError(t, cmd.PersistentFlags().Set("log-level", "debug")) + t.Setenv("NB_LOG_LEVEL", "error") + + require.NoError(t, SetFlagsFromEnvVars(cmd)) + + assert.Equal(t, "debug", raw.logLevel, "an explicit flag should not be overwritten by the environment") +} + +// A container must not start with a configuration other than the one it was +// given, so an unparseable value stops startup instead of leaving the default. +func TestSetFlagsFromEnvVarsRejectsInvalidValue(t *testing.T) { + cmd, raw := newTestCommand() + t.Setenv("NB_ALLOW_PUBLIC_BIND", "yes-please") + + err := SetFlagsFromEnvVars(cmd) + + require.Error(t, err) + assert.Contains(t, err.Error(), "NB_ALLOW_PUBLIC_BIND", "the error should name the variable at fault") + assert.False(t, raw.allowPublicBind, "the default must not be silently kept in effect") +} + +func TestResolveRequiresForward(t *testing.T) { + _, raw := newTestCommand() + + _, err := raw.Resolve() + + require.Error(t, err) + assert.Contains(t, err.Error(), "at least one --forward") +} + +func TestResolveGatesPublicBind(t *testing.T) { + _, raw := newTestCommand() + raw.forwards = []string{"http://0.0.0.0:8080=https://grafana.internal"} + + _, err := raw.Resolve() + + require.Error(t, err) + assert.Contains(t, err.Error(), "allow-public-bind", + "binding a public address should require the explicit opt-in") +} + +func TestResolveAllowsPublicBindWhenOptedIn(t *testing.T) { + _, raw := newTestCommand() + raw.forwards = []string{"http://0.0.0.0:8080=https://grafana.internal"} + raw.allowPublicBind = true + + cfg, err := raw.Resolve() + + require.NoError(t, err) + require.Len(t, cfg.Forwards, 1) + assert.Equal(t, "0.0.0.0:8080", cfg.Forwards[0].Listen) +} + +func TestResolveRejectsDuplicateListener(t *testing.T) { + _, raw := newTestCommand() + raw.forwards = []string{ + "http://8080=https://a.internal", + "http://127.0.0.1:8080=https://b.internal", + } + + _, err := raw.Resolve() + + require.Error(t, err) + assert.Contains(t, err.Error(), "already bound", + "two forwards on one address should be caught before binding") +} + +// Every bad spec should be reported together, so fixing a container +// environment does not take one deploy per mistake. +func TestResolveReportsEveryProblem(t *testing.T) { + _, raw := newTestCommand() + raw.forwards = []string{ + "http://70000=https://a.internal", + "gopher://8081=https://b.internal", + } + + _, err := raw.Resolve() + + require.Error(t, err) + assert.Contains(t, err.Error(), "out of range") + assert.Contains(t, err.Error(), "unknown scheme") +} + +func TestResolveAllowedHosts(t *testing.T) { + _, raw := newTestCommand() + raw.forwards = []string{"http://0.0.0.0:8080=https://a.internal", "http://8081=https://b.internal"} + raw.allowPublicBind = true + raw.allowedHosts = []string{"NBLink.LAN.", "svc.ns.svc.cluster.local"} + + cfg, err := raw.Resolve() + + require.NoError(t, err) + assert.Equal(t, []string{"nblink.lan", "svc.ns.svc.cluster.local"}, cfg.AllowedHosts, + "allowed hosts must be stored in the form they are compared in") + for _, f := range cfg.Forwards { + assert.Equal(t, cfg.AllowedHosts, f.AllowedHosts, "every forward must carry the allowed hosts") + } +} + +func TestResolveRejectsMalformedAllowedHosts(t *testing.T) { + _, raw := newTestCommand() + raw.forwards = []string{"http://8080=https://a.internal"} + raw.allowedHosts = []string{"http://nblink.lan", "nblink.lan:8080", "*.lan", "10.0.0.5", ""} + + _, err := raw.Resolve() + + require.Error(t, err) + for _, entry := range []string{`"http://nblink.lan"`, `"nblink.lan:8080"`, `"*.lan"`, `""`} { + assert.Contains(t, err.Error(), entry, "every malformed entry must be reported") + } + assert.Contains(t, err.Error(), "addresses are already accepted", "an address must be explained, not just refused") +} + +func TestResolveSetupKeyFromFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "key") + // The trailing newline mirrors what a mounted secret usually contains. + require.NoError(t, os.WriteFile(path, []byte("A1B2C3D4-SETUP-KEY\n"), 0o600)) + + _, raw := newTestCommand() + raw.forwards = []string{"http://8080=https://a.internal"} + raw.setupKey = "file:" + path + + cfg, err := raw.Resolve() + + require.NoError(t, err) + assert.Equal(t, "A1B2C3D4-SETUP-KEY", cfg.SetupKey, "a mounted secret should be trimmed") +} + +// Each port-0 forward gets its own OS-assigned port, so sharing the spec is +// not a collision. +func TestResolveAllowsSeveralEphemeralPorts(t *testing.T) { + _, raw := newTestCommand() + raw.forwards = []string{ + "http://0=https://a.internal", + "http://0=https://b.internal", + } + + cfg, err := raw.Resolve() + + require.NoError(t, err) + assert.Len(t, cfg.Forwards, 2, "two ephemeral forwards should both be kept") +} + +func TestResolveSetupKeyFileEmpty(t *testing.T) { + path := filepath.Join(t.TempDir(), "key") + require.NoError(t, os.WriteFile(path, []byte(" \n"), 0o600)) + + _, raw := newTestCommand() + raw.forwards = []string{"http://8080=https://a.internal"} + raw.setupKey = "file:" + path + + _, err := raw.Resolve() + + require.Error(t, err) + assert.Contains(t, err.Error(), "is empty", + "an empty secret should fail rather than fall back to interactive login") +} + +func TestResolveSetupKeyFileMissing(t *testing.T) { + _, raw := newTestCommand() + raw.forwards = []string{"http://8080=https://a.internal"} + raw.setupKey = "file:" + filepath.Join(t.TempDir(), "absent") + + _, err := raw.Resolve() + + require.Error(t, err) + assert.Contains(t, err.Error(), "setup key") +} + +func TestStatePathsFollowStateDir(t *testing.T) { + cfg := &Config{} + assert.Empty(t, cfg.ConfigPath(), "an unset state dir should keep identity in memory") + assert.Empty(t, cfg.StatePath()) + + cfg.StateDir = "/var/lib/nblink" + assert.Equal(t, filepath.Join("/var/lib/nblink", "config.json"), cfg.ConfigPath()) + assert.Equal(t, filepath.Join("/var/lib/nblink", "state.json"), cfg.StatePath()) +} diff --git a/client/link/forward.go b/client/link/forward.go new file mode 100644 index 000000000..25d272931 --- /dev/null +++ b/client/link/forward.go @@ -0,0 +1,217 @@ +// Package link implements nblink, an unprivileged forwarder that exposes local +// listeners and carries their traffic into the NetBird overlay. +// +// The process runs the embedded client in netstack mode, so it needs no TUN +// device and no elevated privileges. Overlay names on the upstream side are +// resolved inside the tunnel rather than through the host resolver. +package link + +import ( + "errors" + "fmt" + "net" + "net/netip" + "net/url" + "regexp" + "strconv" + "strings" +) + +// defaultBindHost is used when a forward spec gives a port but no bind address. +// Binding loopback by default keeps the overlay unreachable from the rest of +// the network until the operator opts in. +const defaultBindHost = "127.0.0.1" + +// ProtoHTTP forwards HTTP requests to an upstream URL over the overlay. +const ProtoHTTP = "http" + +// supportedProtos lists the forward schemes this build accepts. Specs naming a +// known but unimplemented scheme are rejected with a clearer message than an +// unknown one would produce. +var ( + supportedProtos = map[string]bool{ProtoHTTP: true} + plannedProtos = map[string]bool{"tcp": true, "udp": true, "socks5": true} + + // schemePrefix matches a URL scheme at the start of a string, per RFC 3986 + // section 3.1. + schemePrefix = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.\-]*://`) + + // urlPassword matches the password half of a URL's userinfo. Errors quote + // the spec the operator typed, which would otherwise echo a password back + // to the terminal and into whatever captured it. + // + // The password runs to the last '@' of the authority, which is where + // net/url ends the userinfo, so a password containing an '@' is covered + // whole. The username ends at the first ':', as net/url splits it. + urlPassword = regexp.MustCompile(`(//[^/:\s]*):[^/\s]*@`) +) + +// Forward is one local listener and the overlay address it carries traffic to. +type Forward struct { + // Proto is the listener type, taken from the scheme on the left of the spec. + Proto string + // Listen is the local bind address as host:port. + Listen string + // Upstream is the target reached over the overlay. + Upstream *url.URL + // Spec is the original text, used in logs and errors so the operator sees + // what they typed rather than a normalized form. + Spec string + // AllowedHosts are names, beyond loopback names and addresses, that the + // listener accepts in the Host header. + AllowedHosts []string +} + +// String returns the forward in the spec form the operator wrote. +func (f Forward) String() string { + return fmt.Sprintf("%s://%s=%s", f.Proto, f.Listen, f.Upstream) +} + +// ParseForward parses a forward spec of the form scheme://[host:]port=upstream. +// +// The scheme on the left selects the listener type, the right side is the +// overlay target. A spec that gives only a port binds loopback. +func ParseForward(spec string) (Forward, error) { + spec = strings.TrimSpace(spec) + if spec == "" { + return Forward{}, fmt.Errorf("empty forward spec") + } + + schemeEnd := strings.Index(spec, "://") + if schemeEnd < 0 { + return Forward{}, fmt.Errorf("forward %q: missing scheme, want scheme://[host:]port=upstream", redactSpec(spec)) + } + proto := strings.ToLower(spec[:schemeEnd]) + + // The left side cannot contain '=', so the first one always separates the + // listener from the upstream even when the upstream carries a query string. + sep := strings.Index(spec, "=") + if sep < 0 || sep < schemeEnd { + return Forward{}, fmt.Errorf("forward %q: missing '=' between listener and upstream", redactSpec(spec)) + } + + if !supportedProtos[proto] { + if plannedProtos[proto] { + return Forward{}, fmt.Errorf("forward %q: %s forwarding is not supported yet, this build handles %s", redactSpec(spec), proto, ProtoHTTP) + } + return Forward{}, fmt.Errorf("forward %q: unknown scheme %q", redactSpec(spec), proto) + } + + listen, err := parseListen(spec[schemeEnd+len("://") : sep]) + if err != nil { + return Forward{}, fmt.Errorf("forward %q: %w", redactSpec(spec), err) + } + + upstream, err := parseUpstream(spec[sep+1:]) + if err != nil { + return Forward{}, fmt.Errorf("forward %q: %w", redactSpec(spec), err) + } + + return Forward{Proto: proto, Listen: listen, Upstream: upstream, Spec: spec}, nil +} + +// redactSpec replaces any password inside a forward spec, so an error may +// still name the spec it refers to. +func redactSpec(spec string) string { + return urlPassword.ReplaceAllString(spec, "$1:xxxxx@") +} + +// parseListen normalizes the listener side of a spec to host:port, defaulting +// the host to loopback when only a port is given. +func parseListen(raw string) (string, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", fmt.Errorf("missing listen address") + } + + host, port := defaultBindHost, raw + if strings.Contains(raw, ":") { + h, p, err := net.SplitHostPort(raw) + if err != nil { + return "", fmt.Errorf("parse listen address %q: %w", raw, err) + } + if h != "" { + host = h + } + port = p + } + + // Only literal addresses are accepted. A name would be resolved once by + // the loopback check and again by the bind, and an answer that changed in + // between would put a listener on a public address without the opt-in. A + // zone suffix stays allowed: it is part of the literal, resolved the same + // way at both points, and link-local addresses cannot be bound without it. + if strings.EqualFold(host, "localhost") { + host = defaultBindHost + } + if _, err := netip.ParseAddr(host); err != nil { + return "", fmt.Errorf("listen host %q must be an IP address or localhost", host) + } + + n, err := strconv.Atoi(port) + if err != nil { + return "", fmt.Errorf("listen port %q is not a number", port) + } + // Port 0 asks the OS for a free port. The bound address is printed at + // startup, so the operator still learns where to point a client. + if n < 0 || n > 65535 { + return "", fmt.Errorf("listen port %d out of range 0-65535", n) + } + + return net.JoinHostPort(host, port), nil +} + +// parseUpstream validates the overlay target of an HTTP forward. The host is +// resolved later, inside the tunnel, so only the shape is checked here. +func parseUpstream(raw string) (*url.URL, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return nil, fmt.Errorf("missing upstream") + } + // Only a leading scheme counts. A "://" inside a path or query, as in a + // redirect parameter, belongs to the target rather than to this URL. + if !schemePrefix.MatchString(raw) { + raw = "https://" + raw + } + + u, err := url.Parse(raw) + if err != nil { + // url.Error carries the whole URL, password included, so only the + // reason inside it is reported alongside the redacted form. + var parseErr *url.Error + if errors.As(err, &parseErr) { + err = parseErr.Err + } + return nil, fmt.Errorf("parse upstream %q: %w", redactSpec(raw), err) + } + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("upstream scheme %q must be http or https", u.Scheme) + } + if u.Hostname() == "" { + return nil, fmt.Errorf("upstream %q has no host", redactSpec(raw)) + } + // Credentials in the upstream would reach the logs and the --check output, + // and the forwarder does not use them to authenticate anything. + if u.User != nil { + return nil, fmt.Errorf("upstream %q must not carry credentials", u.Redacted()) + } + + return u, nil +} + +// isLoopback reports whether addr binds only the loopback interface. The host +// is always a literal address, because parseListen rejects names, so this +// decides the same address the bind will use. +func isLoopback(addr string) bool { + host, _, err := net.SplitHostPort(addr) + if err != nil { + return false + } + ip, err := netip.ParseAddr(host) + if err != nil { + return false + } + // Unmap first, so a v4-mapped form such as ::ffff:127.0.0.1 is recognised + // as the loopback address it is. + return ip.Unmap().IsLoopback() +} diff --git a/client/link/forward_http.go b/client/link/forward_http.go new file mode 100644 index 000000000..463de5134 --- /dev/null +++ b/client/link/forward_http.go @@ -0,0 +1,422 @@ +package link + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/http/httputil" + "net/netip" + "net/url" + "os" + "strings" + "time" + + log "github.com/sirupsen/logrus" +) + +const ( + // dialTimeout bounds connection establishment only. Request and response + // bodies stream without a deadline so long downloads are not cut off. + dialTimeout = 30 * time.Second + + // readHeaderTimeout bounds how long a caller may take to send request + // headers. + readHeaderTimeout = 10 * time.Second + + // fetchSiteHeader is the browser's own account of how a request was + // initiated. It is the only one of the three guard signals that a no-cors + // GET carries. + fetchSiteHeader = "Sec-Fetch-Site" + + // defaultHTTPPort fills in the port an origin leaves implicit. + defaultHTTPPort = "80" + + idleConnTimeout = 90 * time.Second + tlsHandshakeTimeout = 10 * time.Second + expectContinueTimeout = time.Second + maxIdleConns = 100 + + // maxHeaderBytes caps the request headers a caller may send. + maxHeaderBytes = 32 << 10 + + // maxLoggedPath caps how much of a caller-supplied path reaches the log. + maxLoggedPath = 128 +) + +// bodyIdleTimeout bounds how long a request body may stall without delivering +// more bytes. It is a variable so tests can shorten it. +var bodyIdleTimeout = 30 * time.Second + +// foreignFetchSites are the Sec-Fetch-Site values a browser sends when a page +// other than this listener's own initiated the request. A page on another port +// is same-site here, because a bare address or localhost is its own site, yet +// it is a different application with no more claim on this listener than any +// other page has. +var foreignFetchSites = map[string]bool{"cross-site": true, "same-site": true} + +// DialFunc establishes a connection inside the overlay. The embedded client +// supplies the real implementation; tests supply their own. +type DialFunc func(ctx context.Context, network, addr string) (net.Conn, error) + +// httpForwarder serves one local HTTP listener and proxies what it accepts to +// an upstream reached over the overlay. +type httpForwarder struct { + forward Forward + server *http.Server + listener net.Listener +} + +// idleTimeoutBody applies a fresh read deadline before every read, so a +// transfer that keeps progressing runs as long as it needs while a stalled one +// is cut off. +type idleTimeoutBody struct { + io.ReadCloser + controller *http.ResponseController + idle time.Duration +} + +// newHTTPForwarder binds the local listener and prepares the reverse proxy. +// Binding happens here rather than in Serve so a port clash is reported before +// any forward is announced as ready. +func newHTTPForwarder(fwd Forward, dial DialFunc) (*httpForwarder, error) { + listener, err := net.Listen(listenNetwork(fwd.Listen), fwd.Listen) + if err != nil { + return nil, fmt.Errorf("listen on %s: %w", fwd.Listen, bindHint(fwd.Listen, err)) + } + + upstream := fwd.Upstream + proxy := &httputil.ReverseProxy{ + Rewrite: func(r *httputil.ProxyRequest) { + r.SetURL(upstream) + // The upstream decides routing and TLS verification from Host, and + // it must see its own name rather than the local bind address. + r.Out.Host = upstream.Host + }, + Transport: &http.Transport{ + DialContext: boundedDial(dial), + ForceAttemptHTTP2: true, + MaxIdleConns: maxIdleConns, + // Without this the per-host default of two applies and MaxIdleConns + // never binds, so concurrent requests re-dial the overlay instead + // of reusing a connection. + MaxIdleConnsPerHost: maxIdleConns, + IdleConnTimeout: idleConnTimeout, + TLSHandshakeTimeout: tlsHandshakeTimeout, + ExpectContinueTimeout: expectContinueTimeout, + }, + ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) { + if errors.Is(err, context.Canceled) { + return + } + log.Debugf("%s %s %s: %v", fwd.Listen, r.Method, truncatePath(r.URL.Path), err) + // No upstream answered, so the connection is closed rather than + // kept for a next request. What is left of the body is still + // drained afterwards, bounded by the deadline armed in + // withBodyIdleTimeout. + rejectRequest(w, "upstream unreachable over the overlay", http.StatusBadGateway) + }, + } + + return &httpForwarder{ + forward: fwd, + listener: listener, + server: &http.Server{ + Handler: withBodyIdleTimeout(guardRebinding(proxy, isLoopback(fwd.Listen), fwd.AllowedHosts)), + // Bound how long a caller may take to send headers, so a slow + // sender cannot hold a connection and its goroutine open + // indefinitely. Neither deadline limits body streaming, so large + // uploads and downloads still run as long as they need. + ReadHeaderTimeout: readHeaderTimeout, + IdleTimeout: idleConnTimeout, + // Go's default allows about a megabyte of headers per connection, + // which a caller on a published port can spend freely. + MaxHeaderBytes: maxHeaderBytes, + }, + }, nil +} + +// Addr returns the address actually bound, which differs from the spec when +// the operator asked for port 0. +func (f *httpForwarder) Addr() string { + return f.listener.Addr().String() +} + +// Serve runs until the forwarder is closed. +func (f *httpForwarder) Serve() error { + if err := f.server.Serve(f.listener); err != nil && !errors.Is(err, http.ErrServerClosed) { + return fmt.Errorf("serve %s: %w", f.forward.Listen, err) + } + return nil +} + +// Close stops the listener and waits for in-flight requests to finish, up to +// the deadline carried by ctx. +// +// Shutdown only closes listeners the server has taken over in Serve, so the +// raw listener is closed here as well. Otherwise a forwarder that was built +// but never served, which happens when a later bind in the same set fails, +// would hold its socket until the process exits. +func (f *httpForwarder) Close(ctx context.Context) error { + shutdownErr := f.server.Shutdown(ctx) + if err := f.listener.Close(); err != nil && !errors.Is(err, net.ErrClosed) { + return err + } + return shutdownErr +} + +func (b *idleTimeoutBody) Read(p []byte) (int, error) { + if err := b.setDeadline(time.Now().Add(b.idle)); err != nil { + return 0, err + } + + n, err := b.ReadCloser.Read(p) + if errors.Is(err, io.EOF) { + // The body arrived in full, so its deadline has to go with it. The + // server keeps reading the connection while the handler streams its + // reply, and a deadline left over from the body would cut that reply + // off partway through. + // + // Only EOF clears it. On any other error the caller stalled or the + // connection broke, and the deadline is what stops the server from + // blocking forever as it drains what was never sent. + b.clearDeadline() + } + return n, err +} + +func (b *idleTimeoutBody) setDeadline(t time.Time) error { + // A connection that cannot carry a deadline, which the controller reports + // as unsupported, still reads normally rather than failing the request. + if err := b.controller.SetReadDeadline(t); err != nil && !errors.Is(err, http.ErrNotSupported) { + return err + } + return nil +} + +// clearDeadline removes any read deadline. A failure here cannot be acted on +// and must not mask the error the caller is already returning. +func (b *idleTimeoutBody) clearDeadline() { + if err := b.setDeadline(time.Time{}); err != nil { + log.Debugf("clear read deadline: %v", err) + } +} + +// listenNetwork pins the address family to the literal the operator wrote. +// Plain "tcp" turns 0.0.0.0 into a dual-stack wildcard that also accepts IPv6, +// which exposes more than the spec asked for. A "6" network binds IPv6 only. +func listenNetwork(addr string) string { + host, _, err := net.SplitHostPort(addr) + if err != nil { + return "tcp" + } + ip, err := netip.ParseAddr(host) + if err != nil { + return "tcp" + } + if ip.Is4() { + return "tcp4" + } + return "tcp6" +} + +// boundedDial applies dialTimeout to connection establishment without +// constraining the lifetime of the connection it returns. +func boundedDial(dial DialFunc) DialFunc { + return func(ctx context.Context, network, addr string) (net.Conn, error) { + ctx, cancel := context.WithTimeout(ctx, dialTimeout) + defer cancel() + + start := time.Now() + conn, err := dial(ctx, network, addr) + if err != nil { + log.Debugf("dial %s over the overlay failed after %s: %v", + addr, time.Since(start).Truncate(time.Millisecond), err) + return nil, err + } + log.Debugf("dial %s over the overlay took %s, remote %s", + addr, time.Since(start).Truncate(time.Millisecond), conn.RemoteAddr()) + return conn, nil + } +} + +// bindHint adds the likely cause when a bind fails for a reason an operator +// can act on. The privileged-port boundary is detected from the error rather +// than assumed from the port number, because containers often lower it to zero. +func bindHint(addr string, err error) error { + if errors.Is(err, os.ErrPermission) { + return fmt.Errorf("%w (nblink runs unprivileged, so %s likely needs a port above 1023)", err, addr) + } + return err +} + +// withBodyIdleTimeout cuts off a caller that sends request headers and then +// stops sending the body. +// +// ReadHeaderTimeout covers only the headers, and IdleTimeout applies between +// requests rather than during one, so without this a caller could hold a +// connection and its handler open indefinitely. A total ReadTimeout would +// close that gap but would also break legitimate long uploads, so the deadline +// is refreshed on every read that makes progress instead. +func withBodyIdleTimeout(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Body != nil && r.Body != http.NoBody { + b := &idleTimeoutBody{ + ReadCloser: r.Body, + controller: http.NewResponseController(w), + idle: bodyIdleTimeout, + } + // Armed before the handler runs rather than on the first read, + // because it is the only bound on the drain the server performs + // after the handler returns. That drain reads the connection + // directly and no response header suppresses it, so a handler + // that never touches the body would otherwise leave the + // connection and its goroutine held until the caller goes away. + // + // Replacing r.Body also hides it from net/http's own type-based + // checks, which short-circuit that drain for a body it recognises, + // so this deadline stands in for those too. + if err := b.setDeadline(time.Now().Add(b.idle)); err != nil { + log.Debugf("arm request body deadline: %v", err) + } + r.Body = b + } + next.ServeHTTP(w, r) + }) +} + +// guardRebinding rejects requests that a browser issued on behalf of some +// other site, and requests that reached a loopback listener under a name that +// is not its own. +// +// A loopback forwarder is reachable from any page the user visits. A site can +// point its own hostname at 127.0.0.1, and it can also embed the loopback +// address directly in an img, script or iframe URL. Either way the browser +// sends the request, and this listener would otherwise proxy it into the +// network under the peer's identity. +// +// Three signals separate those from a local caller. Host carries the +// attacker's name in the rebinding variant. Origin is present on any +// cross-origin fetch. Sec-Fetch-Site covers what the first two miss: the +// browser states how the request was initiated, including on the no-cors GETs +// that carry no Origin at all. Non-browser callers send none of the latter +// two and are unaffected. +// +// A browser too old to send Sec-Fetch-Site can still reach a loopback forward +// with an embedded no-cors GET. Browsers have sent it since 2020, so this +// residual needs a deliberately outdated one. +// +// The Host check holds on a publicly bound listener too, which is how a +// container port published to the host is reached. There a rebound page sends +// its own name with a matching Origin, so Host is the only signal left. Other +// machines reach a public listener by address, which is accepted; any name +// they use instead has to be listed with --allowed-host. +func guardRebinding(next http.Handler, loopbackOnly bool, allowedHosts []string) http.Handler { + allowed := make(map[string]bool, len(allowedHosts)) + for _, h := range allowedHosts { + allowed[normalizeHostname(h)] = true + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !isAllowedHost(hostnameOf(r.Host), loopbackOnly, allowed) { + rejectRequest(w, "unexpected Host for this listener, list it with --allowed-host", http.StatusMisdirectedRequest) + return + } + if !isLocalCaller(r) { + rejectRequest(w, "cross-origin request refused", http.StatusForbidden) + return + } + next.ServeHTTP(w, r) + }) +} + +// rejectRequest answers a request the forwarder will not proxy and closes the +// connection. +// +// Closing stops the connection being reused; it does not stop the server +// draining what is left of the request body. That drain runs from +// finishRequest whatever the response said, and it reads the original body +// rather than the handler's wrapper, so the only thing bounding it is the +// deadline withBodyIdleTimeout arms on the connection before the handler +// chain runs. Without that deadline a caller that stopped sending holds the +// connection and its goroutine even though the answer already went out. +func rejectRequest(w http.ResponseWriter, msg string, code int) { + w.Header().Set("Connection", "close") + http.Error(w, msg, code) +} + +// isLocalCaller reports whether a request may be proxied, from what the +// browser says about where it came from. +func isLocalCaller(r *http.Request) bool { + if foreignFetchSites[strings.ToLower(r.Header.Get(fetchSiteHeader))] { + return false + } + + // Only a browser sets Origin, and a legitimate one for this listener is + // same-origin. Anything else is a cross-site caller. + origin := r.Header.Get("Origin") + return origin == "" || isSameOrigin(origin, r.Host) +} + +// hostnameOf strips any port from a Host header value. +func hostnameOf(host string) string { + if h, _, err := net.SplitHostPort(host); err == nil { + return h + } + return host +} + +// isAllowedHost reports whether a request may arrive under host. Loopback +// names always may and listed names always may. A public listener also +// accepts any address, since a page cannot rebind an address it does not +// control. +func isAllowedHost(host string, loopbackOnly bool, allowed map[string]bool) bool { + if isLocalName(host) || allowed[normalizeHostname(host)] { + return true + } + if loopbackOnly { + return false + } + _, err := netip.ParseAddr(strings.Trim(host, "[]")) + return err == nil +} + +// isLocalName reports whether host names this machine's loopback interface. +func isLocalName(host string) bool { + if strings.EqualFold(host, "localhost") { + return true + } + ip, err := netip.ParseAddr(strings.Trim(host, "[]")) + return err == nil && ip.Unmap().IsLoopback() +} + +// isSameOrigin reports whether an Origin header names this listener. Scheme, +// host and port all have to match: the forwarder serves plaintext HTTP, and a +// page on another loopback port is a different origin like any other. +func isSameOrigin(origin, host string) bool { + u, err := url.Parse(origin) + if err != nil || u.Scheme != "http" { + return false + } + return normalizeAuthority(u.Host) == normalizeAuthority(host) +} + +// normalizeAuthority reduces an authority to a comparable host:port, so two +// spellings of one origin match. +func normalizeAuthority(authority string) string { + host, port, err := net.SplitHostPort(authority) + if err != nil { + host, port = authority, defaultHTTPPort + } + return strings.ToLower(strings.Trim(host, "[]")) + ":" + port +} + +// truncatePath shortens a caller-supplied path so a log line cannot be made +// arbitrarily long by the request that produced it. +func truncatePath(path string) string { + if len(path) <= maxLoggedPath { + return path + } + return path[:maxLoggedPath] + "..." +} diff --git a/client/link/forward_http_test.go b/client/link/forward_http_test.go new file mode 100644 index 000000000..1518a565a --- /dev/null +++ b/client/link/forward_http_test.go @@ -0,0 +1,563 @@ +package link + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// errDialNotExpected marks a test that must never reach the overlay, so a dial +// that does happen fails the test rather than passing silently. +var errDialNotExpected = errors.New("dial not expected in this test") + +func refuseDial(context.Context, string, string) (net.Conn, error) { + return nil, errDialNotExpected +} + +// overlayDialer stands in for the embedded client. It records what the +// forwarder asked to reach and then connects to a local test server, which +// lets the whole proxy path be exercised without a live network. +type overlayDialer struct { + target string + calls atomic.Int64 + asked atomic.Value +} + +func (d *overlayDialer) dial(ctx context.Context, network, addr string) (net.Conn, error) { + d.calls.Add(1) + d.asked.Store(addr) + var dialer net.Dialer + return dialer.DialContext(ctx, network, d.target) +} + +func (d *overlayDialer) lastAsked() string { + v, _ := d.asked.Load().(string) + return v +} + +// startForwarder wires a forwarder in front of upstream and returns its local +// base URL. +func startForwarder(t *testing.T, upstream *httptest.Server) (string, *overlayDialer) { + t.Helper() + + upstreamURL, err := url.Parse(upstream.URL) + require.NoError(t, err) + + // The spec names a hostname that does not resolve on this machine, proving + // the forwarder hands resolution to the dialer instead of the host. + fwd, err := ParseForward("http://127.0.0.1:0=http://grafana.internal") + require.NoError(t, err) + + dialer := &overlayDialer{target: upstreamURL.Host} + f, err := newHTTPForwarder(fwd, dialer.dial) + require.NoError(t, err) + + go func() { _ = f.Serve() }() + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = f.Close(ctx) + }) + + return "http://" + f.Addr(), dialer +} + +func TestForwarderProxiesThroughOverlayDialer(t *testing.T) { + var gotHost, gotPath string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotHost, gotPath = r.Host, r.URL.RequestURI() + _, _ = io.WriteString(w, "reached upstream") + })) + defer upstream.Close() + + base, dialer := startForwarder(t, upstream) + + resp, err := http.Get(base + "/dashboard?orgId=1") + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "reached upstream", string(body)) + assert.Equal(t, "/dashboard?orgId=1", gotPath, "path and query should be forwarded unchanged") + assert.Equal(t, "grafana.internal", gotHost, + "upstream must see its own Host, not the local bind address") + assert.Equal(t, int64(1), dialer.calls.Load(), "the overlay dialer should carry the request") + assert.Equal(t, "grafana.internal:80", dialer.lastAsked(), + "the overlay dialer should be asked for the upstream name, so it resolves in the tunnel") +} + +func TestForwarderForwardsRequestBodyAndMethod(t *testing.T) { + var gotMethod, gotBody string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod = r.Method + b, _ := io.ReadAll(r.Body) + gotBody = string(b) + w.WriteHeader(http.StatusCreated) + })) + defer upstream.Close() + + base, _ := startForwarder(t, upstream) + + resp, err := http.Post(base+"/api", "application/json", strings.NewReader(`{"k":"v"}`)) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusCreated, resp.StatusCode) + assert.Equal(t, http.MethodPost, gotMethod) + assert.Equal(t, `{"k":"v"}`, gotBody, "request bodies should reach the upstream intact") +} + +// A forward whose upstream cannot be reached must answer 502 rather than hang +// or leak the dial error to the caller. +func TestForwarderReportsUnreachableUpstream(t *testing.T) { + fwd, err := ParseForward("http://127.0.0.1:0=http://grafana.internal") + require.NoError(t, err) + + refuse := func(context.Context, string, string) (net.Conn, error) { + return nil, &net.OpError{Op: "dial", Err: net.UnknownNetworkError("no route in overlay")} + } + f, err := newHTTPForwarder(fwd, refuse) + require.NoError(t, err) + go func() { _ = f.Serve() }() + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = f.Close(ctx) + }) + + resp, err := http.Get("http://" + f.Addr()) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) +} + +// Binding must fail at construction so a port clash surfaces before any +// forward is announced as ready. +func TestNewHTTPForwarderReportsBindConflict(t *testing.T) { + busy, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer busy.Close() + + fwd, err := ParseForward("http://" + busy.Addr().String() + "=https://grafana.internal") + require.NoError(t, err) + + _, err = newHTTPForwarder(fwd, refuseDial) + + require.Error(t, err) + assert.Contains(t, err.Error(), "listen on") +} + +// A forwarder that was built but never served still owns its socket. The +// bind-all-then-serve path closes earlier forwarders when a later bind fails, +// so Close has to release the port even though Serve never ran. +func TestCloseReleasesAnUnservedListener(t *testing.T) { + fwd, err := ParseForward("http://127.0.0.1:0=https://grafana.internal") + require.NoError(t, err) + + f, err := newHTTPForwarder(fwd, refuseDial) + require.NoError(t, err) + addr := f.Addr() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + require.NoError(t, f.Close(ctx)) + + // The port is free again only if the listener was actually closed. + reclaimed, err := net.Listen("tcp", addr) + require.NoError(t, err, "Close should have released the listener") + require.NoError(t, reclaimed.Close()) +} + +// A caller that sends headers and then stalls its body must be cut off. +// ReadHeaderTimeout does not cover the body and IdleTimeout applies only +// between requests, so without the idle read deadline this connection would be +// held open indefinitely. +func TestStalledRequestBodyIsCutOff(t *testing.T) { + previous := bodyIdleTimeout + bodyIdleTimeout = 150 * time.Millisecond + t.Cleanup(func() { bodyIdleTimeout = previous }) + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + })) + defer upstream.Close() + + base, _ := startForwarder(t, upstream) + + conn, err := net.Dial("tcp", strings.TrimPrefix(base, "http://")) + require.NoError(t, err) + defer conn.Close() + + // Announce a body, send one byte of it, then go quiet. + _, err = conn.Write([]byte("POST / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 1000\r\n\r\nx")) + require.NoError(t, err) + + require.NoError(t, conn.SetReadDeadline(time.Now().Add(10*time.Second))) + _, err = io.ReadAll(conn) + + // The server closes the connection rather than waiting on the rest of the + // body, so the read ends well before the generous deadline above. + require.NoError(t, err, "the connection should be closed by the server, not time out in the test") +} + +func TestRedactSpec(t *testing.T) { + assert.Equal(t, "http://8080=https://user:xxxxx@a.internal", + redactSpec("http://8080=https://user:hunter2@a.internal"), + "a password must not survive into an error message") + assert.Equal(t, "http://8080=https://a.internal", + redactSpec("http://8080=https://a.internal"), + "a spec without credentials should be unchanged") +} + +// The error that rejects a credential-bearing upstream quotes the spec, which +// must not carry the password back to the terminal. +func TestParseForwardDoesNotEchoPassword(t *testing.T) { + _, err := ParseForward("http://8080=https://user:hunter2@a.internal") + + require.Error(t, err) + assert.NotContains(t, err.Error(), "hunter2", "the password must not appear anywhere in the error") + assert.Contains(t, err.Error(), "must not carry credentials") +} + +// A POST whose body is fully delivered must still receive its whole response, +// even when the upstream takes longer to answer than the body idle timeout. +// The deadline that guards the body must not outlive it. +func TestCompletedUploadGetsFullResponse(t *testing.T) { + previous := bodyIdleTimeout + bodyIdleTimeout = 200 * time.Millisecond + t.Cleanup(func() { bodyIdleTimeout = previous }) + + payload := strings.Repeat("y", 4096) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + time.Sleep(600 * time.Millisecond) + _, _ = io.WriteString(w, payload) + })) + defer upstream.Close() + + base, _ := startForwarder(t, upstream) + + resp, err := http.Post(base+"/upload", "text/plain", strings.NewReader(strings.Repeat("x", 2048))) + require.NoError(t, err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err, "the response must not be cut off once the body was fully read") + assert.Equal(t, len(payload), len(body), "the whole response body should arrive") +} + +// A page that points its own hostname at this loopback listener must not be +// able to reach the upstream through it. +func TestLoopbackForwarderRejectsForeignHost(t *testing.T) { + var reached bool + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + })) + defer upstream.Close() + + base, _ := startForwarder(t, upstream) + + req, err := http.NewRequest(http.MethodGet, base, nil) + require.NoError(t, err) + req.Host = "attacker.example" + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusMisdirectedRequest, resp.StatusCode) + assert.False(t, reached, "the upstream must not be reached under a foreign Host") +} + +func TestLoopbackForwarderRejectsCrossOrigin(t *testing.T) { + var reached bool + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + })) + defer upstream.Close() + + base, _ := startForwarder(t, upstream) + + req, err := http.NewRequest(http.MethodGet, base, nil) + require.NoError(t, err) + req.Header.Set("Origin", "https://attacker.example") + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.False(t, reached, "the upstream must not be reached from a cross-site page") +} + +// The ordinary local callers must keep working: no Origin at all, and a +// same-origin browser request. +func TestLoopbackForwarderAllowsLocalCallers(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, "ok") + })) + defer upstream.Close() + + base, _ := startForwarder(t, upstream) + + for _, origin := range []string{"", "http://" + strings.TrimPrefix(base, "http://")} { + req, err := http.NewRequest(http.MethodGet, base, nil) + require.NoError(t, err) + if origin != "" { + req.Header.Set("Origin", origin) + } + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + _ = resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode, "origin %q should be allowed", origin) + assert.Equal(t, "ok", string(body)) + } +} + +// A malformed upstream fails in the URL parser rather than in the credential +// check, so both error layers have to be redacted. +func TestParseForwardRedactsMalformedUpstream(t *testing.T) { + for _, spec := range []string{ + "http://8080=https://user:hunter2@a.internal:bad", + "http://8080=https://user:hunter2@a.internal/%zz", + } { + _, err := ParseForward(spec) + require.Error(t, err) + assert.NotContains(t, err.Error(), "hunter2", + "a password must not survive a parse failure either") + } +} + +// An embedded or