Skip to content

Commit 9ccec94

Browse files
ethanndicksonethanndickson
committed
feat(cli): add aws check to ping p2p diagnostics
1 parent e65eb03 commit 9ccec94

File tree

5 files changed

+344
-11
lines changed

5 files changed

+344
-11
lines changed

‎cli/cliui/agent.go

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -357,10 +357,18 @@ type ConnDiags struct {
357357
LocalNetInfo *tailcfg.NetInfo
358358
LocalInterfaces *healthsdk.InterfacesReport
359359
AgentNetcheck *healthsdk.AgentNetcheckReport
360+
ClientIPIsAWS bool
361+
AgentIPIsAWS bool
360362
// TODO: More diagnostics
361363
}
362364

363365
func ConnDiagnostics(w io.Writer, d ConnDiags) {
366+
if d.PingP2P {
367+
_, _ = fmt.Fprint(w, "✔ You are connected directly (p2p)\n")
368+
} else {
369+
_, _ = fmt.Fprint(w, "❗ You are connected via a DERP relay, not directly (p2p)\n")
370+
}
371+
364372
if d.AgentNetcheck != nil {
365373
for _, msg := range d.AgentNetcheck.Interfaces.Warnings {
366374
_, _ = fmt.Fprintf(w, "❗ Agent: %s\n", msg.Message)
@@ -373,12 +381,6 @@ func ConnDiagnostics(w io.Writer, d ConnDiags) {
373381
}
374382
}
375383

376-
if d.PingP2P {
377-
_, _ = fmt.Fprint(w, "✔ You are connected directly (p2p)\n")
378-
return
379-
}
380-
_, _ = fmt.Fprint(w, "❗ You are connected via a DERP relay, not directly (p2p)\n")
381-
382384
if d.DisableDirect {
383385
_, _ = fmt.Fprint(w, "❗ Direct connections are disabled locally, by `--disable-direct` or `CODER_DISABLE_DIRECT`\n")
384386
return
@@ -389,15 +391,32 @@ func ConnDiagnostics(w io.Writer, d ConnDiags) {
389391
return
390392
}
391393

392-
if d.ConnInfo != nil && d.ConnInfo.DERPMap != nil && !d.ConnInfo.DERPMap.HasSTUN() {
393-
_, _ = fmt.Fprint(w, "✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks\n")
394+
if d.ConnInfo != nil && d.ConnInfo.DERPMap != nil {
395+
if !d.ConnInfo.DERPMap.HasSTUN() {
396+
_, _ = fmt.Fprint(w, "✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks\n")
397+
} else if d.LocalNetInfo != nil && !d.LocalNetInfo.UDP {
398+
_, _ = fmt.Fprint(w, "❗ Client could not connect to STUN over UDP, which may be preventing a direct connection\n")
399+
}
394400
}
395401

396402
if d.LocalNetInfo != nil && d.LocalNetInfo.MappingVariesByDestIP.EqualBool(true) {
397403
_, _ = fmt.Fprint(w, "❗ Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
398404
}
399405

400-
if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil && d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) {
401-
_, _ = fmt.Fprint(w, "❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
406+
if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil {
407+
if d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) {
408+
_, _ = fmt.Fprint(w, "❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
409+
}
410+
if !d.AgentNetcheck.NetInfo.UDP {
411+
_, _ = fmt.Fprint(w, "❗ Agent could not connect to STUN over UDP, which may be preventing a direct connection\n")
412+
}
413+
}
414+
415+
if d.ClientIPIsAWS {
416+
_, _ = fmt.Fprint(w, "❗ Client IP address is within an AWS range, which is known to cause problems with forming direct connections (AWS uses hard NAT)\n")
417+
}
418+
419+
if d.AgentIPIsAWS {
420+
_, _ = fmt.Fprint(w, "❗ Agent IP address is within an AWS range, which is known to cause problems with forming direct connections (AWS uses hard NAT)\n")
402421
}
403422
}

‎cli/cliui/agent_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,58 @@ func TestConnDiagnostics(t *testing.T) {
719719
`✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks`,
720720
},
721721
},
722+
{
723+
name: "ClientHasStunNoUDP",
724+
diags: cliui.ConnDiags{
725+
ConnInfo: &workspacesdk.AgentConnectionInfo{
726+
DERPMap: &tailcfg.DERPMap{
727+
Regions: map[int]*tailcfg.DERPRegion{
728+
999: {
729+
Nodes: []*tailcfg.DERPNode{
730+
{
731+
STUNPort: 1337,
732+
},
733+
},
734+
},
735+
},
736+
},
737+
},
738+
LocalNetInfo: &tailcfg.NetInfo{
739+
UDP: false,
740+
},
741+
},
742+
want: []string{
743+
`❗ You are connected via a DERP relay, not directly (p2p)`,
744+
`❗ Client could not connect to STUN over UDP, which may be preventing a direct connection`,
745+
},
746+
},
747+
{
748+
name: "AgentHasStunNoUDP",
749+
diags: cliui.ConnDiags{
750+
ConnInfo: &workspacesdk.AgentConnectionInfo{
751+
DERPMap: &tailcfg.DERPMap{
752+
Regions: map[int]*tailcfg.DERPRegion{
753+
999: {
754+
Nodes: []*tailcfg.DERPNode{
755+
{
756+
STUNPort: 1337,
757+
},
758+
},
759+
},
760+
},
761+
},
762+
},
763+
AgentNetcheck: &healthsdk.AgentNetcheckReport{
764+
NetInfo: &tailcfg.NetInfo{
765+
UDP: false,
766+
},
767+
},
768+
},
769+
want: []string{
770+
`❗ You are connected via a DERP relay, not directly (p2p)`,
771+
`❗ Agent could not connect to STUN over UDP, which may be preventing a direct connection`,
772+
},
773+
},
722774
{
723775
name: "ClientHardNat",
724776
diags: cliui.ConnDiags{
@@ -782,6 +834,28 @@ func TestConnDiagnostics(t *testing.T) {
782834
`✔ You are connected directly (p2p)`,
783835
},
784836
},
837+
{
838+
name: "ClientAWSIP",
839+
diags: cliui.ConnDiags{
840+
ClientIPIsAWS: true,
841+
AgentIPIsAWS: false,
842+
},
843+
want: []string{
844+
`❗ You are connected via a DERP relay, not directly (p2p)`,
845+
`❗ Client IP address is within an AWS range, which is known to cause problems with forming direct connections (AWS uses hard NAT)`,
846+
},
847+
},
848+
{
849+
name: "AgentAWSIP",
850+
diags: cliui.ConnDiags{
851+
ClientIPIsAWS: false,
852+
AgentIPIsAWS: true,
853+
},
854+
want: []string{
855+
`❗ You are connected via a DERP relay, not directly (p2p)`,
856+
`❗ Agent IP address is within an AWS range, which is known to cause problems with forming direct connections (AWS uses hard NAT)`,
857+
},
858+
},
785859
}
786860
for _, tc := range testCases {
787861
tc := tc

‎cli/cliutil/awscheck.go

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package cliutil
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"io"
7+
"net/http"
8+
"net/netip"
9+
"time"
10+
11+
"golang.org/x/xerrors"
12+
)
13+
14+
const AWSIPRangesURL = "https://ip-ranges.amazonaws.com/ip-ranges.json"
15+
16+
type awsIPv4Prefix struct {
17+
Prefix string `json:"ip_prefix"`
18+
Region string `json:"region"`
19+
Service string `json:"service"`
20+
NetworkBorderGroup string `json:"network_border_group"`
21+
}
22+
23+
type awsIPv6Prefix struct {
24+
Prefix string `json:"ipv6_prefix"`
25+
Region string `json:"region"`
26+
Service string `json:"service"`
27+
NetworkBorderGroup string `json:"network_border_group"`
28+
}
29+
30+
type AWSIPRanges struct {
31+
V4 []netip.Prefix
32+
V6 []netip.Prefix
33+
}
34+
35+
type awsIPRangesResponse struct {
36+
SyncToken string `json:"syncToken"`
37+
CreateDate string `json:"createDate"`
38+
IPV4Prefixes []awsIPv4Prefix `json:"prefixes"`
39+
IPV6Prefixes []awsIPv6Prefix `json:"ipv6_prefixes"`
40+
}
41+
42+
func FetchAWSIPRanges(ctx context.Context, url string) (*AWSIPRanges, error) {
43+
client := &http.Client{}
44+
reqCtx, reqCancel := context.WithTimeout(ctx, 5*time.Second)
45+
defer reqCancel()
46+
req, _ := http.NewRequestWithContext(reqCtx, http.MethodGet, url, nil)
47+
resp, err := client.Do(req)
48+
if err != nil {
49+
return nil, err
50+
}
51+
defer resp.Body.Close()
52+
53+
if resp.StatusCode != http.StatusOK {
54+
b, _ := io.ReadAll(resp.Body)
55+
return nil, xerrors.Errorf("unexpected status code %d: %s", resp.StatusCode, b)
56+
}
57+
58+
var body awsIPRangesResponse
59+
err = json.NewDecoder(resp.Body).Decode(&body)
60+
if err != nil {
61+
return nil, xerrors.Errorf("json decode: %w", err)
62+
}
63+
64+
out := &AWSIPRanges{
65+
V4: make([]netip.Prefix, 0, len(body.IPV4Prefixes)),
66+
V6: make([]netip.Prefix, 0, len(body.IPV6Prefixes)),
67+
}
68+
69+
for _, p := range body.IPV4Prefixes {
70+
prefix, err := netip.ParsePrefix(p.Prefix)
71+
if err != nil {
72+
return nil, xerrors.Errorf("parse ip prefix: %w", err)
73+
}
74+
if prefix.Addr().Is6() {
75+
return nil, xerrors.Errorf("ipv4 prefix contains ipv6 address: %s", p.Prefix)
76+
}
77+
out.V4 = append(out.V4, prefix)
78+
}
79+
80+
for _, p := range body.IPV6Prefixes {
81+
prefix, err := netip.ParsePrefix(p.Prefix)
82+
if err != nil {
83+
return nil, xerrors.Errorf("parse ip prefix: %w", err)
84+
}
85+
if prefix.Addr().Is4() {
86+
return nil, xerrors.Errorf("ipv6 prefix contains ipv4 address: %s", p.Prefix)
87+
}
88+
out.V6 = append(out.V6, prefix)
89+
}
90+
91+
return out, nil
92+
}
93+
94+
// CheckIP checks if the given IP address is an AWS IP.
95+
func (r *AWSIPRanges) CheckIP(ip netip.Addr) bool {
96+
if ip.IsLoopback() || ip.IsLinkLocalMulticast() || ip.IsLinkLocalUnicast() || ip.IsPrivate() {
97+
return false
98+
}
99+
100+
if ip.Is4() {
101+
for _, p := range r.V4 {
102+
if p.Contains(ip) {
103+
return true
104+
}
105+
}
106+
} else {
107+
for _, p := range r.V6 {
108+
if p.Contains(ip) {
109+
return true
110+
}
111+
}
112+
}
113+
return false
114+
}

‎cli/cliutil/awscheck_internal_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package cliutil
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"net/netip"
8+
"testing"
9+
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/coder/coder/v2/coderd/httpapi"
13+
"github.com/coder/coder/v2/testutil"
14+
)
15+
16+
func TestIPV4Check(t *testing.T) {
17+
t.Parallel()
18+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
19+
httpapi.Write(context.Background(), w, http.StatusOK, awsIPRangesResponse{
20+
IPV4Prefixes: []awsIPv4Prefix{
21+
{
22+
Prefix: "3.24.0.0/14",
23+
},
24+
{
25+
Prefix: "15.230.15.29/32",
26+
},
27+
{
28+
Prefix: "47.128.82.100/31",
29+
},
30+
},
31+
IPV6Prefixes: []awsIPv6Prefix{
32+
{
33+
Prefix: "2600:9000:5206::/48",
34+
},
35+
{
36+
Prefix: "2406:da70:8800::/40",
37+
},
38+
{
39+
Prefix: "2600:1f68:5000::/40",
40+
},
41+
},
42+
})
43+
}))
44+
ctx := testutil.Context(t, testutil.WaitShort)
45+
ranges, err := FetchAWSIPRanges(ctx, srv.URL)
46+
require.NoError(t, err)
47+
48+
t.Run("Private/IPV4", func(t *testing.T) {
49+
t.Parallel()
50+
ip, err := netip.ParseAddr("192.168.0.1")
51+
require.NoError(t, err)
52+
isAws := ranges.CheckIP(ip)
53+
require.False(t, isAws)
54+
})
55+
56+
t.Run("AWS/IPV4", func(t *testing.T) {
57+
t.Parallel()
58+
ip, err := netip.ParseAddr("3.25.61.113")
59+
require.NoError(t, err)
60+
isAws := ranges.CheckIP(ip)
61+
require.True(t, isAws)
62+
})
63+
64+
t.Run("NonAWS/IPV4", func(t *testing.T) {
65+
t.Parallel()
66+
ip, err := netip.ParseAddr("159.196.123.40")
67+
require.NoError(t, err)
68+
isAws := ranges.CheckIP(ip)
69+
require.False(t, isAws)
70+
})
71+
72+
t.Run("Private/IPV6", func(t *testing.T) {
73+
t.Parallel()
74+
ip, err := netip.ParseAddr("::1")
75+
require.NoError(t, err)
76+
isAws := ranges.CheckIP(ip)
77+
require.False(t, isAws)
78+
})
79+
80+
t.Run("AWS/IPV6", func(t *testing.T) {
81+
t.Parallel()
82+
ip, err := netip.ParseAddr("2600:9000:5206:0001:0000:0000:0000:0001")
83+
require.NoError(t, err)
84+
isAws := ranges.CheckIP(ip)
85+
require.True(t, isAws)
86+
})
87+
88+
t.Run("NonAWS/IPV6", func(t *testing.T) {
89+
t.Parallel()
90+
ip, err := netip.ParseAddr("2403:5807:885f:0:a544:49d4:58f8:aedf")
91+
require.NoError(t, err)
92+
isAws := ranges.CheckIP(ip)
93+
require.False(t, isAws)
94+
})
95+
}

0 commit comments

Comments
 (0)