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