feat(cli): add aws check to ping p2p diagnostics

coder · ethanndickson · Aug 29, 2024 · Aug 27, 2024 · Aug 29, 2024 · Aug 29, 2024
commit 9ccec941ec793d61e15e2b0f40849b58413a9cf9
diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go
@@ -357,10 +357,18 @@ type ConnDiags struct {
 	LocalNetInfo    *tailcfg.NetInfo
 	LocalInterfaces *healthsdk.InterfacesReport
 	AgentNetcheck   *healthsdk.AgentNetcheckReport
+	ClientIPIsAWS   bool
+	AgentIPIsAWS    bool
 	// TODO: More diagnostics
 }
 
 func ConnDiagnostics(w io.Writer, d ConnDiags) {
+	if d.PingP2P {
+		_, _ = fmt.Fprint(w, "✔ You are connected directly (p2p)\n")
+	} else {
+		_, _ = fmt.Fprint(w, "❗ You are connected via a DERP relay, not directly (p2p)\n")
+	}
+
 	if d.AgentNetcheck != nil {
 		for _, msg := range d.AgentNetcheck.Interfaces.Warnings {
 			_, _ = fmt.Fprintf(w, "❗ Agent: %s\n", msg.Message)
@@ -373,12 +381,6 @@ func ConnDiagnostics(w io.Writer, d ConnDiags) {
 		}
 	}
 
-	if d.PingP2P {
-		_, _ = fmt.Fprint(w, "✔ You are connected directly (p2p)\n")
-		return
-	}
-	_, _ = fmt.Fprint(w, "❗ You are connected via a DERP relay, not directly (p2p)\n")
-
 	if d.DisableDirect {
 		_, _ = fmt.Fprint(w, "❗ Direct connections are disabled locally, by `--disable-direct` or `CODER_DISABLE_DIRECT`\n")
 		return
@@ -389,15 +391,32 @@ func ConnDiagnostics(w io.Writer, d ConnDiags) {
 		return
 	}
 
-	if d.ConnInfo != nil && d.ConnInfo.DERPMap != nil && !d.ConnInfo.DERPMap.HasSTUN() {
-		_, _ = fmt.Fprint(w, "✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks\n")
+	if d.ConnInfo != nil && d.ConnInfo.DERPMap != nil {
+		if !d.ConnInfo.DERPMap.HasSTUN() {
+			_, _ = fmt.Fprint(w, "✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks\n")
+		} else if d.LocalNetInfo != nil && !d.LocalNetInfo.UDP {
+			_, _ = fmt.Fprint(w, "❗ Client could not connect to STUN over UDP, which may be preventing a direct connection\n")
+		}
 	}
 
 	if d.LocalNetInfo != nil && d.LocalNetInfo.MappingVariesByDestIP.EqualBool(true) {
 		_, _ = fmt.Fprint(w, "❗ Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
 	}
 
-	if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil && d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) {
-		_, _ = fmt.Fprint(w, "❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
+	if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil {
+		if d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) {
+			_, _ = fmt.Fprint(w, "❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers\n")
+		}
+		if !d.AgentNetcheck.NetInfo.UDP {
+			_, _ = fmt.Fprint(w, "❗ Agent could not connect to STUN over UDP, which may be preventing a direct connection\n")
+		}
+	}
+
+	if d.ClientIPIsAWS {
+		_, _ = 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")
+	}
+
+	if d.AgentIPIsAWS {
+		_, _ = 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")
 	}
 }
diff --git a/cli/cliui/agent_test.go b/cli/cliui/agent_test.go
@@ -719,6 +719,58 @@ func TestConnDiagnostics(t *testing.T) {
 				`✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks`,
 			},
 		},
+		{
+			name: "ClientHasStunNoUDP",
+			diags: cliui.ConnDiags{
+				ConnInfo: &workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{
+						Regions: map[int]*tailcfg.DERPRegion{
+							999: {
+								Nodes: []*tailcfg.DERPNode{
+									{
+										STUNPort: 1337,
+									},
+								},
+							},
+						},
+					},
+				},
+				LocalNetInfo: &tailcfg.NetInfo{
+					UDP: false,
+				},
+			},
+			want: []string{
+				`❗ You are connected via a DERP relay, not directly (p2p)`,
+				`❗ Client could not connect to STUN over UDP, which may be preventing a direct connection`,
+			},
+		},
+		{
+			name: "AgentHasStunNoUDP",
+			diags: cliui.ConnDiags{
+				ConnInfo: &workspacesdk.AgentConnectionInfo{
+					DERPMap: &tailcfg.DERPMap{
+						Regions: map[int]*tailcfg.DERPRegion{
+							999: {
+								Nodes: []*tailcfg.DERPNode{
+									{
+										STUNPort: 1337,
+									},
+								},
+							},
+						},
+					},
+				},
+				AgentNetcheck: &healthsdk.AgentNetcheckReport{
+					NetInfo: &tailcfg.NetInfo{
+						UDP: false,
+					},
+				},
+			},
+			want: []string{
+				`❗ You are connected via a DERP relay, not directly (p2p)`,
+				`❗ Agent could not connect to STUN over UDP, which may be preventing a direct connection`,
+			},
+		},
 		{
 			name: "ClientHardNat",
 			diags: cliui.ConnDiags{
@@ -782,6 +834,28 @@ func TestConnDiagnostics(t *testing.T) {
 				`✔ You are connected directly (p2p)`,
 			},
 		},
+		{
+			name: "ClientAWSIP",
+			diags: cliui.ConnDiags{
+				ClientIPIsAWS: true,
+				AgentIPIsAWS:  false,
+			},
+			want: []string{
+				`❗ You are connected via a DERP relay, not directly (p2p)`,
+				`❗ Client IP address is within an AWS range, which is known to cause problems with forming direct connections (AWS uses hard NAT)`,
+			},
+		},
+		{
+			name: "AgentAWSIP",
+			diags: cliui.ConnDiags{
+				ClientIPIsAWS: false,
+				AgentIPIsAWS:  true,
+			},
+			want: []string{
+				`❗ You are connected via a DERP relay, not directly (p2p)`,
+				`❗ Agent IP address is within an AWS range, which is known to cause problems with forming direct connections (AWS uses hard NAT)`,
+			},
+		},
 	}
 	for _, tc := range testCases {
 		tc := tc

diff --git a/cli/cliutil/awscheck.go b/cli/cliutil/awscheck.go
@@ -0,0 +1,114 @@
+package cliutil
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/netip"
+	"time"
+
+	"golang.org/x/xerrors"
+)
+
+const AWSIPRangesURL = "https://ip-ranges.amazonaws.com/ip-ranges.json"
+
+type awsIPv4Prefix struct {
+	Prefix             string `json:"ip_prefix"`
+	Region             string `json:"region"`
+	Service            string `json:"service"`
+	NetworkBorderGroup string `json:"network_border_group"`
+}
+
+type awsIPv6Prefix struct {
+	Prefix             string `json:"ipv6_prefix"`
+	Region             string `json:"region"`
+	Service            string `json:"service"`
+	NetworkBorderGroup string `json:"network_border_group"`
+}
+
+type AWSIPRanges struct {
+	V4 []netip.Prefix
+	V6 []netip.Prefix
+}
+
+type awsIPRangesResponse struct {
+	SyncToken    string          `json:"syncToken"`
+	CreateDate   string          `json:"createDate"`
+	IPV4Prefixes []awsIPv4Prefix `json:"prefixes"`
+	IPV6Prefixes []awsIPv6Prefix `json:"ipv6_prefixes"`
+}
+
+func FetchAWSIPRanges(ctx context.Context, url string) (*AWSIPRanges, error) {
+	client := &http.Client{}
+	reqCtx, reqCancel := context.WithTimeout(ctx, 5*time.Second)
+	defer reqCancel()
+	req, _ := http.NewRequestWithContext(reqCtx, http.MethodGet, url, nil)
+	resp, err := client.Do(req)
+	if err != nil {
+		return nil, err
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		b, _ := io.ReadAll(resp.Body)
+		return nil, xerrors.Errorf("unexpected status code %d: %s", resp.StatusCode, b)
+	}
+
+	var body awsIPRangesResponse
+	err = json.NewDecoder(resp.Body).Decode(&body)
+	if err != nil {
+		return nil, xerrors.Errorf("json decode: %w", err)
+	}
+
+	out := &AWSIPRanges{
+		V4: make([]netip.Prefix, 0, len(body.IPV4Prefixes)),
+		V6: make([]netip.Prefix, 0, len(body.IPV6Prefixes)),
+	}
+
+	for _, p := range body.IPV4Prefixes {
+		prefix, err := netip.ParsePrefix(p.Prefix)
+		if err != nil {
+			return nil, xerrors.Errorf("parse ip prefix: %w", err)
+		}
+		if prefix.Addr().Is6() {
+			return nil, xerrors.Errorf("ipv4 prefix contains ipv6 address: %s", p.Prefix)
+		}
+		out.V4 = append(out.V4, prefix)
+	}
+
+	for _, p := range body.IPV6Prefixes {
+		prefix, err := netip.ParsePrefix(p.Prefix)
+		if err != nil {
+			return nil, xerrors.Errorf("parse ip prefix: %w", err)
+		}
+		if prefix.Addr().Is4() {
+			return nil, xerrors.Errorf("ipv6 prefix contains ipv4 address: %s", p.Prefix)
+		}
+		out.V6 = append(out.V6, prefix)
+	}
+
+	return out, nil
+}
+
+// CheckIP checks if the given IP address is an AWS IP.
+func (r *AWSIPRanges) CheckIP(ip netip.Addr) bool {
+	if ip.IsLoopback() || ip.IsLinkLocalMulticast() || ip.IsLinkLocalUnicast() || ip.IsPrivate() {
+		return false
+	}
+
+	if ip.Is4() {
+		for _, p := range r.V4 {
+			if p.Contains(ip) {
+				return true
+			}
+		}
+	} else {
+		for _, p := range r.V6 {
+			if p.Contains(ip) {
+				return true
+			}
+		}
+	}
+	return false
+}
diff --git a/cli/cliutil/awscheck_internal_test.go b/cli/cliutil/awscheck_internal_test.go
@@ -0,0 +1,95 @@
+package cliutil
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/v2/coderd/httpapi"
+	"github.com/coder/coder/v2/testutil"
+)
+
+func TestIPV4Check(t *testing.T) {
+	t.Parallel()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		httpapi.Write(context.Background(), w, http.StatusOK, awsIPRangesResponse{
+			IPV4Prefixes: []awsIPv4Prefix{
+				{
+					Prefix: "3.24.0.0/14",
+				},
+				{
+					Prefix: "15.230.15.29/32",
+				},
+				{
+					Prefix: "47.128.82.100/31",
+				},
+			},
+			IPV6Prefixes: []awsIPv6Prefix{
+				{
+					Prefix: "2600:9000:5206::/48",
+				},
+				{
+					Prefix: "2406:da70:8800::/40",
+				},
+				{
+					Prefix: "2600:1f68:5000::/40",
+				},
+			},
+		})
+	}))
+	ctx := testutil.Context(t, testutil.WaitShort)
+	ranges, err := FetchAWSIPRanges(ctx, srv.URL)
+	require.NoError(t, err)
+
+	t.Run("Private/IPV4", func(t *testing.T) {
+		t.Parallel()
+		ip, err := netip.ParseAddr("192.168.0.1")
+		require.NoError(t, err)
+		isAws := ranges.CheckIP(ip)
+		require.False(t, isAws)
+	})
+
+	t.Run("AWS/IPV4", func(t *testing.T) {
+		t.Parallel()
+		ip, err := netip.ParseAddr("3.25.61.113")
+		require.NoError(t, err)
+		isAws := ranges.CheckIP(ip)
+		require.True(t, isAws)
+	})
+
+	t.Run("NonAWS/IPV4", func(t *testing.T) {
+		t.Parallel()
+		ip, err := netip.ParseAddr("159.196.123.40")
+		require.NoError(t, err)
+		isAws := ranges.CheckIP(ip)
+		require.False(t, isAws)
+	})
+
+	t.Run("Private/IPV6", func(t *testing.T) {
+		t.Parallel()
+		ip, err := netip.ParseAddr("::1")
+		require.NoError(t, err)
+		isAws := ranges.CheckIP(ip)
+		require.False(t, isAws)
+	})
+
+	t.Run("AWS/IPV6", func(t *testing.T) {
+		t.Parallel()
+		ip, err := netip.ParseAddr("2600:9000:5206:0001:0000:0000:0000:0001")
+		require.NoError(t, err)
+		isAws := ranges.CheckIP(ip)
+		require.True(t, isAws)
+	})
+
+	t.Run("NonAWS/IPV6", func(t *testing.T) {
+		t.Parallel()
+		ip, err := netip.ParseAddr("2403:5807:885f:0:a544:49d4:58f8:aedf")
+		require.NoError(t, err)
+		isAws := ranges.CheckIP(ip)
+		require.False(t, isAws)
+	})
+}