From 77e60ef2dac1c89f4a2124bbc4dbe5475a1fc6bb Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Fri, 30 Oct 2026 05:05:20 +0530 Subject: [PATCH 1/4] feat(agent-vault): accept SOCKS5 over TLS on the proxy --- .github/workflows/run-cli-tests.yml | 4 +- packages/agentvault/ca.go | 37 ++ packages/agentvault/dial_guard.go | 78 ++++ packages/agentvault/dial_guard_test.go | 62 +++ packages/agentvault/proxy.go | 146 +++++-- packages/agentvault/run.go | 104 ++++- packages/agentvault/socks5.go | 370 ++++++++++++++++ packages/agentvault/socks5_e2e_test.go | 572 +++++++++++++++++++++++++ packages/agentvault/socks5_test.go | 181 ++++++++ packages/agentvault/upstream_auth.go | 4 +- packages/cmd/agent_vault.go | 203 ++++++++- packages/cmd/agent_vault_proxy_test.go | 124 ++++++ 12 files changed, 1810 insertions(+), 75 deletions(-) create mode 100644 packages/agentvault/dial_guard.go create mode 100644 packages/agentvault/dial_guard_test.go create mode 100644 packages/agentvault/socks5.go create mode 100644 packages/agentvault/socks5_e2e_test.go create mode 100644 packages/agentvault/socks5_test.go diff --git a/.github/workflows/run-cli-tests.yml b/.github/workflows/run-cli-tests.yml index e75f2ce6a..3b28bbf27 100644 --- a/.github/workflows/run-cli-tests.yml +++ b/.github/workflows/run-cli-tests.yml @@ -39,7 +39,9 @@ jobs: - name: Install dependencies run: go get . - name: Race-check the Agent Vault packages - run: go test -race -count=1 ./packages/agentvault/ ./packages/api/ + run: | + go test -race -count=1 ./packages/agentvault/ ./packages/api/ + go test -race -count=1 -run 'AgentVault|ValidateProxyAddr|TrimProxyScheme|NonBlankDrops|OnlyThePinnedCertificate|KeychainTrust' ./packages/cmd/ - name: Race-check the PAM gateway packages run: go test -race -count=1 ./packages/gateway-v2/... ./packages/pam/handlers/clickhouse/ - name: Race-check telemetry diff --git a/packages/agentvault/ca.go b/packages/agentvault/ca.go index 499281e27..68d61bb51 100644 --- a/packages/agentvault/ca.go +++ b/packages/agentvault/ca.go @@ -193,6 +193,43 @@ func LoadOwnCa(in OwnCaInput) (*OwnCa, error) { return own, nil } +// ListenerCert is the operator's own certificate for the SOCKS5 listener. It is read once at start, so a +// renewed certificate takes effect on the next restart. +type ListenerCert struct { + certificate tls.Certificate + leaf *x509.Certificate +} + +func (c *ListenerCert) Fingerprint() string { return FingerprintOf(c.leaf.Raw) } + +func (c *ListenerCert) NotAfter() time.Time { return c.leaf.NotAfter } + +// Any key type and the full chain, unlike a CA, which has to be an ECDSA P-256 key the proxy can sign with. +func LoadListenerCert(in OwnCaInput) (*ListenerCert, error) { + certPEM, ok := loadCaValue(in.Cert) + if !ok { + return nil, fmt.Errorf("%s is not a PEM, base64 PEM or readable file: %s", in.CertSource, shortenValue(in.Cert)) + } + keyPEM, ok := loadCaValue(in.Key) + if !ok { + return nil, fmt.Errorf("%s is not a PEM, base64 PEM or readable file (value hidden)", in.KeySource) + } + pair, err := tls.X509KeyPair(certPEM, keyPEM) + if err != nil { + return nil, fmt.Errorf("the certificate in %s and the key in %s can't be used together: %w", in.CertSource, in.KeySource, err) + } + leaf, err := x509.ParseCertificate(pair.Certificate[0]) + if err != nil { + return nil, fmt.Errorf("the certificate in %s can't be parsed: %w", in.CertSource, err) + } + if now := time.Now(); now.Before(leaf.NotBefore) || now.After(leaf.NotAfter) { + return nil, fmt.Errorf("the certificate in %s isn't valid yet or has expired (valid from %s to %s)", + in.CertSource, leaf.NotBefore.UTC().Format(time.RFC3339), leaf.NotAfter.UTC().Format(time.RFC3339)) + } + pair.Leaf = leaf + return &ListenerCert{certificate: pair, leaf: leaf}, nil +} + func countPEMBlocks(data []byte, blockType string) int { n := 0 for { diff --git a/packages/agentvault/dial_guard.go b/packages/agentvault/dial_guard.go new file mode 100644 index 000000000..4e8dfcf3c --- /dev/null +++ b/packages/agentvault/dial_guard.go @@ -0,0 +1,78 @@ +package agentvault + +import ( + "errors" + "net" + "net/http" + "net/netip" + "syscall" +) + +var errPrivateDestination = errors.New("the destination is a private, loopback or link-local address") + +// The same ranges the backend refuses for outbound requests (backend/src/lib/ip/ipRange.ts), plus the +// IPv4-compatible and site-local IPv6 forms, which can also carry an internal address. +var blockedPrefixes = func() []netip.Prefix { + cidrs := []string{ + "0.0.0.0/8", + "10.0.0.0/8", + "100.64.0.0/10", + "127.0.0.0/8", + "169.254.0.0/16", + "172.16.0.0/12", + "192.0.0.0/24", + "192.0.2.0/24", + "192.88.99.0/24", + "192.168.0.0/16", + "198.18.0.0/15", + "198.51.100.0/24", + "203.0.113.0/24", + "224.0.0.0/4", + "240.0.0.0/4", + "::/96", + "::1/128", + "100::/64", + "2001:2::/48", + "2001:db8::/32", + "2002::/16", + "64:ff9b::/96", + "64:ff9b:1::/48", + "fc00::/7", + "fe80::/10", + "fec0::/10", + "ff00::/8", + } + prefixes := make([]netip.Prefix, 0, len(cidrs)) + for _, cidr := range cidrs { + prefixes = append(prefixes, netip.MustParsePrefix(cidr)) + } + return prefixes +}() + +// Prefix.Contains is false for any address with a zone, so the zone comes off first; an IPv4 address written +// as IPv6 is unwrapped so the IPv4 ranges apply to it. +func blockedDestination(ip netip.Addr) bool { + ip = ip.WithZone("").Unmap() + for _, prefix := range blockedPrefixes { + if prefix.Contains(ip) { + return true + } + } + return false +} + +// Runs on the resolved address of every socket the dialer opens, so a hostname that resolves to an internal +// address is caught however the name was written. +func guardControl(_, address string, _ syscall.RawConn) error { + addrPort, err := netip.ParseAddrPort(address) + if err != nil || blockedDestination(addrPort.Addr()) { + return errPrivateDestination + } + return nil +} + +func newGuardedUpstreamTransport() *http.Transport { + transport := newUpstreamTransport() + transport.DialContext = (&net.Dialer{Control: guardControl}).DialContext + return transport +} diff --git a/packages/agentvault/dial_guard_test.go b/packages/agentvault/dial_guard_test.go new file mode 100644 index 000000000..643a5eef6 --- /dev/null +++ b/packages/agentvault/dial_guard_test.go @@ -0,0 +1,62 @@ +package agentvault + +import ( + "errors" + "net/netip" + "testing" +) + +func TestBlockedDestination(t *testing.T) { + for _, tc := range []struct { + addr string + blocked bool + }{ + {"10.0.0.1", true}, + {"172.16.5.4", true}, + {"192.168.1.1", true}, + {"127.0.0.1", true}, + {"0.0.0.0", true}, + {"100.64.0.1", true}, + {"169.254.169.254", true}, + {"192.0.0.1", true}, + {"198.18.0.1", true}, + {"224.0.0.1", true}, + {"240.0.0.1", true}, + {"255.255.255.255", true}, + {"::", true}, + {"::1", true}, + {"fe80::1", true}, + {"fe80::1%eth0", true}, + {"fc00::1", true}, + {"fd00:ec2::254", true}, + {"fec0::1", true}, + {"ff02::1", true}, + {"::ffff:169.254.169.254", true}, + {"::169.254.169.254", true}, + {"64:ff9b::a9fe:a9fe", true}, + {"64:ff9b::a9fe:a9fe%1", true}, + {"64:ff9b:1::1", true}, + {"2002:a9fe:a9fe::1", true}, + {"8.8.8.8", false}, + {"1.1.1.1", false}, + {"2606:4700:4700::1111", false}, + {"::ffff:8.8.8.8", false}, + } { + t.Run(tc.addr, func(t *testing.T) { + if got := blockedDestination(netip.MustParseAddr(tc.addr)); got != tc.blocked { + t.Errorf("blockedDestination(%s) = %v, want %v", tc.addr, got, tc.blocked) + } + }) + } +} + +func TestGuardControlFailsClosed(t *testing.T) { + for _, address := range []string{"127.0.0.1:443", "[::1]:443", "not-an-address", "127.0.0.1%lo0:443"} { + if err := guardControl("tcp", address, nil); !errors.Is(err, errPrivateDestination) { + t.Errorf("guardControl(%q) = %v, want errPrivateDestination", address, err) + } + } + if err := guardControl("tcp4", "8.8.8.8:443", nil); err != nil { + t.Errorf("a public address was refused: %v", err) + } +} diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go index ddd219890..68069a3bc 100644 --- a/packages/agentvault/proxy.go +++ b/packages/agentvault/proxy.go @@ -15,6 +15,7 @@ import ( "sync" "time" + "github.com/rs/zerolog" "github.com/rs/zerolog/log" ) @@ -65,12 +66,16 @@ const ( ) type Options struct { - Port int + Port int + // Empty binds every interface. + ListenHost string DataDir string ProxyToken func() string ProxyID string ProxyName string OwnCa *OwnCa + // Nil leaves the SOCKS5 listener off. + Socks5 *Socks5Options OnReady func() } @@ -81,6 +86,10 @@ type proxyServer struct { cache *sessionCache sessionLogs *sessionLogRecorder transport http.RoundTripper + // SOCKS5 traffic that no service names goes out through this one, which refuses private destinations + // when it dials. Its own pool, so it never reuses a connection the unchecked transport opened. + guardedTransport http.RoundTripper + socks5AllowPrivate bool configMu sync.RWMutex config ProxyConfig @@ -174,35 +183,36 @@ func (ps *proxyServer) dispatch(w http.ResponseWriter, r *http.Request) { const proxyRevokedBody = "this proxy's access has been revoked" func (ps *proxyServer) denyAtGate(w http.ResponseWriter, sessionToken, hostname, port string, err error) { + refusal := classifyGateRefusal(err) + refusal.event. + Str("host", net.JoinHostPort(hostname, port)). + Str("sessionKey", sessionKey(sessionToken)). + Str("decision", refusal.decision). + Int("status", refusal.status). + Msg(refusal.msg) + http.Error(w, refusal.body, refusal.status) +} + +type gateRefusal struct { + event *zerolog.Event + decision string + status int + body string + msg string +} + +// Shared by both doors so a refused session reads the same in the log whichever way it arrived. +func classifyGateRefusal(err error) gateRefusal { if isProxyTokenRejected(err) { - log.Error(). - Err(err). - Str("host", net.JoinHostPort(hostname, port)). - Str("sessionKey", sessionKey(sessionToken)). - Str("decision", decisionError). - Int("status", http.StatusServiceUnavailable). - Msg("agent-vault: refused, Infisical rejected this proxy's token") - http.Error(w, proxyRevokedBody, http.StatusServiceUnavailable) - return + return gateRefusal{log.Error().Err(err), decisionError, http.StatusServiceUnavailable, proxyRevokedBody, + "agent-vault: refused, Infisical rejected this proxy's token"} } if isSessionGone(err) { - log.Warn(). - Str("host", net.JoinHostPort(hostname, port)). - Str("sessionKey", sessionKey(sessionToken)). - Str("decision", decisionBlocked). - Int("status", http.StatusForbidden). - Msg("agent-vault: refused, the session is no longer valid") - http.Error(w, "the session is no longer valid", http.StatusForbidden) - return + return gateRefusal{log.Warn(), decisionBlocked, http.StatusForbidden, "the session is no longer valid", + "agent-vault: refused, the session is no longer valid"} } - log.Error(). - Err(err). - Str("host", net.JoinHostPort(hostname, port)). - Str("sessionKey", sessionKey(sessionToken)). - Str("decision", decisionError). - Int("status", http.StatusBadGateway). - Msg("agent-vault: refused, the session could not be resolved") - http.Error(w, "failed to resolve the session", http.StatusBadGateway) + return gateRefusal{log.Error().Err(err), decisionError, http.StatusBadGateway, "failed to resolve the session", + "agent-vault: refused, the session could not be resolved"} } func (ps *proxyServer) handleConnect(w http.ResponseWriter, r *http.Request) { @@ -219,8 +229,8 @@ func (ps *proxyServer) handleConnect(w http.ResponseWriter, r *http.Request) { return } - // Resolve before the DNS lookup below and the leaf minting further down, so an unauthenticated caller - // cannot make the proxy work on their behalf. + // Resolve before the leaf minting further down, so an unauthenticated caller cannot make the proxy work + // on their behalf. if _, err := ps.cache.get(sessionToken); err != nil { ps.denyAtGate(w, sessionToken, hostname, port, err) return @@ -250,20 +260,35 @@ func (ps *proxyServer) handleConnect(w http.ResponseWriter, r *http.Request) { return } - // Node's fetch tunnels http:// targets too and speaks plaintext inside, where every other client sends - // absolute-form. A tunnel is opaque bytes to an ordinary proxy, so that works everywhere else; here the - // first byte decides: a TLS record starts with 0x16, anything else is plain HTTP and takes the same path - // as absolute-form. The port stays the one the CONNECT line named, so a service still only matches if - // its pattern covers that port. - buffered := newBufferedConn(clientConn) - _ = clientConn.SetDeadline(time.Now().Add(tlsHandshakeTimeout)) + ps.serveIntercepted(clientConn, tunnel{hostname: hostname, port: port, sessionToken: sessionToken}, leaf) +} + +// tunnel is what a connection was opened for, fixed before any byte of the agent's request is read. +type tunnel struct { + scheme string + hostname string + port string + sessionToken string + viaSocks5 bool +} + +// serveIntercepted takes over a connection the agent has been told is open to the target, from either door. +// Node's fetch tunnels http:// targets too and speaks plaintext inside, where every other client sends +// absolute-form. A tunnel is opaque bytes to an ordinary proxy, so that works everywhere else; here the +// first byte decides: a TLS record starts with 0x16, anything else is plain HTTP and takes the same path +// as absolute-form. The port stays the one the tunnel named, so a service still only matches if its +// pattern covers that port. +func (ps *proxyServer) serveIntercepted(conn net.Conn, t tunnel, leaf tls.Certificate) { + buffered := newBufferedConn(conn) + _ = conn.SetDeadline(time.Now().Add(tlsHandshakeTimeout)) first, err := buffered.reader.Peek(1) if err != nil { return } if first[0] != tlsRecordTypeHandshake { - _ = clientConn.SetDeadline(time.Time{}) - ps.serveTunnel(buffered, "http", hostname, port, sessionToken) + _ = conn.SetDeadline(time.Time{}) + t.scheme = "http" + ps.serveTunnel(buffered, t) return } @@ -275,12 +300,13 @@ func (ps *proxyServer) handleConnect(w http.ResponseWriter, r *http.Request) { NextProtos: []string{"http/1.1"}, }) if err := tlsConn.Handshake(); err != nil { - log.Debug().Err(err).Str("host", net.JoinHostPort(hostname, port)).Msg("agent-vault: tunnel TLS handshake failed") + log.Debug().Err(err).Str("host", net.JoinHostPort(t.hostname, t.port)).Msg("agent-vault: tunnel TLS handshake failed") return } _ = tlsConn.SetDeadline(time.Time{}) - ps.serveTunnel(tlsConn, "https", hostname, port, sessionToken) + t.scheme = "https" + ps.serveTunnel(tlsConn, t) } const tlsRecordTypeHandshake = 0x16 @@ -300,11 +326,11 @@ func (b *bufferedConn) Read(p []byte) (int, error) { return b.reader.Read(p) } // The scheme is the tunnel's, not the inner request's, and the target host comes from the CONNECT line so // an agent cannot address one host through a tunnel to another. -func (ps *proxyServer) serveTunnel(conn net.Conn, scheme, hostname, port, sessionToken string) { +func (ps *proxyServer) serveTunnel(conn net.Conn, t tunnel) { listener := newOneShotListener(conn) srv := &http.Server{ Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ps.forwardHTTP(w, r, scheme, hostname, port, sessionToken) + ps.forwardHTTP(w, r, t) }), ReadHeaderTimeout: tunnelReadHeaderTimeout, ReadTimeout: tunnelReadTimeout, @@ -349,10 +375,11 @@ func (ps *proxyServer) handlePlainForward(w http.ResponseWriter, r *http.Request return } - ps.forwardHTTP(w, r, "http", hostname, port, sessionToken) + ps.forwardHTTP(w, r, tunnel{scheme: "http", hostname: hostname, port: port, sessionToken: sessionToken}) } -func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, scheme, hostname, port, sessionToken string) { +func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, t tunnel) { + scheme, hostname, port := t.scheme, t.hostname, t.port // Before anything reads the path: the policy check, the substitutions and the forward all have to see // the bytes the agent sent, not the ones Go rebuilds. normalizeRequestTarget(r.URL) @@ -361,7 +388,7 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem reqPath := truncatePath(requestPath(r)) reqMethod := truncateLogged(r.Method, maxLoggedMethodLen) - resp, matched, outcome, err := ps.forward(r, scheme, hostname, port, sessionToken) + resp, matched, outcome, err := ps.forward(r, t) // The body is fixed text per outcome, never err.Error(): an APIError carries the control-plane URL and // request id, and a dial error names the upstream address. The detail goes on the log line below. @@ -377,6 +404,9 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem decision, status, body = decisionBlocked, http.StatusBadRequest, err.Error() case errors.Is(err, errOpaqueTarget): decision, status, body = decisionBlocked, http.StatusBadRequest, errOpaqueTarget.Error() + case errors.Is(err, errPrivateDestination): + // The dial error names the address it refused, which would tell the agent about the internal network. + decision, status, body = decisionBlocked, http.StatusForbidden, errPrivateDestination.Error() case isProxyTokenRejected(err): decision, status, body = decisionError, http.StatusServiceUnavailable, proxyRevokedBody case isSessionGone(err): @@ -405,6 +435,9 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem Str("path", reqPath). Str("decision", decision). Int("status", status) + if t.viaSocks5 { + event = event.Str("door", "socks5") + } if matched != nil { event = event.Str("service", matched.name).Str("accessBundle", matched.accessBundleName) } @@ -485,7 +518,8 @@ type forwardOutcome struct { mcp *mcpDetail } -func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessionToken string) (*http.Response, *resolvedService, forwardOutcome, error) { +func (ps *proxyServer) forward(req *http.Request, t tunnel) (*http.Response, *resolvedService, forwardOutcome, error) { + scheme, hostname, port, sessionToken := t.scheme, t.hostname, t.port, t.sessionToken var outcome forwardOutcome services, grant, err := ps.cache.lookup(sessionToken) @@ -610,7 +644,11 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio replay, replayable = replayableBody(req) } - resp, err := ps.transport.RoundTrip(req) + upstream, err := ps.upstreamFor(t, matched) + if err != nil { + return nil, matched, outcome, err + } + resp, err := upstream.RoundTrip(req) if err != nil { return nil, matched, outcome, err } @@ -624,7 +662,7 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio return upstreamAuthFailed(req, staticCredentialRejectedBody(matched.name)), matched, outcome, nil } if sent != nil { - resp, err = ps.handleUpstreamAuth(req, resp, matched, sessionToken, sent, replay, replayable) + resp, err = ps.handleUpstreamAuth(upstream, req, resp, matched, sessionToken, sent, replay, replayable) if err != nil { return nil, matched, outcome, err } @@ -632,6 +670,20 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio return resp, matched, outcome, nil } +// upstreamFor picks the transport a request leaves through. A SOCKS5 port is meant to face the internet, so +// under the any-host policy a destination no service names may not be private: only an admin naming a host +// in a bundle makes an internal address reachable that way. Under bundle-hosts every reachable host is +// already named. Exceptions do not count here, because the UI hides them once the policy is any-host. +func (ps *proxyServer) upstreamFor(t tunnel, matched *resolvedService) (http.RoundTripper, error) { + if !t.viaSocks5 || ps.socks5AllowPrivate || matched != nil || ps.currentConfig().TrafficPolicy != TrafficPolicyAnyHost { + return ps.transport, nil + } + if ps.guardedTransport == nil { + return nil, errPrivateDestination + } + return ps.guardedTransport, nil +} + func containsSurface(surfaces []string, target string) bool { for _, surface := range surfaces { if surface == target { diff --git a/packages/agentvault/run.go b/packages/agentvault/run.go index ee6c50e3c..15893c651 100644 --- a/packages/agentvault/run.go +++ b/packages/agentvault/run.go @@ -10,6 +10,7 @@ import ( "net/http" "os" "os/signal" + "strconv" "syscall" "time" @@ -361,12 +362,16 @@ func serve(opts Options, s startup) error { } ps := &proxyServer{ - opts: opts, - ca: ca, - transport: newUpstreamTransport(), - config: config, - persisted: state, - legacy: s.legacy, + opts: opts, + ca: ca, + transport: newUpstreamTransport(), + guardedTransport: newGuardedUpstreamTransport(), + config: config, + persisted: state, + legacy: s.legacy, + } + if opts.Socks5 != nil { + ps.socks5AllowPrivate = opts.Socks5.AllowPrivateDestinations } resolver, err := newInfisicalResolver(opts.ProxyToken) if err != nil { @@ -381,9 +386,41 @@ func serve(opts Options, s startup) error { ps.sessionLogs = newSessionLogRecorder(state.ProxyID, shipper) // Port 0 is not "unset": it is the ordinary ask for any free port, so it is never substituted. - listener, err := net.Listen("tcp", fmt.Sprintf(":%d", opts.Port)) + listenAddr := net.JoinHostPort(opts.ListenHost, strconv.Itoa(opts.Port)) + listener, err := net.Listen("tcp", listenAddr) if err != nil { - return fmt.Errorf("failed to listen on port %d: %w", opts.Port, err) + return fmt.Errorf("failed to listen on %s: %w", listenAddr, err) + } + + // Opened before anything starts, so a SOCKS5 port that can't be used stops the start instead of leaving a + // proxy that only half works. It binds every interface: it is the door meant to face the network. + var socks *socks5Server + var socksListener net.Listener + if opts.Socks5 != nil { + tlsConfig := socks5TLSConfig(ca, opts.Socks5) + if opts.Socks5.Cert == nil { + if _, err := ca.mintLeaf(opts.Socks5.TLSName); err != nil { + _ = listener.Close() + return fmt.Errorf("failed to make a certificate for %s: %w", opts.Socks5.TLSName, err) + } + } + raw, err := net.Listen("tcp", fmt.Sprintf(":%d", opts.Socks5.Port)) + if err != nil { + _ = listener.Close() + return fmt.Errorf("failed to listen for SOCKS5 on port %d: %w", opts.Socks5.Port, err) + } + socksListener = newLimitListener(raw, maxConcurrentConns, func() { + log.Warn(). + Int("limit", maxConcurrentConns). + Str("door", "socks5"). + Msg("agent-vault: SOCKS5 connection limit reached, new connections are waiting for a slot") + }) + socks = newSocks5Server(ps, tlsConfig) + } + closeSocks := func() { + if socksListener != nil { + _ = socksListener.Close() + } } front := &http.Server{ @@ -407,9 +444,20 @@ func serve(opts Options, s startup) error { serveErr := make(chan error, 1) go func() { serveErr <- front.Serve(limited) }() + // Stays nil without SOCKS5, so its select arm never fires. + var socksErr chan error + if socks != nil { + socksErr = make(chan error, 1) + go func() { socksErr <- socks.serve(socksListener) }() + } + // The bound port, not the requested one: they differ whenever port 0 was asked for. listening := log.Info(). - Int("port", listener.Addr().(*net.TCPAddr).Port). + Int("port", listener.Addr().(*net.TCPAddr).Port) + if opts.ListenHost != "" { + listening = listening.Str("host", opts.ListenHost) + } + listening = listening. Str("proxyId", state.ProxyID). Str("name", state.ProxyName). Str("fingerprint", ca.Fingerprint()) @@ -418,6 +466,28 @@ func serve(opts Options, s startup) error { } listening.Msg("agent-vault: proxy listening") + if socks != nil { + socksListening := log.Info(). + Str("door", "socks5"). + Int("port", socksListener.Addr().(*net.TCPAddr).Port). + Bool("allowPrivateDestinations", opts.Socks5.AllowPrivateDestinations) + if cert := opts.Socks5.Cert; cert != nil { + socksListening = socksListening. + Str("certificate", "own"). + Str("certificateFingerprint", cert.Fingerprint()). + Time("certificateExpiresAt", cert.NotAfter()) + } else { + socksListening = socksListening. + Str("certificate", "minted from the proxy's certificate authority"). + Str("tlsName", opts.Socks5.TLSName) + } + socksListening.Msg("agent-vault: SOCKS5 listening") + if opts.Socks5.AllowPrivateDestinations { + log.Warn().Str("door", "socks5"). + Msg("agent-vault: SOCKS5 traffic may reach private, loopback and link-local addresses") + } + } + signals := make(chan os.Signal, 1) signal.Notify(signals, os.Interrupt, syscall.SIGTERM) @@ -431,15 +501,30 @@ func serve(opts Options, s startup) error { select { case err := <-serveErr: close(stop) + closeSocks() closeSessionLogs() ps.cache.close() if errors.Is(err, http.ErrServerClosed) { return nil } return err + case err := <-socksErr: + // serve returns nil only for a closed listener, and nothing closes it before this select ends. + if err == nil { + err = net.ErrClosed + } + close(stop) + closeSocks() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + _ = front.Shutdown(ctx) + closeSessionLogs() + ps.cache.close() + return fmt.Errorf("the SOCKS5 listener stopped: %w", err) case <-signals: log.Info().Msg("agent-vault: shutting down") close(stop) + closeSocks() ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() _ = front.Shutdown(ctx) @@ -448,6 +533,7 @@ func serve(opts Options, s startup) error { return nil case err := <-fatal: close(stop) + closeSocks() ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() _ = front.Shutdown(ctx) diff --git a/packages/agentvault/socks5.go b/packages/agentvault/socks5.go new file mode 100644 index 000000000..f05c0d6dd --- /dev/null +++ b/packages/agentvault/socks5.go @@ -0,0 +1,370 @@ +package agentvault + +import ( + "crypto/tls" + "encoding/binary" + "errors" + "fmt" + "io" + "net" + "net/netip" + "strconv" + "sync" + "sync/atomic" + "time" + + "github.com/rs/zerolog/log" +) + +// Socks5Options turns on a TLS-wrapped SOCKS5 listener beside the HTTP one, for clients such as E2B that +// tunnel a sandbox's traffic to a proxy from outside the sandbox, with the session token as the password. +type Socks5Options struct { + Port int + // The name the listener's certificate is minted for. Ignored when Cert is set. + TLSName string + // The operator's own certificate for the listener, in place of one minted from the proxy's CA. + Cert *ListenerCert + AllowPrivateDestinations bool +} + +// RFC 1928 and RFC 1929. +const ( + socks5Version = 0x05 + socks5AuthVersion = 0x01 + socks5MethodUserPass = 0x02 + socks5MethodNone = 0xff + socks5CmdConnect = 0x01 + socks5AtypIPv4 = 0x01 + socks5AtypDomain = 0x03 + socks5AtypIPv6 = 0x04 + socks5RepSucceeded = 0x00 + socks5RepFailure = 0x01 + socks5RepCmdUnsup = 0x07 + socks5RepAtypUnsup = 0x08 + socks5AuthStatusOK = 0x00 + socks5AuthStatusRefus = 0x01 +) + +// A session token is "agv_" and 32 random bytes in unpadded base64url (backend +// agent-vault-session-fns.ts). Anything else is refused without asking Infisical. +const ( + sessionTokenPrefix = "agv_" + sessionTokenLen = len(sessionTokenPrefix) + 43 +) + +func plausibleSessionToken(s string) bool { + if len(s) != sessionTokenLen || s[:len(sessionTokenPrefix)] != sessionTokenPrefix { + return false + } + for i := len(sessionTokenPrefix); i < len(s); i++ { + c := s[i] + if !(c >= 'A' && c <= 'Z' || c >= 'a' && c <= 'z' || c >= '0' && c <= '9' || c == '-' || c == '_') { + return false + } + } + return true +} + +var errSocks5Version = errors.New("not a SOCKS5 client") + +// Reads are io.ReadFull straight off the connection, never through a bufio.Reader: whatever the client sends +// after the request belongs to the tunnel, and a buffer here would swallow it. +func readSocks5Greeting(r io.Reader) (offersUserPass bool, err error) { + var head [2]byte + if _, err := io.ReadFull(r, head[:]); err != nil { + return false, err + } + if head[0] != socks5Version { + return false, errSocks5Version + } + methods := make([]byte, head[1]) + if _, err := io.ReadFull(r, methods); err != nil { + return false, err + } + for _, m := range methods { + if m == socks5MethodUserPass { + return true, nil + } + } + return false, nil +} + +// The username is read and dropped: the password alone is the session token, as on the HTTP door. +func readSocks5Auth(r io.Reader) (password string, err error) { + var head [2]byte + if _, err := io.ReadFull(r, head[:]); err != nil { + return "", err + } + if head[0] != socks5AuthVersion { + return "", fmt.Errorf("unsupported auth version %d", head[0]) + } + if _, err := io.CopyN(io.Discard, r, int64(head[1])); err != nil { + return "", err + } + var plen [1]byte + if _, err := io.ReadFull(r, plen[:]); err != nil { + return "", err + } + pass := make([]byte, plen[0]) + if _, err := io.ReadFull(r, pass); err != nil { + return "", err + } + return string(pass), nil +} + +type socks5Request struct { + cmd byte + host string + port uint16 +} + +// rep is the reply code to send when err is set, or 0 when the connection should just close. The reserved +// byte is not checked: other servers ignore it too, and refusing on it would only break odd clients. +func readSocks5Request(r io.Reader) (req socks5Request, rep byte, err error) { + var head [4]byte + if _, err := io.ReadFull(r, head[:]); err != nil { + return req, 0, err + } + if head[0] != socks5Version { + return req, socks5RepFailure, errSocks5Version + } + req.cmd = head[1] + + switch head[3] { + case socks5AtypIPv4: + var b [4]byte + if _, err := io.ReadFull(r, b[:]); err != nil { + return req, 0, err + } + req.host = netip.AddrFrom4(b).String() + case socks5AtypIPv6: + var b [16]byte + if _, err := io.ReadFull(r, b[:]); err != nil { + return req, 0, err + } + req.host = netip.AddrFrom16(b).String() + case socks5AtypDomain: + var n [1]byte + if _, err := io.ReadFull(r, n[:]); err != nil { + return req, 0, err + } + b := make([]byte, n[0]) + if _, err := io.ReadFull(r, b); err != nil { + return req, 0, err + } + req.host = string(b) + default: + // The address length is unknown, so nothing after it can be read. + return req, socks5RepAtypUnsup, fmt.Errorf("unsupported address type %d", head[3]) + } + + var port [2]byte + if _, err := io.ReadFull(r, port[:]); err != nil { + return req, 0, err + } + req.port = binary.BigEndian.Uint16(port[:]) + + // After the whole frame is read, so the refusal still reaches the client. + if req.cmd != socks5CmdConnect { + return req, socks5RepCmdUnsup, fmt.Errorf("unsupported command %d", req.cmd) + } + if req.host == "" || req.port == 0 { + return req, socks5RepFailure, errors.New("the request names no host or port") + } + return req, 0, nil +} + +func writeSocks5Reply(w io.Writer, rep byte) error { + _, err := w.Write([]byte{socks5Version, rep, 0x00, socks5AtypIPv4, 0, 0, 0, 0, 0, 0}) + return err +} + +const ( + socks5StepTimeout = 10 * time.Second + socks5MaxAcceptBackoff = time.Second + socks5MismatchWarnEvery = time.Minute +) + +type socks5Server struct { + ps *proxyServer + tlsConfig *tls.Config + // The client's own steps: TLS, greeting, login and request. Asking Infisical about the token runs on its + // own timeout, so a slow Infisical does not cut off an agent that did nothing wrong. + stepTimeout time.Duration + + conns sync.WaitGroup + + lastMismatchWarn atomic.Int64 +} + +func newSocks5Server(ps *proxyServer, tlsConfig *tls.Config) *socks5Server { + return &socks5Server{ps: ps, tlsConfig: tlsConfig, stepTimeout: socks5StepTimeout} +} + +// serve returns nil once the listener is closed. A temporary accept error, such as running out of file +// descriptors, is waited out the way net/http does rather than ending the listener. +func (s *socks5Server) serve(l net.Listener) error { + var backoff time.Duration + for { + conn, err := l.Accept() + if err != nil { + if errors.Is(err, net.ErrClosed) { + return nil + } + var temporary interface{ Temporary() bool } + if errors.As(err, &temporary) && temporary.Temporary() { + if backoff == 0 { + backoff = 5 * time.Millisecond + } else { + backoff = min(backoff*2, socks5MaxAcceptBackoff) + } + log.Warn().Err(err).Dur("retryIn", backoff).Msg("agent-vault: SOCKS5 accept failed, retrying") + time.Sleep(backoff) + continue + } + return err + } + backoff = 0 + s.conns.Add(1) + go func() { + defer s.conns.Done() + s.handle(conn) + }() + } +} + +// wait blocks until every connection handled so far has returned. +func (s *socks5Server) wait() { s.conns.Wait() } + +func (s *socks5Server) handle(raw net.Conn) { + remote := raw.RemoteAddr().String() + // net/http recovers a panicking handler for the HTTP door; this goroutine has nothing above it, so a panic + // here would take both doors down. + defer func() { + if r := recover(); r != nil { + log.Error().Interface("panic", r).Str("door", "socks5").Str("remote", remote). + Msg("agent-vault: SOCKS5 connection handler panicked") + } + }() + defer raw.Close() + + _ = raw.SetDeadline(time.Now().Add(s.stepTimeout)) + // Peeked rather than left to the TLS handshake: a plain SOCKS5 greeting is shorter than a TLS record + // header, so the handshake would only time out and never say why. + buffered := newBufferedConn(raw) + first, err := buffered.reader.Peek(1) + if err != nil { + return + } + if first[0] == socks5Version { + s.warnMismatch(remote) + return + } + conn := tls.Server(buffered, s.tlsConfig) + if err := conn.Handshake(); err != nil { + log.Debug().Err(err).Str("door", "socks5").Str("remote", remote).Msg("agent-vault: SOCKS5 TLS handshake failed") + return + } + + offersUserPass, err := readSocks5Greeting(conn) + if err != nil { + log.Debug().Err(err).Str("door", "socks5").Str("remote", remote).Msg("agent-vault: SOCKS5 greeting unreadable") + return + } + if !offersUserPass { + _, _ = conn.Write([]byte{socks5Version, socks5MethodNone}) + log.Debug().Str("door", "socks5").Str("remote", remote).Msg("agent-vault: refused, the client offered no username and password") + return + } + if _, err := conn.Write([]byte{socks5Version, socks5MethodUserPass}); err != nil { + return + } + + sessionToken, err := readSocks5Auth(conn) + if err != nil { + log.Debug().Err(err).Str("door", "socks5").Str("remote", remote).Msg("agent-vault: SOCKS5 login unreadable") + return + } + if !plausibleSessionToken(sessionToken) { + _, _ = conn.Write([]byte{socks5AuthVersion, socks5AuthStatusRefus}) + log.Debug().Str("door", "socks5").Str("remote", remote).Msg("agent-vault: refused, the password is not a session token") + return + } + + _ = raw.SetDeadline(time.Time{}) + _, err = s.ps.cache.get(sessionToken) + _ = raw.SetDeadline(time.Now().Add(s.stepTimeout)) + if err != nil { + refusal := classifyGateRefusal(err) + refusal.event. + Str("door", "socks5"). + Str("remote", remote). + Str("sessionKey", sessionKey(sessionToken)). + Str("decision", refusal.decision). + Msg(refusal.msg) + _, _ = conn.Write([]byte{socks5AuthVersion, socks5AuthStatusRefus}) + return + } + if _, err := conn.Write([]byte{socks5AuthVersion, socks5AuthStatusOK}); err != nil { + return + } + + req, rep, err := readSocks5Request(conn) + if err != nil { + if rep != 0 { + _ = writeSocks5Reply(conn, rep) + } + log.Debug().Err(err).Str("door", "socks5").Str("remote", remote).Msg("agent-vault: SOCKS5 request refused") + return + } + hostname, port, err := checkedTarget(req.host, strconv.Itoa(int(req.port))) + if err != nil { + _ = writeSocks5Reply(conn, socks5RepFailure) + log.Debug().Err(err).Str("door", "socks5").Str("remote", remote).Msg("agent-vault: SOCKS5 request refused") + return + } + + leaf, err := s.ps.ca.mintLeaf(hostname) + if err != nil { + _ = writeSocks5Reply(conn, socks5RepFailure) + log.Error().Err(err).Str("door", "socks5").Str("host", hostname).Msg("agent-vault: failed to mint a certificate") + return + } + if err := writeSocks5Reply(conn, socks5RepSucceeded); err != nil { + return + } + _ = raw.SetDeadline(time.Time{}) + + s.ps.serveIntercepted(conn, tunnel{hostname: hostname, port: port, sessionToken: sessionToken, viaSocks5: true}, leaf) +} + +// A first byte of 0x05 on the TLS port means the client was set up for plain SOCKS5, the likeliest setup +// mistake. Rate limited, since a misconfigured client repeats it on every connection. +func (s *socks5Server) warnMismatch(remote string) { + now := time.Now().UnixNano() + last := s.lastMismatchWarn.Load() + if now-last < int64(socks5MismatchWarnEvery) || !s.lastMismatchWarn.CompareAndSwap(last, now) { + return + } + log.Warn().Str("door", "socks5").Str("remote", remote). + Msg("agent-vault: a client spoke plain SOCKS5 to the TLS port; turn on TLS in the client's proxy settings") +} + +func socks5TLSConfig(ca *caManager, opts *Socks5Options) *tls.Config { + cfg := &tls.Config{MinVersion: tls.VersionTLS12} + if opts.Cert != nil { + cfg.Certificates = []tls.Certificate{opts.Cert.certificate} + return cfg + } + // Per handshake rather than once, so the 24-hour leaf is replaced before it expires. The client's SNI is + // ignored on purpose: an unauthenticated client must not choose what gets minted. + name := opts.TLSName + cfg.GetCertificate = func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + leaf, err := ca.mintLeaf(name) + if err != nil { + return nil, err + } + return &leaf, nil + } + return cfg +} diff --git a/packages/agentvault/socks5_e2e_test.go b/packages/agentvault/socks5_e2e_test.go new file mode 100644 index 000000000..823d5046a --- /dev/null +++ b/packages/agentvault/socks5_e2e_test.go @@ -0,0 +1,572 @@ +package agentvault + +import ( + "bufio" + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/binary" + "encoding/pem" + "errors" + "fmt" + "io" + "math/big" + "net" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/Infisical/infisical-merge/packages/api" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +var testSocks5Token = "agv_" + strings.Repeat("t", 43) + +type socks5Fixture struct { + ps *proxyServer + addr string + proxyCAs *x509.CertPool + server *socks5Server + upstream *httptest.Server + hits *atomic.Int64 +} + +// A proxy whose SOCKS5 port is served over TLS with a certificate minted for 127.0.0.1, in front of an +// echoing TLS upstream on 127.0.0.1. Both upstream transports trust the upstream's certificate. +func newSocks5Fixture(t *testing.T, resolver sessionResolver, configure func(*proxyServer, *Socks5Options, *socks5Server)) *socks5Fixture { + t.Helper() + + hits := &atomic.Int64{} + upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"method":%q,"path":%q,"headers":{"X-Injected":[%q]}}`, r.Method, r.URL.Path, r.Header.Get("X-Injected")) + })) + t.Cleanup(upstream.Close) + upstreamPool := x509.NewCertPool() + upstreamPool.AddCert(upstream.Certificate()) + + key, cert, err := generateRootCa() + if err != nil { + t.Fatal(err) + } + transport := newUpstreamTransport() + transport.TLSClientConfig = &tls.Config{RootCAs: upstreamPool} + guarded := newGuardedUpstreamTransport() + guarded.TLSClientConfig = &tls.Config{RootCAs: upstreamPool} + + ps := &proxyServer{transport: transport, guardedTransport: guarded, ca: newCaManager(key, cert)} + ps.setConfig(ProxyConfig{TrafficPolicy: TrafficPolicyAnyHost}) + ps.cache = newSessionCache(resolver, ps.pollInterval) + + opts := &Socks5Options{TLSName: "127.0.0.1"} + server := newSocks5Server(ps, nil) + server.stepTimeout = 2 * time.Second + if configure != nil { + configure(ps, opts, server) + } + server.tlsConfig = socks5TLSConfig(ps.ca, opts) + + raw, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { done <- server.serve(raw) }() + t.Cleanup(func() { + _ = raw.Close() + <-done + server.wait() + }) + + pool := x509.NewCertPool() + pool.AddCert(cert) + return &socks5Fixture{ps: ps, addr: raw.Addr().String(), proxyCAs: pool, server: server, upstream: upstream, hits: hits} +} + +func (f *socks5Fixture) upstreamPort(t *testing.T) uint16 { + t.Helper() + u, err := url.Parse(f.upstream.URL) + if err != nil { + t.Fatal(err) + } + port, _ := strconv.Atoi(u.Port()) + return uint16(port) +} + +// dialSocks5 does what E2B's host does: TLS to the proxy, then the SOCKS5 greeting, login and CONNECT. It +// returns the reply codes so a test can check a refusal, and the connection when the tunnel opened. +func dialSocks5(t *testing.T, addr string, outer *tls.Config, user, password, host string, port uint16) (net.Conn, byte, byte) { + t.Helper() + raw, err := net.DialTimeout("tcp", addr, 5*time.Second) + if err != nil { + t.Fatal(err) + } + conn := tls.Client(raw, outer) + _ = conn.SetDeadline(time.Now().Add(10 * time.Second)) + + frame := []byte{5, 1, socks5MethodUserPass} + frame = append(frame, 1, byte(len(user))) + frame = append(frame, user...) + frame = append(frame, byte(len(password))) + frame = append(frame, password...) + if _, err := conn.Write(frame); err != nil { + t.Fatal(err) + } + var method [2]byte + if _, err := io.ReadFull(conn, method[:]); err != nil { + t.Fatalf("no method selection: %v", err) + } + var auth [2]byte + if _, err := io.ReadFull(conn, auth[:]); err != nil { + conn.Close() + return nil, 0xff, 0 + } + if auth[1] != socks5AuthStatusOK { + conn.Close() + return nil, auth[1], 0 + } + + req := []byte{5, socks5CmdConnect, 0} + if ip := net.ParseIP(host); ip != nil && ip.To4() != nil { + req = append(req, socks5AtypIPv4) + req = append(req, ip.To4()...) + } else { + req = append(req, socks5AtypDomain, byte(len(host))) + req = append(req, host...) + } + req = binary.BigEndian.AppendUint16(req, port) + if _, err := conn.Write(req); err != nil { + t.Fatal(err) + } + var reply [10]byte + if _, err := io.ReadFull(conn, reply[:]); err != nil { + t.Fatalf("no reply: %v", err) + } + if reply[1] != socks5RepSucceeded { + conn.Close() + return nil, auth[1], reply[1] + } + _ = conn.SetDeadline(time.Time{}) + return conn, auth[1], reply[1] +} + +func (f *socks5Fixture) outerTLS() *tls.Config { + return &tls.Config{RootCAs: f.proxyCAs, ServerName: "127.0.0.1"} +} + +// get sends one HTTPS request through a SOCKS5 tunnel to host:port and returns the status and body. +func (f *socks5Fixture) get(t *testing.T, host string, port uint16) (int, string) { + t.Helper() + conn, auth, rep := dialSocks5(t, f.addr, f.outerTLS(), ProxyAuthUsername, testSocks5Token, host, port) + if conn == nil { + t.Fatalf("tunnel refused: auth=%#x rep=%#x", auth, rep) + } + defer conn.Close() + inner := tls.Client(conn, &tls.Config{RootCAs: f.proxyCAs, ServerName: host}) + req, _ := http.NewRequest(http.MethodGet, "https://"+net.JoinHostPort(host, strconv.Itoa(int(port)))+"/x", nil) + if err := req.Write(inner); err != nil { + t.Fatal(err) + } + resp, err := http.ReadResponse(bufio.NewReader(inner), req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + return resp.StatusCode, strings.TrimSpace(string(body)) +} + +func bearerService(host string) *resolvedService { + return &resolvedService{ + name: "api", + accessBundleName: "bundle", + hostPatterns: parseHostPatterns(host), + credential: credential{kind: credentialBearer, headerName: "X-Injected", value: []byte("s3cr3t")}, + } +} + +func TestSocks5BrokersARequestOverTLS(t *testing.T) { + var f *socks5Fixture + f = newSocks5Fixture(t, resolverFunc(func() (*resolveResult, error) { + return &resolveResult{SessionID: "s1", Services: []*resolvedService{bearerService("127.0.0.1:" + strconv.Itoa(int(f.upstreamPort(t))))}}, nil + }), nil) + + status, body := f.get(t, "127.0.0.1", f.upstreamPort(t)) + if status != http.StatusOK || !strings.Contains(body, "s3cr3t") { + t.Fatalf("status=%d body=%s, want the injected credential echoed", status, body) + } +} + +func TestSocks5ServesTheOperatorsOwnCertificate(t *testing.T) { + certPEM, keyPEM, pool := selfSignedListenerCert(t, "127.0.0.1", time.Now().Add(time.Hour)) + own, err := LoadListenerCert(OwnCaInput{Cert: certPEM, CertSource: "--socks5-tls-cert", Key: keyPEM, KeySource: "--socks5-tls-key"}) + if err != nil { + t.Fatal(err) + } + var f *socks5Fixture + f = newSocks5Fixture(t, resolverFunc(func() (*resolveResult, error) { + return &resolveResult{SessionID: "s1", Services: []*resolvedService{bearerService("127.0.0.1:" + strconv.Itoa(int(f.upstreamPort(t))))}}, nil + }), func(_ *proxyServer, opts *Socks5Options, _ *socks5Server) { opts.Cert = own }) + + conn, auth, rep := dialSocks5(t, f.addr, &tls.Config{RootCAs: pool, ServerName: "127.0.0.1"}, "", testSocks5Token, "127.0.0.1", f.upstreamPort(t)) + if conn == nil { + t.Fatalf("tunnel refused: auth=%#x rep=%#x", auth, rep) + } + conn.Close() +} + +func TestLoadListenerCertRefusesAnExpiredCertificate(t *testing.T) { + certPEM, keyPEM, _ := selfSignedListenerCert(t, "proxy.example.com", time.Now().Add(-time.Minute)) + _, err := LoadListenerCert(OwnCaInput{Cert: certPEM, CertSource: "--socks5-tls-cert", Key: keyPEM, KeySource: "--socks5-tls-key"}) + if err == nil || !strings.Contains(err.Error(), "expired") { + t.Fatalf("err = %v, want an expiry error", err) + } +} + +func TestSocks5RefusesAtLogin(t *testing.T) { + for _, tc := range []struct { + name string + err error + msg string + }{ + {"revoked or expired session", &api.APIError{StatusCode: 401, Name: infisicalUnauthorizedName}, "the session is no longer valid"}, + {"proxy token rejected", &api.APIError{StatusCode: 401, Name: proxyTokenRejectedName}, "Infisical rejected this proxy's token"}, + {"Infisical unreachable", errors.New("dial tcp: connection refused"), "the session could not be resolved"}, + } { + t.Run(tc.name, func(t *testing.T) { + var logs bytes.Buffer + restore := log.Logger + log.Logger = zerolog.New(&syncWriter{w: &logs}) + t.Cleanup(func() { log.Logger = restore }) + + f := newSocks5Fixture(t, &stubResolver{err: tc.err}, nil) + conn, auth, _ := dialSocks5(t, f.addr, f.outerTLS(), ProxyAuthUsername, testSocks5Token, "127.0.0.1", 443) + if conn != nil || auth != socks5AuthStatusRefus { + t.Fatalf("auth = %#x, want the login refused", auth) + } + f.server.wait() + out := logs.String() + if !strings.Contains(out, tc.msg) || !strings.Contains(out, `"door":"socks5"`) || !strings.Contains(out, `"remote":`) { + t.Fatalf("refusal not logged as expected: %s", out) + } + if strings.Contains(out, testSocks5Token) { + t.Fatal("the session token reached the log") + } + }) + } +} + +func TestSocks5JunkPasswordNeverAsksInfisical(t *testing.T) { + resolver := &stubResolver{result: &resolveResult{SessionID: "s1"}} + f := newSocks5Fixture(t, resolver, nil) + for _, password := range []string{"", "hunter2", testSocks5Token + "\n"} { + conn, auth, _ := dialSocks5(t, f.addr, f.outerTLS(), ProxyAuthUsername, password, "127.0.0.1", 443) + if conn != nil || auth != socks5AuthStatusRefus { + t.Fatalf("password %q: auth = %#x, want refused", password, auth) + } + } + if n := resolver.callCount(); n != 0 { + t.Fatalf("Infisical was asked %d times about passwords that are not session tokens", n) + } +} + +func TestSocks5AcceptsAnEmptyUsername(t *testing.T) { + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, nil) + conn, auth, rep := dialSocks5(t, f.addr, f.outerTLS(), "", testSocks5Token, "127.0.0.1", 443) + if conn == nil { + t.Fatalf("tunnel refused: auth=%#x rep=%#x", auth, rep) + } + conn.Close() +} + +func TestSocks5SessionRevokedMidTunnelGets403(t *testing.T) { + f := newSocks5Fixture(t, expiringResolver{ttl: 300 * time.Millisecond}, func(ps *proxyServer, _ *Socks5Options, _ *socks5Server) { + ps.socks5AllowPrivate = true + }) + conn, auth, rep := dialSocks5(t, f.addr, f.outerTLS(), ProxyAuthUsername, testSocks5Token, "127.0.0.1", f.upstreamPort(t)) + if conn == nil { + t.Fatalf("tunnel refused: auth=%#x rep=%#x", auth, rep) + } + defer conn.Close() + time.Sleep(400 * time.Millisecond) + + inner := tls.Client(conn, &tls.Config{RootCAs: f.proxyCAs, ServerName: "127.0.0.1"}) + req, _ := http.NewRequest(http.MethodGet, "https://127.0.0.1/x", nil) + if err := req.Write(inner); err != nil { + t.Fatal(err) + } + resp, err := http.ReadResponse(bufio.NewReader(inner), req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusForbidden || !strings.Contains(string(body), "no longer valid") { + t.Fatalf("status=%d body=%s, want 403 session no longer valid", resp.StatusCode, body) + } +} + +func TestSocks5PrivateDestinations(t *testing.T) { + t.Run("any-host: a host no service names that resolves to loopback is refused", func(t *testing.T) { + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, nil) + status, body := f.get(t, "localhost", f.upstreamPort(t)) + if status != http.StatusForbidden || !strings.Contains(body, errPrivateDestination.Error()) { + t.Fatalf("status=%d body=%s, want 403 private destination", status, body) + } + if strings.Contains(body, "127.0.0.1") || strings.Contains(body, "::1") { + t.Fatalf("the refusal named the internal address: %s", body) + } + if f.hits.Load() != 0 { + t.Fatal("the upstream was reached") + } + }) + + t.Run("any-host: a private host a service names is allowed", func(t *testing.T) { + var f *socks5Fixture + f = newSocks5Fixture(t, resolverFunc(func() (*resolveResult, error) { + return &resolveResult{SessionID: "s1", Services: []*resolvedService{bearerService("127.0.0.1:" + strconv.Itoa(int(f.upstreamPort(t))))}}, nil + }), nil) + if status, body := f.get(t, "127.0.0.1", f.upstreamPort(t)); status != http.StatusOK { + t.Fatalf("status=%d body=%s", status, body) + } + }) + + t.Run("any-host: a hidden exception does not make a private host reachable", func(t *testing.T) { + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, func(ps *proxyServer, _ *Socks5Options, _ *socks5Server) { + ps.setConfig(ProxyConfig{TrafficPolicy: TrafficPolicyAnyHost, AllowedHosts: "127.0.0.1"}) + }) + if status, _ := f.get(t, "127.0.0.1", f.upstreamPort(t)); status != http.StatusForbidden { + t.Fatalf("status=%d, want 403", status) + } + }) + + t.Run("bundle-hosts: an exception the admin set is allowed", func(t *testing.T) { + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, func(ps *proxyServer, _ *Socks5Options, _ *socks5Server) { + ps.setConfig(ProxyConfig{TrafficPolicy: TrafficPolicyBundleHosts, AllowedHosts: "127.0.0.1"}) + }) + if status, body := f.get(t, "127.0.0.1", f.upstreamPort(t)); status != http.StatusOK { + t.Fatalf("status=%d body=%s", status, body) + } + }) + + t.Run("the opt-out allows it", func(t *testing.T) { + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, func(ps *proxyServer, _ *Socks5Options, _ *socks5Server) { + ps.socks5AllowPrivate = true + }) + if status, body := f.get(t, "127.0.0.1", f.upstreamPort(t)); status != http.StatusOK { + t.Fatalf("status=%d body=%s", status, body) + } + }) + + t.Run("the HTTP door is unchanged", func(t *testing.T) { + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, nil) + front := httptest.NewServer(http.HandlerFunc(f.ps.dispatch)) + t.Cleanup(front.Close) + proxyURL, _ := url.Parse(front.URL) + proxyURL.User = url.UserPassword(ProxyAuthUsername, testSocks5Token) + client := &http.Client{Transport: &http.Transport{ + Proxy: http.ProxyURL(proxyURL), + TLSClientConfig: &tls.Config{RootCAs: f.proxyCAs}, + }} + status, body := do(t, client, http.MethodGet, fmt.Sprintf("https://127.0.0.1:%d/x", f.upstreamPort(t)), "") + if status != http.StatusOK { + t.Fatalf("status=%d body=%s", status, body) + } + }) + + t.Run("a missing guarded transport refuses rather than skipping the check", func(t *testing.T) { + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, func(ps *proxyServer, _ *Socks5Options, _ *socks5Server) { + ps.guardedTransport = nil + }) + if status, _ := f.get(t, "localhost", f.upstreamPort(t)); status != http.StatusForbidden { + t.Fatalf("status=%d, want 403", status) + } + }) +} + +func TestSocks5ClosesAClientThatSaysNothing(t *testing.T) { + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, func(_ *proxyServer, _ *Socks5Options, s *socks5Server) { + s.stepTimeout = 200 * time.Millisecond + }) + conn, err := net.Dial("tcp", f.addr) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + if _, err := conn.Read(make([]byte, 1)); err == nil || errors.Is(err, os.ErrDeadlineExceeded) { + t.Fatalf("the connection was not closed by the server: %v", err) + } +} + +func TestSocks5WaitsOutASlowInfisical(t *testing.T) { + resolver := &stubResolver{result: &resolveResult{SessionID: "s1"}, delay: 600 * time.Millisecond} + f := newSocks5Fixture(t, resolver, func(_ *proxyServer, _ *Socks5Options, s *socks5Server) { + s.stepTimeout = 300 * time.Millisecond + }) + conn, auth, rep := dialSocks5(t, f.addr, f.outerTLS(), ProxyAuthUsername, testSocks5Token, "127.0.0.1", 443) + if conn == nil { + t.Fatalf("a slow Infisical cut the agent off: auth=%#x rep=%#x", auth, rep) + } + conn.Close() +} + +type panickingResolver struct{} + +func (panickingResolver) resolve(string, *sessionLogGrant) (*resolveResult, error) { + panic("boom") +} + +func TestSocks5RecoversAPanickingConnection(t *testing.T) { + var logs bytes.Buffer + restore := log.Logger + log.Logger = zerolog.New(&syncWriter{w: &logs}) + t.Cleanup(func() { log.Logger = restore }) + + f := newSocks5Fixture(t, panickingResolver{}, nil) + _, _, _ = dialSocks5(t, f.addr, f.outerTLS(), ProxyAuthUsername, testSocks5Token, "127.0.0.1", 443) + f.server.wait() + if !strings.Contains(logs.String(), "panicked") { + t.Fatalf("the panic was not logged: %s", logs.String()) + } + // The listener still accepts. + conn, err := net.Dial("tcp", f.addr) + if err != nil { + t.Fatalf("the listener stopped after a panic: %v", err) + } + conn.Close() +} + +func TestSocks5WarnsWhenAClientSkipsTLS(t *testing.T) { + var logs bytes.Buffer + restore := log.Logger + log.Logger = zerolog.New(&syncWriter{w: &logs}) + t.Cleanup(func() { log.Logger = restore }) + + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, nil) + for i := 0; i < 2; i++ { + conn, err := net.Dial("tcp", f.addr) + if err != nil { + t.Fatal(err) + } + _, _ = conn.Write([]byte{5, 1, 2}) + _, _ = io.ReadAll(conn) + conn.Close() + } + f.server.wait() + if n := strings.Count(logs.String(), "spoke plain SOCKS5"); n != 1 { + t.Fatalf("warned %d times, want once: %s", n, logs.String()) + } +} + +// A listener whose first Accept fails with a temporary error, then hands out a real connection. +type flakyListener struct { + net.Listener + failed atomic.Bool +} + +type temporaryErr struct{} + +func (temporaryErr) Error() string { return "too many open files" } +func (temporaryErr) Temporary() bool { return true } +func (temporaryErr) Timeout() bool { return false } + +func (l *flakyListener) Accept() (net.Conn, error) { + if l.failed.CompareAndSwap(false, true) { + return nil, temporaryErr{} + } + return l.Listener.Accept() +} + +func TestSocks5RetriesATemporaryAcceptError(t *testing.T) { + raw, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + key, cert, err := generateRootCa() + if err != nil { + t.Fatal(err) + } + ps := &proxyServer{transport: newUpstreamTransport(), guardedTransport: newGuardedUpstreamTransport(), ca: newCaManager(key, cert)} + ps.cache = newSessionCache(&stubResolver{result: &resolveResult{SessionID: "s1"}}, ps.pollInterval) + server := newSocks5Server(ps, socks5TLSConfig(ps.ca, &Socks5Options{TLSName: "127.0.0.1"})) + done := make(chan error, 1) + go func() { done <- server.serve(&flakyListener{Listener: raw}) }() + + pool := x509.NewCertPool() + pool.AddCert(cert) + conn, auth, rep := dialSocks5(t, raw.Addr().String(), &tls.Config{RootCAs: pool, ServerName: "127.0.0.1"}, "", testSocks5Token, "127.0.0.1", 443) + if conn == nil { + t.Fatalf("no tunnel after a temporary accept error: auth=%#x rep=%#x", auth, rep) + } + conn.Close() + + _ = raw.Close() + if err := <-done; err != nil { + t.Fatalf("serve returned %v after the listener closed, want nil", err) + } + server.wait() +} + +type resolverFunc func() (*resolveResult, error) + +func (f resolverFunc) resolve(string, *sessionLogGrant) (*resolveResult, error) { return f() } + +// Handlers log from their own goroutines, so the buffer a test reads must be safe to write concurrently. +type syncWriter struct { + mu sync.Mutex + w io.Writer +} + +func (s *syncWriter) Write(p []byte) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.w.Write(p) +} + +func selfSignedListenerCert(t *testing.T, name string, notAfter time.Time) (certPEM, keyPEM string, pool *x509.CertPool) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: name}, + NotBefore: notAfter.Add(-2 * time.Hour), + NotAfter: notAfter, + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + } + if ip := net.ParseIP(name); ip != nil { + template.IPAddresses = []net.IP{ip} + } else { + template.DNSNames = []string{name} + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + keyDER, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatal(err) + } + parsed, _ := x509.ParseCertificate(der) + pool = x509.NewCertPool() + pool.AddCert(parsed) + return string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})), + string(pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER})), pool +} diff --git a/packages/agentvault/socks5_test.go b/packages/agentvault/socks5_test.go new file mode 100644 index 000000000..44d4e6b22 --- /dev/null +++ b/packages/agentvault/socks5_test.go @@ -0,0 +1,181 @@ +package agentvault + +import ( + "bytes" + "errors" + "io" + "strings" + "testing" +) + +func TestSocks5SessionTokenShape(t *testing.T) { + valid := "agv_" + strings.Repeat("aZ0-_", 8) + "abc" + for _, tc := range []struct { + name string + token string + want bool + }{ + {"a token as Infisical issues it", valid, true}, + {"empty", "", false}, + {"no prefix", "xyz_" + valid[4:], false}, + {"one character short", valid[:len(valid)-1], false}, + {"one character long", valid + "a", false}, + {"padding is not base64url", valid[:len(valid)-1] + "=", false}, + {"a trailing newline from an env file", valid[:len(valid)-1] + "\n", false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := plausibleSessionToken(tc.token); got != tc.want { + t.Errorf("plausibleSessionToken(%q) = %v, want %v", tc.token, got, tc.want) + } + }) + } +} + +func TestSocks5Greeting(t *testing.T) { + for _, tc := range []struct { + name string + frame []byte + offers bool + wantErr bool + }{ + {"username and password offered", []byte{5, 2, 0, 2}, true, false}, + {"only no-auth offered", []byte{5, 1, 0}, false, false}, + {"no methods at all", []byte{5, 0}, false, false}, + {"SOCKS4", []byte{4, 1, 0}, false, true}, + {"cut short", []byte{5, 2, 0}, false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + offers, err := readSocks5Greeting(bytes.NewReader(tc.frame)) + if (err != nil) != tc.wantErr || offers != tc.offers { + t.Errorf("got offers=%v err=%v, want offers=%v err=%v", offers, err, tc.offers, tc.wantErr) + } + }) + } +} + +func TestSocks5AuthIgnoresTheUsername(t *testing.T) { + for _, tc := range []struct { + name string + frame []byte + password string + wantErr bool + }{ + {"username and password", append([]byte{1, 3, 'b', 'o', 'b', 3}, "pwd"...), "pwd", false}, + {"empty username", append([]byte{1, 0, 3}, "pwd"...), "pwd", false}, + {"empty password", []byte{1, 3, 'b', 'o', 'b', 0}, "", false}, + {"wrong sub-negotiation version", append([]byte{5, 0, 3}, "pwd"...), "", true}, + {"cut short", []byte{1, 3, 'b'}, "", true}, + } { + t.Run(tc.name, func(t *testing.T) { + password, err := readSocks5Auth(bytes.NewReader(tc.frame)) + if (err != nil) != tc.wantErr || password != tc.password { + t.Errorf("got %q err=%v, want %q err=%v", password, err, tc.password, tc.wantErr) + } + }) + } +} + +func TestSocks5Request(t *testing.T) { + domain := func(cmd byte, host string, port uint16) []byte { + frame := []byte{5, cmd, 0, socks5AtypDomain, byte(len(host))} + frame = append(frame, host...) + return append(frame, byte(port>>8), byte(port)) + } + for _, tc := range []struct { + name string + frame []byte + host string + port uint16 + rep byte + }{ + {"domain", domain(1, "api.example.com", 443), "api.example.com", 443, 0}, + {"IPv4", []byte{5, 1, 0, socks5AtypIPv4, 10, 0, 0, 1, 0, 80}, "10.0.0.1", 80, 0}, + {"IPv6", append([]byte{5, 1, 0, socks5AtypIPv6, 0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, 1, 187), "2001:db8::1", 443, 0}, + {"a non-zero reserved byte is accepted", []byte{5, 1, 7, socks5AtypIPv4, 1, 2, 3, 4, 0, 80}, "1.2.3.4", 80, 0}, + {"BIND is refused", domain(2, "api.example.com", 443), "", 0, socks5RepCmdUnsup}, + {"UDP ASSOCIATE is refused", domain(3, "api.example.com", 443), "", 0, socks5RepCmdUnsup}, + {"unknown address type", []byte{5, 1, 0, 9, 1, 2, 3, 4, 0, 80}, "", 0, socks5RepAtypUnsup}, + {"empty domain", domain(1, "", 443), "", 0, socks5RepFailure}, + {"port 0", domain(1, "api.example.com", 0), "", 0, socks5RepFailure}, + {"wrong version", []byte{4, 1, 0, socks5AtypIPv4, 1, 2, 3, 4, 0, 80}, "", 0, socks5RepFailure}, + } { + t.Run(tc.name, func(t *testing.T) { + req, rep, err := readSocks5Request(bytes.NewReader(tc.frame)) + if tc.rep != 0 { + if err == nil || rep != tc.rep { + t.Fatalf("got rep=%#x err=%v, want rep=%#x", rep, err, tc.rep) + } + return + } + if err != nil || req.host != tc.host || req.port != tc.port { + t.Fatalf("got %+v err=%v, want %s:%d", req, err, tc.host, tc.port) + } + }) + } +} + +// Whatever the client sends after the request is the tunnel's first bytes, so the parsers must not read +// past their own frames. +func TestSocks5ParsersLeavePipelinedBytesUnread(t *testing.T) { + var stream bytes.Buffer + stream.Write([]byte{5, 1, 2}) + stream.Write(append([]byte{1, 1, 'u', 3}, "pwd"...)) + stream.Write([]byte{5, 1, 0, socks5AtypIPv4, 127, 0, 0, 1, 1, 187}) + stream.WriteString("\x16\x03\x01rest-of-the-client-hello") + + r := &stream + if _, err := readSocks5Greeting(r); err != nil { + t.Fatal(err) + } + if _, err := readSocks5Auth(r); err != nil { + t.Fatal(err) + } + if _, _, err := readSocks5Request(r); err != nil { + t.Fatal(err) + } + rest, _ := io.ReadAll(r) + if string(rest) != "\x16\x03\x01rest-of-the-client-hello" { + t.Fatalf("the tunnel's bytes were consumed: %q", rest) + } +} + +func FuzzSocks5Request(f *testing.F) { + f.Add([]byte{5, 1, 0, socks5AtypDomain, 3, 'a', '.', 'b', 1, 187}) + f.Add([]byte{5, 1, 0, socks5AtypIPv4, 127, 0, 0, 1, 0, 80}) + f.Add([]byte{5, 1, 0, socks5AtypIPv6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 80}) + f.Add([]byte{5, 2, 0, 9}) + f.Add([]byte{5, 1, 0, socks5AtypDomain, 255}) + f.Fuzz(func(t *testing.T, data []byte) { + r := bytes.NewReader(data) + req, rep, err := readSocks5Request(r) + if err != nil { + if rep != 0 && rep != socks5RepFailure && rep != socks5RepCmdUnsup && rep != socks5RepAtypUnsup { + t.Fatalf("unexpected reply code %#x", rep) + } + return + } + if req.host == "" || req.port == 0 || req.cmd != socks5CmdConnect { + t.Fatalf("accepted an incomplete request: %+v", req) + } + if consumed := len(data) - r.Len(); consumed > len(data) { + t.Fatal("read past the input") + } + }) +} + +func TestSocks5ReplyIsAlwaysWellFormed(t *testing.T) { + var buf bytes.Buffer + if err := writeSocks5Reply(&buf, socks5RepCmdUnsup); err != nil { + t.Fatal(err) + } + if got := buf.Bytes(); !bytes.Equal(got, []byte{5, socks5RepCmdUnsup, 0, 1, 0, 0, 0, 0, 0, 0}) { + t.Fatalf("reply = %v", got) + } +} + +func TestSocks5GreetingFromANonSocksClient(t *testing.T) { + _, err := readSocks5Greeting(strings.NewReader("GET / HTTP/1.1\r\n")) + if !errors.Is(err, errSocks5Version) { + t.Fatalf("err = %v, want errSocks5Version", err) + } +} diff --git a/packages/agentvault/upstream_auth.go b/packages/agentvault/upstream_auth.go index 9924aa54d..18b838b48 100644 --- a/packages/agentvault/upstream_auth.go +++ b/packages/agentvault/upstream_auth.go @@ -69,7 +69,7 @@ func replayableBody(req *http.Request) ([]byte, bool) { // handleUpstreamAuth runs only for a refreshable credential the proxy injected, so every other credential // sees the upstream's response untouched. -func (ps *proxyServer) handleUpstreamAuth(req *http.Request, resp *http.Response, svc *resolvedService, sessionToken string, sent *credential, body []byte, replayable bool) (*http.Response, error) { +func (ps *proxyServer) handleUpstreamAuth(upstream http.RoundTripper, req *http.Request, resp *http.Response, svc *resolvedService, sessionToken string, sent *credential, body []byte, replayable bool) (*http.Response, error) { switch resp.StatusCode { case http.StatusUnauthorized: // An auth-error body can stay open indefinitely; draining it would stall the refresh. @@ -103,7 +103,7 @@ func (ps *proxyServer) handleUpstreamAuth(req *http.Request, resp *http.Response retry.Body = &bufferedBody{Reader: bytes.NewReader(body), data: body} } injectCredential(retry, &fresh) - retryResp, err := ps.transport.RoundTrip(retry) + retryResp, err := upstream.RoundTrip(retry) if err != nil { return nil, err } diff --git a/packages/cmd/agent_vault.go b/packages/cmd/agent_vault.go index 69ca4095b..a77b021ad 100644 --- a/packages/cmd/agent_vault.go +++ b/packages/cmd/agent_vault.go @@ -3,6 +3,7 @@ package cmd import ( "errors" "fmt" + "net/netip" "os" "runtime" "strconv" @@ -47,10 +48,16 @@ token. Infisical issues the certificate authority the proxy serves. To serve your own instead, pass --ca-cert and --ca-key, each as a PEM, the base64 of a PEM, or a file path. +The proxy also accepts SOCKS5 over TLS on a second port, for clients such as E2B that send a sandbox's +traffic to the proxy from outside the sandbox, with the session token as the password. Pass --socks5-port +and --socks5-tls-name, the name clients reach the proxy by; the proxy makes the port's certificate from its +certificate authority. To use your own certificate instead, pass --socks5-tls-cert and --socks5-tls-key. + A proxy's settings - its traffic policy, the exceptions to it, and how often it refreshes - are set in Infisical and reach the proxy while it runs, so they have no flags here. Every HTTPS host is intercepted either way; an exception is not blocked, not un-intercepted.`, - Example: "INFISICAL_AGENT_VAULT_PROXY_TOKEN=eyJhb... infisical agent-vault proxy", + Example: ` INFISICAL_AGENT_VAULT_PROXY_TOKEN=eyJhb... infisical agent-vault proxy + INFISICAL_AGENT_VAULT_PROXY_TOKEN=eyJhb... infisical agent-vault proxy --listen-host 127.0.0.1 --socks5-port 1080 --socks5-tls-name proxy.example.com`, DisableFlagsInUseLine: true, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { @@ -79,16 +86,22 @@ not un-intercepted.`, } opts := agentvault.Options{ - Port: in.port, - DataDir: in.dataDir, - OwnCa: in.ownCa, + Port: in.port, + ListenHost: in.listenHost, + DataDir: in.dataDir, + OwnCa: in.ownCa, + Socks5: in.socks5, OnReady: func() { Telemetry.CaptureEventAsync("cli-command:agent-vault proxy", posthog.NewProperties(). Set("version", util.CLI_VERSION). Set("platform", runtime.GOOS). Set("customPort", in.port != agentvault.DefaultPort). + Set("customListenHost", in.listenHost != ""). Set("customDataDir", in.dataDir != ""). Set("ownCa", in.ownCa != nil). + Set("socks5", in.socks5 != nil). + Set("socks5OwnCert", in.socks5 != nil && in.socks5.Cert != nil). + Set("socks5AllowPrivate", in.socks5 != nil && in.socks5.AllowPrivateDestinations). Set("logFormat", logFormat)) }, } @@ -103,7 +116,9 @@ type agentVaultProxyInputs struct { proxyToken string dataDir string port int + listenHost string ownCa *agentvault.OwnCa + socks5 *agentvault.Socks5Options warnings []string } @@ -124,20 +139,21 @@ func readAgentVaultProxyInputs(cmd *cobra.Command) (agentVaultProxyInputs, error return in, fmt.Errorf("unable to read --data-dir: %w", err) } - in.port, err = cmd.Flags().GetInt("port") + port, portSource, _, err := agentVaultPortFlagOrEnv(cmd, "port", "INFISICAL_AGENT_VAULT_PORT") if err != nil { - return in, fmt.Errorf("unable to read --port: %w", err) + return in, err } - portSource := "--port" - if raw := strings.TrimSpace(os.Getenv("INFISICAL_AGENT_VAULT_PORT")); raw != "" && !cmd.Flags().Changed("port") { - portSource = "INFISICAL_AGENT_VAULT_PORT" - in.port, err = strconv.Atoi(raw) - if err != nil { - return in, fmt.Errorf("INFISICAL_AGENT_VAULT_PORT must be a whole number, got %q", raw) - } + if port < 0 || port > 65535 { + return in, fmt.Errorf("%s must be between 0 and 65535, got %d. 0 asks for any free port", portSource, port) } - if in.port < 0 || in.port > 65535 { - return in, fmt.Errorf("%s must be between 0 and 65535, got %d. 0 asks for any free port", portSource, in.port) + in.port = port + + listenHost, listenHostSource, err := agentVaultFlagOrEnv(cmd, "listen-host", "INFISICAL_AGENT_VAULT_LISTEN_HOST") + if err != nil { + return in, err + } + if in.listenHost, err = hostOnly(listenHost, listenHostSource); err != nil { + return in, err } caCert, caCertSource, err := agentVaultFlagOrEnv(cmd, "ca-cert", "INFISICAL_AGENT_VAULT_CA_CERT") @@ -166,9 +182,158 @@ func readAgentVaultProxyInputs(cmd *cobra.Command) (agentVaultProxyInputs, error in.warnings = append(in.warnings, in.ownCa.Warnings()...) } + if in.socks5, err = readAgentVaultSocks5Inputs(cmd, &in); err != nil { + return in, err + } + return in, nil } +func readAgentVaultSocks5Inputs(cmd *cobra.Command, in *agentVaultProxyInputs) (*agentvault.Socks5Options, error) { + port, portSource, portSet, err := agentVaultPortFlagOrEnv(cmd, "socks5-port", "INFISICAL_AGENT_VAULT_SOCKS5_PORT") + if err != nil { + return nil, err + } + tlsName, tlsNameSource, err := agentVaultFlagOrEnv(cmd, "socks5-tls-name", "INFISICAL_AGENT_VAULT_SOCKS5_TLS_NAME") + if err != nil { + return nil, err + } + tlsCert, tlsCertSource, err := agentVaultFlagOrEnv(cmd, "socks5-tls-cert", "INFISICAL_AGENT_VAULT_SOCKS5_TLS_CERT") + if err != nil { + return nil, err + } + tlsKey, tlsKeySource, err := agentVaultFlagOrEnv(cmd, "socks5-tls-key", "INFISICAL_AGENT_VAULT_SOCKS5_TLS_KEY") + if err != nil { + return nil, err + } + allowPrivate, allowPrivateSource, err := agentVaultBoolFlagOrEnv(cmd, "socks5-allow-private-destinations", "INFISICAL_AGENT_VAULT_SOCKS5_ALLOW_PRIVATE_DESTINATIONS") + if err != nil { + return nil, err + } + + if !portSet { + for _, set := range []struct { + source string + given bool + }{ + {tlsNameSource, tlsName != ""}, + {tlsCertSource, tlsCert != ""}, + {tlsKeySource, tlsKey != ""}, + {allowPrivateSource, allowPrivate}, + } { + if set.given { + return nil, fmt.Errorf("%s only applies with --socks5-port", set.source) + } + } + return nil, nil + } + + // 0 is not "off" or "any free port" here: a SOCKS5 client has to be told the port in advance. + if port < 1 || port > 65535 { + return nil, fmt.Errorf("%s must be between 1 and 65535, got %d", portSource, port) + } + if port == in.port { + return nil, fmt.Errorf("%s and --port can't be the same port (%d)", portSource, port) + } + + opts := &agentvault.Socks5Options{Port: port, AllowPrivateDestinations: allowPrivate} + + if (tlsCert == "") != (tlsKey == "") { + return nil, errors.New("--socks5-tls-cert and --socks5-tls-key go together") + } + if tlsCert != "" { + if tlsKeySource == "--socks5-tls-key" && strings.Contains(tlsKey, "-----BEGIN ") { + in.warnings = append(in.warnings, "--socks5-tls-key is visible in the process list; prefer the env var INFISICAL_AGENT_VAULT_SOCKS5_TLS_KEY") + } + opts.Cert, err = agentvault.LoadListenerCert(agentvault.OwnCaInput{ + Cert: tlsCert, CertSource: tlsCertSource, + Key: tlsKey, KeySource: tlsKeySource, + }) + if err != nil { + return nil, err + } + return opts, nil + } + + if tlsName == "" { + return nil, errors.New("--socks5-port needs --socks5-tls-name, the DNS name or IP address clients reach the proxy by, so the proxy can make the port's certificate. To use your own certificate instead, pass --socks5-tls-cert and --socks5-tls-key") + } + if opts.TLSName, err = socks5TLSName(tlsName, tlsNameSource); err != nil { + return nil, err + } + return opts, nil +} + +// A certificate made for "Proxy.Example.com." or "proxy.example.com:1080" is one clients reject, so the name +// is reduced to the form a client checks against. +func socks5TLSName(raw, source string) (string, error) { + name, err := hostOnly(raw, source) + if err != nil { + return "", err + } + name = strings.ToLower(strings.TrimRight(name, ".")) + if name == "" || strings.ContainsAny(name, "/* \t") { + return "", fmt.Errorf("%s must be a DNS name or an IP address, got %q", source, raw) + } + return name, nil +} + +// hostOnly accepts a host name or an IP address, with or without the brackets an IPv6 address is often written +// in, and refuses a value that also names a port, since the port has a flag of its own. +func hostOnly(raw, source string) (string, error) { + host := strings.TrimSpace(raw) + if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { + host = host[1 : len(host)-1] + } + if strings.Contains(host, ":") { + if _, err := netip.ParseAddr(host); err != nil { + return "", fmt.Errorf("%s takes a host only, got %q. Set the port with its own flag", source, raw) + } + } + return host, nil +} + +// set reports whether the value came from the flag or the env var rather than the flag's default. +func agentVaultPortFlagOrEnv(cmd *cobra.Command, flag, env string) (port int, source string, set bool, err error) { + port, err = cmd.Flags().GetInt(flag) + if err != nil { + return 0, "", false, fmt.Errorf("unable to read --%s: %w", flag, err) + } + if cmd.Flags().Changed(flag) { + return port, "--" + flag, true, nil + } + if raw := strings.TrimSpace(os.Getenv(env)); raw != "" { + port, err = strconv.Atoi(raw) + if err != nil { + return 0, env, true, fmt.Errorf("%s must be a whole number, got %q", env, raw) + } + return port, env, true, nil + } + return port, "--" + flag, false, nil +} + +// Stricter than util.GetBoolFlagOrEnv, which turns an unreadable value into false with a warning: this switch +// turns a protection off, so a value that says neither must stop the start rather than be guessed at. +func agentVaultBoolFlagOrEnv(cmd *cobra.Command, flag, env string) (value bool, source string, err error) { + if cmd.Flags().Changed(flag) { + value, err = cmd.Flags().GetBool(flag) + if err != nil { + return false, "", fmt.Errorf("unable to read --%s: %w", flag, err) + } + return value, "--" + flag, nil + } + switch raw := strings.ToLower(strings.TrimSpace(os.Getenv(env))); raw { + case "": + return false, "--" + flag, nil + case "true", "1": + return true, env, nil + case "false", "0": + return false, env, nil + default: + return false, env, fmt.Errorf("%s must be true, false, 1 or 0, got %q", env, raw) + } +} + // The source is the flag's name or the env var's, whichever the value came from, so errors can point at it. func agentVaultFlagOrEnv(cmd *cobra.Command, flag, env string) (value, source string, err error) { value, err = cmd.Flags().GetString(flag) @@ -195,7 +360,13 @@ func addAgentVaultProxyFlags(cmd *cobra.Command) { fmt.Sprintf("where the proxy saves its settings and certificate authority for starts while Infisical can't be reached (default: %s)", defaultDataDirHelp())) cmd.Flags().String("ca-cert", "", "your own certificate authority to serve, instead of the one Infisical issues: PEM, base64 PEM or file path") cmd.Flags().String("ca-key", "", "private key for --ca-cert: PEM, base64 PEM or file path") - cmd.Flags().Int("port", agentvault.DefaultPort, "port to listen on; 0 binds any free port, which the startup line then reports") + cmd.Flags().Int("port", agentvault.DefaultPort, "port for the HTTP proxy; 0 binds any free port, which the startup line then reports") + cmd.Flags().String("listen-host", "", "host or IP address the HTTP proxy listens on, such as 127.0.0.1 (default: all interfaces)") + cmd.Flags().Int("socks5-port", 0, "port for SOCKS5 over TLS, which listens on all interfaces (default: off)") + cmd.Flags().String("socks5-tls-name", "", "DNS name or IP address clients reach the SOCKS5 port by; the proxy makes the port's certificate for it") + cmd.Flags().String("socks5-tls-cert", "", "your own certificate for the SOCKS5 port, instead of one the proxy makes: PEM, base64 PEM or file path") + cmd.Flags().String("socks5-tls-key", "", "private key for --socks5-tls-cert: PEM, base64 PEM or file path") + cmd.Flags().Bool("socks5-allow-private-destinations", false, "let SOCKS5 traffic that no service covers reach private, loopback and link-local addresses") cmd.Flags().String("log-format", "console", "log output format: console | json") cmd.Flags().String("log-file", "", "path to also write logs to") // Units written for enrollment still pass this on every start; failing on an unknown flag would take an diff --git a/packages/cmd/agent_vault_proxy_test.go b/packages/cmd/agent_vault_proxy_test.go index bd586811a..d5a476c02 100644 --- a/packages/cmd/agent_vault_proxy_test.go +++ b/packages/cmd/agent_vault_proxy_test.go @@ -23,6 +23,12 @@ var agentVaultProxyEnv = []string{ "INFISICAL_AGENT_VAULT_CA_CERT", "INFISICAL_AGENT_VAULT_CA_KEY", "INFISICAL_AGENT_VAULT_ENROLLMENT_TOKEN", + "INFISICAL_AGENT_VAULT_LISTEN_HOST", + "INFISICAL_AGENT_VAULT_SOCKS5_PORT", + "INFISICAL_AGENT_VAULT_SOCKS5_TLS_NAME", + "INFISICAL_AGENT_VAULT_SOCKS5_TLS_CERT", + "INFISICAL_AGENT_VAULT_SOCKS5_TLS_KEY", + "INFISICAL_AGENT_VAULT_SOCKS5_ALLOW_PRIVATE_DESTINATIONS", } func readProxyInputs(t *testing.T, env map[string]string, args ...string) (agentVaultProxyInputs, error) { @@ -177,3 +183,121 @@ func TestAgentVaultProxyNamesTheEnvVarAValueCameFrom(t *testing.T) { t.Fatalf("port from flag: got %v", err) } } + +func TestAgentVaultProxyListenHostTakesAHostOnly(t *testing.T) { + for _, tc := range []struct { + value string + want string + wantErr bool + }{ + {"127.0.0.1", "127.0.0.1", false}, + {"localhost", "localhost", false}, + {"::1", "::1", false}, + {"[::1]", "::1", false}, + {"127.0.0.1:17323", "", true}, + {"[::1]:17323", "", true}, + {"localhost:80", "", true}, + } { + t.Run(tc.value, func(t *testing.T) { + in, err := readProxyInputs(t, nil, "--listen-host", tc.value) + if tc.wantErr { + if err == nil || !strings.Contains(err.Error(), "takes a host only") { + t.Fatalf("got %v, want a host-only error", err) + } + return + } + if err != nil || in.listenHost != tc.want { + t.Fatalf("got %q err=%v, want %q", in.listenHost, err, tc.want) + } + }) + } +} + +func TestAgentVaultProxyListenHostFromEnv(t *testing.T) { + in, err := readProxyInputs(t, map[string]string{"INFISICAL_AGENT_VAULT_LISTEN_HOST": "127.0.0.1"}) + if err != nil || in.listenHost != "127.0.0.1" { + t.Fatalf("got %q err=%v", in.listenHost, err) + } +} + +func TestAgentVaultProxySocks5IsOffByDefault(t *testing.T) { + in, err := readProxyInputs(t, nil) + if err != nil || in.socks5 != nil { + t.Fatalf("got %+v err=%v, want SOCKS5 off", in.socks5, err) + } +} + +func TestAgentVaultProxySocks5FromFlagsAndEnv(t *testing.T) { + in, err := readProxyInputs(t, nil, "--socks5-port", "1080", "--socks5-tls-name", "Proxy.Example.com.") + if err != nil { + t.Fatal(err) + } + if in.socks5 == nil || in.socks5.Port != 1080 || in.socks5.TLSName != "proxy.example.com" || in.socks5.AllowPrivateDestinations { + t.Fatalf("got %+v", in.socks5) + } + + in, err = readProxyInputs(t, map[string]string{ + "INFISICAL_AGENT_VAULT_SOCKS5_PORT": "1080", + "INFISICAL_AGENT_VAULT_SOCKS5_TLS_NAME": "10.0.0.5", + "INFISICAL_AGENT_VAULT_SOCKS5_ALLOW_PRIVATE_DESTINATIONS": "TRUE", + }) + if err != nil { + t.Fatal(err) + } + if in.socks5 == nil || in.socks5.TLSName != "10.0.0.5" || !in.socks5.AllowPrivateDestinations { + t.Fatalf("env should turn SOCKS5 on: got %+v", in.socks5) + } +} + +func TestAgentVaultProxySocks5Validation(t *testing.T) { + certPEM, keyPEM := caPair(t) + for _, tc := range []struct { + name string + env map[string]string + args []string + want string + }{ + {"port 0", nil, []string{"--socks5-port", "0", "--socks5-tls-name", "p.example.com"}, "must be between 1 and 65535"}, + {"port out of range", nil, []string{"--socks5-port", "70000", "--socks5-tls-name", "p.example.com"}, "must be between 1 and 65535"}, + {"port from env not a number", map[string]string{"INFISICAL_AGENT_VAULT_SOCKS5_PORT": "x"}, nil, "must be a whole number"}, + {"same port as HTTP", nil, []string{"--socks5-port", "17323", "--socks5-tls-name", "p.example.com"}, "can't be the same port"}, + {"no TLS name and no certificate", nil, []string{"--socks5-port", "1080"}, "needs --socks5-tls-name"}, + {"TLS name with a port", nil, []string{"--socks5-port", "1080", "--socks5-tls-name", "p.example.com:1080"}, "takes a host only"}, + {"wildcard TLS name", nil, []string{"--socks5-port", "1080", "--socks5-tls-name", "*.example.com"}, "must be a DNS name or an IP address"}, + {"certificate without key", nil, []string{"--socks5-port", "1080", "--socks5-tls-cert", certPEM}, "go together"}, + {"TLS name without port", nil, []string{"--socks5-tls-name", "p.example.com"}, "--socks5-tls-name only applies with --socks5-port"}, + {"certificate without port", nil, []string{"--socks5-tls-cert", certPEM, "--socks5-tls-key", keyPEM}, "--socks5-tls-cert only applies with --socks5-port"}, + {"allow-private without port", nil, []string{"--socks5-allow-private-destinations"}, "only applies with --socks5-port"}, + {"allow-private env not a boolean", map[string]string{"INFISICAL_AGENT_VAULT_SOCKS5_ALLOW_PRIVATE_DESTINATIONS": "yes"}, nil, "must be true, false, 1 or 0"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := readProxyInputs(t, tc.env, tc.args...) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("got %v, want an error containing %q", err, tc.want) + } + }) + } +} + +func TestAgentVaultProxySocks5OwnCertificate(t *testing.T) { + certPEM, keyPEM := caPair(t) + in, err := readProxyInputs(t, map[string]string{"INFISICAL_AGENT_VAULT_SOCKS5_TLS_KEY": keyPEM}, + "--socks5-port", "1080", "--socks5-tls-cert", certPEM) + if err != nil { + t.Fatal(err) + } + if in.socks5 == nil || in.socks5.Cert == nil || in.socks5.TLSName != "" { + t.Fatalf("got %+v", in.socks5) + } + if len(in.warnings) != 0 { + t.Fatalf("a key from the env var should not warn: %v", in.warnings) + } + + in, err = readProxyInputs(t, nil, "--socks5-port", "1080", "--socks5-tls-cert", certPEM, "--socks5-tls-key", keyPEM) + if err != nil { + t.Fatal(err) + } + if len(in.warnings) != 1 || !strings.Contains(in.warnings[0], "--socks5-tls-key is visible in the process list") { + t.Fatalf("a key on the command line should warn: %v", in.warnings) + } +} From 5fe3366be6a4f877181139f89d209dd4a5cdcfaf Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:15:00 +0530 Subject: [PATCH 2/4] fix(agent-vault): read settings once per request and log non-web SOCKS5 traffic --- packages/agentvault/match_test.go | 4 +- packages/agentvault/proxy.go | 65 ++++++++++++++++++++++---- packages/agentvault/socks5_e2e_test.go | 30 ++++++++++++ 3 files changed, 89 insertions(+), 10 deletions(-) diff --git a/packages/agentvault/match_test.go b/packages/agentvault/match_test.go index 1331fbe15..aab3831d0 100644 --- a/packages/agentvault/match_test.go +++ b/packages/agentvault/match_test.go @@ -188,7 +188,7 @@ func TestExceptionOverridesBundleHostsPolicy(t *testing.T) { t.Fatalf("bestMatch(%q) = %v, want %v", tc.host, matched, tc.wantMatch) } - if blocked := ps.blocksOffBundle(matched, tc.host, "443"); blocked != tc.wantBlock { + if blocked := ps.currentConfig().blocksOffBundle(matched, tc.host, "443"); blocked != tc.wantBlock { t.Errorf("blocked = %v, want %v", blocked, tc.wantBlock) } }) @@ -214,7 +214,7 @@ func TestExceptionPortScope(t *testing.T) { {"pkg.example.com", "443", false, "naming a port still scopes the entry to it"}, {"other.example.com", "80", false, "a host that is not on the list"}, } { - if got := ps.isAllowedHost(tc.host, tc.port); got != tc.want { + if got := ps.currentConfig().isAllowedHost(tc.host, tc.port); got != tc.want { t.Errorf("isAllowedHost(%q, %q) = %v, want %v: %s", tc.host, tc.port, got, tc.want, tc.why) } } diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go index 68069a3bc..90ae037c3 100644 --- a/packages/agentvault/proxy.go +++ b/packages/agentvault/proxy.go @@ -13,6 +13,7 @@ import ( "net/http" "strings" "sync" + "sync/atomic" "time" "github.com/rs/zerolog" @@ -90,6 +91,7 @@ type proxyServer struct { // when it dials. Its own pool, so it never reuses a connection the unchecked transport opened. guardedTransport http.RoundTripper socks5AllowPrivate bool + lastNotWebWarn atomic.Int64 configMu sync.RWMutex config ProxyConfig @@ -283,9 +285,19 @@ func (ps *proxyServer) serveIntercepted(conn net.Conn, t tunnel, leaf tls.Certif _ = conn.SetDeadline(time.Now().Add(tlsHandshakeTimeout)) first, err := buffered.reader.Peek(1) if err != nil { + // Nothing arrived: a protocol where the server speaks first, such as SMTP or MySQL. + ps.warnNotWebTraffic(t, "the client sent nothing") return } if first[0] != tlsRecordTypeHandshake { + // A SOCKS5 provider sends every TCP flow here, so SSH or a database driver would otherwise get an + // HTTP 400 and leave no trace above debug level. + if t.viaSocks5 { + if head, err := buffered.reader.Peek(4); err != nil || !looksLikeHTTPRequest(head) { + ps.warnNotWebTraffic(t, "the client didn't send TLS or an HTTP request") + return + } + } _ = conn.SetDeadline(time.Time{}) t.scheme = "http" ps.serveTunnel(buffered, t) @@ -301,6 +313,8 @@ func (ps *proxyServer) serveIntercepted(conn net.Conn, t tunnel, leaf tls.Certif }) if err := tlsConn.Handshake(); err != nil { log.Debug().Err(err).Str("host", net.JoinHostPort(t.hostname, t.port)).Msg("agent-vault: tunnel TLS handshake failed") + // An HTTP/2-only client such as gRPC, or one that doesn't trust the proxy's CA, ends here. + ps.warnNotWebTraffic(t, "the TLS handshake failed: "+err.Error()) return } _ = tlsConn.SetDeadline(time.Time{}) @@ -311,6 +325,37 @@ func (ps *proxyServer) serveIntercepted(conn net.Conn, t tunnel, leaf tls.Certif const tlsRecordTypeHandshake = 0x16 +var httpMethodPrefixes = []string{"GET ", "HEAD", "POST", "PUT ", "PATC", "DELE", "OPTI", "CONN", "TRAC"} + +func looksLikeHTTPRequest(head []byte) bool { + for _, prefix := range httpMethodPrefixes { + if string(head) == prefix { + return true + } + } + return false +} + +const notWebTrafficWarnEvery = time.Minute + +// Only the SOCKS5 door warns: the HTTP door is reached by clients that chose to use a web proxy, and its +// logs stay as they were. Rate limited, since a sandbox retrying a git push over SSH repeats it constantly. +func (ps *proxyServer) warnNotWebTraffic(t tunnel, reason string) { + if !t.viaSocks5 { + return + } + now := time.Now().UnixNano() + last := ps.lastNotWebWarn.Load() + if now-last < int64(notWebTrafficWarnEvery) || !ps.lastNotWebWarn.CompareAndSwap(last, now) { + return + } + log.Warn(). + Str("door", "socks5"). + Str("host", net.JoinHostPort(t.hostname, t.port)). + Str("reason", reason). + Msg("agent-vault: closed a SOCKS5 connection the proxy can't handle; it handles only HTTP/1.1 web traffic from clients that trust its CA") +} + // bufferedConn lets the first byte be peeked and then handed on, to tls.Server or to the plain HTTP // server, without the byte being consumed. type bufferedConn struct { @@ -507,8 +552,8 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, t tun } // The one place the block decision lives, so the test that covers exceptions runs what forward runs. -func (ps *proxyServer) blocksOffBundle(matched *resolvedService, hostname, port string) bool { - return matched == nil && ps.currentConfig().TrafficPolicy == TrafficPolicyBundleHosts && !ps.isAllowedHost(hostname, port) +func (cfg ProxyConfig) blocksOffBundle(matched *resolvedService, hostname, port string) bool { + return matched == nil && cfg.TrafficPolicy == TrafficPolicyBundleHosts && !cfg.isAllowedHost(hostname, port) } type forwardOutcome struct { @@ -543,6 +588,10 @@ func (ps *proxyServer) forward(req *http.Request, t tunnel) (*http.Response, *re } matched := bestMatch(services, hostname, port) + // One copy for every decision this request makes. A poll can swap the settings at any moment, and a + // request judged by two different policies could pass the gate under one and skip the private-address + // check under the other. + cfg := ps.currentConfig() // Set only when the proxy injected a credential Infisical can refresh, which is what decides whether an // upstream 401 or 403 is handled here or passed to the agent. var sent *credential @@ -550,7 +599,7 @@ func (ps *proxyServer) forward(req *http.Request, t tunnel) (*http.Response, *re // stored credential is wrong, which the agent cannot fix by logging in itself. staticMCP := false - if ps.blocksOffBundle(matched, hostname, port) { + if cfg.blocksOffBundle(matched, hostname, port) { return nil, nil, outcome, fmt.Errorf("no service covers host %q: %w", hostname, errHostBlocked) } @@ -644,7 +693,7 @@ func (ps *proxyServer) forward(req *http.Request, t tunnel) (*http.Response, *re replay, replayable = replayableBody(req) } - upstream, err := ps.upstreamFor(t, matched) + upstream, err := ps.upstreamFor(t, matched, cfg) if err != nil { return nil, matched, outcome, err } @@ -674,8 +723,8 @@ func (ps *proxyServer) forward(req *http.Request, t tunnel) (*http.Response, *re // under the any-host policy a destination no service names may not be private: only an admin naming a host // in a bundle makes an internal address reachable that way. Under bundle-hosts every reachable host is // already named. Exceptions do not count here, because the UI hides them once the policy is any-host. -func (ps *proxyServer) upstreamFor(t tunnel, matched *resolvedService) (http.RoundTripper, error) { - if !t.viaSocks5 || ps.socks5AllowPrivate || matched != nil || ps.currentConfig().TrafficPolicy != TrafficPolicyAnyHost { +func (ps *proxyServer) upstreamFor(t tunnel, matched *resolvedService, cfg ProxyConfig) (http.RoundTripper, error) { + if !t.viaSocks5 || ps.socks5AllowPrivate || matched != nil || cfg.TrafficPolicy != TrafficPolicyAnyHost { return ps.transport, nil } if ps.guardedTransport == nil { @@ -693,8 +742,8 @@ func containsSurface(surfaces []string, target string) bool { return false } -func (ps *proxyServer) isAllowedHost(hostname, port string) bool { - raw := ps.currentConfig().AllowedHosts +func (cfg ProxyConfig) isAllowedHost(hostname, port string) bool { + raw := cfg.AllowedHosts if raw == "" { return false } diff --git a/packages/agentvault/socks5_e2e_test.go b/packages/agentvault/socks5_e2e_test.go index 823d5046a..c0dc7044e 100644 --- a/packages/agentvault/socks5_e2e_test.go +++ b/packages/agentvault/socks5_e2e_test.go @@ -570,3 +570,33 @@ func selfSignedListenerCert(t *testing.T, name string, notAfter time.Time) (cert return string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})), string(pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER})), pool } + +func TestSocks5WarnsAboutTrafficThatIsNotWeb(t *testing.T) { + var logs bytes.Buffer + restore := log.Logger + log.Logger = zerolog.New(&syncWriter{w: &logs}) + t.Cleanup(func() { log.Logger = restore }) + + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, nil) + for i := 0; i < 2; i++ { + conn, auth, rep := dialSocks5(t, f.addr, f.outerTLS(), ProxyAuthUsername, testSocks5Token, "ssh.github.com", 443) + if conn == nil { + t.Fatalf("tunnel refused: auth=%#x rep=%#x", auth, rep) + } + _, _ = conn.Write([]byte("SSH-2.0-OpenSSH_9.6\r\n")) + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + if _, err := io.ReadAll(conn); err != nil && !errors.Is(err, io.EOF) { + t.Fatalf("the connection was not closed: %v", err) + } + conn.Close() + } + f.server.wait() + + out := logs.String() + if n := strings.Count(out, "closed a SOCKS5 connection the proxy can't handle"); n != 1 { + t.Fatalf("warned %d times, want once (rate limited): %s", n, out) + } + if !strings.Contains(out, `"host":"ssh.github.com:443"`) { + t.Fatalf("the warning doesn't name the target: %s", out) + } +} From bad8e53d12c980531a449628be8019d2449a4fe7 Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:20:48 +0530 Subject: [PATCH 3/4] fix(agent-vault): accept any capital-letter HTTP method on SOCKS5 tunnels --- packages/agentvault/proxy.go | 15 +++++---- packages/agentvault/socks5_e2e_test.go | 46 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go index 90ae037c3..50ecff1f1 100644 --- a/packages/agentvault/proxy.go +++ b/packages/agentvault/proxy.go @@ -325,15 +325,18 @@ func (ps *proxyServer) serveIntercepted(conn net.Conn, t tunnel, leaf tls.Certif const tlsRecordTypeHandshake = 0x16 -var httpMethodPrefixes = []string{"GET ", "HEAD", "POST", "PUT ", "PATC", "DELE", "OPTI", "CONN", "TRAC"} - +// Any method in capitals passes, so WebDAV's PROPFIND or MKCOL reach the HTTP parser like GET does. Every +// method is at least three letters, so a space can only come fourth. SSH ("SSH-") and binary protocols fail. func looksLikeHTTPRequest(head []byte) bool { - for _, prefix := range httpMethodPrefixes { - if string(head) == prefix { - return true + for i, c := range head { + if c == ' ' && i == 3 { + continue + } + if c < 'A' || c > 'Z' { + return false } } - return false + return true } const notWebTrafficWarnEvery = time.Minute diff --git a/packages/agentvault/socks5_e2e_test.go b/packages/agentvault/socks5_e2e_test.go index c0dc7044e..f988ab51b 100644 --- a/packages/agentvault/socks5_e2e_test.go +++ b/packages/agentvault/socks5_e2e_test.go @@ -600,3 +600,49 @@ func TestSocks5WarnsAboutTrafficThatIsNotWeb(t *testing.T) { t.Fatalf("the warning doesn't name the target: %s", out) } } + +func TestLooksLikeHTTPRequest(t *testing.T) { + for _, tc := range []struct { + head string + want bool + }{ + {"GET ", true}, {"PUT ", true}, {"POST", true}, {"PROP", true}, {"MKCO", true}, {"LOCK", true}, + {"SSH-", false}, {"get ", false}, {"\x00\x00\x00\x08", false}, {"GE T", false}, + } { + if got := looksLikeHTTPRequest([]byte(tc.head)); got != tc.want { + t.Errorf("looksLikeHTTPRequest(%q) = %v, want %v", tc.head, got, tc.want) + } + } +} + +func TestSocks5ForwardsAPlainHTTPExtensionMethod(t *testing.T) { + plain := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, "method="+r.Method) + })) + t.Cleanup(plain.Close) + u, _ := url.Parse(plain.URL) + port, _ := strconv.Atoi(u.Port()) + + f := newSocks5Fixture(t, &stubResolver{result: &resolveResult{SessionID: "s1"}}, func(ps *proxyServer, _ *Socks5Options, _ *socks5Server) { + ps.socks5AllowPrivate = true + }) + conn, auth, rep := dialSocks5(t, f.addr, f.outerTLS(), ProxyAuthUsername, testSocks5Token, "127.0.0.1", uint16(port)) + if conn == nil { + t.Fatalf("tunnel refused: auth=%#x rep=%#x", auth, rep) + } + defer conn.Close() + + req, _ := http.NewRequest("PROPFIND", "http://127.0.0.1/x", nil) + if err := req.Write(conn); err != nil { + t.Fatal(err) + } + resp, err := http.ReadResponse(bufio.NewReader(conn), req) + if err != nil { + t.Fatalf("the PROPFIND request was dropped: %v", err) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusOK || string(body) != "method=PROPFIND" { + t.Fatalf("status=%d body=%q, want 200 method=PROPFIND", resp.StatusCode, body) + } +} From 7c8a5ad5b8172cfdf42bb48cd6d293c3533dd65d Mon Sep 17 00:00:00 2001 From: saif <11242541+saifsmailbox98@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:43:05 +0530 Subject: [PATCH 4/4] fix(agent-vault): count SOCKS5 connections without a WaitGroup --- packages/agentvault/socks5.go | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/agentvault/socks5.go b/packages/agentvault/socks5.go index f05c0d6dd..0efa787bc 100644 --- a/packages/agentvault/socks5.go +++ b/packages/agentvault/socks5.go @@ -192,13 +192,19 @@ type socks5Server struct { // own timeout, so a slow Infisical does not cut off an agent that did nothing wrong. stepTimeout time.Duration - conns sync.WaitGroup + // A count under a lock rather than a WaitGroup: a test waits for the connections so far and then opens + // more, and a WaitGroup may not be added to again once a Wait has seen it reach zero. + connsMu sync.Mutex + connsN int + idle *sync.Cond lastMismatchWarn atomic.Int64 } func newSocks5Server(ps *proxyServer, tlsConfig *tls.Config) *socks5Server { - return &socks5Server{ps: ps, tlsConfig: tlsConfig, stepTimeout: socks5StepTimeout} + s := &socks5Server{ps: ps, tlsConfig: tlsConfig, stepTimeout: socks5StepTimeout} + s.idle = sync.NewCond(&s.connsMu) + return s } // serve returns nil once the listener is closed. A temporary accept error, such as running out of file @@ -225,16 +231,31 @@ func (s *socks5Server) serve(l net.Listener) error { return err } backoff = 0 - s.conns.Add(1) + s.connsMu.Lock() + s.connsN++ + s.connsMu.Unlock() go func() { - defer s.conns.Done() + defer func() { + s.connsMu.Lock() + s.connsN-- + if s.connsN == 0 { + s.idle.Broadcast() + } + s.connsMu.Unlock() + }() s.handle(conn) }() } } -// wait blocks until every connection handled so far has returned. -func (s *socks5Server) wait() { s.conns.Wait() } +// wait blocks until no connection is being handled. +func (s *socks5Server) wait() { + s.connsMu.Lock() + defer s.connsMu.Unlock() + for s.connsN > 0 { + s.idle.Wait() + } +} func (s *socks5Server) handle(raw net.Conn) { remote := raw.RemoteAddr().String()