diff --git a/README.md b/README.md index 44d0b3ca..c06921bc 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,7 @@ boundary [flags] -- command [args...] --log-level Set log level (error, warn, info, debug). Default: warn --log-dir Directory to write logs to (default: stderr) --proxy-port HTTP proxy port (default: 8080) + --upstream-proxy Forward allowed requests through an upstream HTTP(S) proxy --pprof Enable pprof profiling server --pprof-port pprof server port (default: 6060) --disable-audit-logs Disable sending audit logs to the workspace agent @@ -168,7 +169,14 @@ boundary [flags] -- command [args...] -h, --help Print help ``` -Environment variables: `BOUNDARY_CONFIG`, `BOUNDARY_ALLOW`, `BOUNDARY_LOG_LEVEL`, `BOUNDARY_LOG_DIR`, `PROXY_PORT`, `BOUNDARY_PPROF`, `BOUNDARY_PPROF_PORT`, `DISABLE_AUDIT_LOGS`, `CODER_AGENT_BOUNDARY_LOG_PROXY_SOCKET_PATH` +Environment variables: `BOUNDARY_CONFIG`, `BOUNDARY_ALLOW`, `BOUNDARY_LOG_LEVEL`, `BOUNDARY_LOG_DIR`, `PROXY_PORT`, `BOUNDARY_UPSTREAM_PROXY`, `BOUNDARY_PPROF`, `BOUNDARY_PPROF_PORT`, `DISABLE_AUDIT_LOGS`, `CODER_AGENT_BOUNDARY_LOG_PROXY_SOCKET_PATH` + +When `--upstream-proxy` is set, Boundary still evaluates allow rules and writes audit logs before forwarding allowed requests through the upstream proxy: + +```bash +boundary --upstream-proxy http://proxy.corp:3128 \ + --allow "domain=github.com" -- curl https://github.com +``` ## Development @@ -188,4 +196,4 @@ For detailed information about how `boundary` works internally, see [docs/archit ## License -MIT License - see LICENSE file for details. \ No newline at end of file +MIT License - see LICENSE file for details. diff --git a/cli/cli.go b/cli/cli.go index 0042b384..bf0cb5c0 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -39,6 +39,10 @@ func NewCommand(version string) *serpent.Command { # Use allowlist from config file with additional CLI allow rules boundary --allow "domain=example.com" -- curl https://example.com + # Forward allowed requests through a corporate proxy + boundary --upstream-proxy http://proxy.corp:3128 \ + --allow "domain=github.com" -- curl https://github.com + # Block everything by default (implicit) # Enable session correlation inside a Coder workspace @@ -122,6 +126,13 @@ func BaseCommand(version string) *serpent.Command { Value: &cliConfig.ProxyPort, YAML: "proxy_port", }, + { + Flag: config.UpstreamProxyFlag, + Env: config.UpstreamProxyEnv, + Description: "Forward allowed requests through this upstream HTTP(S) proxy URL.", + Value: &cliConfig.UpstreamProxy, + YAML: "upstream_proxy", + }, { Flag: "pprof", Env: "BOUNDARY_PPROF", diff --git a/config/config.go b/config/config.go index 3e745b62..8da302e2 100644 --- a/config/config.go +++ b/config/config.go @@ -2,7 +2,10 @@ package config import ( "fmt" + "net" + "net/url" "path/filepath" + "strconv" "strings" "github.com/coder/serpent" @@ -16,6 +19,12 @@ type JailType string const ( NSJailType JailType = "nsjail" LandjailType JailType = "landjail" + + // UpstreamProxyEnv configures Boundary's parent-side forwarding transport. + UpstreamProxyEnv = "BOUNDARY_UPSTREAM_PROXY" + + // UpstreamProxyFlag configures Boundary's parent-side forwarding transport. + UpstreamProxyFlag = "upstream-proxy" ) func NewJailTypeFromString(str string) (JailType, error) { @@ -64,6 +73,7 @@ type CliConfig struct { LogLevel serpent.String `yaml:"log_level"` LogDir serpent.String `yaml:"log_dir"` ProxyPort serpent.Int64 `yaml:"proxy_port"` + UpstreamProxy serpent.String `yaml:"upstream_proxy"` PprofEnabled serpent.Bool `yaml:"pprof_enabled"` PprofPort serpent.Int64 `yaml:"pprof_port"` JailType serpent.String `yaml:"jail_type"` @@ -83,6 +93,7 @@ type AppConfig struct { LogLevel string LogDir string ProxyPort int64 + UpstreamProxy string `json:"-"` PprofEnabled bool PprofPort int64 JailType JailType @@ -119,6 +130,34 @@ func confinedProcessName(targetCMD []string) string { return filepath.Base(targetCMD[0]) } +// StripUpstreamProxyArgs removes Boundary's parent-only upstream proxy flag +// before argv is passed to the confined child helper process. +func StripUpstreamProxyArgs(args []string) []string { + stripped := make([]string, 0, len(args)) + flag := "--" + UpstreamProxyFlag + flagWithValue := flag + "=" + + for i := 0; i < len(args); i++ { + arg := args[i] + if arg == "--" { + stripped = append(stripped, args[i:]...) + return stripped + } + if arg == flag { + if i+1 < len(args) { + i++ + } + continue + } + if strings.HasPrefix(arg, flagWithValue) { + continue + } + stripped = append(stripped, arg) + } + + return stripped +} + func NewAppConfigFromCliConfig(cfg CliConfig, targetCMD []string, environ []string) (AppConfig, error) { // Merge allowlist from config file with allow from CLI flags allowListStrings := cfg.AllowListStrings.Value() @@ -140,11 +179,17 @@ func NewAppConfigFromCliConfig(cfg CliConfig, targetCMD []string, environ []stri return AppConfig{}, fmt.Errorf("session correlation config: %w", err) } + upstreamProxy, err := normalizeUpstreamProxy(cfg.UpstreamProxy.Value(), cfg.ProxyPort.Value()) + if err != nil { + return AppConfig{}, err + } + return AppConfig{ AllowRules: allAllowStrings, LogLevel: cfg.LogLevel.Value(), LogDir: cfg.LogDir.Value(), ProxyPort: cfg.ProxyPort.Value(), + UpstreamProxy: upstreamProxy, PprofEnabled: cfg.PprofEnabled.Value(), PprofPort: cfg.PprofPort.Value(), JailType: jailType, @@ -159,6 +204,85 @@ func NewAppConfigFromCliConfig(cfg CliConfig, targetCMD []string, environ []stri }, nil } +func normalizeUpstreamProxy(raw string, boundaryProxyPort int64) (string, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", nil + } + + upstreamURL, err := url.Parse(raw) + if err != nil { + return "", fmt.Errorf("invalid upstream proxy URL") + } + if upstreamURL.Scheme != "http" && upstreamURL.Scheme != "https" { + return "", fmt.Errorf("upstream proxy URL must use http or https scheme") + } + if upstreamURL.Hostname() == "" { + return "", fmt.Errorf("upstream proxy URL must include a host") + } + if hasInvalidPort(upstreamURL) || hasOutOfRangePort(upstreamURL) { + return "", fmt.Errorf("upstream proxy URL has an invalid port") + } + if pointsToBoundaryProxy(upstreamURL, boundaryProxyPort) { + return "", fmt.Errorf("upstream proxy URL must not point to boundary's local proxy listener") + } + + return upstreamURL.String(), nil +} + +func hasInvalidPort(upstreamURL *url.URL) bool { + if upstreamURL.Port() != "" { + return false + } + + host := upstreamURL.Host + if strings.HasPrefix(host, "[") { + bracket := strings.LastIndex(host, "]") + return bracket >= 0 && len(host) > bracket+1 + } + + return strings.Count(host, ":") > 0 +} + +func hasOutOfRangePort(upstreamURL *url.URL) bool { + port := upstreamURL.Port() + if port == "" { + return false + } + + portNumber, err := strconv.Atoi(port) + return err != nil || portNumber < 1 || portNumber > 65535 +} + +func pointsToBoundaryProxy(upstreamURL *url.URL, boundaryProxyPort int64) bool { + port := upstreamURL.Port() + if port == "" { + port = defaultPortForScheme(upstreamURL.Scheme) + } + if port == "" || port != strconv.FormatInt(boundaryProxyPort, 10) { + return false + } + + host := strings.ToLower(upstreamURL.Hostname()) + if host == "localhost" { + return true + } + + ip := net.ParseIP(host) + return ip != nil && (ip.IsLoopback() || ip.IsUnspecified()) +} + +func defaultPortForScheme(scheme string) string { + switch scheme { + case "http": + return "80" + case "https": + return "443" + default: + return "" + } +} + // buildSessionCorrelation merges CLI and YAML inject target sources // and validates the resulting configuration. Inject targets use the same // "domain=... path=..." syntax as --allow rules so that matching semantics diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 00000000..ffe0f7dd --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,186 @@ +package config + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestNewAppConfigFromCliConfig_UpstreamProxy(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw string + port string + want string + wantErr bool + }{ + { + name: "empty by default", + }, + { + name: "http upstream proxy", + raw: "http://proxy.corp:3128", + want: "http://proxy.corp:3128", + }, + { + name: "https upstream proxy", + raw: "https://proxy.corp:8443", + want: "https://proxy.corp:8443", + }, + { + name: "upstream proxy with credentials", + raw: "http://user:pass@proxy.corp:3128", + want: "http://user:pass@proxy.corp:3128", + }, + { + name: "unsupported scheme", + raw: "socks5://proxy.corp:1080", + wantErr: true, + }, + { + name: "missing host", + raw: "http:///proxy", + wantErr: true, + }, + { + name: "invalid port", + raw: "http://proxy.corp:badport", + wantErr: true, + }, + { + name: "out of range port", + raw: "http://proxy.corp:70000", + wantErr: true, + }, + { + name: "IPv6 proxy host", + raw: "http://[2001:db8::1]:3128", + want: "http://[2001:db8::1]:3128", + }, + { + name: "localhost boundary proxy loop", + raw: "http://localhost:8080", + wantErr: true, + }, + { + name: "loopback IP boundary proxy loop", + raw: "http://127.0.0.1:8080", + wantErr: true, + }, + { + name: "same custom boundary proxy port loop", + raw: "http://localhost:18080", + port: "18080", + wantErr: true, + }, + { + name: "local proxy on a different port is allowed", + raw: "http://localhost:3128", + want: "http://localhost:3128", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + c := baseCliConfig() + _ = c.ProxyPort.Set("8080") + if tc.port != "" { + _ = c.ProxyPort.Set(tc.port) + } + if tc.raw != "" { + _ = c.UpstreamProxy.Set(tc.raw) + } + + got, err := NewAppConfigFromCliConfig(c, []string{"echo", "hello"}, nil) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.UpstreamProxy != tc.want { + t.Fatalf("UpstreamProxy: got %q, want %q", got.UpstreamProxy, tc.want) + } + }) + } +} + +func TestAppConfigJSONOmitsUpstreamProxy(t *testing.T) { + t.Parallel() + + c := baseCliConfig() + _ = c.ProxyPort.Set("8080") + _ = c.UpstreamProxy.Set("http://user:pass@proxy.corp:3128") + + got, err := NewAppConfigFromCliConfig(c, []string{"echo", "hello"}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.UpstreamProxy == "" { + t.Fatal("expected upstream proxy to remain available at runtime") + } + + payload, err := json.Marshal(got) + if err != nil { + t.Fatalf("unexpected marshal error: %v", err) + } + configJSON := string(payload) + + for _, forbidden := range []string{"user:pass", "UpstreamProxy", "upstream_proxy"} { + if strings.Contains(configJSON, forbidden) { + t.Fatalf("marshaled AppConfig leaked %q in %s", forbidden, configJSON) + } + } +} + +func TestStripUpstreamProxyArgs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args []string + want []string + }{ + { + name: "separate flag value", + args: []string{"boundary", "--upstream-proxy", "http://user:pass@proxy.corp:3128", "--allow", "domain=example.com", "--", "curl"}, + want: []string{"boundary", "--allow", "domain=example.com", "--", "curl"}, + }, + { + name: "equals flag value", + args: []string{"boundary", "--upstream-proxy=http://user:pass@proxy.corp:3128", "--", "curl"}, + want: []string{"boundary", "--", "curl"}, + }, + { + name: "target args are preserved after separator", + args: []string{"boundary", "--allow", "domain=example.com", "--", "tool", "--upstream-proxy", "http://target.example"}, + want: []string{"boundary", "--allow", "domain=example.com", "--", "tool", "--upstream-proxy", "http://target.example"}, + }, + { + name: "args without upstream proxy are copied", + args: []string{"boundary", "--allow", "domain=example.com", "--", "curl"}, + want: []string{"boundary", "--allow", "domain=example.com", "--", "curl"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := StripUpstreamProxyArgs(tc.args) + if strings.Join(got, "\x00") != strings.Join(tc.want, "\x00") { + t.Fatalf("args: got %#v, want %#v", got, tc.want) + } + if len(tc.args) > 0 && &got[0] == &tc.args[0] { + t.Fatal("expected returned args to use a new backing array") + } + }) + } +} diff --git a/docs/agent-guide.md b/docs/agent-guide.md index b635aebb..d8c75c61 100644 --- a/docs/agent-guide.md +++ b/docs/agent-guide.md @@ -24,6 +24,7 @@ Important CLI behavior: - `--allow` is repeatable and CLI-only. - YAML `allowlist` is merged with CLI `--allow` rules. - `--jail-type` defaults to `nsjail`. +- `--upstream-proxy` configures Boundary's outbound forwarding transport for corporate proxies. Do not point child proxy environment variables at this proxy. - `--use-real-dns` intentionally permits DNS exfiltration. Do not enable it by accident. - `--disable-audit-logs` disables workspace-agent socket forwarding. It does not remove stderr logging. - `--enable-session-correlation` requires configured inject targets or a valid fallback from `CODER_AGENT_URL`. diff --git a/docs/architecture.md b/docs/architecture.md index df75a053..cc3505cc 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -151,6 +151,8 @@ When a client uses Boundary as an explicit HTTP proxy for HTTPS, it sends a CONN For allowed requests, the proxy creates a new upstream request, copies appropriate headers, optionally injects session-correlation headers, and writes the upstream response back to the client. +If `upstream_proxy` / `--upstream-proxy` is configured, the proxy forwards allowed requests through that upstream HTTP(S) proxy after policy evaluation and audit. The confined child process still sends traffic to Boundary first; the upstream proxy is a Boundary outbound transport setting, not a child `HTTP_PROXY` setting. + For denied requests, the proxy returns HTTP 403 with a short message and example allow rules. Every HTTP request that reaches the proxy is audited before the allow or deny handling completes. CONNECT handshake requests themselves are not audited; only the HTTP requests inside the resulting tunnel are audited. diff --git a/landjail/child.go b/landjail/child.go index ad9c9dc9..d47f4717 100644 --- a/landjail/child.go +++ b/landjail/child.go @@ -76,7 +76,7 @@ func RunChild(logger *slog.Logger, config config.AppConfig) error { // Returns environment variables intended to be set on the child process, // so they can later be inherited by the target process. func getEnvsForTargetProcess(configDir string, caCertPath string, httpProxyPort int) []string { - e := os.Environ() + e := util.RemoveEnvs(os.Environ(), config.UpstreamProxyEnv) proxyAddr := fmt.Sprintf("http://localhost:%d", httpProxyPort) e = util.MergeEnvs(e, map[string]string{ diff --git a/landjail/child_test.go b/landjail/child_test.go new file mode 100644 index 00000000..aad2cec9 --- /dev/null +++ b/landjail/child_test.go @@ -0,0 +1,47 @@ +package landjail + +import ( + "strings" + "testing" + + "github.com/coder/boundary/config" +) + +func TestGetEnvsForTargetProcessStripsUpstreamProxy(t *testing.T) { + t.Setenv(config.UpstreamProxyEnv, "http://user:pass@proxy.corp:3128") + t.Setenv("BOUNDARY_TEST_KEEP", "keep") + + env := getEnvsForTargetProcess("/tmp/boundary-config", "/tmp/boundary-config/ca.pem", 18080) + + if containsEnvKey(env, config.UpstreamProxyEnv) { + t.Fatalf("target environment includes %s", config.UpstreamProxyEnv) + } + if !containsEnv(env, "BOUNDARY_TEST_KEEP=keep") { + t.Fatal("target environment did not preserve unrelated variables") + } + if !containsEnv(env, "HTTP_PROXY=http://localhost:18080") { + t.Fatal("target environment did not include Boundary HTTP proxy") + } + if !containsEnv(env, "SSL_CERT_FILE=/tmp/boundary-config/ca.pem") { + t.Fatal("target environment did not include CA certificate path") + } +} + +func containsEnv(env []string, want string) bool { + for _, entry := range env { + if entry == want { + return true + } + } + return false +} + +func containsEnvKey(env []string, want string) bool { + for _, entry := range env { + key, _, ok := strings.Cut(entry, "=") + if ok && key == want { + return true + } + } + return false +} diff --git a/landjail/manager.go b/landjail/manager.go index 92b08ee7..f9c60d68 100644 --- a/landjail/manager.go +++ b/landjail/manager.go @@ -37,17 +37,26 @@ func NewLandJail( return nil, fmt.Errorf("build inject engine: %w", err) } + forwardTransport, err := proxy.NewForwardTransport(config.UpstreamProxy) + if err != nil { + return nil, fmt.Errorf("build upstream proxy transport: %w", err) + } + if config.UpstreamProxy != "" { + logger.Info("Using upstream proxy", "upstream_proxy", proxy.RedactProxyURL(config.UpstreamProxy)) + } + // Create proxy server proxyServer := proxy.NewProxyServer(proxy.Config{ - HTTPPort: int(config.ProxyPort), - RuleEngine: ruleEngine, - Auditor: auditor, - Logger: logger, - TLSConfig: tlsConfig, - PprofEnabled: config.PprofEnabled, - PprofPort: int(config.PprofPort), - InjectEngine: injectEngine, - SessionID: config.SessionID.String(), + HTTPPort: int(config.ProxyPort), + RuleEngine: ruleEngine, + Auditor: auditor, + Logger: logger, + TLSConfig: tlsConfig, + PprofEnabled: config.PprofEnabled, + PprofPort: int(config.PprofPort), + InjectEngine: injectEngine, + SessionID: config.SessionID.String(), + ForwardTransport: forwardTransport, }) return &LandJail{ @@ -80,7 +89,7 @@ func (b *LandJail) Run(ctx context.Context) error { childErr := make(chan error, 1) go func() { defer cancel() - childErr <- b.RunChildProcess(os.Args) + childErr <- b.RunChildProcess(config.StripUpstreamProxyArgs(os.Args)) }() // Setup signal handling BEFORE any setup @@ -112,7 +121,7 @@ func (b *LandJail) Run(ctx context.Context) error { func (b *LandJail) RunChildProcess(command []string) error { childCmd := b.getChildCommand(command) - b.logger.Debug("Executing command in boundary", "command", strings.Join(os.Args, " ")) + b.logger.Debug("Executing command in boundary", "command", strings.Join(command, " ")) err := childCmd.Start() if err != nil { b.logger.Error("Command failed to start", "error", err) diff --git a/nsjail_manager/manager.go b/nsjail_manager/manager.go index 51169b69..c49c0ad2 100644 --- a/nsjail_manager/manager.go +++ b/nsjail_manager/manager.go @@ -39,17 +39,26 @@ func NewNSJailManager( return nil, fmt.Errorf("build inject engine: %w", err) } + forwardTransport, err := proxy.NewForwardTransport(config.UpstreamProxy) + if err != nil { + return nil, fmt.Errorf("build upstream proxy transport: %w", err) + } + if config.UpstreamProxy != "" { + logger.Info("Using upstream proxy", "upstream_proxy", proxy.RedactProxyURL(config.UpstreamProxy)) + } + // Create proxy server proxyServer := proxy.NewProxyServer(proxy.Config{ - HTTPPort: int(config.ProxyPort), - RuleEngine: ruleEngine, - Auditor: auditor, - Logger: logger, - TLSConfig: tlsConfig, - PprofEnabled: config.PprofEnabled, - PprofPort: int(config.PprofPort), - InjectEngine: injectEngine, - SessionID: config.SessionID.String(), + HTTPPort: int(config.ProxyPort), + RuleEngine: ruleEngine, + Auditor: auditor, + Logger: logger, + TLSConfig: tlsConfig, + PprofEnabled: config.PprofEnabled, + PprofPort: int(config.PprofPort), + InjectEngine: injectEngine, + SessionID: config.SessionID.String(), + ForwardTransport: forwardTransport, }) return &NSJailManager{ @@ -83,7 +92,7 @@ func (b *NSJailManager) Run(ctx context.Context) error { childErr := make(chan error, 1) go func() { defer cancel() - childErr <- b.RunChildProcess(os.Args) + childErr <- b.RunChildProcess(config.StripUpstreamProxyArgs(os.Args)) }() // Setup signal handling BEFORE any setup @@ -115,7 +124,7 @@ func (b *NSJailManager) Run(ctx context.Context) error { func (b *NSJailManager) RunChildProcess(command []string) error { cmd := b.jailer.Command(command) - b.logger.Debug("Executing command in boundary", "command", strings.Join(os.Args, " ")) + b.logger.Debug("Executing command in boundary", "command", strings.Join(command, " ")) err := cmd.Start() if err != nil { b.logger.Error("Command failed to start", "error", err) diff --git a/nsjail_manager/nsjail/env.go b/nsjail_manager/nsjail/env.go index 6918844f..a34cfb2f 100644 --- a/nsjail_manager/nsjail/env.go +++ b/nsjail_manager/nsjail/env.go @@ -3,13 +3,14 @@ package nsjail import ( "os" + "github.com/coder/boundary/config" "github.com/coder/boundary/util" ) // Returns environment variables intended to be set on the child process, // so they can later be inherited by the target process. func getEnvsForTargetProcess(configDir string, caCertPath string) []string { - e := os.Environ() + e := util.RemoveEnvs(os.Environ(), config.UpstreamProxyEnv) e = util.MergeEnvs(e, map[string]string{ // Set standard CA certificate environment variables for common tools diff --git a/nsjail_manager/nsjail/env_test.go b/nsjail_manager/nsjail/env_test.go new file mode 100644 index 00000000..ce28384c --- /dev/null +++ b/nsjail_manager/nsjail/env_test.go @@ -0,0 +1,44 @@ +package nsjail + +import ( + "strings" + "testing" + + "github.com/coder/boundary/config" +) + +func TestGetEnvsForTargetProcessStripsUpstreamProxy(t *testing.T) { + t.Setenv(config.UpstreamProxyEnv, "http://user:pass@proxy.corp:3128") + t.Setenv("BOUNDARY_TEST_KEEP", "keep") + + env := getEnvsForTargetProcess("/tmp/boundary-config", "/tmp/boundary-config/ca.pem") + + if containsEnvKey(env, config.UpstreamProxyEnv) { + t.Fatalf("target environment includes %s", config.UpstreamProxyEnv) + } + if !containsEnv(env, "BOUNDARY_TEST_KEEP=keep") { + t.Fatal("target environment did not preserve unrelated variables") + } + if !containsEnv(env, "SSL_CERT_FILE=/tmp/boundary-config/ca.pem") { + t.Fatal("target environment did not include CA certificate path") + } +} + +func containsEnv(env []string, want string) bool { + for _, entry := range env { + if entry == want { + return true + } + } + return false +} + +func containsEnvKey(env []string, want string) bool { + for _, entry := range env { + key, _, ok := strings.Cut(entry, "=") + if ok && key == want { + return true + } + } + return false +} diff --git a/proxy/upstream.go b/proxy/upstream.go new file mode 100644 index 00000000..4849209b --- /dev/null +++ b/proxy/upstream.go @@ -0,0 +1,42 @@ +package proxy + +import ( + "fmt" + "net/http" + "net/url" + "strings" +) + +// NewForwardTransport builds the outbound transport Boundary uses after a +// request has passed policy evaluation. When upstreamProxy is empty, callers +// should leave Config.ForwardTransport nil to preserve the default transport. +func NewForwardTransport(upstreamProxy string) (http.RoundTripper, error) { + upstreamProxy = strings.TrimSpace(upstreamProxy) + if upstreamProxy == "" { + return nil, nil + } + + upstreamURL, err := url.Parse(upstreamProxy) + if err != nil { + return nil, fmt.Errorf("parse upstream proxy URL: %w", err) + } + + defaultTransport, ok := http.DefaultTransport.(*http.Transport) + if !ok { + return nil, fmt.Errorf("default HTTP transport is %T, want *http.Transport", http.DefaultTransport) + } + + transport := defaultTransport.Clone() + transport.Proxy = http.ProxyURL(upstreamURL) + return transport, nil +} + +// RedactProxyURL removes userinfo from a proxy URL before it is logged. +func RedactProxyURL(raw string) string { + u, err := url.Parse(raw) + if err != nil { + return raw + } + u.User = nil + return u.String() +} diff --git a/proxy/upstream_test.go b/proxy/upstream_test.go new file mode 100644 index 00000000..55cb3c89 --- /dev/null +++ b/proxy/upstream_test.go @@ -0,0 +1,230 @@ +package proxy + +import ( + "context" + "crypto/tls" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestProxyServerUpstreamProxy_ForwardsAllowedHTTPAndHTTPS(t *testing.T) { + upstream := newRecordingUpstreamProxy(t) + defer upstream.Close() + + upstreamURL, err := url.Parse(upstream.URL) + require.NoError(t, err) + + forwardTransport := http.DefaultTransport.(*http.Transport).Clone() + forwardTransport.Proxy = http.ProxyURL(upstreamURL) + forwardTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //nolint:gosec + forwardTransport.DisableKeepAlives = true + + httpBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("http-ok")) + })) + defer httpBackend.Close() + + httpsBackend := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("https-ok")) + })) + defer httpsBackend.Close() + + httpsBackendURL, err := url.Parse(httpsBackend.URL) + require.NoError(t, err) + httpsURL := "https://localhost:" + httpsBackendURL.Port() + + pt := NewProxyTest(t, + WithProxyPort(freeTCPPort(t)), + WithCertManager(t.TempDir()), + WithAllowedDomain("127.0.0.1"), + WithAllowedDomain("localhost"), + WithForwardTransport(forwardTransport), + ).Start() + defer pt.Stop() + + resp, err := pt.proxyClient.Get(httpBackend.URL) + require.NoError(t, err) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, "http-ok", string(body)) + + resp, err = pt.proxyClient.Get(httpsURL) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, "https-ok", string(body)) + + require.Equal(t, int32(1), upstream.HTTPRequests()) + require.Equal(t, int32(1), upstream.CONNECTRequests()) + require.Contains(t, upstream.URLs(), httpBackend.URL+"/") +} + +func TestProxyServerUpstreamProxy_DeniedRequestsNeverReachUpstream(t *testing.T) { + upstream := newRecordingUpstreamProxy(t) + defer upstream.Close() + + upstreamURL, err := url.Parse(upstream.URL) + require.NoError(t, err) + + forwardTransport := http.DefaultTransport.(*http.Transport).Clone() + forwardTransport.Proxy = http.ProxyURL(upstreamURL) + forwardTransport.DisableKeepAlives = true + + pt := NewProxyTest(t, + WithProxyPort(freeTCPPort(t)), + WithCertManager(t.TempDir()), + WithForwardTransport(forwardTransport), + ).Start() + defer pt.Stop() + + pt.ExpectGetViaProxy("http://denied.example.test/exfil", http.StatusForbidden) + pt.ExpectGetViaProxy("https://denied.example.test/exfil", http.StatusForbidden) + + require.Equal(t, int32(0), upstream.HTTPRequests()) + require.Equal(t, int32(0), upstream.CONNECTRequests()) +} + +func TestNewForwardTransport_UsesConfiguredUpstreamProxy(t *testing.T) { + t.Parallel() + + transport, err := NewForwardTransport("http://user:pass@proxy.corp:3128") + require.NoError(t, err) + require.NotNil(t, transport) + + req, err := http.NewRequest(http.MethodGet, "http://example.com", nil) + require.NoError(t, err) + + proxyURL, err := transport.(*http.Transport).Proxy(req) + require.NoError(t, err) + require.Equal(t, "http://user:pass@proxy.corp:3128", proxyURL.String()) + require.Equal(t, "http://proxy.corp:3128", RedactProxyURL(proxyURL.String())) +} + +type recordingUpstreamProxy struct { + *httptest.Server + + httpRequests atomic.Int32 + connectRequests atomic.Int32 + + mu sync.Mutex + urls []string +} + +func newRecordingUpstreamProxy(t *testing.T) *recordingUpstreamProxy { + t.Helper() + + proxy := &recordingUpstreamProxy{} + proxy.Server = httptest.NewServer(http.HandlerFunc(proxy.handle)) + return proxy +} + +func (p *recordingUpstreamProxy) HTTPRequests() int32 { + return p.httpRequests.Load() +} + +func (p *recordingUpstreamProxy) CONNECTRequests() int32 { + return p.connectRequests.Load() +} + +func (p *recordingUpstreamProxy) URLs() []string { + p.mu.Lock() + defer p.mu.Unlock() + + urls := make([]string, len(p.urls)) + copy(urls, p.urls) + return urls +} + +func (p *recordingUpstreamProxy) handle(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodConnect { + p.connectRequests.Add(1) + p.handleCONNECT(w, r) + return + } + + p.httpRequests.Add(1) + p.recordURL(r.URL.String()) + + if !r.URL.IsAbs() { + http.Error(w, "proxy request URL must be absolute", http.StatusBadRequest) + return + } + + outReq := r.Clone(context.Background()) + outReq.RequestURI = "" + + resp, err := http.DefaultTransport.RoundTrip(outReq) + if err != nil { + http.Error(w, err.Error(), http.StatusBadGateway) + return + } + defer resp.Body.Close() //nolint:errcheck + + for name, values := range resp.Header { + for _, value := range values { + w.Header().Add(name, value) + } + } + w.WriteHeader(resp.StatusCode) + _, _ = io.Copy(w, resp.Body) +} + +func (p *recordingUpstreamProxy) handleCONNECT(w http.ResponseWriter, r *http.Request) { + hijacker, ok := w.(http.Hijacker) + if !ok { + http.Error(w, "hijacking unsupported", http.StatusInternalServerError) + return + } + + clientConn, _, err := hijacker.Hijack() + if err != nil { + return + } + defer clientConn.Close() //nolint:errcheck + + targetConn, err := net.DialTimeout("tcp", r.Host, 5*time.Second) + if err != nil { + _, _ = fmt.Fprintf(clientConn, "HTTP/1.1 502 Bad Gateway\r\n\r\n") + return + } + defer targetConn.Close() //nolint:errcheck + + _, _ = fmt.Fprintf(clientConn, "HTTP/1.1 200 Connection Established\r\n\r\n") + + go func() { + _, _ = io.Copy(targetConn, clientConn) + _ = targetConn.Close() + }() + _, _ = io.Copy(clientConn, targetConn) +} + +func (p *recordingUpstreamProxy) recordURL(raw string) { + p.mu.Lock() + defer p.mu.Unlock() + + p.urls = append(p.urls, raw) +} + +func freeTCPPort(t *testing.T) int { + t.Helper() + + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer ln.Close() //nolint:errcheck + + return ln.Addr().(*net.TCPAddr).Port +} diff --git a/util/env.go b/util/env.go index 15f28360..fe1c4159 100644 --- a/util/env.go +++ b/util/env.go @@ -2,6 +2,33 @@ package util import "strings" +// RemoveEnvs returns a copy of base without entries matching keys. +func RemoveEnvs(base []string, keys ...string) []string { + if len(keys) == 0 { + return append([]string(nil), base...) + } + + remove := make(map[string]struct{}, len(keys)) + for _, key := range keys { + remove[key] = struct{}{} + } + + filtered := make([]string, 0, len(base)) + for _, env := range base { + key, _, ok := strings.Cut(env, "=") + if !ok { + filtered = append(filtered, env) + continue + } + if _, ok := remove[key]; ok { + continue + } + filtered = append(filtered, env) + } + + return filtered +} + func MergeEnvs(base []string, extra map[string]string) []string { envMap := make(map[string]string) for _, env := range base {