Skip to content

Commit

Permalink
Escape labels on Node Join scripts (#52698)
Browse files Browse the repository at this point in the history
  • Loading branch information
@marcoandredinis
marcoandredinis authored Mar 3, 2025
1 parent 2f0a9e9 commit d646170
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 18 deletions.
153 changes: 137 additions & 16 deletions lib/web/join_tokens_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ func TestGetNodeJoinScript(t *testing.T) {
settings scriptSettings
errAssert require.ErrorAssertionFunc
token *types.ProvisionTokenV2
extraAssertions func(script string)
extraAssertions func(t *testing.T, script string)
}{
{
desc: "zero value",
Expand Down Expand Up @@ -740,7 +740,7 @@ func TestGetNodeJoinScript(t *testing.T) {
},
},
},
extraAssertions: func(script string) {
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, validToken)
require.Contains(t, script, hostname)
require.Contains(t, script, strconv.Itoa(port))
Expand Down Expand Up @@ -773,7 +773,7 @@ func TestGetNodeJoinScript(t *testing.T) {
},
},
errAssert: require.NoError,
extraAssertions: func(script string) {
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, "JOIN_METHOD='iam'")
},
},
Expand All @@ -791,7 +791,7 @@ func TestGetNodeJoinScript(t *testing.T) {
},
},
},
extraAssertions: func(script string) {
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, "--labels ")
require.Contains(t, script, fmt.Sprintf("%s=%s", types.InternalResourceIDLabel, internalResourceID))
},
Expand All @@ -810,13 +810,62 @@ func TestGetNodeJoinScript(t *testing.T) {
},
},
errAssert: require.NoError,
extraAssertions: func(script string) {
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, `APP_NAME='app-name'`)
require.Contains(t, script, `APP_URI='app-uri'`)
require.Contains(t, script, `public_addr`)
require.Contains(t, script, fmt.Sprintf(" labels:\n %s: %s", types.InternalResourceIDLabel, internalResourceID))
},
},
{
desc: "app server labels with shell injection attempt",
settings: scriptSettings{token: validToken, appInstallMode: true, appName: "app-name", appURI: "app-uri"},
token: &types.ProvisionTokenV2{
Metadata: types.Metadata{
Name: validToken,
},
Spec: types.ProvisionTokenSpecV2{
SuggestedLabels: types.Labels{
types.InternalResourceIDLabel: apiutils.Strings{internalResourceID},
"env": []string{"bad label value | ; & $ > < ' !"},
"bad label key | ; & $ > < ' !": []string{"env"},
},
},
},
errAssert: require.NoError,
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, `APP_NAME='app-name'`)
require.Contains(t, script, `APP_URI='app-uri'`)
require.Contains(t, script, `public_addr`)
require.Contains(t, script, `
labels:
bad label key | ; & $ > < ' !: env
env: bad\ label\ value\ \|\ \;\ \&\ \$\ \>\ \<\ \'\ \!
teleport.internal/resource-id: `+internalResourceID,
)
},
},
{
desc: "attempt to shell injection using suggested labels",
settings: scriptSettings{token: validToken},
token: &types.ProvisionTokenV2{
Metadata: types.Metadata{
Name: validToken,
},
Spec: types.ProvisionTokenSpecV2{
SuggestedLabels: types.Labels{
types.InternalResourceIDLabel: apiutils.Strings{internalResourceID},
"env": []string{"bad label value | ; & $ > < ' !"},
"bad label key | ; & $ > < ' !": []string{"env"},
},
},
},
errAssert: require.NoError,
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, `bad\ label\ key\ \|\ \;\ \&\ \$\ \>\ \<\ \'\ \!=env`)
require.Contains(t, script, `env=bad\ label\ value\ \|\ \;\ \&\ \$\ \>\ \<\ \'\ \!`)
},
},
} {
t.Run(test.desc, func(t *testing.T) {
h := newAutoupdateTestHandler(t, autoupdateTestHandlerConfig{
Expand All @@ -831,7 +880,7 @@ func TestGetNodeJoinScript(t *testing.T) {
}

if test.extraAssertions != nil {
test.extraAssertions(script)
test.extraAssertions(t, script)
}
})
}
Expand Down Expand Up @@ -1133,7 +1182,7 @@ func TestGetDatabaseJoinScript(t *testing.T) {
settings scriptSettings
token *types.ProvisionTokenV2
errAssert require.ErrorAssertionFunc
extraAssertions func(script string)
extraAssertions func(t *testing.T, script string)
}{
{
desc: "two installation methods",
Expand All @@ -1153,23 +1202,98 @@ func TestGetDatabaseJoinScript(t *testing.T) {
token: validToken,
},
errAssert: require.NoError,
extraAssertions: func(script string) {
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, validToken)
require.Contains(t, script, hostname)
require.Contains(t, script, strconv.Itoa(port))
require.Contains(t, script, "sha256:")
require.Contains(t, script, "--labels ")
require.Contains(t, script, fmt.Sprintf("%s=%s", types.InternalResourceIDLabel, internalResourceID))
require.Contains(t, script, `
db_service:
enabled: "yes"
resources:
- labels:
env: prod
os:
- mac
- linux
product: '*'
`)
},
},
{
desc: "discover flow with wildcard label matcher",
token: &types.ProvisionTokenV2{
Metadata: types.Metadata{
Name: validToken,
},
Spec: types.ProvisionTokenSpecV2{
SuggestedLabels: types.Labels{
types.InternalResourceIDLabel: apiutils.Strings{internalResourceID},
},
SuggestedAgentMatcherLabels: types.Labels{
"*": apiutils.Strings{"*"},
},
},
},
settings: scriptSettings{
databaseInstallMode: true,
token: validToken,
},
errAssert: require.NoError,
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, validToken)
require.Contains(t, script, hostname)
require.Contains(t, script, "sha256:")
require.Contains(t, script, "--labels ")
require.Contains(t, script, fmt.Sprintf("%s=%s", types.InternalResourceIDLabel, internalResourceID))
require.Contains(t, script, `
- labels:
'*': '*'
`)
},
},
{
desc: "discover flow with shell injection attempt in resource matcher labels",
token: &types.ProvisionTokenV2{
Metadata: types.Metadata{
Name: validToken,
},
Spec: types.ProvisionTokenSpecV2{
SuggestedLabels: types.Labels{
types.InternalResourceIDLabel: apiutils.Strings{internalResourceID},
},
SuggestedAgentMatcherLabels: types.Labels{
"*": apiutils.Strings{"*"},
"spa ces": apiutils.Strings{"spa ces"},
"EOF": apiutils.Strings{"test heredoc"},
`"EOF"`: apiutils.Strings{"test quoted heredoc"},
"#'; <>\\#": apiutils.Strings{"try to escape yaml"},
"&<>'\"$A,./;'BCD ${ABCD}": apiutils.Strings{"key with special characters"},
"value with special characters": apiutils.Strings{"&<>'\"$A,./;'BCD ${ABCD}", "#&<>'\"$A,./;'BCD ${ABCD}"},
},
},
},
settings: scriptSettings{
databaseInstallMode: true,
token: validToken,
},
errAssert: require.NoError,
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, validToken)
require.Contains(t, script, hostname)
require.Contains(t, script, "sha256:")
require.Contains(t, script, "--labels ")
require.Contains(t, script, fmt.Sprintf("%s=%s", types.InternalResourceIDLabel, internalResourceID))
require.Contains(t, script, `
- labels:
'"EOF"': test quoted heredoc
'#''; <>\#': try to escape yaml
'&<>''"$A,./;''BCD ${ABCD}': key with special characters
'*': '*'
EOF: test heredoc
spa ces: spa ces
value with special characters:
- '&<>''"$A,./;''BCD ${ABCD}'
- '#&<>''"$A,./;''BCD ${ABCD}'
`)
},
},
Expand All @@ -1181,17 +1305,14 @@ db_service:
token: emptySuggestedAgentMatcherLabelsToken,
},
errAssert: require.NoError,
extraAssertions: func(script string) {
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, emptySuggestedAgentMatcherLabelsToken)
require.Contains(t, script, hostname)
require.Contains(t, script, strconv.Itoa(port))
require.Contains(t, script, "sha256:")
require.Contains(t, script, "--labels ")
require.Contains(t, script, fmt.Sprintf("%s=%s", types.InternalResourceIDLabel, internalResourceID))
require.Contains(t, script, `
db_service:
enabled: "yes"
resources:
- labels:
{}
`)
Expand All @@ -1212,7 +1333,7 @@ db_service:
}

if test.extraAssertions != nil {
test.extraAssertions(script)
test.extraAssertions(t, script)
}
})
}
Expand Down
6 changes: 5 additions & 1 deletion lib/web/scripts/install_node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ func GetNodeInstallScript(ctx context.Context, opts InstallNodeScriptOptions) (s
// Computing service configuration-related values
labelsList := []string{}
for labelKey, labelValues := range opts.Labels {
labelKey = shsprintf.EscapeDefaultContext(labelKey)
for i := range labelValues {
labelValues[i] = shsprintf.EscapeDefaultContext(labelValues[i])
}
labels := strings.Join(labelValues, " ")
labelsList = append(labelsList, fmt.Sprintf("%s=%s", labelKey, labels))
}
Expand Down Expand Up @@ -155,7 +159,7 @@ func GetNodeInstallScript(ctx context.Context, opts InstallNodeScriptOptions) (s
// This section relies on Go's default zero values to make sure that the settings
// are correct when not installing an app.
err = installNodeBashScript.Execute(&buf, map[string]interface{}{
"token": opts.Token,
"token": shsprintf.EscapeDefaultContext(opts.Token),
"hostname": hostname,
"port": portStr,
// The install.sh script has some manually generated configs and some
Expand Down
10 changes: 9 additions & 1 deletion lib/web/scripts/node-join/install.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ JOIN_METHOD_FLAG=""
[ -n "$JOIN_METHOD" ] && JOIN_METHOD_FLAG="--join-method ${JOIN_METHOD}"

# inject labels into the configuration
LABELS='{{.labels}}'
LABELS="{{.labels}}"
LABELS_FLAG=()
[ -n "$LABELS" ] && LABELS_FLAG=(--labels "${LABELS}")

Expand Down Expand Up @@ -478,6 +478,10 @@ app_service:
- name: "${APP_NAME}"
uri: "${APP_URI}"
public_addr: ${APP_PUBLIC_ADDR}
EOF

# Quoting the EOF heredoc indicates to shell to treat this as a literal string and does not try to interpolate or execute anything.
cat << "EOF" >> ${TELEPORT_CONFIG_PATH}
labels:{{range $index, $line := .appServerResourceLabels}}
{{$line -}}
{{end}}
Expand Down Expand Up @@ -512,6 +516,10 @@ proxy_service:
db_service:
enabled: "yes"
resources:
EOF

# Quoting the EOF heredoc indicates to shell to treat this as a literal string and does not try to interpolate or execute anything.
cat << "EOF" >> ${TELEPORT_CONFIG_PATH}
- labels:{{range $index, $line := .db_service_resource_labels}}
{{$line -}}
{{end}}
Expand Down

0 comments on commit d646170

Please sign in to comment.