Skip to content

Commit

Permalink
Escape labels on Node Join scripts (#52350) (#52706)
Browse files Browse the repository at this point in the history
  • Loading branch information
@marcoandredinis
marcoandredinis authored Mar 5, 2025
1 parent ca49736 commit 6a6966c
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 28 deletions.
6 changes: 5 additions & 1 deletion lib/web/join_tokens.go
View file
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
View file
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
View file
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

0 comments on commit 6a6966c

Please sign in to comment.