Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v15] Escape labels on Node Join scripts #52706

Merged
merged 1 commit into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/web/join_tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ func getJoinScript(ctx context.Context, settings scriptSettings, m nodeAPIGetter

labelsList := []string{}
for labelKey, labelValues := range token.GetSuggestedLabels() {
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 @@ -498,7 +502,7 @@ func getJoinScript(ctx context.Context, settings scriptSettings, m nodeAPIGetter
// This section relies on Go's default zero values to make sure that the settings
// are correct when not installing an app.
err = scripts.InstallNodeBashScript.Execute(&buf, map[string]interface{}{
"token": settings.token,
"token": shsprintf.EscapeDefaultContext(settings.token),
"hostname": hostname,
"port": portStr,
// The install.sh script has some manually generated configs and some
Expand Down
126 changes: 100 additions & 26 deletions lib/web/join_tokens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ func toHex(s string) string { return hex.EncodeToString([]byte(s)) }

func TestGetNodeJoinScript(t *testing.T) {
validToken := "f18da1c9f6630a51e8daf121e7451daa"
validTokenWithLabelsWithSpecialChars := "f18da1c9f6630a51e8daf121e7451dbb"
validIAMToken := "valid-iam-token"
internalResourceID := "967d38ff-7a61-4f42-bd2d-c61965b44db0"

Expand All @@ -365,27 +366,33 @@ func TestGetNodeJoinScript(t *testing.T) {
return &proto.GetClusterCACertResponse{TLSCA: fakeBytes}, nil
},
mockGetToken: func(_ context.Context, token string) (types.ProvisionToken, error) {
if token == validToken || token == validIAMToken {
return &types.ProvisionTokenV2{
Metadata: types.Metadata{
Name: token,
},
Spec: types.ProvisionTokenSpecV2{
SuggestedLabels: types.Labels{
types.InternalResourceIDLabel: utils.Strings{internalResourceID},
},
baseToken := &types.ProvisionTokenV2{
Metadata: types.Metadata{
Name: token,
},
Spec: types.ProvisionTokenSpecV2{
SuggestedLabels: types.Labels{
types.InternalResourceIDLabel: utils.Strings{internalResourceID},
},
}, nil
},
}
return nil, trace.NotFound("token does not exist")
switch token {
case validToken, validIAMToken:
case validTokenWithLabelsWithSpecialChars:
baseToken.Spec.SuggestedLabels["env"] = []string{"bad label value | ; & $ > < ' !"}
baseToken.Spec.SuggestedLabels["bad label key | ; & $ > < ' !"] = []string{"env"}
default:
return nil, trace.NotFound("token does not exist")
}
return baseToken, nil
},
}

for _, test := range []struct {
desc string
settings scriptSettings
errAssert require.ErrorAssertionFunc
extraAssertions func(script string)
extraAssertions func(t *testing.T, script string)
}{
{
desc: "zero value",
Expand All @@ -406,7 +413,7 @@ func TestGetNodeJoinScript(t *testing.T) {
desc: "valid",
settings: scriptSettings{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, "test-host")
require.Contains(t, script, "12345678")
Expand All @@ -429,19 +436,28 @@ func TestGetNodeJoinScript(t *testing.T) {
joinMethod: string(types.JoinMethodIAM),
},
errAssert: require.NoError,
extraAssertions: func(script string) {
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, "JOIN_METHOD='iam'")
},
},
{
desc: "internal resourceid label",
settings: scriptSettings{token: validToken},
errAssert: require.NoError,
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))
},
},
{
desc: "attempt to shell injection using suggested labels",
settings: scriptSettings{token: validTokenWithLabelsWithSpecialChars},
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) {
script, err := getJoinScript(context.Background(), test.settings, m)
Expand All @@ -451,7 +467,7 @@ func TestGetNodeJoinScript(t *testing.T) {
}

if test.extraAssertions != nil {
test.extraAssertions(script)
test.extraAssertions(t, script)
}
})
}
Expand Down Expand Up @@ -646,6 +662,8 @@ func TestGetAppJoinScript(t *testing.T) {
func TestGetDatabaseJoinScript(t *testing.T) {
validToken := "f18da1c9f6630a51e8daf121e7451daa"
emptySuggestedAgentMatcherLabelsToken := "f18da1c9f6630a51e8daf121e7451000"
wildcardLabelMatcherToken := "f18da1c9f6630a51e8daf121e7451001"
tokenWithSpecialChars := "f18da1c9f6630a51e8daf121e7451002"
internalResourceID := "967d38ff-7a61-4f42-bd2d-c61965b44db0"

m := &mockedNodeAPIGetter{
Expand Down Expand Up @@ -682,6 +700,22 @@ func TestGetDatabaseJoinScript(t *testing.T) {
provisionToken.Spec.SuggestedAgentMatcherLabels = types.Labels{}
return provisionToken, nil
}
if token == wildcardLabelMatcherToken {
provisionToken.Spec.SuggestedAgentMatcherLabels = types.Labels{"*": []string{"*"}}
return provisionToken, nil
}
if token == tokenWithSpecialChars {
provisionToken.Spec.SuggestedAgentMatcherLabels = types.Labels{
"*": utils.Strings{"*"},
"spa ces": utils.Strings{"spa ces"},
"EOF": utils.Strings{"test heredoc"},
`"EOF"`: utils.Strings{"test quoted heredoc"},
"#'; <>\\#": utils.Strings{"try to escape yaml"},
"&<>'\"$A,./;'BCD ${ABCD}": utils.Strings{"key with special characters"},
"value with special characters": utils.Strings{"&<>'\"$A,./;'BCD ${ABCD}", "#&<>'\"$A,./;'BCD ${ABCD}"},
}
return provisionToken, nil
}
return nil, trace.NotFound("token does not exist")
},
}
Expand All @@ -690,7 +724,7 @@ func TestGetDatabaseJoinScript(t *testing.T) {
desc string
settings scriptSettings
errAssert require.ErrorAssertionFunc
extraAssertions func(script string)
extraAssertions func(t *testing.T, script string)
}{
{
desc: "two installation methods",
Expand All @@ -708,22 +742,65 @@ 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, "test-host")
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",
settings: scriptSettings{
databaseInstallMode: true,
token: wildcardLabelMatcherToken,
},
errAssert: require.NoError,
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, wildcardLabelMatcherToken)
require.Contains(t, script, "test-host")
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",
settings: scriptSettings{
databaseInstallMode: true,
token: tokenWithSpecialChars,
},
errAssert: require.NoError,
extraAssertions: func(t *testing.T, script string) {
require.Contains(t, script, tokenWithSpecialChars)
require.Contains(t, script, "test-host")
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 @@ -734,16 +811,13 @@ 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, "test-host")
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 @@ -758,7 +832,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/node-join/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,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 @@ -494,6 +494,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
Loading