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

Escape labels on Node Join scripts #52350

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

marcoandredinis
Copy link
Contributor

Fixes: https://github.com/gravitational/teleport-private/issues/1883

changelog: Escape user provided labels when creating the shell script that enrolls servers, applications and databases into Teleport.

Enrolling new servers using the UI (Enroll new resource button), generates a script that installs teleport into the machine and enrolls it into teleport.
We now support adding labels to this script, so that when the teleport service presents itself to the cluster, it will advertise those labels.

However, those labels were added into the script without escaping their values.

This PR changes 3 places where user input data was being used:

  • when installing the node, it runs teleport node configure --labels <labels>
  • when installing as a Application Service, it would put the labels in the teleport.yaml file
  • when installing as a Database Service, it would put the labels as resource label matchers in the teleport.yaml file

The source of the first two installation modes (node and app service), labels are sourced from Token.Spec.SuggestedLabels.
For the Database Service installation mode, labels are sourced from Token.Spec.SuggestedAgentMatcherLabels.

For the node, the labels are now escaped before being added into the teleport node configure --labels command.

For the Application and Database installation modes, we keep using the same method we were using but now prevent any escape from the expected flow by creating a new HEREDOC using a quoted starting identifier ("EOF"), which prevents any interpolation of the HEREDOC contents.


I think we should be more strict about the labels we accept in the first place, but I'll not add those checks here to keep the PR as specific as possible.

Example A
When the teleport node configure --labels x=y is called, teleport will validate the labels using client.ParseLabelSpec which might return invalid even tho the labels were set as SuggestedLabels.

Example B
Even if the teleport.yaml was generated using teleport node configure --labels x=y it might be the case that the label is invalid.
There's a validation using types.IsValidLabelKey, which is different from the above.

Copy link
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the EOF heredocs?

@marcoandredinis marcoandredinis force-pushed the marco/escape_labels_joinscript branch from 35ac83d to 0c4b004 Compare February 20, 2025 18:14
@marcoandredinis
Copy link
Contributor Author

what about the EOF heredocs?

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants