diff --git a/go.mod b/go.mod index 7e3989593753f..906990b0724f2 100644 --- a/go.mod +++ b/go.mod @@ -102,6 +102,7 @@ require ( github.com/google/go-querystring v1.1.0 github.com/google/go-tpm-tools v0.4.2 github.com/google/renameio/v2 v2.0.0 + github.com/google/safetext v0.0.0-20240104143208-7a7d9b3d812f github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/google/uuid v1.5.0 github.com/googleapis/gax-go/v2 v2.12.0 @@ -516,6 +517,7 @@ require ( k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect k8s.io/metrics v0.29.0 // indirect k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect + mvdan.cc/sh/v3 v3.7.0 // indirect oras.land/oras-go v1.2.5 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/kustomize/api v0.13.5-0.20230601165947-6ce0bf390ce3 // indirect diff --git a/go.sum b/go.sum index 05785fede1d0d..c0dc9047b3bcd 100644 --- a/go.sum +++ b/go.sum @@ -510,8 +510,8 @@ github.com/fortytw2/leaktest v1.3.0 h1:u8491cBMTQ8ft8aeV+adlcytMZylmA5nnwwkRZjI8 github.com/fortytw2/leaktest v1.3.0/go.mod h1:jDsjWgpAGjm2CA7WthBh/CdZYEPF31XHquHwclZch5g= github.com/foxcpp/go-mockdns v1.0.0 h1:7jBqxd3WDWwi/6WhDvacvH1XsN3rOLXyHM1uhvIx6FI= github.com/foxcpp/go-mockdns v1.0.0/go.mod h1:lgRN6+KxQBawyIghpnl5CezHFGS9VLzvtVlwxvzXTQ4= -github.com/frankban/quicktest v1.14.4 h1:g2rn0vABPOOXmZUj+vbmUp0lPoXEMuhTpIluN0XL9UY= -github.com/frankban/quicktest v1.14.4/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= +github.com/frankban/quicktest v1.14.5 h1:dfYrrRyLtiqT9GyKXgdh+k4inNeTvmGbuSgZ3lx3GhA= +github.com/frankban/quicktest v1.14.5/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/fsnotify/fsnotify v1.5.4/go.mod h1:OVB6XrOHzAwXMpEM7uPOzcehqUV2UqJxmVXmkdnm1bU= @@ -775,6 +775,8 @@ github.com/google/renameio/v2 v2.0.0 h1:UifI23ZTGY8Tt29JbYFiuyIU3eX+RNFtUwefq9qA github.com/google/renameio/v2 v2.0.0/go.mod h1:BtmJXm5YlszgC+TD4HOEEUFgkJP3nLxehU6hfe7jRt4= github.com/google/s2a-go v0.1.7 h1:60BLSyTrOV4/haCDW4zb1guZItoSq8foHCXrAnjBo/o= github.com/google/s2a-go v0.1.7/go.mod h1:50CgR4k1jNlWBu4UfS4AcfhVe1r6pdZPygJ3R8F0Qdw= +github.com/google/safetext v0.0.0-20240104143208-7a7d9b3d812f h1:o2yGZLlsOj5H5uvtQNEdi6DeA0GbUP3lm0gWW5RvY0s= +github.com/google/safetext v0.0.0-20240104143208-7a7d9b3d812f/go.mod h1:H3K1Iu/utuCfa10JO+GsmKUYSWi7ug57Rk6GaDRHaaQ= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/tink/go v1.7.0 h1:6Eox8zONGebBFcCBqkVmt60LaWZa6xg1cl/DwAh/J1w= @@ -2206,6 +2208,8 @@ k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSn k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= launchpad.net/gocheck v0.0.0-20140225173054-000000000087 h1:Izowp2XBH6Ya6rv+hqbceQyw/gSGoXfH/UPoTGduL54= launchpad.net/gocheck v0.0.0-20140225173054-000000000087/go.mod h1:hj7XX3B/0A+80Vse0e+BUHsHMTEhd0O4cpUHr/e/BUM= +mvdan.cc/sh/v3 v3.7.0 h1:lSTjdP/1xsddtaKfGg7Myu7DnlHItd3/M2tomOcNNBg= +mvdan.cc/sh/v3 v3.7.0/go.mod h1:K2gwkaesF/D7av7Kxl0HbF5kGOd2ArupNTX3X44+8l8= oras.land/oras-go v1.2.5 h1:XpYuAwAb0DfQsunIyMfeET92emK8km3W4yEzZvUbsTo= oras.land/oras-go v1.2.5/go.mod h1:PuAwRShRZCsZb7g8Ar3jKKQR/2A/qN+pkYxIOd/FAoo= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/lib/utils/cli.go b/lib/utils/cli.go index 7e9e772fbfb70..49930aff2aff5 100644 --- a/lib/utils/cli.go +++ b/lib/utils/cli.go @@ -29,7 +29,6 @@ import ( stdlog "log" "log/slog" "os" - "regexp" "runtime" "strconv" "strings" @@ -420,24 +419,6 @@ func SplitIdentifiers(s string) []string { }) } -var unixShellQuoteCharacters = regexp.MustCompile( - "[^" + // Match any character that is NOT one of the following: - "\\w" + // Word characters (letter, number, underscore) - "@%+=:,./-" + // Safe symbols that don't typically have a special meaning in shells - "]") - -// UnixShellQuote returns the string in quotes if quoting is necessary to prevent possible execution or injection for -// UNIX-like systems. This is intended to be used when building shell scripts for Linux or macOS. -func UnixShellQuote(s string) string { - if unixShellQuoteCharacters.MatchString(s) { - s = strings.ReplaceAll(s, "\n", "\\n") - s = strings.ReplaceAll(s, "\r", "\\r") - return "'" + strings.ReplaceAll(s, "'", "'\"'\"'") + "'" - } - - return s -} - // EscapeControl escapes all ANSI escape sequences from string and returns a // string that is safe to print on the CLI. This is to ensure that malicious // servers can not hide output. For more details, see: diff --git a/lib/utils/cli_test.go b/lib/utils/cli_test.go index 0a0fe236988eb..dcfccdeaab9cf 100644 --- a/lib/utils/cli_test.go +++ b/lib/utils/cli_test.go @@ -68,143 +68,6 @@ func TestUserMessageFromError(t *testing.T) { } } -func TestUnixShellQuote(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - in string - out string - }{ - { - name: "emptyString", - in: "", - out: "", - }, - { - name: "noQuote", - in: "foo", - out: "foo", - }, - { - name: "bang", - in: "foo!", - out: "'foo!'", - }, - { - name: "variable", - in: "foo$BAR", - out: "'foo$BAR'", - }, - { - name: "semicolon", - in: "foo;bar", - out: "'foo;bar'", - }, - { - name: "singleQuoteStart", - in: "'foo", - out: "''\"'\"'foo'", - }, - { - name: "singleQuoteMid", - in: "foo'bar", - out: "'foo'\"'\"'bar'", - }, - { - name: "singleQuoteEnd", - in: "foo'", - out: "'foo'\"'\"''", - }, - { - name: "singleQuotesSurrounding", - in: "'foo'", - out: "''\"'\"'foo'\"'\"''", - }, - { - name: "space", - in: "foo bar", - out: "'foo bar'", - }, - { - name: "path", - in: "/usr/local/bin", - out: "/usr/local/bin", - }, - { - name: "commandSubstitution", - in: "$(ls -la)", - out: "'$(ls -la)'", - }, - { - name: "backticks", - in: "`echo foo`", - out: "'`echo foo`'", - }, - { - name: "doubleQuotes", - in: "foo\"bar", - out: "'foo\"bar'", - }, - { - name: "brackets", - in: "[1,2,3]", - out: "'[1,2,3]'", - }, - { - name: "parentheses", - in: "(1+2)", - out: "'(1+2)'", - }, - { - name: "braceExpansion", - in: "{a,b}", - out: "'{a,b}'", - }, - { - name: "escapeCharacters", - in: "foo\\bar", - out: "'foo\\bar'", - }, - { - name: "wildcards", - in: "*", - out: "'*'", - }, - { - name: "pipe", - in: "foo | bar", - out: "'foo | bar'", - }, - { - name: "andOperator", - in: "foo && bar", - out: "'foo && bar'", - }, - { - name: "newline", - in: "foo\nbar", - out: "'foo\\nbar'", - }, - { - name: "carriageReturn", - in: "foo\rbar", - out: "'foo\\rbar'", - }, - { - name: "tab", - in: "foo\tbar", - out: "'foo\tbar'", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.out, UnixShellQuote(tt.in)) - }) - } -} - // TestEscapeControl tests escape control func TestEscapeControl(t *testing.T) { t.Parallel() diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index cbb77a5fd0137..5fef3df54eb2b 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -39,6 +39,7 @@ import ( "sync/atomic" "time" + "github.com/google/safetext/shsprintf" "github.com/google/uuid" "github.com/gorilla/websocket" "github.com/gravitational/oxy/ratelimit" @@ -1936,11 +1937,11 @@ func (h *Handler) installer(w http.ResponseWriter, r *http.Request, p httprouter tmpl := installers.Template{ PublicProxyAddr: h.PublicProxyAddr(), - MajorVersion: utils.UnixShellQuote(version), + MajorVersion: shsprintf.EscapeDefaultContext(version), TeleportPackage: teleportPackage, - RepoChannel: utils.UnixShellQuote(repoChannel), + RepoChannel: shsprintf.EscapeDefaultContext(repoChannel), AutomaticUpgrades: strconv.FormatBool(installUpdater), - AzureClientID: utils.UnixShellQuote(azureClientID), + AzureClientID: shsprintf.EscapeDefaultContext(azureClientID), } err = instTmpl.Execute(w, tmpl) return nil, trace.Wrap(err) diff --git a/lib/web/integrations_awsoidc.go b/lib/web/integrations_awsoidc.go index 4ebcbdd78bf28..ac4579ccb472a 100644 --- a/lib/web/integrations_awsoidc.go +++ b/lib/web/integrations_awsoidc.go @@ -28,6 +28,7 @@ import ( "slices" "strings" + "github.com/google/safetext/shsprintf" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" @@ -45,7 +46,6 @@ import ( "github.com/gravitational/teleport/lib/integrations/awsoidc/deployserviceconfig" "github.com/gravitational/teleport/lib/reversetunnelclient" "github.com/gravitational/teleport/lib/services" - libutils "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/web/scripts/oneoff" "github.com/gravitational/teleport/lib/web/ui" ) @@ -267,11 +267,11 @@ func (h *Handler) awsOIDCConfigureDeployServiceIAM(w http.ResponseWriter, r *htt // teleport integration configure deployservice-iam argsList := []string{ "integration", "configure", "deployservice-iam", - fmt.Sprintf("--cluster=%s", libutils.UnixShellQuote(clusterName)), - fmt.Sprintf("--name=%s", libutils.UnixShellQuote(integrationName)), - fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), - fmt.Sprintf("--task-role=%s", libutils.UnixShellQuote(taskRole)), + fmt.Sprintf("--cluster=%s", shsprintf.EscapeDefaultContext(clusterName)), + fmt.Sprintf("--name=%s", shsprintf.EscapeDefaultContext(integrationName)), + fmt.Sprintf("--aws-region=%s", shsprintf.EscapeDefaultContext(awsRegion)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), + fmt.Sprintf("--task-role=%s", shsprintf.EscapeDefaultContext(taskRole)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), @@ -306,8 +306,8 @@ func (h *Handler) awsOIDCConfigureEICEIAM(w http.ResponseWriter, r *http.Request // teleport integration configure eice-iam argsList := []string{ "integration", "configure", "eice-iam", - fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), + fmt.Sprintf("--aws-region=%s", shsprintf.EscapeDefaultContext(awsRegion)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), @@ -341,8 +341,8 @@ func (h *Handler) awsOIDCConfigureEKSIAM(w http.ResponseWriter, r *http.Request, // "teleport integration configure eks-iam" argsList := []string{ "integration", "configure", "eks-iam", - fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), + fmt.Sprintf("--aws-region=%s", shsprintf.EscapeDefaultContext(awsRegion)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), @@ -886,10 +886,10 @@ func (h *Handler) awsOIDCConfigureIdP(w http.ResponseWriter, r *http.Request, p // teleport integration configure awsoidc-idp argsList := []string{ "integration", "configure", "awsoidc-idp", - fmt.Sprintf("--cluster=%s", libutils.UnixShellQuote(clusterName)), - fmt.Sprintf("--name=%s", libutils.UnixShellQuote(integrationName)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), - fmt.Sprintf("--s3-bucket-uri=%s", libutils.UnixShellQuote(s3URI.String())), + fmt.Sprintf("--cluster=%s", shsprintf.EscapeDefaultContext(clusterName)), + fmt.Sprintf("--name=%s", shsprintf.EscapeDefaultContext(integrationName)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), + fmt.Sprintf("--s3-bucket-uri=%s", shsprintf.EscapeDefaultContext(s3URI.String())), fmt.Sprintf("--s3-jwks-base64=%s", base64.StdEncoding.EncodeToString(jwksJSON)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ @@ -924,8 +924,8 @@ func (h *Handler) awsOIDCConfigureListDatabasesIAM(w http.ResponseWriter, r *htt // teleport integration configure listdatabases-iam argsList := []string{ "integration", "configure", "listdatabases-iam", - fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), + fmt.Sprintf("--aws-region=%s", shsprintf.EscapeDefaultContext(awsRegion)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), @@ -965,7 +965,7 @@ func (h *Handler) awsAccessGraphOIDCSync(w http.ResponseWriter, r *http.Request, // "teleport integration configure access-graph aws-iam" argsList := []string{ "integration", "configure", "access-graph", "aws-iam", - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), diff --git a/lib/web/integrations_awsoidc_test.go b/lib/web/integrations_awsoidc_test.go index 71ca08b39e200..539eec69e37ca 100644 --- a/lib/web/integrations_awsoidc_test.go +++ b/lib/web/integrations_awsoidc_test.go @@ -92,7 +92,7 @@ func TestBuildDeployServiceConfigureIAMScript(t *testing.T) { `--cluster=localhost ` + `--name=myintegration ` + `--aws-region=us-east-1 ` + - `--role=Test+1=2,3.4@5-6_7 ` + + `--role=Test\+1=2,3.4\@5-6_7 ` + `--task-role=taskRole`, }, { @@ -206,7 +206,7 @@ func TestBuildEICEConfigureIAMScript(t *testing.T) { errCheck: require.NoError, expectedTeleportArgs: "integration configure eice-iam " + "--aws-region=us-east-1 " + - "--role=Test+1=2,3.4@5-6_7", + "--role=Test\\+1=2,3.4\\@5-6_7", }, { name: "missing aws-region", @@ -295,7 +295,7 @@ func TestBuildEKSConfigureIAMScript(t *testing.T) { errCheck: require.NoError, expectedTeleportArgs: "integration configure eks-iam " + "--aws-region=us-east-1 " + - "--role=Test+1=2,3.4@5-6_7", + "--role=Test\\+1=2,3.4\\@5-6_7", }, { name: "missing aws-region", @@ -398,7 +398,7 @@ func TestBuildAWSOIDCIdPConfigureScript(t *testing.T) { expectedTeleportArgs: "integration configure awsoidc-idp " + "--cluster=localhost " + "--name=myintegration " + - "--role=Test+1=2,3.4@5-6_7 " + + "--role=Test\\+1=2,3.4\\@5-6_7 " + `--s3-bucket-uri=s3://my-bucket/prefix ` + "--s3-jwks-base64=" + jwksBase64, }, @@ -514,7 +514,7 @@ func TestBuildListDatabasesConfigureIAMScript(t *testing.T) { errCheck: require.NoError, expectedTeleportArgs: "integration configure listdatabases-iam " + "--aws-region=us-east-1 " + - "--role=Test+1=2,3.4@5-6_7", + "--role=Test\\+1=2,3.4\\@5-6_7", }, { name: "missing aws-region", diff --git a/lib/web/integrations_samlidp.go b/lib/web/integrations_samlidp.go index 2b086b82fe5da..8359d81f2b263 100644 --- a/lib/web/integrations_samlidp.go +++ b/lib/web/integrations_samlidp.go @@ -23,12 +23,12 @@ import ( "net/http" "strings" + "github.com/google/safetext/shsprintf" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" "github.com/gravitational/teleport/lib/httplib" "github.com/gravitational/teleport/lib/integrations/samlidp/samlidpconfig" - libutils "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/web/scripts/oneoff" ) @@ -50,10 +50,10 @@ func (h *Handler) gcpWorkforceConfigScript(w http.ResponseWriter, r *http.Reques // teleport integration configure samlidp gcp-workforce argsList := []string{ "integration", "configure", "samlidp", "gcp-workforce", - fmt.Sprintf("--org-id=%s", libutils.UnixShellQuote(queryParams.Get("orgId"))), - fmt.Sprintf("--pool-name=%s", libutils.UnixShellQuote(queryParams.Get("poolName"))), - fmt.Sprintf("--pool-provider-name=%s", libutils.UnixShellQuote(queryParams.Get("poolProviderName"))), - fmt.Sprintf("--idp-metadata-url=%s", libutils.UnixShellQuote(samlIdPMetadataURL)), + fmt.Sprintf("--org-id=%s", shsprintf.EscapeDefaultContext(queryParams.Get("orgId"))), + fmt.Sprintf("--pool-name=%s", shsprintf.EscapeDefaultContext(queryParams.Get("poolName"))), + fmt.Sprintf("--pool-provider-name=%s", shsprintf.EscapeDefaultContext(queryParams.Get("poolProviderName"))), + fmt.Sprintf("--idp-metadata-url=%s", shsprintf.EscapeDefaultContext(samlIdPMetadataURL)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), diff --git a/lib/web/join_tokens.go b/lib/web/join_tokens.go index 331b7cd1a709f..9b5bfd459aaf3 100644 --- a/lib/web/join_tokens.go +++ b/lib/web/join_tokens.go @@ -33,6 +33,7 @@ import ( "strings" "time" + "github.com/google/safetext/shsprintf" "github.com/google/uuid" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" @@ -510,16 +511,16 @@ func getJoinScript(ctx context.Context, settings scriptSettings, m nodeAPIGetter "packageName": packageName, "repoChannel": repoChannel, "installUpdater": strconv.FormatBool(settings.installUpdater), - "version": utils.UnixShellQuote(version), + "version": shsprintf.EscapeDefaultContext(version), "appInstallMode": strconv.FormatBool(settings.appInstallMode), - "appName": utils.UnixShellQuote(settings.appName), - "appURI": utils.UnixShellQuote(settings.appURI), - "joinMethod": utils.UnixShellQuote(settings.joinMethod), + "appName": shsprintf.EscapeDefaultContext(settings.appName), + "appURI": shsprintf.EscapeDefaultContext(settings.appURI), + "joinMethod": shsprintf.EscapeDefaultContext(settings.joinMethod), "labels": strings.Join(labelsList, ","), "databaseInstallMode": strconv.FormatBool(settings.databaseInstallMode), "db_service_resource_labels": dbServiceResourceLabels, "discoveryInstallMode": settings.discoveryInstallMode, - "discoveryGroup": utils.UnixShellQuote(settings.discoveryGroup), + "discoveryGroup": shsprintf.EscapeDefaultContext(settings.discoveryGroup), }) if err != nil { return "", trace.Wrap(err)