From 02d88fdeb06495479bb0c81a5a7d82685ce9c134 Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 14 Sep 2026 09:18:02 +0200 Subject: [PATCH] [client] Refuse --upload-bundle-insecure only when an upload is requested A request with no URL, no upload and uploadInsecure set was denied, although uploadInsecure has no effect on a local-only bundle: there is no destination to weaken, and the caller only wanted the file on disk. Pass the upload intent into the gate and apply the empty-URL branch only when the request asks to upload. Reported by cubic on #7514. --- client/server/debug.go | 2 +- client/server/debug_gate.go | 16 +++++++++------- client/server/debug_gate_test.go | 7 +++++-- client/server/server_privileged_test.go | 2 +- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/client/server/debug.go b/client/server/debug.go index a01e6a319..d1315d3d3 100644 --- a/client/server/debug.go +++ b/client/server/debug.go @@ -27,7 +27,7 @@ import ( // DebugBundle creates a debug bundle and returns the location. func (s *Server) DebugBundle(callerCtx context.Context, req *proto.DebugBundleRequest) (resp *proto.DebugBundleResponse, err error) { - if err := requirePrivilegeForUploadURL(callerCtx, req.GetUploadURL(), req.GetUploadInsecure()); err != nil { + if err := requirePrivilegeForUploadURL(callerCtx, req.GetUploadURL(), req.GetUploadInsecure(), req.GetUpload()); err != nil { return nil, err } diff --git a/client/server/debug_gate.go b/client/server/debug_gate.go index a75d574cf..ba473c397 100644 --- a/client/server/debug_gate.go +++ b/client/server/debug_gate.go @@ -50,14 +50,16 @@ func uiLogOpener(id ipcauth.Identity, identified bool) debug.LogOpener { // // insecure relaxes transport security (http, or an untrusted TLS certificate) // for a self-hosted server. It weakens a root-privileged upload, so it is -// refused for an unprivileged caller regardless of the host. -func requirePrivilegeForUploadURL(ctx context.Context, rawURL string, insecure bool) error { +// refused for an unprivileged caller regardless of the host. upload says whether +// the request asks for an upload at all; without one there is nothing to weaken. +func requirePrivilegeForUploadURL(ctx context.Context, rawURL string, insecure, upload bool) error { if rawURL == "" { - // An empty URL is not "no upload": the daemon then resolves the - // destination the management server published. Relaxing TLS on the way - // there exposes the bundle exactly as naming the host outright would, so - // it needs the same privilege. - if insecure { + // An empty URL with upload requested is not "no upload": the daemon then + // resolves the destination the management server published. Relaxing TLS + // on the way there exposes the bundle exactly as naming the host outright + // would, so it needs the same privilege. Without an upload there is no + // destination to weaken and a local-only bundle must not be refused. + if insecure && upload { return denyPrivileged(ctx, "uploading a debug bundle without transport security (--upload-bundle-insecure)", ipcauth.ElevatedCommand("netbird debug bundle -U --upload-bundle-insecure")) diff --git a/client/server/debug_gate_test.go b/client/server/debug_gate_test.go index c677887eb..a8ce2da0e 100644 --- a/client/server/debug_gate_test.go +++ b/client/server/debug_gate_test.go @@ -111,6 +111,7 @@ func TestRequirePrivilegeForUploadURL(t *testing.T) { name string url string insecure bool + noUpload bool unprivOK bool invalid bool rootAlso bool @@ -119,6 +120,8 @@ func TestRequirePrivilegeForUploadURL(t *testing.T) { // An empty URL resolves to the destination management published, so // relaxing TLS towards it needs the same privilege as naming a host. {name: "insecure with no URL", url: "", insecure: true, rootAlso: true}, + // insecure only weakens an upload; a local-only bundle must still pass. + {name: "insecure with no URL and no upload", url: "", insecure: true, noUpload: true, unprivOK: true}, {name: "default service", url: types.DefaultBundleURL, unprivOK: true}, {name: "default service, other path", url: "https://upload.debug.netbird.io/other", unprivOK: true}, {name: "loopback exfiltration endpoint", url: "https://127.0.0.1:8080/upload-url", rootAlso: true}, @@ -137,7 +140,7 @@ func TestRequirePrivilegeForUploadURL(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - err := requirePrivilegeForUploadURL(userCtx(), tc.url, tc.insecure) + err := requirePrivilegeForUploadURL(userCtx(), tc.url, tc.insecure, !tc.noUpload) switch { case tc.invalid: @@ -153,7 +156,7 @@ func TestRequirePrivilegeForUploadURL(t *testing.T) { } if tc.rootAlso { - assertAllowed(t, requirePrivilegeForUploadURL(rootCtx(), tc.url, tc.insecure)) + assertAllowed(t, requirePrivilegeForUploadURL(rootCtx(), tc.url, tc.insecure, !tc.noUpload)) } }) } diff --git a/client/server/server_privileged_test.go b/client/server/server_privileged_test.go index aa6e99026..ca2218550 100644 --- a/client/server/server_privileged_test.go +++ b/client/server/server_privileged_test.go @@ -10,9 +10,9 @@ import ( "testing" "time" - "go.uber.org/mock/gomock" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel" + "go.uber.org/mock/gomock" "github.com/netbirdio/netbird/management/server/integrations/integrated_validator/validator"