mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-14 18:59:08 +02:00
[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.
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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"))
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user