Skip to content

puttyconfig: Switch to string-based Validity format and deprecate MatchHosts#32598

Merged
webvictim merged 17 commits intomasterfrom
gus/tsh-putty-matchhosts-validity
Oct 2, 2023
Merged

puttyconfig: Switch to string-based Validity format and deprecate MatchHosts#32598
webvictim merged 17 commits intomasterfrom
gus/tsh-putty-matchhosts-validity

Conversation

@webvictim
Copy link
Copy Markdown
Contributor

@webvictim webvictim commented Sep 26, 2023

At some point during the development of PuTTY 0.78 which added support for SSH user and host certificates, the author switched from storing the list of hostnames to trust for a given public key in a REG_MULTI_SZ called MatchHosts, to storing them in a REG_SZ called Validity.

tsh puttyconfig was written to use the MatchHosts format, PuTTY still contains code to process that format and tsh only needs to work with single hostnames or wildcards, so there was no sense in delaying the PR further by switching to the more-complicated string-based Validity format instead which involves boolean logic and a tokenizer/lexer etc.

However, the author of WinSCP is now adding support for importing saved sessions from and automatically using the same SSH host CAs as configured in PuTTY. WinSCP only supports the newer Validity format, so it's time for this PR which switches tsh puttyconfig to the same.

If a MatchHosts value is set in the registry key, tsh puttyconfig will automatically migrate any entries to the Validity format, save the key to the registry and then delete the MatchHosts value if this was successful.

Additionally, Teleport ignores any values from the Validity key which are inside parentheses and passes them through verbatim to avoid needing to implement a lexer of its own. I suspect it's unlikely that users will manually tinker with the Validity values in keys added by tsh puttyconfig, but I wanted to do to try and account for this anyway.

tsh puttyconfig will error if the Validity key does not match the managed <hostname> || <hostname> || ... format that it uses. There is now a troubleshooting section added to the docs to help people fix these errors if they encounter them.

References:

@webvictim webvictim added tsh tsh - Teleport's command line tool for logging into nodes running Teleport. backport/branch/v14 labels Sep 26, 2023
@webvictim webvictim self-assigned this Sep 26, 2023
Comment thread lib/puttyhosts/puttyhosts.go Outdated
@webvictim webvictim force-pushed the gus/tsh-putty-matchhosts-validity branch from 04de663 to 739115e Compare September 27, 2023 15:02
@webvictim webvictim temporarily deployed to vercel September 27, 2023 15:02 — with GitHub Actions Inactive
@webvictim webvictim temporarily deployed to vercel September 27, 2023 15:05 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-95dzuasqm-goteleport.vercel.app/docs/ver/14.x

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-h27kl3f1l-goteleport.vercel.app/docs/ver/14.x

@webvictim webvictim temporarily deployed to vercel September 27, 2023 15:30 — with GitHub Actions Inactive
@webvictim webvictim requested a review from nklaassen September 27, 2023 15:32
@webvictim webvictim temporarily deployed to vercel September 27, 2023 15:35 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-278a7fzyj-goteleport.vercel.app/docs/ver/14.x

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-h52ccn83z-goteleport.vercel.app/docs/ver/14.x

Comment thread lib/puttyhosts/puttyhosts.go Outdated
Comment thread lib/puttyhosts/puttyhosts_test.go Outdated
Comment thread lib/puttyhosts/puttyhosts_test.go Outdated
Comment thread lib/puttyhosts/puttyhosts.go Outdated
Comment thread lib/puttyhosts/puttyhosts_test.go Outdated
Comment thread lib/puttyhosts/puttyhosts.go Outdated
Comment thread tool/tsh/common/putty_config_windows.go Outdated
@webvictim webvictim temporarily deployed to vercel September 27, 2023 18:12 — with GitHub Actions Inactive
@webvictim webvictim temporarily deployed to vercel September 27, 2023 18:22 — with GitHub Actions Inactive
webvictim and others added 2 commits September 28, 2023 22:07
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
@webvictim webvictim temporarily deployed to vercel September 29, 2023 01:07 — with GitHub Actions Inactive
@webvictim webvictim temporarily deployed to vercel September 29, 2023 01:07 — with GitHub Actions Inactive
@webvictim webvictim temporarily deployed to vercel September 29, 2023 01:07 — with GitHub Actions Inactive
@webvictim webvictim requested a review from zmb3 September 29, 2023 01:08
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-hrdb0nz00-goteleport.vercel.app/docs/ver/14.x

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-1pkyragit-goteleport.vercel.app/docs/ver/14.x

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-4w0jscrid-goteleport.vercel.app/docs/ver/14.x

@webvictim webvictim added this pull request to the merge queue Oct 2, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 2, 2023
@webvictim webvictim temporarily deployed to vercel October 2, 2023 15:10 — with GitHub Actions Inactive
@webvictim webvictim enabled auto-merge October 2, 2023 15:10
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 2, 2023

🤖 Vercel preview here: https://docs-4tvdg1e02-goteleport.vercel.app/docs/ver/14.x

@webvictim webvictim added this pull request to the merge queue Oct 2, 2023
Merged via the queue into master with commit a4b3248 Oct 2, 2023
@webvictim webvictim deleted the gus/tsh-putty-matchhosts-validity branch October 2, 2023 16:26
@public-teleport-github-review-bot
Copy link
Copy Markdown

@webvictim See the table below for backport results.

Branch Result
branch/v14 Create PR

webvictim added a commit that referenced this pull request Oct 16, 2023
…chHosts (#32598)

* puttyconfig: Switch to string-based Validity format and deprecate MatchHosts

* Switch to more restrictive, reliable parsing

* Add validity string errors to docs

* Remove invalid test case

* Add test case

* Remove any spaces from user-provided input and use sanitized hostname

* Apply fixes from code review

* Tidy up errors, provide consistent detail about which field contains an error

* Disable docs lint for dots in heading

This is needed here, as there are 5 error messages which all start the same way but end differently.

* Catch a few more error cases

* Only delete old MatchHosts key after new Validity key has been written successfully

* Apply suggestions from code review

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Address Zac's comments from code review

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
github-merge-queue Bot pushed a commit that referenced this pull request Oct 20, 2023
* [v13] tsh: Implement puttyconfig command to add saved PuTTY sessions to Windows registry

* docs: `tsh puttyconfig`

* docs: Minor cosmetic tweaks to tsh puttyconfig (#29831)

Fixes an extra full stop and tenant name which was included accidentally.

* puttyconfig: Switch to string-based Validity format and deprecate MatchHosts (#32598)

* puttyconfig: Switch to string-based Validity format and deprecate MatchHosts

* Switch to more restrictive, reliable parsing

* Add validity string errors to docs

* Remove invalid test case

* Add test case

* Remove any spaces from user-provided input and use sanitized hostname

* Apply fixes from code review

* Tidy up errors, provide consistent detail about which field contains an error

* Disable docs lint for dots in heading

This is needed here, as there are 5 error messages which all start the same way but end differently.

* Catch a few more error cases

* Only delete old MatchHosts key after new Validity key has been written successfully

* Apply suggestions from code review

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Address Zac's comments from code review

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Add tsh puttyconfig to CLI reference

* Add winscp to cspell whitelist

* Add WinSCP to PuTTY client instructions (#32868)

* Remove duplicate tsh puttyconfig from CLI reference

* Fix link to CLI reference

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/md tsh tsh - Teleport's command line tool for logging into nodes running Teleport.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants