[management,client] Take the debug-bundle upload destination from management

The debug-bundle paths that upload without a human picking a destination
compiled the vendor endpoint in: the mobile clients and the desktop UI hold
`https://upload.debug.netbird.io/upload-url` as a constant, the CLI defaults its
flag to it, and the remote job falls back to it when nothing else is set. A
self-hosted deployment therefore shipped peer logs, routes, DNS and firewall
state to NetBird-run infrastructure without its operator ever configuring that,
and had no way to point those paths anywhere else. #7147 and #7153 gave the
remote job a per-job URL and an MDM override, but neither reaches the mobile,
UI or CLI paths, and both fail open when unset.

Publish the destination from the management server instead, on the channel that
already carries stun/turn/signal/relay/flow/metrics:

- `NetbirdConfig.debug.upload_url`, sourced from the new account setting
  `debug_bundle_upload_url` (REST + dashboard) and falling back to the new
  `DebugUpload.URL` in the management server config, which a self-hosted install
  can set once so a fresh account is not left on the vendor default. Both are
  validated as https-with-host where they are written; a change fans out to
  connected peers rather than waiting for the next login.
- One resolver on the client, `debug.ResolveUploadURL`, used by every path:
  MDM override > explicitly named URL > destination published by management >
  the NetBird service, but only for a peer enrolled with NetBird's cloud.
  Anything else fails closed with ErrNoUploadDestination and the bundle stays
  local, which is the behaviour change: a self-hosted deployment that names no
  upload service no longer uploads at all.
- The engine keeps the published value (`Engine.DebugUploadURL`) so the bundle
  paths, which run off the engine loop, do not have to read it back out of the
  opt-in sync-response store.
- The daemon request grows `upload`, so "upload to wherever this deployment
  says" is expressible; an empty `uploadURL` no longer has to mean "no upload".
  The privilege gate is unchanged and still applies only to a URL the local
  caller named — a destination published by management is the operator naming
  their own service.
- The desktop UI stops carrying a vendor URL of its own and sends the intent.

