Skip to content

Validate desktop names#31739

Merged
zmb3 merged 1 commit intomasterfrom
zmb3/desktop-name-validation
Sep 12, 2023
Merged

Validate desktop names#31739
zmb3 merged 1 commit intomasterfrom
zmb3/desktop-name-validation

Conversation

@zmb3
Copy link
Copy Markdown
Collaborator

@zmb3 zmb3 commented Sep 11, 2023

Desktops cannot contain '.' characters in their names, as we use SNI to pass the desktop name in the TLS handshake. This wasn't a problem in the past when Teleport was the only thing that created desktops, as we were sure to sanitize names. With non-AD access we encourage users to register their own desktops via API or tctl, and they often encounter a confusing TLS error when they use an invalid name.

With this change:

➜ tctl create -f desktop-resource.yaml
ERROR: invalid name "this-is-a.test": desktop names cannot contain periods

Fixes #31644

Desktops cannot contain '.' characters in their names, as we use SNI
to pass the desktop name in the TLS handshake. This wasn't a problem
in the past when Teleport was the only thing that created desktops,
as we were sure to sanitize names. With non-AD access we encourage
users to register their own desktops via API or tctl, and they often
encounter a confusing TLS error when they use an invalid name.

With this change:

    ➜ tctl create -f desktop-resource.yaml
    ERROR: invalid name "this-is-a.test": desktop names cannot contain periods

Fixes #31644
@zmb3 zmb3 enabled auto-merge September 12, 2023 13:59
Copy link
Copy Markdown
Contributor

@ibeckermayer ibeckermayer left a comment

Choose a reason for hiding this comment

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

It seems like we don't have any examples of manually adding a windows desktop in the docs, is that something we want to add?

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from camscale September 12, 2023 18:20
@zmb3 zmb3 added this pull request to the merge queue Sep 12, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 12, 2023
@zmb3
Copy link
Copy Markdown
Collaborator Author

zmb3 commented Sep 12, 2023

It seems like we don't have any examples of manually adding a windows desktop in the docs, is that something we want to add?

We do:

Note that without Active Directory, Teleport cannot automatically discover your
Desktops. Instead you must define the Windows systems configured for access through
Teleport in your config file, or use Teleport's [API](../api/introduction.mdx)
to build your own integration. An example API integration is available on
[GitHub](https://github.com/gravitational/teleport/tree/master/examples/desktop-registration).

@zmb3 zmb3 added this pull request to the merge queue Sep 12, 2023
Merged via the queue into master with commit 34a172c Sep 12, 2023
@zmb3 zmb3 deleted the zmb3/desktop-name-validation branch September 12, 2023 19:15
@public-teleport-github-review-bot
Copy link
Copy Markdown

@zmb3 See the table below for backport results.

Branch Result
branch/v12 Create PR
branch/v13 Create PR
branch/v14 Create PR

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve Error Handling when Desktop Name has a "."

3 participants