Compare commits

...

4 Commits

Author SHA1 Message Date
Zoltan Papp
8197764a21 [client] Reuse existing sessionExpired label for expired tray row
Drop the newly added tray.session.expired string and render the expired
tray row with the existing tray.status.sessionExpired label, which is
already translated in every locale and short enough for the tray.
2026-07-21 16:47:04 +02:00
Zoltan Papp
7d80fca4a7 [client] Round session remaining-time label up to nearest unit
Ceiling each unit so the tray label never shows less time than actually
remains, and shift the unit thresholds (<=59m, <=23h) so ceiling never
overflows into 60 minutes or 24 hours.
2026-07-21 16:25:29 +02:00
Zoltán Papp
a642f4c9e4 [client] Keep session deadline visible across reconnects and after expiry
The tray session row flapped on every transient engine restart and went
blank at the exact moment the session expired:

- Watcher.Close zeroed the server-scoped recorder deadline on every
  engine restart (sleep/wake, network change, stream errors), hiding the
  tray row until the next successful login re-seeded it. Close now only
  stops the timers; the client run loop clears the recorder when it
  exits for real.
- Status.GetSessionExpiresAt masked past deadlines as zero, so the
  status snapshot dropped the field right when the UI should have shown
  "session expired". The getter now returns the raw value and the tray
  renders past deadlines as "Session expired — sign in again", routing
  the click to the login flow.
- The watcher rejected recently-expired deadlines (30s skew) delivered
  by login/sync, clearing the recorder instead of surfacing the expiry.
  Deadlines up to 30 days past are now recorded as expired; only older
  values are rejected as garbage.
