diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go index a0079d3b3c81e..9f528a6d69f20 100644 --- a/cli/cliui/agent.go +++ b/cli/cliui/agent.go @@ -351,53 +351,100 @@ func PeerDiagnostics(w io.Writer, d tailnet.PeerDiagnostics) { } type ConnDiags struct { - ConnInfo *workspacesdk.AgentConnectionInfo + ConnInfo workspacesdk.AgentConnectionInfo PingP2P bool DisableDirect bool LocalNetInfo *tailcfg.NetInfo LocalInterfaces *healthsdk.InterfacesReport AgentNetcheck *healthsdk.AgentNetcheckReport + ClientIPIsAWS bool + AgentIPIsAWS bool + Verbose bool // TODO: More diagnostics } -func ConnDiagnostics(w io.Writer, d ConnDiags) { +func (d ConnDiags) Write(w io.Writer) { + _, _ = fmt.Fprintln(w, "") + general, client, agent := d.splitDiagnostics() + for _, msg := range general { + _, _ = fmt.Fprintln(w, msg) + } + if len(client) > 0 { + _, _ = fmt.Fprint(w, "Possible client-side issues with direct connection:\n\n") + for _, msg := range client { + _, _ = fmt.Fprintf(w, " - %s\n\n", msg) + } + } + if len(agent) > 0 { + _, _ = fmt.Fprint(w, "Possible agent-side issues with direct connections:\n\n") + for _, msg := range agent { + _, _ = fmt.Fprintf(w, " - %s\n\n", msg) + } + } +} + +func (d ConnDiags) splitDiagnostics() (general, client, agent []string) { + if d.PingP2P { + general = append(general, "✔ You are connected directly (p2p)") + } else { + general = append(general, "❗ You are connected via a DERP relay, not directly (p2p)") + } + if d.AgentNetcheck != nil { for _, msg := range d.AgentNetcheck.Interfaces.Warnings { - _, _ = fmt.Fprintf(w, "❗ Agent: %s\n", msg.Message) + agent = append(agent, msg.Message) } } if d.LocalInterfaces != nil { for _, msg := range d.LocalInterfaces.Warnings { - _, _ = fmt.Fprintf(w, "❗ Client: %s\n", msg.Message) + client = append(client, msg.Message) } } - if d.PingP2P { - _, _ = fmt.Fprint(w, "✔ You are connected directly (p2p)\n") - return + if d.PingP2P && !d.Verbose { + return general, client, agent } - _, _ = 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 + general = append(general, "❗ Direct connections are disabled locally, by `--disable-direct` or `CODER_DISABLE_DIRECT`") + if !d.Verbose { + return general, client, agent + } } - if d.ConnInfo != nil && d.ConnInfo.DisableDirectConnections { - _, _ = fmt.Fprint(w, "❗ Your Coder administrator has blocked direct connections\n") - return + if d.ConnInfo.DisableDirectConnections { + general = append(general, "❗ Your Coder administrator has blocked direct connections") + if !d.Verbose { + return general, client, agent + } } - 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.DERPMap.HasSTUN() { + general = append(general, "The DERP map is not configured to use STUN") + } else if d.LocalNetInfo != nil && !d.LocalNetInfo.UDP { + client = append(client, "Client could not connect to STUN over UDP") } 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") + client = append(client, "Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers") + } + + if d.AgentNetcheck != nil && d.AgentNetcheck.NetInfo != nil { + if d.AgentNetcheck.NetInfo.MappingVariesByDestIP.EqualBool(true) { + agent = append(agent, "Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers") + } + if !d.AgentNetcheck.NetInfo.UDP { + agent = append(agent, "Agent could not connect to STUN over UDP") + } + } + + if d.ClientIPIsAWS { + client = append(client, "Client IP address is within an AWS range (AWS uses hard NAT)") } - 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.AgentIPIsAWS { + agent = append(agent, "Agent IP address is within an AWS range (AWS uses hard NAT)") } + return general, client, agent } diff --git a/cli/cliui/agent_test.go b/cli/cliui/agent_test.go index fced0c67ea1b9..aeb41130d344e 100644 --- a/cli/cliui/agent_test.go +++ b/cli/cliui/agent_test.go @@ -686,7 +686,9 @@ func TestConnDiagnostics(t *testing.T) { { name: "Direct", diags: cliui.ConnDiags{ - ConnInfo: &workspacesdk.AgentConnectionInfo{}, + ConnInfo: workspacesdk.AgentConnectionInfo{ + DERPMap: &tailcfg.DERPMap{}, + }, PingP2P: true, LocalNetInfo: &tailcfg.NetInfo{}, }, @@ -697,7 +699,8 @@ func TestConnDiagnostics(t *testing.T) { { name: "DirectBlocked", diags: cliui.ConnDiags{ - ConnInfo: &workspacesdk.AgentConnectionInfo{ + ConnInfo: workspacesdk.AgentConnectionInfo{ + DERPMap: &tailcfg.DERPMap{}, DisableDirectConnections: true, }, }, @@ -709,32 +712,89 @@ func TestConnDiagnostics(t *testing.T) { { name: "NoStun", diags: cliui.ConnDiags{ - ConnInfo: &workspacesdk.AgentConnectionInfo{ + ConnInfo: workspacesdk.AgentConnectionInfo{ DERPMap: &tailcfg.DERPMap{}, }, LocalNetInfo: &tailcfg.NetInfo{}, }, want: []string{ `❗ You are connected via a DERP relay, not directly (p2p)`, - `✘ The DERP map is not configured to use STUN, which will prevent direct connections from starting outside of local networks`, + `The DERP map is not configured to use STUN`, + }, + }, + { + 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`, + }, + }, + { + 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`, }, }, { name: "ClientHardNat", diags: cliui.ConnDiags{ + ConnInfo: workspacesdk.AgentConnectionInfo{ + DERPMap: &tailcfg.DERPMap{}, + }, LocalNetInfo: &tailcfg.NetInfo{ MappingVariesByDestIP: "true", }, }, want: []string{ `❗ You are connected via a DERP relay, not directly (p2p)`, - `❗ Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`, + `Client is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`, }, }, { name: "AgentHardNat", diags: cliui.ConnDiags{ - ConnInfo: &workspacesdk.AgentConnectionInfo{}, + ConnInfo: workspacesdk.AgentConnectionInfo{ + DERPMap: &tailcfg.DERPMap{}, + }, PingP2P: false, LocalNetInfo: &tailcfg.NetInfo{}, AgentNetcheck: &healthsdk.AgentNetcheckReport{ @@ -743,43 +803,77 @@ func TestConnDiagnostics(t *testing.T) { }, want: []string{ `❗ You are connected via a DERP relay, not directly (p2p)`, - `❗ Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`, + `Agent is potentially behind a hard NAT, as multiple endpoints were retrieved from different STUN servers`, }, }, { name: "AgentInterfaceWarnings", diags: cliui.ConnDiags{ + ConnInfo: workspacesdk.AgentConnectionInfo{ + DERPMap: &tailcfg.DERPMap{}, + }, PingP2P: true, AgentNetcheck: &healthsdk.AgentNetcheckReport{ Interfaces: healthsdk.InterfacesReport{ BaseReport: healthsdk.BaseReport{ Warnings: []health.Message{ - health.Messagef(health.CodeInterfaceSmallMTU, "network interface eth0 has MTU 1280, (less than 1378), which may cause problems with direct connections"), + health.Messagef(health.CodeInterfaceSmallMTU, "Network interface eth0 has MTU 1280, (less than 1378), which may degrade the quality of direct connections"), }, }, }, }, }, want: []string{ - `❗ Agent: network interface eth0 has MTU 1280, (less than 1378), which may cause problems with direct connections`, `✔ You are connected directly (p2p)`, + `Network interface eth0 has MTU 1280, (less than 1378), which may degrade the quality of direct connections`, }, }, { name: "LocalInterfaceWarnings", diags: cliui.ConnDiags{ + ConnInfo: workspacesdk.AgentConnectionInfo{ + DERPMap: &tailcfg.DERPMap{}, + }, PingP2P: true, LocalInterfaces: &healthsdk.InterfacesReport{ BaseReport: healthsdk.BaseReport{ Warnings: []health.Message{ - health.Messagef(health.CodeInterfaceSmallMTU, "network interface eth1 has MTU 1310, (less than 1378), which may cause problems with direct connections"), + health.Messagef(health.CodeInterfaceSmallMTU, "Network interface eth1 has MTU 1310, (less than 1378), which may degrade the quality of direct connections"), }, }, }, }, want: []string{ - `❗ Client: network interface eth1 has MTU 1310, (less than 1378), which may cause problems with direct connections`, `✔ You are connected directly (p2p)`, + `Network interface eth1 has MTU 1310, (less than 1378), which may degrade the quality of direct connections`, + }, + }, + { + name: "ClientAWSIP", + diags: cliui.ConnDiags{ + ConnInfo: workspacesdk.AgentConnectionInfo{ + DERPMap: &tailcfg.DERPMap{}, + }, + ClientIPIsAWS: true, + AgentIPIsAWS: false, + }, + want: []string{ + `❗ You are connected via a DERP relay, not directly (p2p)`, + `Client IP address is within an AWS range (AWS uses hard NAT)`, + }, + }, + { + name: "AgentAWSIP", + diags: cliui.ConnDiags{ + ConnInfo: workspacesdk.AgentConnectionInfo{ + DERPMap: &tailcfg.DERPMap{}, + }, + ClientIPIsAWS: false, + AgentIPIsAWS: true, + }, + want: []string{ + `❗ You are connected via a DERP relay, not directly (p2p)`, + `Agent IP address is within an AWS range (AWS uses hard NAT)`, }, }, } @@ -790,7 +884,7 @@ func TestConnDiagnostics(t *testing.T) { r, w := io.Pipe() go func() { defer w.Close() - cliui.ConnDiagnostics(w, tc.diags) + tc.diags.Write(w) }() bytes, err := io.ReadAll(r) require.NoError(t, err) diff --git a/cli/cliutil/awscheck.go b/cli/cliutil/awscheck.go new file mode 100644 index 0000000000000..20a5960a45fb2 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..8688bf51e7e28 --- /dev/null +++ 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) + }) +} diff --git a/cli/ping.go b/cli/ping.go index e36a518b41c7a..cadb69b5c21f5 100644 --- a/cli/ping.go +++ b/cli/ping.go @@ -5,9 +5,11 @@ import ( "errors" "fmt" "net/http" + "net/netip" "time" "golang.org/x/xerrors" + "tailscale.com/tailcfg" "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" @@ -15,6 +17,7 @@ import ( "github.com/coder/pretty" "github.com/coder/coder/v2/cli/cliui" + "github.com/coder/coder/v2/cli/cliutil" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/healthsdk" "github.com/coder/coder/v2/codersdk/workspacesdk" @@ -145,31 +148,42 @@ func (r *RootCmd) ping() *serpent.Command { break } } - ctx, cancel = context.WithTimeout(inv.Context(), 30*time.Second) - defer cancel() + diagCtx, diagCancel := context.WithTimeout(inv.Context(), 30*time.Second) + defer diagCancel() diags := conn.GetPeerDiagnostics() cliui.PeerDiagnostics(inv.Stdout, diags) + ni := conn.GetNetInfo() connDiags := cliui.ConnDiags{ PingP2P: didP2p, DisableDirect: r.disableDirect, - LocalNetInfo: conn.GetNetInfo(), + LocalNetInfo: ni, + Verbose: r.verbose, } - connInfo, err := client.AgentConnectionInfoGeneric(ctx) - if err == nil { - connDiags.ConnInfo = &connInfo - } else { - _, _ = fmt.Fprintf(inv.Stdout, "Failed to retrieve connection info from server: %v\n", err) + + awsRanges, err := cliutil.FetchAWSIPRanges(diagCtx, cliutil.AWSIPRangesURL) + if err != nil { + opts.Logger.Debug(inv.Context(), "failed to retrieve AWS IP ranges", slog.Error(err)) + } + + connDiags.ClientIPIsAWS = isAWSIP(awsRanges, ni) + + connInfo, err := client.AgentConnectionInfoGeneric(diagCtx) + if err != nil || connInfo.DERPMap == nil { + return xerrors.Errorf("Failed to retrieve connection info from server: %w\n", err) } + connDiags.ConnInfo = connInfo ifReport, err := healthsdk.RunInterfacesReport() if err == nil { connDiags.LocalInterfaces = &ifReport } else { _, _ = fmt.Fprintf(inv.Stdout, "Failed to retrieve local interfaces report: %v\n", err) } - agentNetcheck, err := conn.Netcheck(ctx) + + agentNetcheck, err := conn.Netcheck(diagCtx) if err == nil { connDiags.AgentNetcheck = &agentNetcheck + connDiags.AgentIPIsAWS = isAWSIP(awsRanges, agentNetcheck.NetInfo) } else { var sdkErr *codersdk.Error if errors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound { @@ -178,7 +192,8 @@ func (r *RootCmd) ping() *serpent.Command { _, _ = fmt.Fprintf(inv.Stdout, "Failed to retrieve connection report from agent: %v\n", err) } } - cliui.ConnDiagnostics(inv.Stdout, connDiags) + + connDiags.Write(inv.Stdout) return nil }, } @@ -207,3 +222,22 @@ func (r *RootCmd) ping() *serpent.Command { } return cmd } + +func isAWSIP(awsRanges *cliutil.AWSIPRanges, ni *tailcfg.NetInfo) bool { + if awsRanges == nil { + return false + } + if ni.GlobalV4 != "" { + ip, err := netip.ParseAddr(ni.GlobalV4) + if err == nil && awsRanges.CheckIP(ip) { + return true + } + } + if ni.GlobalV6 != "" { + ip, err := netip.ParseAddr(ni.GlobalV6) + if err == nil && awsRanges.CheckIP(ip) { + return true + } + } + return false +} diff --git a/cli/ping_test.go b/cli/ping_test.go index 7a4dcee5e15f3..f4ce8aefa7a00 100644 --- a/cli/ping_test.go +++ b/cli/ping_test.go @@ -67,6 +67,7 @@ func TestPing(t *testing.T) { pty.ExpectMatch("pong from " + workspace.Name) pty.ExpectMatch("✔ received remote agent data from Coder networking coordinator") + pty.ExpectMatch("✔ You are connected directly (p2p)") cancel() <-cmdDone }) diff --git a/codersdk/healthsdk/interfaces.go b/codersdk/healthsdk/interfaces.go index 6f4365aaeefac..714e1ecbdb411 100644 --- a/codersdk/healthsdk/interfaces.go +++ b/codersdk/healthsdk/interfaces.go @@ -72,7 +72,7 @@ func generateInterfacesReport(st *interfaces.State) (report InterfacesReport) { report.Severity = health.SeverityWarning report.Warnings = append(report.Warnings, health.Messagef(health.CodeInterfaceSmallMTU, - "network interface %s has MTU %d (less than %d), which may cause problems with direct connections", iface.Name, iface.MTU, safeMTU), + "Network interface %s has MTU %d (less than %d), which may degrade the quality of direct connections", iface.Name, iface.MTU, safeMTU), ) } }