Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions proxy/cmd/proxy/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ var (
certLockMethod string
wgPort int
proxyProtocol bool
preSharedKey string
)

var rootCmd = &cobra.Command{
Expand Down Expand Up @@ -84,6 +85,7 @@ func init() {
rootCmd.Flags().StringVar(&certLockMethod, "cert-lock-method", envStringOrDefault("NB_PROXY_CERT_LOCK_METHOD", "auto"), "Certificate lock method for cross-replica coordination: auto, flock, or k8s-lease")
rootCmd.Flags().IntVar(&wgPort, "wg-port", envIntOrDefault("NB_PROXY_WG_PORT", 0), "WireGuard listen port (0 = random). Fixed port only works with single-account deployments")
rootCmd.Flags().BoolVar(&proxyProtocol, "proxy-protocol", envBoolOrDefault("NB_PROXY_PROXY_PROTOCOL", false), "Enable PROXY protocol on TCP listeners to preserve client IPs behind L4 proxies")
rootCmd.Flags().StringVar(&preSharedKey, "pre-shared-key", envStringOrDefault("NB_PROXY_PRE_SHARED_KEY", ""), "Define a pre-shared key for the tunnel between proxy and peers")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

PSK exposed in process listing via CLI flag.

WireGuard's own tooling (wg(8)) explicitly states that "command line arguments are not considered private on most systems" for pre-shared keys — they are visible to other processes via ps aux and /proc/<pid>/cmdline.

The existing ProxyToken deliberately has no CLI flag (lines 109–112) and is env-var-only precisely to avoid this leak. The WireGuard PSK is a cryptographic secret and warrants the same treatment. NB_PROXY_PRE_SHARED_KEY is already available as a safe alternative.

Consider removing the --pre-shared-key flag entirely and reading the PSK exclusively from the environment variable, mirroring the ProxyToken pattern:

🔒 Proposed fix: env-var-only PSK
-	rootCmd.Flags().StringVar(&preSharedKey, "pre-shared-key", envStringOrDefault("NB_PROXY_PRE_SHARED_KEY", ""), "Define a pre-shared key for the tunnel between proxy and peers")

In runServer:

+	preSharedKey := os.Getenv("NB_PROXY_PRE_SHARED_KEY")
 	srv := proxy.Server{

And remove the package-level preSharedKey variable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@proxy/cmd/proxy/cmd/root.go` at line 88, Remove the CLI flag registration
that exposes the PSK (rootCmd.Flags().StringVar(&preSharedKey, "pre-shared-key",
...)) and stop using the package-level preSharedKey variable; instead read the
pre-shared key exclusively from the environment using
envStringOrDefault("NB_PROXY_PRE_SHARED_KEY", "") at runtime (same pattern used
for ProxyToken) and use that value inside runServer and any other consumers;
also delete the package-level preSharedKey declaration so there is no flag or
global var exposing the secret.

}

// Execute runs the root command.
Expand Down Expand Up @@ -156,6 +158,7 @@ func runServer(cmd *cobra.Command, args []string) error {
CertLockMethod: nbacme.CertLockMethod(certLockMethod),
WireguardPort: wgPort,
ProxyProtocol: proxyProtocol,
PreSharedKey: preSharedKey,
}

ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT)
Expand Down
5 changes: 4 additions & 1 deletion proxy/internal/roundtrip/netbird.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ type NetBird struct {
proxyID string
proxyAddr string
wgPort int
preSharedKey string
logger *log.Logger
mgmtClient managementClient
transportCfg transportConfig
Expand Down Expand Up @@ -234,6 +235,7 @@ func (n *NetBird) createClientEntry(ctx context.Context, accountID types.Account
LogLevel: log.WarnLevel.String(),
BlockInbound: true,
WireguardPort: &n.wgPort,
PreSharedKey: n.preSharedKey,
})
if err != nil {
return nil, fmt.Errorf("create netbird client: %w", err)
Expand Down Expand Up @@ -539,7 +541,7 @@ func (n *NetBird) ListClientsForStartup() map[types.AccountID]*embed.Client {
// NewNetBird creates a new NetBird transport. Set wgPort to 0 for a random
// OS-assigned port. A fixed port only works with single-account deployments;
// multiple accounts will fail to bind the same port.
func NewNetBird(mgmtAddr, proxyID, proxyAddr string, wgPort int, logger *log.Logger, notifier statusNotifier, mgmtClient managementClient) *NetBird {
func NewNetBird(mgmtAddr, proxyID, proxyAddr string, wgPort int, preSharedKey string, logger *log.Logger, notifier statusNotifier, mgmtClient managementClient) *NetBird {
if logger == nil {
logger = log.StandardLogger()
}
Expand All @@ -548,6 +550,7 @@ func NewNetBird(mgmtAddr, proxyID, proxyAddr string, wgPort int, logger *log.Log
proxyID: proxyID,
proxyAddr: proxyAddr,
wgPort: wgPort,
preSharedKey: preSharedKey,
logger: logger,
clients: make(map[types.AccountID]*clientEntry),
statusNotifier: notifier,
Expand Down
6 changes: 3 additions & 3 deletions proxy/internal/roundtrip/netbird_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (m *mockStatusNotifier) calls() []statusCall {
// mockNetBird creates a NetBird instance for testing without actually connecting.
// It uses an invalid management URL to prevent real connections.
func mockNetBird() *NetBird {
return NewNetBird("http://invalid.test:9999", "test-proxy", "invalid.test", 0, nil, nil, &mockMgmtClient{})
return NewNetBird("http://invalid.test:9999", "test-proxy", "invalid.test", 0, "", nil, nil, &mockMgmtClient{})
}

func TestNetBird_AddPeer_CreatesClientForNewAccount(t *testing.T) {
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestNetBird_RoundTrip_RequiresExistingClient(t *testing.T) {

func TestNetBird_AddPeer_ExistingStartedClient_NotifiesStatus(t *testing.T) {
notifier := &mockStatusNotifier{}
nb := NewNetBird("http://invalid.test:9999", "test-proxy", "invalid.test", 0, nil, notifier, &mockMgmtClient{})
nb := NewNetBird("http://invalid.test:9999", "test-proxy", "invalid.test", 0, "", nil, notifier, &mockMgmtClient{})
accountID := types.AccountID("account-1")

// Add first domain — creates a new client entry.
Expand All @@ -308,7 +308,7 @@ func TestNetBird_AddPeer_ExistingStartedClient_NotifiesStatus(t *testing.T) {

func TestNetBird_RemovePeer_NotifiesDisconnection(t *testing.T) {
notifier := &mockStatusNotifier{}
nb := NewNetBird("http://invalid.test:9999", "test-proxy", "invalid.test", 0, nil, notifier, &mockMgmtClient{})
nb := NewNetBird("http://invalid.test:9999", "test-proxy", "invalid.test", 0, "", nil, notifier, &mockMgmtClient{})
accountID := types.AccountID("account-1")

err := nb.AddPeer(context.Background(), accountID, domain.Domain("domain1.test"), "key-1", "svc-1")
Expand Down
4 changes: 3 additions & 1 deletion proxy/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ type Server struct {
// When enabled, the real client IP is extracted from the PROXY header
// sent by upstream L4 proxies that support PROXY protocol.
ProxyProtocol bool
// PreSharedKey used for tunnel between proxy and peers (set globally not per account)
PreSharedKey string
}

// NotifyStatus sends a status update to management about tunnel connectivity
Expand Down Expand Up @@ -163,7 +165,7 @@ func (s *Server) ListenAndServe(ctx context.Context, addr string) (err error) {

// Initialize the netbird client, this is required to build peer connections
// to proxy over.
s.netbird = roundtrip.NewNetBird(s.ManagementAddress, s.ID, s.ProxyURL, s.WireguardPort, s.Logger, s, s.mgmtClient)
s.netbird = roundtrip.NewNetBird(s.ManagementAddress, s.ID, s.ProxyURL, s.WireguardPort, s.PreSharedKey, s.Logger, s, s.mgmtClient)

tlsConfig, err := s.configureTLS(ctx)
if err != nil {
Expand Down
Loading