2026-07-21 12:37:56 +02:00
Zoltan Papp
724c6a06e6 [relay] only trust X-Real-Ip headers from configured trusted proxies (#6833)
The WS listener unconditionally trusted X-Real-Ip/X-Real-Port headers,
letting any client forge the source address the relay logs. Gate header
trust behind a trusted-proxy allowlist; ignore the headers unless the
immediate peer matches a configured prefix. Defaults to never trusting
the headers when the allowlist is empty.

## Describe your changes

## Issue ticket number and link

## Stack

<!-- branch-stack -->

### Checklist
- [x] Is it a bug fix
- [ ] Is a typo/documentation fix
- [ ] Is a feature enhancement
- [ ] It is a refactor
- [ ] Created tests that fail without the change (if possible)
- [ ] This change does **not** modify the public API, gRPC protocols,
functionality behavior, CLI / service flags, or introduce a new feature
— **OR** I have discussed it with the NetBird team beforehand (link the
issue / Slack thread in the description). See
[CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first).

> By submitting this pull request, you confirm that you have read and
agree to the terms of the [Contributor License
Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md).

## Documentation
Select exactly one:

- [ ] I added/updated documentation for this change
- [x] Documentation is **not needed** for this change (explain why)

### Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

<!-- codesmith:footer -->
---
<a
href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6833"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img
alt="View with Codesmith"
src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a>
<a
href="https://backend.blacksmith.sh/track/enable-autofix?expires=1787141580&installation_id=146802194&pr_number=6833&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6833&signature=9cf182cc7be248e457dfdb56e8a047401276d8cc567ed8ae715ec1cc809f1b6a"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img
alt="Autofix with Codesmith"
src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a>
<sup>Need help on this PR? Tag <code>/codesmith</code> with what you
need. Autofix is disabled.</sup>

<!-- codesmith:autofix:disabled -->
<!-- /codesmith:footer -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added `--trusted-proxies` to configure a comma-separated allowlist of
trusted upstream proxy IPs/CIDRs.
* **Behavior Changes**
* Relay WebSocket now uses `X-Real-Ip` / `X-Real-Port` only when the
immediate peer is from the configured trusted set; otherwise it falls
back to the direct remote address.
* Proxy client IP resolution is now consistent and honors
`X-Forwarded-For` only through trusted hops.
* **Operational**
  * Invalid `--trusted-proxies` values fail fast on startup.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-07-20 18:15:03 +02:00
24 changed files with 561 additions and 491 deletions

View File

@@ -24,11 +24,7 @@ import (
)
const (
// Skew tolerates a small clock difference between the management
// server and this peer before treating a deadline as "in the past".
// Slightly above typical NTP drift; tight enough that the UI doesn't
// paint a stale expiry as if it were valid.
Skew = 30 * time.Second
maxPastHorizon = 30 * 24 * time.Hour
// maxDeadlineHorizon caps how far in the future an accepted deadline
// can sit. A timestamp beyond this is almost certainly a protocol
@@ -57,7 +53,7 @@ var (
ErrDeadlineTooFarFuture = errors.New("session deadline too far in the future")
// ErrDeadlineInPast is returned by Update when the supplied deadline
// is more than Skew in the past.
// is more than maxPastHorizon in the past.
ErrDeadlineInPast = errors.New("session deadline in the past")
)
@@ -66,15 +62,14 @@ var (
// for deadline change/clear, PublishEvent for the two warnings); tests pass
// a fake recorder so the same surface is observable without an engine.
//
// The watcher is the single owner of the deadline propagated to the
// recorder: every set, clear, sanity-check rejection and Close routes the
// value through SetSessionExpiresAt, so the SubscribeStatus snapshot the UI
// reads can never drift from the watcher's timer state. (SetSessionExpiresAt
// fans out its own state-change notification, so no separate notify is
// needed.) The recorder is server-scoped and outlives this engine-scoped
// watcher — without the Close-time clear a teardown (Down, or the Down+Up of
// a profile switch) would leave the next session showing the previous one's
// stale "expires in" value.
// While the watcher runs, it owns the deadline propagated to the recorder:
// every set, clear and sanity-check rejection routes the value through
// SetSessionExpiresAt, so the SubscribeStatus snapshot the UI reads can
// never drift from the watcher's timer state. (SetSessionExpiresAt fans
// out its own state-change notification, so no separate notify is needed.)
// The recorder is server-scoped and outlives this engine-scoped watcher;
// Close deliberately leaves the recorder value in place so transient engine
// restarts don't blank it — the client run loop clears it on real teardown.
//
// PublishEvent's signature mirrors peer.Status.PublishEvent: the watcher
// composes the metadata internally so the wire format (MetaSession*) is
@@ -135,10 +130,13 @@ func NewWithLeads(lead, final time.Duration, recorder StatusRecorder) *Watcher {
// was disabled).
//
// Same-value updates are no-ops. A different non-zero value cancels any
// pending timer, resets the "already fired" guard, and arms a new one.
// pending timer, resets the "already fired" guards, and — when the
// deadline lies in the future — arms fresh warning timers. A deadline
// already in the past (within maxPastHorizon) is recorded as-is with no
// timers: the session has expired and consumers render it that way.
//
// Returns one of the sentinel Err* values when the deadline fails the
// sanity checks (pre-epoch, far future, or in the past beyond Skew).
// sanity checks (pre-epoch, far future, or past beyond maxPastHorizon).
// In every error case the watcher first clears its state so it stays
// consistent with what the caller will push into its other sinks (e.g.
// applySessionDeadline forces a zero deadline into the status recorder
@@ -163,7 +161,7 @@ func (w *Watcher) Update(deadline time.Time) error {
case deadline.After(now.Add(maxDeadlineHorizon)):
w.clearLocked()
return fmt.Errorf("%w: %v", ErrDeadlineTooFarFuture, deadline)
case deadline.Before(now.Add(-Skew)):
case deadline.Before(now.Add(-maxPastHorizon)):
w.clearLocked()
return fmt.Errorf("%w: %v (now=%v)", ErrDeadlineInPast, deadline, now)
}
@@ -183,7 +181,9 @@ func (w *Watcher) Update(deadline time.Time) error {
w.finalFiredAt = time.Time{}
w.dismissedAt = time.Time{}
w.armTimerLocked(deadline)
if deadline.After(now) {
w.armTimerLocked(deadline)
}
recorder := w.recorder
w.mu.Unlock()
if recorder != nil {
@@ -227,30 +227,25 @@ func (w *Watcher) Dismiss() {
log.Infof("auth session final-warning dismissed for deadline %s", w.current.Format(time.RFC3339))
}
// Close stops any pending timer and drops the deadline on the status
// recorder. Update calls after Close are ignored. Clearing the recorder
// here is what keeps a teardown (Down, or the Down+Up of a profile switch)
// from leaving the next session showing this one's stale "expires in"
// value — the recorder is server-scoped and outlives this engine-scoped
// watcher, so nothing else drops the anchor on teardown.
// Close stops any pending timer. Update calls after Close are ignored.
// The recorder keeps its deadline: the watcher is engine-scoped and closes
// on every engine restart (network change, sleep/wake, stream errors)
// while the SSO deadline stays valid across those, so clearing here would
// blank the UI's "expires in" row on every transient reconnect. The
// client run loop clears the server-scoped recorder when it exits for
// real (Down, profile switch, permanent login failure).
func (w *Watcher) Close() {
w.mu.Lock()
defer w.mu.Unlock()
if w.closed {
w.mu.Unlock()
return
}
w.closed = true
w.stopTimerLocked()
hadDeadline := !w.current.IsZero()
w.current = time.Time{}
w.firedAt = time.Time{}
w.finalFiredAt = time.Time{}
w.dismissedAt = time.Time{}
recorder := w.recorder
w.mu.Unlock()
if recorder != nil && hadDeadline {
recorder.SetSessionExpiresAt(time.Time{})
}
}
// clearLocked drops the tracked deadline and notifies the recorder so

View File

@@ -224,11 +224,13 @@ func TestNewDeadlineCancelsPriorTimer(t *testing.T) {
func TestRefreshAfterFireArmsNewWarning(t *testing.T) {
r := &fakeRecorder{}
lead := 30 * time.Millisecond
lead := 150 * time.Millisecond
w := newWatcher(lead, r)
defer w.Close()
first := time.Now().Add(50 * time.Millisecond)
// Warning fires ~20ms in; the deadline itself stays 150ms away so the
// replacement below lands well before it.
first := time.Now().Add(170 * time.Millisecond)
_ = w.Update(first)
// Wait for stateChange + warning of the first cycle.
@@ -306,7 +308,29 @@ func TestUpdateRejectsTooFarFuture(t *testing.T) {
}
}
func TestUpdateInPastClearsDeadline(t *testing.T) {
func TestUpdateRecentPastRecordedAsExpired(t *testing.T) {
r := &fakeRecorder{}
w := newWatcher(50*time.Millisecond, r)
defer w.Close()
d := time.Now().Add(-1 * time.Hour)
if err := w.Update(d); err != nil {
t.Fatalf("recent-past Update should succeed, got %v", err)
}
if !w.Deadline().Equal(d) {
t.Fatalf("expected deadline to be recorded, got %v want %v", w.Deadline(), d)
}
if got := r.deadline(); !got.Equal(d) {
t.Fatalf("recorder deadline = %v, want %v", got, d)
}
time.Sleep(80 * time.Millisecond)
if n := countWhere(r.snapshot(), func(e event) bool { return e.kind == publish }); n != 0 {
t.Fatalf("no warning events may fire for an already-past deadline, got %+v", r.snapshot())
}
}
func TestUpdateAncientPastRejected(t *testing.T) {
r := &fakeRecorder{}
w := newWatcher(50*time.Millisecond, r)
defer w.Close()
@@ -318,12 +342,12 @@ func TestUpdateInPastClearsDeadline(t *testing.T) {
// Drain the stateChange from the seed.
waitForEvents(t, r, 1)
err := w.Update(time.Now().Add(-1 * time.Hour))
err := w.Update(time.Now().Add(-31 * 24 * time.Hour))
if !errors.Is(err, ErrDeadlineInPast) {
t.Fatalf("want ErrDeadlineInPast, got %v", err)
}
if !w.Deadline().IsZero() {
t.Fatalf("in-past update must clear the deadline, got %v", w.Deadline())
t.Fatalf("rejected ancient-past update must clear the deadline, got %v", w.Deadline())
}
events := waitForEvents(t, r, 2)
if events[1].kind != stateChange {
@@ -331,21 +355,6 @@ func TestUpdateInPastClearsDeadline(t *testing.T) {
}
}
func TestUpdateWithinSkewAccepted(t *testing.T) {
r := &fakeRecorder{}
w := newWatcher(50*time.Millisecond, r)
defer w.Close()
// 5 seconds in the past is within the 30s Skew tolerance — accept it.
d := time.Now().Add(-5 * time.Second)
if err := w.Update(d); err != nil {
t.Fatalf("within-skew Update should succeed, got %v", err)
}
if !w.Deadline().Equal(d) {
t.Fatalf("expected deadline to be applied, got %v want %v", w.Deadline(), d)
}
}
func TestCloseSilencesUpdates(t *testing.T) {
r := &fakeRecorder{}
w := newWatcher(50*time.Millisecond, r)
@@ -359,11 +368,12 @@ func TestCloseSilencesUpdates(t *testing.T) {
}
}
// TestCloseClearsRecorderDeadline pins the profile-switch fix: a watcher
// holding a live deadline must zero the recorder on Close so the next
// engine's watcher (and the UI reading the shared server-scoped recorder)
// doesn't start out showing the previous session's stale "expires in".
func TestCloseClearsRecorderDeadline(t *testing.T) {
// TestCloseKeepsRecorderDeadline pins the reconnect-flap fix: the watcher
// closes on every engine restart (network change, sleep/wake) while the
// SSO deadline stays valid across those, so Close must leave the
// server-scoped recorder's value in place. The client run loop clears the
// recorder when it exits for real.
func TestCloseKeepsRecorderDeadline(t *testing.T) {
r := &fakeRecorder{}
w := newWatcher(time.Hour, r)
@@ -377,8 +387,8 @@ func TestCloseClearsRecorderDeadline(t *testing.T) {
w.Close()
if got := r.deadline(); !got.IsZero() {
t.Fatalf("recorder deadline after Close = %v, want zero", got)
if got := r.deadline(); !got.Equal(d) {
t.Fatalf("recorder deadline after Close = %v, want %v", got, d)
}
}

View File

@@ -257,7 +257,10 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan
log.Errorf("failed to clean up temporary installer file: %v", err)
}
defer c.statusRecorder.ClientStop()
defer func() {
c.statusRecorder.SetSessionExpiresAt(time.Time{})
c.statusRecorder.ClientStop()
}()
operation := func() error {
// if context cancelled we not start new backoff cycle
if c.ctx.Err() != nil {

View File

@@ -75,4 +75,14 @@ func TestApplySessionDeadline_ThreeState(t *testing.T) {
require.True(t, e.statusRecorder.GetSessionExpiresAt().IsZero(),
"invalid timestamp must clear the deadline")
})
t.Run("recently expired timestamp stays visible as expired", func(t *testing.T) {
e := newEngine()
expired := time.Now().Add(-5 * time.Minute).UTC().Truncate(time.Second)
e.ApplySessionDeadline(timestamppb.New(expired))
require.True(t, e.statusRecorder.GetSessionExpiresAt().Equal(expired),
"recently-expired deadline must stay on the recorder so consumers render it as expired")
})
}

View File

@@ -813,19 +813,14 @@ func (d *Status) SetSessionExpiresAt(deadline time.Time) {
}
// GetSessionExpiresAt returns the most recently recorded SSO session deadline,
// or the zero value when no deadline is tracked. A deadline that has already
// slipped into the past reports as "none": once the session has expired it is
// no longer a meaningful countdown, and the sessionwatch.Watcher does not
// arm a timer at the deadline itself to clear it (only the two pre-expiry
// warnings). Without this guard the UI would keep painting a stale
// "expires in …" against a moment that has passed until the next login,
// extend, or teardown rewrote the value.
// or the zero value when no deadline is tracked. A deadline in the past is
// returned as-is: it means the session has expired, and consumers (tray row,
// CLI status) render it as "expired" rather than hiding it — masking it as
// "none" would blank the UI at the exact moment it should say the session
// ended.
func (d *Status) GetSessionExpiresAt() time.Time {
d.mux.Lock()
defer d.mux.Unlock()
if !d.sessionExpiresAt.IsZero() && d.sessionExpiresAt.Before(time.Now()) {
return time.Time{}
}
return d.sessionExpiresAt
}

View File

@@ -315,8 +315,7 @@ func (t *Tray) relayoutMenu() {
if sessionDeadline.IsZero() {
t.sessionExpiresItem.SetHidden(true)
} else {
remaining := t.formatSessionRemaining(time.Until(sessionDeadline))
t.sessionExpiresItem.SetLabel(t.loc.T("tray.session.expiresIn", "remaining", remaining))
t.sessionExpiresItem.SetLabel(t.sessionRowLabel(sessionDeadline))
t.sessionExpiresItem.SetHidden(false)
}
}

View File

@@ -87,30 +87,39 @@ func (t *Tray) refreshSessionExpiresLabel() {
if deadline.IsZero() {
return
}
remaining := t.formatSessionRemaining(time.Until(deadline))
item.SetLabel(t.loc.T("tray.session.expiresIn", "remaining", remaining))
item.SetLabel(t.sessionRowLabel(deadline))
}
func (t *Tray) sessionRowLabel(deadline time.Time) string {
remaining := time.Until(deadline)
if remaining <= 0 {
return t.loc.T("tray.status.sessionExpired")
}
return t.loc.T("tray.session.expiresIn", "remaining", t.formatSessionRemaining(remaining))
}
// formatSessionRemaining renders d as a localised long-form string picking the largest non-zero unit.
// Each unit is rounded up so the label never claims less time than actually remains, matching the
// upper-bound sense of the sub-minute "less than a minute" fragment.
// Singular/plural keys are split per language for proper translation.
func (t *Tray) formatSessionRemaining(d time.Duration) string {
switch {
case d < time.Minute:
return t.loc.T("tray.session.unit.lessThanMinute")
case d < time.Hour:
m := int(d / time.Minute)
case d <= 59*time.Minute:
m := ceilDiv(d, time.Minute)
if m == 1 {
return t.loc.T("tray.session.unit.minute")
}
return t.loc.T("tray.session.unit.minutes", "count", strconv.Itoa(m))
case d < 24*time.Hour:
h := int((d + 30*time.Minute) / time.Hour)
case d <= 23*time.Hour:
h := ceilDiv(d, time.Hour)
if h == 1 {
return t.loc.T("tray.session.unit.hour")
}
return t.loc.T("tray.session.unit.hours", "count", strconv.Itoa(h))
default:
days := int((d + 12*time.Hour) / (24 * time.Hour))
days := ceilDiv(d, 24*time.Hour)
if days == 1 {
return t.loc.T("tray.session.unit.day")
}
@@ -118,6 +127,11 @@ func (t *Tray) formatSessionRemaining(d time.Duration) string {
}
}
// ceilDiv divides d by unit rounding up, assuming d > 0.
func ceilDiv(d, unit time.Duration) int {
return int((d + unit - time.Nanosecond) / unit)
}
// registerSessionWarningCategory wires the OS notification category and response handler for the expiry warning.
// Errors are swallowed since the worst case is a plain notification without buttons.
func (t *Tray) registerSessionWarningCategory() {
@@ -252,11 +266,9 @@ func (t *Tray) openSessionExpiration() {
}
// openSessionExtendFlow opens the SessionExpiration window seeded with the cached deadline's remaining time,
// for the "Expires in …" tray row. No-ops when the deadline is unknown or elapsed.
// for the "Expires in …" tray row. Once the deadline has elapsed the row reads "Session expired" and the
// click routes to the login flow instead. No-op when the deadline is unknown.
func (t *Tray) openSessionExtendFlow() {
if t.svc.WindowManager == nil {
return
}
t.sessionMu.Lock()
deadline := t.sessionExpiresAt
t.sessionMu.Unlock()
@@ -265,6 +277,14 @@ func (t *Tray) openSessionExtendFlow() {
}
seconds := int(time.Until(deadline).Seconds())
if seconds <= 0 {
if t.window != nil {
t.window.SetURL("/#/login")
t.window.Show()
t.window.Focus()
}
return
}
if t.svc.WindowManager == nil {
return
}
t.svc.WindowManager.OpenSessionExpiration(seconds)

View File

@@ -18,6 +18,7 @@ import (
"github.com/netbirdio/netbird/client/embed"
"github.com/netbirdio/netbird/proxy"
nbacme "github.com/netbirdio/netbird/proxy/internal/acme"
"github.com/netbirdio/netbird/trustedproxy"
"github.com/netbirdio/netbird/util"
)
@@ -209,7 +210,7 @@ func runServer(cmd *cobra.Command, args []string) error {
return fmt.Errorf("invalid domain value %q: %w", proxyDomain, err)
}
parsedTrustedProxies, err := proxy.ParseTrustedProxies(trustedProxies)
parsedTrustedProxies, err := trustedproxy.Parse(trustedProxies)
if err != nil {
return fmt.Errorf("invalid --trusted-proxies: %w", err)
}

View File

@@ -16,6 +16,7 @@ import (
"github.com/netbirdio/netbird/proxy/auth"
"github.com/netbirdio/netbird/proxy/internal/types"
"github.com/netbirdio/netbird/shared/management/proto"
"github.com/netbirdio/netbird/trustedproxy"
)
const (
@@ -66,7 +67,7 @@ type denyBucket struct {
type Logger struct {
client gRPCClient
logger *log.Logger
trustedProxies []netip.Prefix
trustedProxies *trustedproxy.List
usageMux sync.Mutex
domainUsage map[string]*domainUsage
@@ -82,7 +83,7 @@ type Logger struct {
// NewLogger creates a new access log Logger. The trustedProxies parameter
// configures which upstream proxy IP ranges are trusted for extracting
// the real client IP from X-Forwarded-For headers.
func NewLogger(client gRPCClient, logger *log.Logger, trustedProxies []netip.Prefix) *Logger {
func NewLogger(client gRPCClient, logger *log.Logger, trustedProxies *trustedproxy.List) *Logger {
if logger == nil {
logger = log.StandardLogger()
}

View File

@@ -4,13 +4,13 @@ import (
"net/http"
"net/netip"
"github.com/netbirdio/netbird/proxy/internal/proxy"
"github.com/netbirdio/netbird/trustedproxy"
)
// extractSourceIP resolves the real client IP from the request using trusted
// proxy configuration. When trustedProxies is non-empty and the direct
// connection is from a trusted source, it walks X-Forwarded-For right-to-left
// skipping trusted IPs. Otherwise it returns RemoteAddr directly.
func extractSourceIP(r *http.Request, trustedProxies []netip.Prefix) netip.Addr {
return proxy.ResolveClientIP(r.RemoteAddr, r.Header.Get("X-Forwarded-For"), trustedProxies)
func extractSourceIP(r *http.Request, trustedProxies *trustedproxy.List) netip.Addr {
return trustedProxies.ResolveClientIP(r.RemoteAddr, r.Header.Get("X-Forwarded-For"))
}

View File

@@ -22,6 +22,7 @@ import (
"github.com/netbirdio/netbird/proxy/internal/roundtrip"
"github.com/netbirdio/netbird/proxy/internal/types"
"github.com/netbirdio/netbird/proxy/web"
"github.com/netbirdio/netbird/trustedproxy"
)
type ReverseProxy struct {
@@ -29,10 +30,10 @@ type ReverseProxy struct {
// forwardedProto overrides the X-Forwarded-Proto header value.
// Valid values: "auto" (detect from TLS), "http", "https".
forwardedProto string
// trustedProxies is a list of IP prefixes for trusted upstream proxies.
// When the direct connection comes from a trusted proxy, forwarding
// headers are preserved and appended to instead of being stripped.
trustedProxies []netip.Prefix
// trustedProxies is the set of trusted upstream proxies. When the direct
// connection comes from a trusted proxy, forwarding headers are preserved
// and appended to instead of being stripped.
trustedProxies *trustedproxy.List
mappingsMux sync.RWMutex
mappings map[string]Mapping
logger *log.Logger
@@ -63,7 +64,7 @@ func WithMiddlewareManager(m *middleware.Manager) Option {
// between requested URLs and targets.
// The internal mappings can be modified using the AddMapping
// and RemoveMapping functions.
func NewReverseProxy(transport http.RoundTripper, forwardedProto string, trustedProxies []netip.Prefix, logger *log.Logger, opts ...Option) *ReverseProxy {
func NewReverseProxy(transport http.RoundTripper, forwardedProto string, trustedProxies *trustedproxy.List, logger *log.Logger, opts ...Option) *ReverseProxy {
if logger == nil {
logger = log.StandardLogger()
}
@@ -527,7 +528,7 @@ func (p *ReverseProxy) isSelfTargetLoop(r *http.Request, target *url.URL) bool {
if !types.IsOverlayOrigin(r.Context()) {
return false
}
srcIP := extractHostIP(r.RemoteAddr)
srcIP := trustedproxy.ExtractHostIP(r.RemoteAddr)
if !srcIP.IsValid() {
return false
}
@@ -578,9 +579,9 @@ func (p *ReverseProxy) rewriteFunc(target *url.URL, matchedPath string, passHost
stampNetBirdIdentity(r)
clientIP := extractHostIP(r.In.RemoteAddr)
clientIP := trustedproxy.ExtractHostIP(r.In.RemoteAddr)
if isTrustedAddr(clientIP, p.trustedProxies) {
if p.trustedProxies.Contains(clientIP) {
p.setTrustedForwardingHeaders(r, clientIP)
} else {
p.setUntrustedForwardingHeaders(r, clientIP)
@@ -664,7 +665,7 @@ func (p *ReverseProxy) setTrustedForwardingHeaders(r *httputil.ProxyRequest, cli
if realIP := r.In.Header.Get("X-Real-IP"); realIP != "" {
r.Out.Header.Set("X-Real-IP", realIP)
} else {
resolved := ResolveClientIP(r.In.RemoteAddr, r.In.Header.Get("X-Forwarded-For"), p.trustedProxies)
resolved := p.trustedProxies.ResolveClientIP(r.In.RemoteAddr, r.In.Header.Get("X-Forwarded-For"))
r.Out.Header.Set("X-Real-IP", resolved.String())
}

View File

@@ -23,6 +23,7 @@ import (
"github.com/netbirdio/netbird/proxy/internal/roundtrip"
"github.com/netbirdio/netbird/proxy/internal/types"
"github.com/netbirdio/netbird/proxy/web"
"github.com/netbirdio/netbird/trustedproxy"
)
func TestRewriteFunc_HostRewriting(t *testing.T) {
@@ -302,7 +303,7 @@ func TestExtractHostIP(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.expected, extractHostIP(tt.remoteAddr))
assert.Equal(t, tt.expected, trustedproxy.ExtractHostIP(tt.remoteAddr))
})
}
}
@@ -330,7 +331,7 @@ func TestExtractForwardedPort(t *testing.T) {
func TestRewriteFunc_TrustedProxy(t *testing.T) {
target, _ := url.Parse("http://backend.internal:8080")
trusted := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}
trusted := trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")})
t.Run("appends to X-Forwarded-For", func(t *testing.T) {
p := &ReverseProxy{forwardedProto: "auto", trustedProxies: trusted}

View File

@@ -1,81 +0,0 @@
package proxy
import (
"net/netip"
"strings"
)
// IsTrustedProxy checks if the given IP string falls within any of the trusted prefixes.
func IsTrustedProxy(ipStr string, trusted []netip.Prefix) bool {
addr, err := netip.ParseAddr(ipStr)
if err != nil || len(trusted) == 0 {
return false
}
return isTrustedAddr(addr.Unmap(), trusted)
}
// ResolveClientIP extracts the real client IP from X-Forwarded-For using the trusted proxy list.
// It walks the XFF chain right-to-left, skipping IPs that match trusted prefixes.
// The first untrusted IP is the real client.
//
// If the trusted list is empty or remoteAddr is not trusted, it returns the
// remoteAddr IP directly (ignoring any forwarding headers).
func ResolveClientIP(remoteAddr, xff string, trusted []netip.Prefix) netip.Addr {
remoteIP := extractHostIP(remoteAddr)
if len(trusted) == 0 || !isTrustedAddr(remoteIP, trusted) {
return remoteIP
}
if xff == "" {
return remoteIP
}
parts := strings.Split(xff, ",")
for i := len(parts) - 1; i >= 0; i-- {
ip := strings.TrimSpace(parts[i])
if ip == "" {
continue
}
addr, err := netip.ParseAddr(ip)
if err != nil {
continue
}
addr = addr.Unmap()
if !isTrustedAddr(addr, trusted) {
return addr
}
}
// All IPs in XFF are trusted; return the leftmost as best guess.
if first := strings.TrimSpace(parts[0]); first != "" {
if addr, err := netip.ParseAddr(first); err == nil {
return addr.Unmap()
}
}
return remoteIP
}
// extractHostIP parses the IP from a host:port string and returns it unmapped.
func extractHostIP(hostPort string) netip.Addr {
if ap, err := netip.ParseAddrPort(hostPort); err == nil {
return ap.Addr().Unmap()
}
if addr, err := netip.ParseAddr(hostPort); err == nil {
return addr.Unmap()
}
return netip.Addr{}
}
// isTrustedAddr checks if the given address falls within any of the trusted prefixes.
func isTrustedAddr(addr netip.Addr, trusted []netip.Prefix) bool {
if !addr.IsValid() {
return false
}
for _, prefix := range trusted {
if prefix.Contains(addr) {
return true
}
}
return false
}

View File

@@ -1,129 +0,0 @@
package proxy
import (
"net/netip"
"testing"
"github.com/stretchr/testify/assert"
)
func TestIsTrustedProxy(t *testing.T) {
trusted := []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("192.168.1.0/24"),
netip.MustParsePrefix("fd00::/8"),
}
tests := []struct {
name string
ip string
trusted []netip.Prefix
want bool
}{
{"empty trusted list", "10.0.0.1", nil, false},
{"IP within /8 prefix", "10.1.2.3", trusted, true},
{"IP within /24 prefix", "192.168.1.100", trusted, true},
{"IP outside all prefixes", "203.0.113.50", trusted, false},
{"boundary IP just outside prefix", "192.168.2.1", trusted, false},
{"unparsable IP", "not-an-ip", trusted, false},
{"IPv6 in trusted range", "fd00::1", trusted, true},
{"IPv6 outside range", "2001:db8::1", trusted, false},
{"empty string", "", trusted, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, IsTrustedProxy(tt.ip, tt.trusted))
})
}
}
func TestResolveClientIP(t *testing.T) {
trusted := []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("172.16.0.0/12"),
}
tests := []struct {
name string
remoteAddr string
xff string
trusted []netip.Prefix
want netip.Addr
}{
{
name: "empty trusted list returns RemoteAddr",
remoteAddr: "203.0.113.50:9999",
xff: "1.2.3.4",
trusted: nil,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "untrusted RemoteAddr ignores XFF",
remoteAddr: "203.0.113.50:9999",
xff: "1.2.3.4, 10.0.0.1",
trusted: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "trusted RemoteAddr with single client in XFF",
remoteAddr: "10.0.0.1:5000",
xff: "203.0.113.50",
trusted: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "trusted RemoteAddr walks past trusted entries in XFF",
remoteAddr: "10.0.0.1:5000",
xff: "203.0.113.50, 10.0.0.2, 172.16.0.5",
trusted: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "trusted RemoteAddr with empty XFF falls back to RemoteAddr",
remoteAddr: "10.0.0.1:5000",
xff: "",
trusted: trusted,
want: netip.MustParseAddr("10.0.0.1"),
},
{
name: "all XFF IPs trusted returns leftmost",
remoteAddr: "10.0.0.1:5000",
xff: "10.0.0.2, 172.16.0.1, 10.0.0.3",
trusted: trusted,
want: netip.MustParseAddr("10.0.0.2"),
},
{
name: "XFF with whitespace",
remoteAddr: "10.0.0.1:5000",
xff: " 203.0.113.50 , 10.0.0.2 ",
trusted: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "XFF with empty segments",
remoteAddr: "10.0.0.1:5000",
xff: "203.0.113.50,,10.0.0.2",
trusted: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "multi-hop with mixed trust",
remoteAddr: "10.0.0.1:5000",
xff: "8.8.8.8, 203.0.113.50, 172.16.0.1",
trusted: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "RemoteAddr without port",
remoteAddr: "10.0.0.1",
xff: "203.0.113.50",
trusted: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, ResolveClientIP(tt.remoteAddr, tt.xff, tt.trusted))
})
}
}

View File

@@ -2,13 +2,13 @@ package proxy
import (
"context"
"net/netip"
"time"
log "github.com/sirupsen/logrus"
"github.com/netbirdio/netbird/client/embed"
"github.com/netbirdio/netbird/proxy/internal/acme"
"github.com/netbirdio/netbird/trustedproxy"
)
// Config bundles every knob the proxy reads at construction time. It mirrors
@@ -83,9 +83,9 @@ type Config struct {
// ForwardedProto overrides the X-Forwarded-Proto value sent to
// backends. Valid values: "auto", "http", "https".
ForwardedProto string
// TrustedProxies is a list of IP prefixes for trusted upstream
// proxies that may set forwarding headers.
TrustedProxies []netip.Prefix
// TrustedProxies is the set of trusted upstream proxies that may set
// forwarding headers.
TrustedProxies *trustedproxy.List
// WireguardPort is the UDP port for the embedded NetBird tunnel.
// Zero asks the OS for a random port.
WireguardPort uint16

View File

@@ -10,12 +10,14 @@ import (
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/trustedproxy"
)
func TestWrapProxyProtocol_OverridesRemoteAddr(t *testing.T) {
srv := &Server{
Logger: log.StandardLogger(),
TrustedProxies: []netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")},
TrustedProxies: trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("127.0.0.1/32")}),
ProxyProtocol: true,
}
@@ -66,7 +68,7 @@ func TestWrapProxyProtocol_OverridesRemoteAddr(t *testing.T) {
func TestProxyProtocolPolicy_TrustedRequires(t *testing.T) {
srv := &Server{
Logger: log.StandardLogger(),
TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
TrustedProxies: trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}),
}
opts := proxyproto.ConnPolicyOptions{
@@ -80,7 +82,7 @@ func TestProxyProtocolPolicy_TrustedRequires(t *testing.T) {
func TestProxyProtocolPolicy_UntrustedIgnores(t *testing.T) {
srv := &Server{
Logger: log.StandardLogger(),
TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
TrustedProxies: trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}),
}
opts := proxyproto.ConnPolicyOptions{
@@ -94,7 +96,7 @@ func TestProxyProtocolPolicy_UntrustedIgnores(t *testing.T) {
func TestProxyProtocolPolicy_InvalidIPRejects(t *testing.T) {
srv := &Server{
Logger: log.StandardLogger(),
TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
TrustedProxies: trustedproxy.FromPrefixes([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}),
}
opts := proxyproto.ConnPolicyOptions{

View File

@@ -67,6 +67,7 @@ import (
"github.com/netbirdio/netbird/proxy/web"
"github.com/netbirdio/netbird/shared/management/domain"
"github.com/netbirdio/netbird/shared/management/proto"
"github.com/netbirdio/netbird/trustedproxy"
"github.com/netbirdio/netbird/util/embeddedroots"
)
@@ -79,19 +80,19 @@ type portRouter struct {
type Server struct {
ctx context.Context
mgmtClient proto.ProxyServiceClient
proxy *proxy.ReverseProxy
netbird *roundtrip.NetBird
acme *acme.Manager
mgmtClient proto.ProxyServiceClient
proxy *proxy.ReverseProxy
netbird *roundtrip.NetBird
acme *acme.Manager
staticCertWatcher *certwatch.Watcher
auth *auth.Middleware
http *http.Server
https *http.Server
debug *http.Server
healthServer *health.Server
healthChecker *health.Checker
meter *proxymetrics.Metrics
accessLog *accesslog.Logger
auth *auth.Middleware
http *http.Server
https *http.Server
debug *http.Server
healthServer *health.Server
healthChecker *health.Checker
meter *proxymetrics.Metrics
accessLog *accesslog.Logger
// middlewareManager drives per-target middleware dispatch. Always
// constructed during boot; an empty registry produces empty chains and
// the reverse-proxy stays on the no-capture fast path.
@@ -99,16 +100,16 @@ type Server struct {
// middlewareRegistry is the source of registered middleware factories.
// Concrete middlewares register themselves through init().
middlewareRegistry *middleware.Registry
mainRouter *nbtcp.Router
mainPort uint16
udpMu sync.Mutex
udpRelays map[types.ServiceID]*udprelay.Relay
udpRelayWg sync.WaitGroup
portMu sync.RWMutex
portRouters map[uint16]*portRouter
svcPorts map[types.ServiceID][]uint16
lastMappings map[types.ServiceID]*proto.ProxyMapping
portRouterWg sync.WaitGroup
mainRouter *nbtcp.Router
mainPort uint16
udpMu sync.Mutex
udpRelays map[types.ServiceID]*udprelay.Relay
udpRelayWg sync.WaitGroup
portMu sync.RWMutex
portRouters map[uint16]*portRouter
svcPorts map[types.ServiceID][]uint16
lastMappings map[types.ServiceID]*proto.ProxyMapping
portRouterWg sync.WaitGroup
// hijackTracker tracks hijacked connections (e.g. WebSocket upgrades)
// so they can be closed during graceful shutdown, since http.Server.Shutdown
@@ -192,10 +193,10 @@ type Server struct {
// ForwardedProto overrides the X-Forwarded-Proto value sent to backends.
// Valid values: "auto" (detect from TLS), "http", "https".
ForwardedProto string
// TrustedProxies is a list of IP prefixes for trusted upstream proxies.
// When set, forwarding headers from these sources are preserved and
// appended to instead of being stripped.
TrustedProxies []netip.Prefix
// TrustedProxies is the set of trusted upstream proxies. When set,
// forwarding headers from these sources are preserved and appended to
// instead of being stripped.
TrustedProxies *trustedproxy.List
// WireguardPort is the port for the NetBird tunnel interface. Use 0
// for a random OS-assigned port. A fixed port only works with
// single-account deployments; multiple accounts will fail to bind
@@ -718,7 +719,7 @@ func (s *Server) wrapProxyProtocol(ln net.Listener) net.Listener {
Listener: ln,
ReadHeaderTimeout: proxyProtoHeaderTimeout,
}
if len(s.TrustedProxies) > 0 {
if !s.TrustedProxies.Empty() {
ppListener.ConnPolicy = s.proxyProtocolPolicy
} else {
s.Logger.Warn("PROXY protocol enabled without trusted proxies; any source may send PROXY headers")
@@ -742,10 +743,8 @@ func (s *Server) proxyProtocolPolicy(opts proxyproto.ConnPolicyOptions) (proxypr
addr = addr.Unmap()
// called per accept
for _, prefix := range s.TrustedProxies {
if prefix.Contains(addr) {
return proxyproto.REQUIRE, nil
}
if s.TrustedProxies.Contains(addr) {
return proxyproto.REQUIRE, nil
}
return proxyproto.IGNORE, nil
}

View File

@@ -1,43 +0,0 @@
package proxy
import (
"fmt"
"net/netip"
"strings"
)
// ParseTrustedProxies parses a comma-separated list of CIDR prefixes or bare IPs
// into a slice of netip.Prefix values suitable for trusted proxy configuration.
// Bare IPs are converted to single-host prefixes (/32 or /128).
func ParseTrustedProxies(raw string) ([]netip.Prefix, error) {
if raw == "" {
return nil, nil
}
parts := strings.Split(raw, ",")
prefixes := make([]netip.Prefix, 0, len(parts))
for _, part := range parts {
part = strings.TrimSpace(part)
if part == "" {
continue
}
prefix, err := netip.ParsePrefix(part)
if err == nil {
prefixes = append(prefixes, prefix)
continue
}
addr, addrErr := netip.ParseAddr(part)
if addrErr != nil {
return nil, fmt.Errorf("parse trusted proxy %q: not a valid CIDR or IP: %w", part, addrErr)
}
bits := 32
if addr.Is6() {
bits = 128
}
prefixes = append(prefixes, netip.PrefixFrom(addr, bits))
}
return prefixes, nil
}

View File

@@ -1,90 +0,0 @@
package proxy
import (
"net/netip"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestParseTrustedProxies(t *testing.T) {
tests := []struct {
name string
raw string
want []netip.Prefix
wantErr bool
}{
{
name: "empty string returns nil",
raw: "",
want: nil,
},
{
name: "single CIDR",
raw: "10.0.0.0/8",
want: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
},
{
name: "single bare IPv4",
raw: "1.2.3.4",
want: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32")},
},
{
name: "single bare IPv6",
raw: "::1",
want: []netip.Prefix{netip.MustParsePrefix("::1/128")},
},
{
name: "comma-separated CIDRs",
raw: "10.0.0.0/8, 192.168.1.0/24",
want: []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("192.168.1.0/24"),
},
},
{
name: "mixed CIDRs and bare IPs",
raw: "10.0.0.0/8, 1.2.3.4, fd00::/8",
want: []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("1.2.3.4/32"),
netip.MustParsePrefix("fd00::/8"),
},
},
{
name: "whitespace around entries",
raw: " 10.0.0.0/8 , 192.168.0.0/16 ",
want: []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("192.168.0.0/16"),
},
},
{
name: "trailing comma produces no extra entry",
raw: "10.0.0.0/8,",
want: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
},
{
name: "invalid entry",
raw: "not-an-ip",
wantErr: true,
},
{
name: "partially invalid",
raw: "10.0.0.0/8, garbage",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ParseTrustedProxies(tt.raw)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tt.want, got)
})
}
}

View File

@@ -24,6 +24,7 @@ import (
"github.com/netbirdio/netbird/shared/metrics"
"github.com/netbirdio/netbird/shared/relay/auth"
"github.com/netbirdio/netbird/stun"
"github.com/netbirdio/netbird/trustedproxy"
"github.com/netbirdio/netbird/util"
)
@@ -45,6 +46,9 @@ type Config struct {
LogLevel string
LogFile string
HealthcheckListenAddress string
// TrustedProxies is a comma-separated list of upstream proxy CIDRs/IPs whose
// X-Real-Ip/X-Real-Port headers are trusted. Empty means never trust these headers.
TrustedProxies string
// STUN server configuration
EnableSTUN bool
STUNPorts []int
@@ -116,6 +120,7 @@ func init() {
rootCmd.PersistentFlags().StringVar(&cobraConfig.LogLevel, "log-level", "info", "log level")
rootCmd.PersistentFlags().StringVar(&cobraConfig.LogFile, "log-file", "console", "log file")
rootCmd.PersistentFlags().StringVarP(&cobraConfig.HealthcheckListenAddress, "health-listen-address", "H", ":9000", "listen address of healthcheck server")
rootCmd.PersistentFlags().StringVar(&cobraConfig.TrustedProxies, "trusted-proxies", "", "comma-separated list of upstream proxy CIDRs or IPs whose X-Real-Ip/X-Real-Port headers are trusted; leave empty to always use the direct connection address")
rootCmd.PersistentFlags().BoolVar(&cobraConfig.EnableSTUN, "enable-stun", false, "enable embedded STUN server")
rootCmd.PersistentFlags().IntSliceVar(&cobraConfig.STUNPorts, "stun-ports", []int{3478}, "ports for the embedded STUN server (can be specified multiple times or comma-separated)")
rootCmd.PersistentFlags().StringVar(&cobraConfig.STUNLogLevel, "stun-log-level", "info", "log level for STUN server (panic, fatal, error, warn, info, debug, trace)")
@@ -155,8 +160,15 @@ func execute(cmd *cobra.Command, args []string) error {
return fmt.Errorf("setup metrics: %v", err)
}
trustedProxies, err := trustedproxy.Parse(cobraConfig.TrustedProxies)
if err != nil {
log.Debugf("failed to parse trusted proxies: %s", err)
return fmt.Errorf("failed to parse trusted proxies: %s", err)
}
srvListenerCfg := server.ListenerConfig{
Address: cobraConfig.ListenAddress,
Address: cobraConfig.ListenAddress,
TrustedProxies: trustedProxies,
}
tlsConfig, tlsSupport, err := handleTLSConfig(cobraConfig)

View File

@@ -15,6 +15,7 @@ import (
"github.com/netbirdio/netbird/relay/protocol"
relaylistener "github.com/netbirdio/netbird/relay/server/listener"
"github.com/netbirdio/netbird/shared/relay"
"github.com/netbirdio/netbird/trustedproxy"
)
const (
@@ -27,6 +28,9 @@ type Listener struct {
Address string
// TLSConfig is the TLS configuration for the server.
TLSConfig *tls.Config
// TrustedProxies is the set of upstream proxies whose X-Real-Ip/X-Real-Port
// headers are trusted. Headers from any other immediate peer are ignored.
TrustedProxies *trustedproxy.List
server *http.Server
acceptFn func(conn relaylistener.Conn)
@@ -75,7 +79,7 @@ func (l *Listener) Shutdown(ctx context.Context) error {
}
func (l *Listener) onAccept(w http.ResponseWriter, r *http.Request) {
connRemoteAddr := remoteAddr(r)
connRemoteAddr := remoteAddr(r, l.TrustedProxies)
acceptOptions := &websocket.AcceptOptions{
OriginPatterns: []string{"*"},
@@ -102,9 +106,17 @@ func (l *Listener) onAccept(w http.ResponseWriter, r *http.Request) {
l.acceptFn(conn)
}
func remoteAddr(r *http.Request) string {
if r.Header.Get("X-Real-Ip") == "" || r.Header.Get("X-Real-Port") == "" {
func remoteAddr(r *http.Request, trustedProxies *trustedproxy.List) string {
realIP := r.Header.Get("X-Real-Ip")
realPort := r.Header.Get("X-Real-Port")
if realIP == "" || realPort == "" {
return r.RemoteAddr
}
return net.JoinHostPort(r.Header.Get("X-Real-Ip"), r.Header.Get("X-Real-Port"))
if !trustedProxies.IsTrusted(r.RemoteAddr) {
log.Debugf("ignoring X-Real-Ip header from untrusted peer %s", r.RemoteAddr)
return r.RemoteAddr
}
return net.JoinHostPort(realIP, realPort)
}

View File

@@ -15,14 +15,17 @@ import (
"github.com/netbirdio/netbird/relay/server/listener/quic"
"github.com/netbirdio/netbird/relay/server/listener/ws"
quictls "github.com/netbirdio/netbird/shared/relay/tls"
"github.com/netbirdio/netbird/trustedproxy"
)
// ListenerConfig is the configuration for the listener.
// Address: the address to bind the listener to. It could be an address behind a reverse proxy.
// TLSConfig: the TLS configuration for the listener.
// TrustedProxies: upstream proxy prefixes whose forwarding headers (X-Real-Ip/X-Real-Port) are trusted.
type ListenerConfig struct {
Address string
TLSConfig *tls.Config
Address string
TLSConfig *tls.Config
TrustedProxies *trustedproxy.List
}
// Server is the main entry point for the relay server.
@@ -62,8 +65,9 @@ func NewServer(config Config) (*Server, error) {
// Listen starts the relay server.
func (r *Server) Listen(cfg ListenerConfig) error {
wSListener := &ws.Listener{
Address: cfg.Address,
TLSConfig: cfg.TLSConfig,
Address: cfg.Address,
TLSConfig: cfg.TLSConfig,
TrustedProxies: cfg.TrustedProxies,
}
r.listenerMux.Lock()

View File

@@ -0,0 +1,132 @@
package trustedproxy
import (
"fmt"
"net/netip"
"strings"
)
// List holds a parsed set of trusted upstream proxy prefixes and answers trust
// questions against it. The zero value (and a nil *List) is a valid, empty list
// that never trusts any address, so callers can use it without a nil check.
type List struct {
prefixes []netip.Prefix
}
// Parse parses a comma-separated list of CIDR prefixes or bare IPs into a List.
// Bare IPs are converted to single-host prefixes (/32 or /128). An empty input
// yields an empty List that trusts nothing.
func Parse(raw string) (*List, error) {
if raw == "" {
return &List{}, nil
}
parts := strings.Split(raw, ",")
prefixes := make([]netip.Prefix, 0, len(parts))
for _, part := range parts {
part = strings.TrimSpace(part)
if part == "" {
continue
}
prefix, err := netip.ParsePrefix(part)
if err == nil {
prefixes = append(prefixes, prefix)
continue
}
addr, addrErr := netip.ParseAddr(part)
if addrErr != nil {
return nil, fmt.Errorf("parse trusted proxy %q: not a valid CIDR or IP: %w", part, addrErr)
}
bits := 32
if addr.Is6() {
bits = 128
}
prefixes = append(prefixes, netip.PrefixFrom(addr, bits))
}
return &List{prefixes: prefixes}, nil
}
// FromPrefixes wraps an already-parsed set of prefixes in a List.
func FromPrefixes(prefixes []netip.Prefix) *List {
return &List{prefixes: prefixes}
}
// Empty reports whether the list contains no prefixes.
func (l *List) Empty() bool {
return l == nil || len(l.prefixes) == 0
}
// IsTrusted reports whether the given host:port or bare IP falls within the list.
func (l *List) IsTrusted(remoteAddr string) bool {
if l.Empty() {
return false
}
return l.Contains(ExtractHostIP(remoteAddr))
}
// Contains reports whether the given address falls within any trusted prefix.
func (l *List) Contains(addr netip.Addr) bool {
if l.Empty() || !addr.IsValid() {
return false
}
for _, prefix := range l.prefixes {
if prefix.Contains(addr) {
return true
}
}
return false
}
// ResolveClientIP extracts the real client IP from X-Forwarded-For using the
// list. It walks the XFF chain right-to-left, skipping IPs that match trusted
// prefixes; the first untrusted IP is the real client. If the list is empty or
// remoteAddr is not trusted, it returns the remoteAddr IP directly, ignoring any
// forwarding headers.
func (l *List) ResolveClientIP(remoteAddr, xff string) netip.Addr {
remoteIP := ExtractHostIP(remoteAddr)
if l.Empty() || !l.Contains(remoteIP) {
return remoteIP
}
if xff == "" {
return remoteIP
}
parts := strings.Split(xff, ",")
for i := len(parts) - 1; i >= 0; i-- {
ip := strings.TrimSpace(parts[i])
if ip == "" {
continue
}
addr, err := netip.ParseAddr(ip)
if err != nil {
continue
}
addr = addr.Unmap()
if !l.Contains(addr) {
return addr
}
}
if first := strings.TrimSpace(parts[0]); first != "" {
if addr, err := netip.ParseAddr(first); err == nil {
return addr.Unmap()
}
}
return remoteIP
}
// ExtractHostIP parses the IP from a host:port string and returns it unmapped.
func ExtractHostIP(hostPort string) netip.Addr {
if ap, err := netip.ParseAddrPort(hostPort); err == nil {
return ap.Addr().Unmap()
}
if addr, err := netip.ParseAddr(hostPort); err == nil {
return addr.Unmap()
}
return netip.Addr{}
}

View File

@@ -0,0 +1,216 @@
package trustedproxy
import (
"net/netip"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestParse(t *testing.T) {
tests := []struct {
name string
raw string
want []netip.Prefix
wantErr bool
}{
{
name: "empty string returns empty list",
raw: "",
want: nil,
},
{
name: "single CIDR",
raw: "10.0.0.0/8",
want: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
},
{
name: "single bare IPv4",
raw: "1.2.3.4",
want: []netip.Prefix{netip.MustParsePrefix("1.2.3.4/32")},
},
{
name: "single bare IPv6",
raw: "::1",
want: []netip.Prefix{netip.MustParsePrefix("::1/128")},
},
{
name: "comma-separated CIDRs",
raw: "10.0.0.0/8, 192.168.1.0/24",
want: []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("192.168.1.0/24"),
},
},
{
name: "mixed CIDRs and bare IPs",
raw: "10.0.0.0/8, 1.2.3.4, fd00::/8",
want: []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("1.2.3.4/32"),
netip.MustParsePrefix("fd00::/8"),
},
},
{
name: "whitespace around entries",
raw: " 10.0.0.0/8 , 192.168.0.0/16 ",
want: []netip.Prefix{
netip.MustParsePrefix("10.0.0.0/8"),
netip.MustParsePrefix("192.168.0.0/16"),
},
},
{
name: "trailing comma produces no extra entry",
raw: "10.0.0.0/8,",
want: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
},
{
name: "invalid entry",
raw: "not-an-ip",
wantErr: true,
},
{
name: "partially invalid",
raw: "10.0.0.0/8, garbage",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := Parse(tt.raw)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tt.want, got.prefixes)
})
}
}
func TestListIsTrusted(t *testing.T) {
list, err := Parse("10.0.0.0/8, 192.168.1.0/24, fd00::/8")
require.NoError(t, err)
tests := []struct {
name string
addr string
list *List
want bool
}{
{"nil list", "10.0.0.1", nil, false},
{"empty list", "10.0.0.1", &List{}, false},
{"IP within /8 prefix", "10.1.2.3", list, true},
{"IP within /24 prefix", "192.168.1.100", list, true},
{"IP outside all prefixes", "203.0.113.50", list, false},
{"boundary IP just outside prefix", "192.168.2.1", list, false},
{"unparsable IP", "not-an-ip", list, false},
{"IPv6 in trusted range", "fd00::1", list, true},
{"IPv6 outside range", "2001:db8::1", list, false},
{"empty string", "", list, false},
{"host:port within prefix", "10.1.2.3:9999", list, true},
{"host:port outside prefix", "203.0.113.50:9999", list, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.list.IsTrusted(tt.addr))
})
}
}
func TestListResolveClientIP(t *testing.T) {
trusted, err := Parse("10.0.0.0/8, 172.16.0.0/12")
require.NoError(t, err)
tests := []struct {
name string
remoteAddr string
xff string
list *List
want netip.Addr
}{
{
name: "empty list returns RemoteAddr",
remoteAddr: "203.0.113.50:9999",
xff: "1.2.3.4",
list: &List{},
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "nil list returns RemoteAddr",
remoteAddr: "203.0.113.50:9999",
xff: "1.2.3.4",
list: nil,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "untrusted RemoteAddr ignores XFF",
remoteAddr: "203.0.113.50:9999",
xff: "1.2.3.4, 10.0.0.1",
list: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "trusted RemoteAddr with single client in XFF",
remoteAddr: "10.0.0.1:5000",
xff: "203.0.113.50",
list: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "trusted RemoteAddr walks past trusted entries in XFF",
remoteAddr: "10.0.0.1:5000",
xff: "203.0.113.50, 10.0.0.2, 172.16.0.5",
list: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "trusted RemoteAddr with empty XFF falls back to RemoteAddr",
remoteAddr: "10.0.0.1:5000",
xff: "",
list: trusted,
want: netip.MustParseAddr("10.0.0.1"),
},
{
name: "all XFF IPs trusted returns leftmost",
remoteAddr: "10.0.0.1:5000",
xff: "10.0.0.2, 172.16.0.1, 10.0.0.3",
list: trusted,
want: netip.MustParseAddr("10.0.0.2"),
},
{
name: "XFF with whitespace",
remoteAddr: "10.0.0.1:5000",
xff: " 203.0.113.50 , 10.0.0.2 ",
list: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "XFF with empty segments",
remoteAddr: "10.0.0.1:5000",
xff: "203.0.113.50,,10.0.0.2",
list: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "multi-hop with mixed trust",
remoteAddr: "10.0.0.1:5000",
xff: "8.8.8.8, 203.0.113.50, 172.16.0.1",
list: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
{
name: "RemoteAddr without port",
remoteAddr: "10.0.0.1",
xff: "203.0.113.50",
list: trusted,
want: netip.MustParseAddr("203.0.113.50"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.list.ResolveClientIP(tt.remoteAddr, tt.xff))
})
}
}