Skip to content

Commit 5cc55c9

Browse files
committed
fix(health): use TCP dialing instead of ping
- `HEALTH_TARGET_ADDRESS` to replace `HEALTH_ADDRESS_TO_PING` - Remove `github.com/go-ping/ping` dependency - Dial TCP the target address, appending `:443` if port is not set
1 parent 55e609c commit 5cc55c9

12 files changed

Lines changed: 133 additions & 226 deletions

File tree

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ ENV VPNSP=pia \
126126
LOG_LEVEL=info \
127127
# Health
128128
HEALTH_SERVER_ADDRESS=127.0.0.1:9999 \
129-
HEALTH_ADDRESS_TO_PING=github.com \
129+
HEALTH_TARGET_ADDRESS=github.com:443 \
130130
HEALTH_VPN_DURATION_INITIAL=6s \
131131
HEALTH_VPN_DURATION_ADDITION=5s \
132132
# DNS over TLS

go.mod

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ go 1.17
55
require (
66
github.com/breml/rootcerts v0.2.1
77
github.com/fatih/color v1.13.0
8-
github.com/go-ping/ping v0.0.0-20210911151512-381826476871
98
github.com/golang/mock v1.6.0
109
github.com/qdm12/dns v1.11.0
1110
github.com/qdm12/golibs v0.0.0-20210822203818-5c568b0777b6
@@ -26,7 +25,6 @@ require (
2625
require (
2726
github.com/davecgh/go-spew v1.1.1 // indirect
2827
github.com/google/go-cmp v0.5.5 // indirect
29-
github.com/google/uuid v1.2.0 // indirect
3028
github.com/josharian/native v0.0.0-20200817173448-b6b71def0850 // indirect
3129
github.com/mattn/go-colorable v0.1.9 // indirect
3230
github.com/mattn/go-isatty v0.0.14 // indirect
@@ -41,6 +39,5 @@ require (
4139
go4.org/unsafe/assume-no-moving-gc v0.0.0-20201222180813-1025295fd063 // indirect
4240
golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97 // indirect
4341
golang.org/x/net v0.0.0-20210504132125-bbd867fde50d // indirect
44-
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
4542
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
4643
)

go.sum

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ github.com/go-openapi/spec v0.17.0/go.mod h1:XkF/MOi14NmjsfZ8VtAKf8pIlbZzyoTvZsd
3232
github.com/go-openapi/strfmt v0.17.0/go.mod h1:P82hnJI0CXkErkXi8IKjPbNBM6lV6+5pLP5l494TcyU=
3333
github.com/go-openapi/swag v0.17.0/go.mod h1:AByQ+nYG6gQg71GINrmuDXCPWdL640yX49/kXLo40Tg=
3434
github.com/go-openapi/validate v0.17.0/go.mod h1:Uh4HdOzKt19xGIGm1qHf/ofbX1YQ4Y+MYsct2VUrAJ4=
35-
github.com/go-ping/ping v0.0.0-20210911151512-381826476871 h1:wtjTfjwAR/BYYMJ+QOLI/3J/qGEI0fgrkZvgsEWK2/Q=
36-
github.com/go-ping/ping v0.0.0-20210911151512-381826476871/go.mod h1:xIFjORFzTxqIV/tDVGO4eDy/bLuSyawEeojSm3GfRGk=
3735
github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8=
3836
github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc=
3937
github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs=
@@ -47,8 +45,6 @@ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
4745
github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
4846
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
4947
github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
50-
github.com/google/uuid v1.2.0 h1:qJYtXnJRWmpe7m/3XlyhrsLrEURqHRM2kxzoxXqyUDs=
51-
github.com/google/uuid v1.2.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
5248
github.com/gotify/go-api-client/v2 v2.0.4/go.mod h1:VKiah/UK20bXsr0JObE1eBVLW44zbBouzjuri9iwjFU=
5349
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
5450
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
@@ -176,7 +172,6 @@ golang.org/x/net v0.0.0-20201216054612-986b41b23924/go.mod h1:m0MpNAwzfU5UDzcl9v
176172
golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
177173
golang.org/x/net v0.0.0-20210119194325-5f4716e94777/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
178174
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
179-
golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLdyRGr576XBO4/greRjx4P4O3yc=
180175
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
181176
golang.org/x/net v0.0.0-20210504132125-bbd867fde50d h1:nTDGCTeAu2LhcsHTRzjyIUbZHCJ4QePArsm27Hka0UM=
182177
golang.org/x/net v0.0.0-20210504132125-bbd867fde50d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
@@ -210,7 +205,6 @@ golang.org/x/sys v0.0.0-20210123111255-9b0068b26619/go.mod h1:h1NjWce9XRLGQEsW7w
210205
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
211206
golang.org/x/sys v0.0.0-20210216163648-f7da38b97c65/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
212207
golang.org/x/sys v0.0.0-20210309040221-94ec62e08169/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
213-
golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
214208
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
215209
golang.org/x/sys v0.0.0-20210403161142-5e06dd20ab57/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
216210
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

internal/configuration/settings/health.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ type Health struct {
1515
// for the health check server.
1616
// It cannot be the empty string in the internal state.
1717
ServerAddress string
18-
// AddressToPing is the IP address or domain name to
19-
// ping periodically for the health check.
18+
// TargetAddress is the address (host or host:port)
19+
// to TCP dial to periodically for the health check.
2020
// It cannot be the empty string in the internal state.
21-
AddressToPing string
21+
TargetAddress string
2222
VPN HealthyWait
2323
}
2424

@@ -42,7 +42,7 @@ func (h Health) Validate() (err error) {
4242
func (h *Health) copy() (copied Health) {
4343
return Health{
4444
ServerAddress: h.ServerAddress,
45-
AddressToPing: h.AddressToPing,
45+
TargetAddress: h.TargetAddress,
4646
VPN: h.VPN.copy(),
4747
}
4848
}
@@ -51,7 +51,7 @@ func (h *Health) copy() (copied Health) {
5151
// unset field of the receiver settings object.
5252
func (h *Health) MergeWith(other Health) {
5353
h.ServerAddress = helpers.MergeWithString(h.ServerAddress, other.ServerAddress)
54-
h.AddressToPing = helpers.MergeWithString(h.AddressToPing, other.AddressToPing)
54+
h.TargetAddress = helpers.MergeWithString(h.TargetAddress, other.TargetAddress)
5555
h.VPN.mergeWith(other.VPN)
5656
}
5757

@@ -60,13 +60,13 @@ func (h *Health) MergeWith(other Health) {
6060
// settings.
6161
func (h *Health) OverrideWith(other Health) {
6262
h.ServerAddress = helpers.OverrideWithString(h.ServerAddress, other.ServerAddress)
63-
h.AddressToPing = helpers.OverrideWithString(h.AddressToPing, other.AddressToPing)
63+
h.TargetAddress = helpers.OverrideWithString(h.TargetAddress, other.TargetAddress)
6464
h.VPN.overrideWith(other.VPN)
6565
}
6666

6767
func (h *Health) SetDefaults() {
6868
h.ServerAddress = helpers.DefaultString(h.ServerAddress, "127.0.0.1:9999")
69-
h.AddressToPing = helpers.DefaultString(h.AddressToPing, "github.com")
69+
h.TargetAddress = helpers.DefaultString(h.TargetAddress, "github.com:443")
7070
h.VPN.setDefaults()
7171
}
7272

@@ -77,7 +77,7 @@ func (h Health) String() string {
7777
func (h Health) toLinesNode() (node *gotree.Node) {
7878
node = gotree.New("Health settings:")
7979
node.Appendf("Server listening address: %s", h.ServerAddress)
80-
node.Appendf("Address to ping: %s", h.AddressToPing)
80+
node.Appendf("Target address: %s", h.TargetAddress)
8181
node.AppendNode(h.VPN.toLinesNode("VPN"))
8282
return node
8383
}

internal/configuration/settings/settings_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func Test_Settings_String(t *testing.T) {
6666
| └── Log level: INFO
6767
├── Health settings:
6868
| ├── Server listening address: 127.0.0.1:9999
69-
| ├── Address to ping: github.com
69+
| ├── Target address: github.com:443
7070
| └── VPN wait durations:
7171
| ├── Initial duration: 6s
7272
| └── Additional duration: 5s

internal/configuration/sources/env/health.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import (
1010

1111
func (r *Reader) ReadHealth() (health settings.Health, err error) {
1212
health.ServerAddress = os.Getenv("HEALTH_SERVER_ADDRESS")
13-
health.AddressToPing = os.Getenv("HEALTH_ADDRESS_TO_PING")
13+
health.TargetAddress = os.Getenv("HEALTH_ADDRESS_TO_PING")
14+
if health.TargetAddress == "" {
15+
health.TargetAddress = os.Getenv("HEALTH_TARGET_ADDRESS")
16+
}
1417

1518
health.VPN.Initial, err = r.readDurationWithRetro(
1619
"HEALTH_VPN_DURATION_INITIAL",

internal/healthcheck/health.go

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ package healthcheck
33

44
import (
55
"context"
6+
"errors"
7+
"fmt"
8+
"net"
69
"time"
710
)
811

@@ -14,7 +17,12 @@ func (s *Server) runHealthcheckLoop(ctx context.Context, done chan<- struct{}) {
1417
for {
1518
previousErr := s.handler.getErr()
1619

17-
err := healthCheck(ctx, s.pinger)
20+
const healthcheckTimeout = 3 * time.Second
21+
healthcheckCtx, healthcheckCancel := context.WithTimeout(
22+
ctx, healthcheckTimeout)
23+
err := s.healthCheck(healthcheckCtx)
24+
healthcheckCancel()
25+
1826
s.handler.setErr(err)
1927

2028
if previousErr != nil && err == nil {
@@ -56,23 +64,40 @@ func (s *Server) runHealthcheckLoop(ctx context.Context, done chan<- struct{}) {
5664
}
5765
}
5866

59-
func healthCheck(ctx context.Context, pinger Pinger) (err error) {
67+
func (s *Server) healthCheck(ctx context.Context) (err error) {
6068
// TODO use mullvad API if current provider is Mullvad
61-
// If we run without root, you need to run this on the gluetun binary:
62-
// setcap cap_net_raw=+ep /path/to/your/compiled/binary
63-
// Alternatively, we could have a separate binary just for healthcheck to
64-
// reduce attack surface.
65-
errCh := make(chan error)
66-
go func() {
67-
errCh <- pinger.Run()
68-
}()
6969

70-
select {
71-
case <-ctx.Done():
72-
pinger.Stop()
73-
<-errCh
74-
return ctx.Err()
75-
case err = <-errCh:
70+
address, err := makeAddressToDial(s.config.TargetAddress)
71+
if err != nil {
7672
return err
7773
}
74+
75+
const dialNetwork = "tcp4"
76+
connection, err := s.dialer.DialContext(ctx, dialNetwork, address)
77+
if err != nil {
78+
return fmt.Errorf("cannot dial: %w", err)
79+
}
80+
81+
err = connection.Close()
82+
if err != nil {
83+
return fmt.Errorf("cannot close connection: %w", err)
84+
}
85+
86+
return nil
87+
}
88+
89+
func makeAddressToDial(address string) (addressToDial string, err error) {
90+
host, port, err := net.SplitHostPort(address)
91+
if err != nil {
92+
addrErr := new(net.AddrError)
93+
ok := errors.As(err, &addrErr)
94+
if !ok || addrErr.Err != "missing port in address" {
95+
return "", fmt.Errorf("cannot split host and port from address: %w", err)
96+
}
97+
host = address
98+
const defaultPort = "443"
99+
port = defaultPort
100+
}
101+
address = net.JoinHostPort(host, port)
102+
return address, nil
78103
}

internal/healthcheck/health_ping_test.go

Lines changed: 0 additions & 46 deletions
This file was deleted.

0 commit comments

Comments
 (0)