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

Detect/sanitize homoglyphs in Vizier names #1653

Closed
aimichelle opened this issue Aug 4, 2023 · 1 comment · Fixed by #1658
Closed

Detect/sanitize homoglyphs in Vizier names #1653

aimichelle opened this issue Aug 4, 2023 · 1 comment · Fixed by #1658
Labels
area/control-plane good first issue Good for newcomers kind/feature New feature or request priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@aimichelle
Copy link
Member

Is your feature request related to a problem? Please describe.
Users can currently specify a name for their Vizier by passing in the cluster name at deploy time. For example px deploy --cluster_name=test.
Vizier names are unique across the control plane. So, if someone were to already have a cluster name of test, the cluster would be assigned a name of test_<random-hash>. This is all done here when a Vizier connects to the control plane: https://github.com/pixie-io/pixie/blob/main/src/cloud/vzmgr/controllers/server.go#L775

There's a possibility that someone could confuse another user by trying to deploy a cluster name using homoglyphs. For example: "cluster" vs "clυster". Users may mistakenly believe that "clυster" is cluster which they are looking for.

Describe the solution you'd like
When a user inputs a cluster name with homoglyphs, we should detect that and sanitize the name. For example, replacing υ with u.

Describe alternatives you've considered
Rather than sanitization, we could just choose to present the Vizier name differently in the UI so the characters are visually different.

@aimichelle aimichelle added kind/feature New feature or request good first issue Good for newcomers priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. area/control-plane labels Aug 4, 2023
@vihangm
Copy link
Member

vihangm commented Aug 8, 2023

After some discussion, I think detection at input time and sanitizing is probably not the best solution since it doesn't address viziers that are already deployed.
Instead I think we should handle this similarly to how browsers handle various domain. We can piggyback on the IDNA/Punycode implementation and use the same encoding when rendering cluster names.

vihangm pushed a commit that referenced this issue Aug 9, 2023
Summary: Display a punycode version of vizier names to guard against
homoglyphs (e.g. `clυster` - which looks similar to `cluster` - would
display as `xn--clster-q1e`).

Relevant Issues: Fixes #1653

Type of change: /kind feature

Test Plan: unittest in
[vizier_cluster_test.go](https://github.com/pixie-io/pixie/blob/4219a60d9fb6f5735f866059c701c9c763659e5e/src/cloud/api/controllers/vizier_cluster_test.go#L297)

Signed-off-by: Benjamin Kilimnik <bkilimnik@pixielabs.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/control-plane good first issue Good for newcomers kind/feature New feature or request priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants