mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-12 08:29:06 +02:00
[client] Treat a named upload URL as a request to upload in the privilege gate
DebugBundle uploads when either the upload flag or the upload URL is set, but the privilege gate was told only about the flag. With an MDM policy pinning the destination the gate blanks the caller's URL, so a request carrying a URL and no flag reached the empty-URL branch and found upload false: nothing to weaken, let it through. The daemon then uploaded as root with TLS verification disabled on behalf of an unprivileged caller. A client older than the upload flag sends exactly that shape, a URL and nothing else, so this needs no crafted request to reach. The gate now derives the upload intent from the URL rather than trusting the caller's flag, and DebugBundle computes it once so the gate and the local-only early return cannot drift apart.
This commit is contained in:
@@ -27,8 +27,13 @@ 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) {
|
||||
// One source of truth for "this request uploads", used by both the gate and
|
||||
// the local-only early return below. A client older than the upload flag asks
|
||||
// for an upload by naming a URL and nothing else, so the two must not drift.
|
||||
wantsUpload := req.GetUpload() || req.GetUploadURL() != ""
|
||||
|
||||
mdmUploadURL := s.mdmDebugUploadURL()
|
||||
if err := requirePrivilegeForUploadURL(callerCtx, req.GetUploadURL(), req.GetUploadInsecure(), req.GetUpload(), mdmUploadURL != ""); err != nil {
|
||||
if err := requirePrivilegeForUploadURL(callerCtx, req.GetUploadURL(), req.GetUploadInsecure(), wantsUpload, mdmUploadURL != ""); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -42,7 +47,7 @@ func (s *Server) DebugBundle(callerCtx context.Context, req *proto.DebugBundleRe
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if !req.GetUpload() && req.GetUploadURL() == "" {
|
||||
if !wantsUpload {
|
||||
return &proto.DebugBundleResponse{Path: path}, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -58,6 +58,13 @@ func uiLogOpener(id ipcauth.Identity, identified bool) debug.LogOpener {
|
||||
// that was going to the pinned host anyway into a refusal. Transport security
|
||||
// still is gated: relaxing TLS towards the pinned host is a real weakening.
|
||||
func requirePrivilegeForUploadURL(ctx context.Context, rawURL string, insecure, upload, mdmPinned bool) error {
|
||||
// A named URL is itself a request to upload, since DebugBundle uploads when
|
||||
// either the flag or the URL is set and a client older than the flag sends
|
||||
// only the URL. Derived here rather than taken on trust from the caller:
|
||||
// blanking rawURL below would otherwise turn an absent flag into "no upload
|
||||
// to weaken" and let an unprivileged caller relax TLS on a root-run upload.
|
||||
upload = upload || rawURL != ""
|
||||
|
||||
if mdmPinned {
|
||||
rawURL = ""
|
||||
}
|
||||
|
||||
@@ -133,6 +133,12 @@ func TestRequirePrivilegeForUploadURL(t *testing.T) {
|
||||
{name: "custom URL ignored when MDM pins the destination", url: "https://attacker.example/upload-url", mdmPinned: true, unprivOK: true},
|
||||
// Transport security is still the caller's to weaken, pinned or not.
|
||||
{name: "insecure still gated when MDM pins the destination", url: "https://attacker.example/upload-url", insecure: true, mdmPinned: true, rootAlso: true},
|
||||
// A named URL is itself a request to upload: DebugBundle uploads when
|
||||
// either the flag or the URL is set. A client older than the upload flag
|
||||
// sends exactly this shape, URL and no flag, and with MDM blanking the URL
|
||||
// the gate must not read the missing flag as "no upload to weaken".
|
||||
{name: "insecure, URL set, no upload flag, MDM pinned", url: "https://x.example/upload-url", insecure: true, noUpload: true, mdmPinned: true, rootAlso: true},
|
||||
{name: "insecure, URL set, no upload flag", url: "https://x.example/upload-url", insecure: true, noUpload: true, rootAlso: true},
|
||||
{name: "plaintext default host", url: "http://upload.debug.netbird.io/upload-url", invalid: true},
|
||||
{name: "plaintext custom host", url: "http://attacker.example/upload-url", invalid: true},
|
||||
{name: "unsupported scheme", url: "file:///etc/shadow", invalid: true},
|
||||
|
||||
Reference in New Issue
Block a user