Reported privately as GHSA-hf99-43rj-h577.
This commit is contained in:
riccardom
2026-09-10 16:38:41 +02:00
parent 08718d072c
commit 9f6d17b9e8
34 changed files with 1923 additions and 1319 deletions
+41
View File
@@ -0,0 +1,41 @@
package debug
import (
"errors"
"github.com/netbirdio/netbird/client/internal/metrics"
"github.com/netbirdio/netbird/upload-server/types"
)
// ErrNoUploadDestination reports that a bundle has nowhere to go: the
// management server of this deployment publishes no upload service, and the
// peer is not enrolled with NetBird's cloud either. A debug bundle carries the
// peer's logs, routes, DNS and firewall state, so the default is to keep it
// inside the operator's control sphere rather than fall back to the service
// NetBird runs.
var ErrNoUploadDestination = errors.New("this deployment publishes no debug bundle upload service; set it on the account settings or in the management server config, or pass an explicit upload URL")
// ResolveUploadURL decides where a debug bundle may be uploaded.
//
// requested is a destination a caller named explicitly (a CLI flag, the daemon
// request); it always wins, and the callers that accept one gate it separately.
// published is what the management server of this deployment advertises, which
// the engine holds (Engine.DebugUploadURL). With neither, only a peer enrolled
// with NetBird's cloud falls back to the service NetBird runs — for anyone else
// that would carry the bundle out of the deployment the operator controls, so it
// fails closed with ErrNoUploadDestination.
func ResolveUploadURL(requested, published, managementURL string) (string, error) {
if requested != "" {
return requested, nil
}
if published != "" {
return published, nil
}
if metrics.DetermineDeploymentType(managementURL) == metrics.DeploymentTypeCloud {
return types.DefaultBundleURL, nil
}
return "", ErrNoUploadDestination
}
+86
View File
@@ -0,0 +1,86 @@
package debug
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/upload-server/types"
)
func TestResolveUploadURL(t *testing.T) {
const (
cloudMgm = "https://api.netbird.io:443"
selfHostedMgm = "https://netbird.example.com:33073"
operatorURL = "https://upload.example.com/upload-url"
requestedURL = "https://requested.example.com/upload-url"
)
tests := []struct {
name string
requested string
published string
managementURL string
want string
wantErr bool
}{
{
name: "requested wins over published",
requested: requestedURL,
published: operatorURL,
managementURL: selfHostedMgm,
want: requestedURL,
},
{
name: "requested wins on cloud too",
requested: requestedURL,
managementURL: cloudMgm,
want: requestedURL,
},
{
name: "published used when nothing requested",
published: operatorURL,
managementURL: selfHostedMgm,
want: operatorURL,
},
{
// A cloud deployment publishing its own destination must not be
// overridden by the compiled-in default.
name: "published wins over the cloud fallback",
published: operatorURL,
managementURL: cloudMgm,
want: operatorURL,
},
{
name: "cloud falls back to the NetBird service",
managementURL: cloudMgm,
want: types.DefaultBundleURL,
},
{
// The whole point of GHSA-hf99-43rj-h577: no silent hop to a
// vendor-controlled destination.
name: "self-hosted with no destination fails closed",
managementURL: selfHostedMgm,
wantErr: true,
},
{
name: "unknown management URL fails closed",
managementURL: "",
wantErr: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got, err := ResolveUploadURL(tc.requested, tc.published, tc.managementURL)
if tc.wantErr {
require.ErrorIs(t, err, ErrNoUploadDestination)
assert.Empty(t, got)
return
}
require.NoError(t, err)
assert.Equal(t, tc.want, got)
})
}
}
+43 -4
View File
@@ -14,6 +14,7 @@ import (
"sort"
"strings"
"sync"
"sync/atomic"
"time"
"github.com/hashicorp/go-multierror"
@@ -226,6 +227,14 @@ type Engine struct {
TURNs []*stun.URI
stunTurn icemaker.StunTurn
// debugUploadURL is the debug-bundle upload service the management server
// publishes for this deployment, refreshed on every NetbirdConfig update.
// Atomic because the bundle paths (remote job, daemon RPC, mobile SDK) read
// it off the engine loop. Empty when the deployment publishes none, which is
// what makes a self-hosted peer keep its bundle local instead of shipping it
// to the upload service NetBird runs.
debugUploadURL atomic.Pointer[string]
clientCtx context.Context
clientCancel context.CancelFunc
@@ -1144,6 +1153,8 @@ func (e *Engine) updateNetbirdConfig(wCfg *mgmProto.NetbirdConfig) error {
e.handleMetricsUpdate(wCfg.GetMetrics())
e.handleDebugUploadUpdate(wCfg.GetDebug())
if err := e.PopulateNetbirdConfig(wCfg, nil); err != nil {
log.Warnf("Failed to update DNS server config: %v", err)
}
@@ -1221,6 +1232,26 @@ func (e *Engine) handleMetricsUpdate(config *mgmProto.MetricsConfig) {
e.clientMetrics.UpdatePushFromMgm(e.metricsCtx, config.GetEnabled())
}
// handleDebugUploadUpdate records the debug-bundle destination the management
// server published. A nil DebugConfig clears it: a management server that stops
// publishing a destination must take it away from the peer, not leave the peer
// uploading to a host the operator has since removed.
func (e *Engine) handleDebugUploadUpdate(config *mgmProto.DebugConfig) {
url := config.GetUploadUrl()
e.debugUploadURL.Store(&url)
}
// DebugUploadURL returns the debug-bundle upload service the management server
// published, or empty when it published none or the engine never synced. The
// callers treat empty as "no destination from this deployment" and fail closed
// unless the peer is enrolled with NetBird's cloud; see debug.ResolveUploadURL.
func (e *Engine) DebugUploadURL() string {
if url := e.debugUploadURL.Load(); url != nil {
return *url
}
return ""
}
func toFlowLoggerConfig(config *mgmProto.FlowConfig) (*nftypes.FlowConfig, error) {
if config.GetInterval() == nil {
return nil, errors.New("flow interval is nil")
@@ -1428,9 +1459,17 @@ func (e *Engine) handleBundle(params *mgmProto.BundleParameters) (*mgmProto.JobR
params.GetAnonymize(), params.GetAnonymizeLevel(), params.GetLogFileCount(), params.GetBundleFor(), params.GetBundleForTime())
log.Debugf("remote debug bundle request parameters: %s", params.String())
syncResponse, err := e.GetLatestSyncResponse()
if err != nil {
log.Warnf("get latest sync response: %v", err)
}
// Resolve the upload destination: an MDM override, when set, takes
// precedence over the management-supplied URL. Both are validated the same
// way; an empty result falls back to the default upload server downstream.
// precedence over the job's URL. Both are validated the same way. With
// neither, the destination this deployment publishes is used, and only a
// peer enrolled with NetBird's cloud falls back to the service NetBird runs
// — a self-hosted deployment that named no upload service gets no upload
// rather than one that leaves the operator's control sphere.
uploadURL := params.GetUploadUrl()
if override := e.config.ProfileConfig.DebugBundleUploadURL; override != "" {
log.Infof("using MDM debug bundle upload URL override instead of the management-supplied value")
@@ -1440,9 +1479,9 @@ func (e *Engine) handleBundle(params *mgmProto.BundleParameters) (*mgmProto.JobR
return nil, err
}
syncResponse, err := e.GetLatestSyncResponse()
uploadURL, err = debug.ResolveUploadURL(uploadURL, e.DebugUploadURL(), e.config.ProfileConfig.ManagementURL.String())
if err != nil {
log.Warnf("get latest sync response: %v", err)
return nil, err
}
bundleDeps := debug.GeneratorDependencies{
+22 -1
View File
@@ -5,6 +5,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
mgmProto "github.com/netbirdio/netbird/shared/management/proto"
)
// TestValidateBundleUploadURL covers the sanity check applied to a
@@ -15,7 +17,7 @@ func TestValidateBundleUploadURL(t *testing.T) {
raw string
wantErr bool
}{
{name: "empty falls back to default", raw: ""},
{name: "empty defers to the deployment destination", raw: ""},
{name: "https with host", raw: "https://upload.debug.netbird.io/upload"},
{name: "https self-hosted host", raw: "https://upload.example.com"},
{name: "plaintext rejected", raw: "http://upload.example.com", wantErr: true},
@@ -34,3 +36,22 @@ func TestValidateBundleUploadURL(t *testing.T) {
})
}
}
// TestEngineDebugUploadURL covers the destination the management server
// publishes: the engine keeps the last value it saw so the bundle paths, which
// run off the engine loop, do not have to re-read a sync response.
func TestEngineDebugUploadURL(t *testing.T) {
e := &Engine{}
assert.Empty(t, e.DebugUploadURL(), "a peer that never synced publishes no destination")
e.handleDebugUploadUpdate(nil)
assert.Empty(t, e.DebugUploadURL(), "a management server predating the field publishes none")
e.handleDebugUploadUpdate(&mgmProto.DebugConfig{UploadUrl: "https://upload.example.com/upload-url"})
assert.Equal(t, "https://upload.example.com/upload-url", e.DebugUploadURL())
// An operator that removes the destination must take it away from the peer,
// not leave it uploading to a host that no longer exists.
e.handleDebugUploadUpdate(&mgmProto.DebugConfig{})
assert.Empty(t, e.DebugUploadURL())
}