From 3cba8738fcdb6da10602af4f0be072b3d22588bd Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Wed, 18 Dec 2024 16:18:56 -0500 Subject: [PATCH 1/4] Fix an issue "tsh aws ssm start-session" fails when KMS encryption is enabled --- lib/srv/alpnproxy/forward_proxy.go | 27 ++++++++++++++++- lib/srv/alpnproxy/forward_proxy_test.go | 40 +++++++++++++++++++++++++ tool/tsh/common/app_aws.go | 31 ------------------- 3 files changed, 66 insertions(+), 32 deletions(-) diff --git a/lib/srv/alpnproxy/forward_proxy.go b/lib/srv/alpnproxy/forward_proxy.go index 1ea1811ff004d..7584218700b1a 100644 --- a/lib/srv/alpnproxy/forward_proxy.go +++ b/lib/srv/alpnproxy/forward_proxy.go @@ -24,10 +24,12 @@ import ( "log/slog" "net" "net/http" + "net/http/httputil" "net/url" "strings" "github.com/gravitational/trace" + "github.com/sirupsen/logrus" "golang.org/x/net/http/httpproxy" apidefaults "github.com/gravitational/teleport/api/defaults" @@ -175,7 +177,30 @@ func MatchAllRequests(req *http.Request) bool { // MatchAWSRequests is a MatchFunc that returns true if request is an AWS API // request. func MatchAWSRequests(req *http.Request) bool { - return awsapiutils.IsAWSEndpoint(req.Host) + if dump, err := httputil.DumpRequest(req, true); err == nil { + logrus.Debugf("=== dump req %s", string(dump)) + } + return awsapiutils.IsAWSEndpoint(req.Host) && + // Avoid proxying SSM session WebSocket requests and let the forward proxy + // send it directly to AWS. + // + // `aws ssm start-session` first calls ssm..amazonaws.com to get + // a stream URL and a token. Then it makes a wss connection with the + // provided token to the provided stream URL. The stream URL looks like: + // wss://ssmmessages.region.amazonaws.com/v1/data-channel/session-id?stream=(input|output) + // + // The wss request currently respects HTTPS_PROXY but does not + // respect local CA bundle we provided thus causing a failure. The + // request is not signed with SigV4 either. + // + // Reference: + // https://github.com/aws/session-manager-plugin/ + !isAWSSSMWebsocketRequest(req) +} + +func isAWSSSMWebsocketRequest(req *http.Request) bool { + return awsapiutils.IsAWSEndpoint(req.Host) && + strings.HasPrefix(req.Host, "ssmmessages.") } // MatchAzureRequests is a MatchFunc that returns true if request is an Azure API diff --git a/lib/srv/alpnproxy/forward_proxy_test.go b/lib/srv/alpnproxy/forward_proxy_test.go index 118d0c3f825f6..c5970e08d9085 100644 --- a/lib/srv/alpnproxy/forward_proxy_test.go +++ b/lib/srv/alpnproxy/forward_proxy_test.go @@ -247,3 +247,43 @@ func TestMatchGCPRequests(t *testing.T) { }) } } + +func TestMatchAWSRequests(t *testing.T) { + makeRequest := func(url string) *http.Request { + // Forward proxy always receives CONNECT requests. + request, err := http.NewRequest("CONNECT", url, nil) + require.NoError(t, err) + return request + } + tests := []struct { + name string + req *http.Request + check require.BoolAssertionFunc + }{ + { + name: "AWS request", + req: makeRequest("http://s3.ca-central-1.amazonaws.com"), + check: require.True, + }, + { + name: "non-AWS request", + req: makeRequest("https://registry.terraform.io"), + check: require.False, + }, + { + name: "SSM API", + req: makeRequest("https://ssm.ca-central-1.amazonaws.com"), + check: require.True, + }, + { + name: "SSM session WebSocket", + req: makeRequest("wss://ssmmessages.ca-central-1.amazonaws.com"), + check: require.False, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.check(t, MatchAWSRequests(tt.req)) + }) + } +} diff --git a/tool/tsh/common/app_aws.go b/tool/tsh/common/app_aws.go index aaa6adeec6b01..dee17ab5f388f 100644 --- a/tool/tsh/common/app_aws.go +++ b/tool/tsh/common/app_aws.go @@ -52,11 +52,6 @@ func onAWS(cf *CLIConf) error { return trace.Wrap(err) } - if shouldUseAWSEndpointURLMode(cf) { - log.Debugf("Forcing endpoint URL mode for AWS command %q.", cf.AWSCommandArgs) - cf.AWSEndpointURLMode = true - } - err = awsApp.StartLocalProxies(cf.Context) if err != nil { return trace.Wrap(err) @@ -82,32 +77,6 @@ func onAWS(cf *CLIConf) error { return awsApp.RunCommand(cmd) } -func shouldUseAWSEndpointURLMode(cf *CLIConf) bool { - inputAWSCommand := strings.Join(removeAWSCommandFlags(cf.AWSCommandArgs), " ") - switch inputAWSCommand { - // `aws ssm start-session` first calls ssm..amazonaws.com to get an - // stream URL and an token. Then it makes a wss connection with the - // provided token to the provided stream URL. The wss request currently - // respects HTTPS_PROXY but does not respect local CA bundle we provided - // thus causing a failure. Even if this is resolved one day, the wss send - // the token through websocket data channel for authentication, instead of - // sigv4, which likely we won't support. - // - // When using the endpoint URL mode, only the first request goes through - // Teleport Proxy. The wss connection does not respect the endpoint URL and - // goes to AWS directly (thus working fine). - // - // Reference: - // https://github.com/aws/session-manager-plugin/ - // - // "aws ecs execute-command" also start SSM sessions. - case "ssm start-session", "ecs execute-command": - return true - default: - return false - } -} - func removeAWSCommandFlags(args []string) (ret []string) { for i := 0; i < len(args); i++ { switch { From 57633ce6c0f7ca89a5ca55d49e622bd849bd4871 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Wed, 18 Dec 2024 17:05:02 -0500 Subject: [PATCH 2/4] remove httputil dump --- lib/srv/alpnproxy/forward_proxy.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/srv/alpnproxy/forward_proxy.go b/lib/srv/alpnproxy/forward_proxy.go index 7584218700b1a..f1fec62a8fab1 100644 --- a/lib/srv/alpnproxy/forward_proxy.go +++ b/lib/srv/alpnproxy/forward_proxy.go @@ -24,12 +24,10 @@ import ( "log/slog" "net" "net/http" - "net/http/httputil" "net/url" "strings" "github.com/gravitational/trace" - "github.com/sirupsen/logrus" "golang.org/x/net/http/httpproxy" apidefaults "github.com/gravitational/teleport/api/defaults" @@ -177,9 +175,6 @@ func MatchAllRequests(req *http.Request) bool { // MatchAWSRequests is a MatchFunc that returns true if request is an AWS API // request. func MatchAWSRequests(req *http.Request) bool { - if dump, err := httputil.DumpRequest(req, true); err == nil { - logrus.Debugf("=== dump req %s", string(dump)) - } return awsapiutils.IsAWSEndpoint(req.Host) && // Avoid proxying SSM session WebSocket requests and let the forward proxy // send it directly to AWS. From 71a9181553265fbd1fa4fc5069fd632a41ba8302 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Thu, 19 Dec 2024 09:37:47 -0500 Subject: [PATCH 3/4] fix ut --- tool/tsh/common/app_aws_test.go | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/tool/tsh/common/app_aws_test.go b/tool/tsh/common/app_aws_test.go index 15297426e7d39..98782f73dee15 100644 --- a/tool/tsh/common/app_aws_test.go +++ b/tool/tsh/common/app_aws_test.go @@ -149,39 +149,6 @@ func TestAWS(t *testing.T) { setCmdRunner(validateCmd), ) require.NoError(t, err) - - t.Run("aws ssm start-session", func(t *testing.T) { - // Validate --endpoint-url 127.0.0.1: is added to the command. - validateCmd := func(cmd *exec.Cmd) error { - require.Len(t, cmd.Args, 9) - require.Equal(t, []string{"aws", "ssm", "--region", "us-west-1", "start-session", "--target", "target-id", "--endpoint-url"}, cmd.Args[:8]) - require.Contains(t, cmd.Args[8], "127.0.0.1:") - return nil - } - err = Run( - context.Background(), - []string{"aws", "ssm", "--region", "us-west-1", "start-session", "--target", "target-id"}, - setHomePath(tmpHomePath), - setCmdRunner(validateCmd), - ) - require.NoError(t, err) - }) - t.Run("aws ecs execute-command", func(t *testing.T) { - // Validate --endpoint-url 127.0.0.1: is added to the command. - validateCmd := func(cmd *exec.Cmd) error { - require.Len(t, cmd.Args, 13) - require.Equal(t, []string{"aws", "ecs", "execute-command", "--debug", "--cluster", "cluster-name", "--task", "task-name", "--command", "/bin/bash", "--interactive", "--endpoint-url"}, cmd.Args[:12]) - require.Contains(t, cmd.Args[12], "127.0.0.1:") - return nil - } - err = Run( - context.Background(), - []string{"aws", "ecs", "execute-command", "--debug", "--cluster", "cluster-name", "--task", "task-name", "--command", "/bin/bash", "--interactive"}, - setHomePath(tmpHomePath), - setCmdRunner(validateCmd), - ) - require.NoError(t, err) - }) } // TestAWSConsoleLogins given a AWS console application, execute a app login From b0fd7c25ced30de91e3f3d6edfc330376c2b9f10 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Mon, 6 Jan 2025 14:09:37 -0500 Subject: [PATCH 4/4] remove unused funcs --- tool/tsh/common/app_aws.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/tool/tsh/common/app_aws.go b/tool/tsh/common/app_aws.go index 8cc3d501c2c2e..c09f1a69039db 100644 --- a/tool/tsh/common/app_aws.go +++ b/tool/tsh/common/app_aws.go @@ -24,7 +24,6 @@ import ( "io" "os" "os/exec" - "strings" "sync" "github.com/aws/aws-sdk-go-v2/aws" @@ -77,29 +76,6 @@ func onAWS(cf *CLIConf) error { return awsApp.RunCommand(cmd) } -func removeAWSCommandFlags(args []string) (ret []string) { - for i := 0; i < len(args); i++ { - switch { - case isAWSFlag(args, i): - // Skip next arg, if next arg is not a flag but a flag value. - if !isAWSFlag(args, i+1) { - i++ - } - continue - default: - ret = append(ret, args[i]) - } - } - return -} - -func isAWSFlag(args []string, i int) bool { - if i >= len(args) { - return false - } - return strings.HasPrefix(args[i], "--") -} - // awsApp is an AWS app that can start local proxies to serve AWS APIs. type awsApp struct { *localProxyApp