diff --git a/README.md b/README.md index 91fc765..9d73025 100644 --- a/README.md +++ b/README.md @@ -193,11 +193,24 @@ only thing limiting them. ### Health -`enable_health_check="on"` makes the proxy count the responses it serves: a run -of consecutive failures marks it unhealthy and a single success clears the run. +`enable_health_check="on"` makes the proxy count how its work goes: a run of +consecutive failures marks it unhealthy and a single success clears the run. `/health` on `http_listen` answers 200 while it is healthy and 503 once it is not, which is what the container image's healthcheck probes. +What counts differs slightly between the two frontends: + +* the SOCKS frontend reports on whether it reached the target. Opening a tunnel + is a success and failing to dial one is a failure; a client refused before the + proxy tried to reach anything — wrong password, disallowed network, disallowed + port — records nothing, because it says something about that client rather + than about this proxy; +* the HTTP frontend counts every response it serves, and additionally treats a + 407 as a failure. A `Proxy-Authorization` challenge in the response stream may + have come from an upstream proxy whose credentials have gone stale, which is + the proxy's problem, but it may equally be this proxy challenging its own + client, which is not. + A program embedding the package gets the same thing through the API, and can read the state directly rather than over HTTP: diff --git a/health.go b/health.go index ba38af9..2f897eb 100644 --- a/health.go +++ b/health.go @@ -98,15 +98,30 @@ func (s *Server) Health() *Health { // health check counted, so it is kept: a proxy whose upstream credentials have // gone stale answers nothing but 407. func (s *Server) recordHealth(resp *http.Response) { - if s.health == nil { + if resp == nil || resp.StatusCode == http.StatusProxyAuthRequired { + s.recordHealthFailure() + return } - if resp == nil || resp.StatusCode == http.StatusProxyAuthRequired { - s.health.RecordFailure() + s.recordHealthSuccess() +} - return +// recordHealthSuccess and recordHealthFailure are what the frontends report +// through. They do nothing unless health tracking was asked for. +// +// The SOCKS frontend reports on whether it reached the target, and says nothing +// about the connections it refuses before trying: a client presenting the wrong +// password, or coming from a network that is not allowed, is a statement about +// that client rather than about this proxy's ability to reach the world. +func (s *Server) recordHealthSuccess() { + if s.health != nil { + s.health.RecordSuccess() } +} - s.health.RecordSuccess() +func (s *Server) recordHealthFailure() { + if s.health != nil { + s.health.RecordFailure() + } } diff --git a/socks.go b/socks.go index 6b024d7..0c25c06 100644 --- a/socks.go +++ b/socks.go @@ -394,8 +394,11 @@ func (s *Server) socksDial(client net.Conn, target string) (net.Conn, error) { return nil, fmt.Errorf("%w: %v is not an allowed port", errSOCKSRefused, target) } + // From here the proxy is the one being asked to reach something, so the + // outcome is what its health is made of. route, err := s.route(target) if err != nil { + s.recordHealthFailure() _ = writeSOCKSReply(client, replyGeneralFailure, nil) return nil, fmt.Errorf("couldn't route %v: %w", target, err) @@ -403,11 +406,14 @@ func (s *Server) socksDial(client net.Conn, target string) (net.Conn, error) { upstream, err := s.dialRoute(context.Background(), route, "tcp", target) if err != nil { + s.recordHealthFailure() _ = writeSOCKSReply(client, replyForDialError(err), nil) return nil, fmt.Errorf("couldn't connect to %v: %w", target, err) } + s.recordHealthSuccess() + if err := writeSOCKSReply(client, replySuccess, upstream.LocalAddr()); err != nil { upstream.Close() diff --git a/socks_test.go b/socks_test.go index 5100a0c..067dbd5 100644 --- a/socks_test.go +++ b/socks_test.go @@ -3,6 +3,7 @@ package microproxy import ( "context" "crypto/tls" + "errors" "net" "net/http" "net/http/httptest" @@ -319,3 +320,112 @@ func TestSOCKSShutdown(t *testing.T) { t.Error("expected the listener to be closed") } } + +// unhealthy drives the tracker below its threshold, so that a later success is +// visible as a change rather than as the state it started in. +func unhealthy(t *testing.T, health *Health) { + t.Helper() + + for i := 0; i < DefaultHealthFailureLimit; i++ { + health.RecordFailure() + } + + if health.Healthy() { + t.Fatal("expected the proxy to be unhealthy to begin with") + } +} + +// A tunnel the proxy managed to open says it can reach the world. +func TestSOCKSRecordsHealthOnSuccess(t *testing.T) { + background := httptest.NewServer(constantHandler("Hello, World!")) + defer background.Close() + + server, addr := newTestSOCKS(t, Config{ + AllowedConnectPorts: []int{portOf(t, background.URL)}, + HealthCheckEnabled: "on", + }) + + unhealthy(t, server.Health()) + + resp, err := socksClient(t, addr, "", "").Get(background.URL) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + + if !server.Health().Healthy() { + t.Error("expected an established tunnel to record a success") + } +} + +// A target the proxy could not reach is what the health endpoint exists to +// report. +func TestSOCKSRecordsHealthOnDialFailure(t *testing.T) { + background := httptest.NewServer(constantHandler("Hello, World!")) + defer background.Close() + + server, addr := newTestSOCKS(t, + Config{ + AllowedConnectPorts: []int{portOf(t, background.URL)}, + HealthCheckEnabled: "on", + }, + WithRouter(RouterFunc(func(host string) (Route, error) { + return Route{Dialer: DialerFunc(func(context.Context, string, string) (net.Conn, error) { + return nil, errors.New("the tunnel is down") + })}, nil + }))) + + if _, err := socksClient(t, addr, "", "").Get(background.URL); err == nil { + t.Fatal("expected the connection to fail") + } + + if failures := server.Health().Failures(); failures != 1 { + t.Errorf("expected one failure to be recorded, got %v", failures) + } +} + +// A client refused before the proxy tried to reach anything says nothing about +// the proxy, so it must not count against it. +func TestSOCKSRefusalDoesNotAffectHealth(t *testing.T) { + background := httptest.NewServer(constantHandler("Hello, World!")) + defer background.Close() + + server, addr := newTestSOCKS(t, + Config{ + AllowedConnectPorts: []int{portOf(t, background.URL)}, + HealthCheckEnabled: "on", + }, + WithCredentials(testBasicUsers())) + + if _, err := socksClient(t, addr, user, "wrong").Get(background.URL); err == nil { + t.Fatal("expected the connection to be refused") + } + + if failures := server.Health().Failures(); failures != 0 { + t.Errorf("expected a refused client to record nothing, got %v failures", failures) + } + + if !server.Health().Healthy() { + t.Error("expected the proxy to still be healthy") + } +} + +// A client the network ACLs turn away is refused for the same reason. +func TestSOCKSDeniedNetworkDoesNotAffectHealth(t *testing.T) { + background := httptest.NewServer(constantHandler("Hello, World!")) + defer background.Close() + + server, addr := newTestSOCKS(t, Config{ + AllowedNetworks: []string{"172.16.11.0/24"}, + AllowedConnectPorts: []int{portOf(t, background.URL)}, + HealthCheckEnabled: "on", + }) + + if _, err := socksClient(t, addr, "", "").Get(background.URL); err == nil { + t.Fatal("expected the connection to be refused") + } + + if failures := server.Health().Failures(); failures != 0 { + t.Errorf("expected a denied client to record nothing, got %v failures", failures) + } +}