From 6c7e22934492b14f7139d53c2b6b99940e0dd272 Mon Sep 17 00:00:00 2001 From: Nic Klaassen Date: Wed, 25 Jun 2025 13:41:54 -0700 Subject: [PATCH 1/2] [vnet] fix: avoid empty host matchers in generated SSH config --- lib/vnet/opensshconfig.go | 59 +++++++++++++++++++++------------- lib/vnet/opensshconfig_test.go | 28 ++++++++++++---- 2 files changed, 58 insertions(+), 29 deletions(-) diff --git a/lib/vnet/opensshconfig.go b/lib/vnet/opensshconfig.go index 65f2587f63796..bee92a7028633 100644 --- a/lib/vnet/opensshconfig.go +++ b/lib/vnet/opensshconfig.go @@ -22,6 +22,7 @@ import ( "context" "encoding/pem" "io" + "maps" "os" "path/filepath" "slices" @@ -37,7 +38,6 @@ import ( "github.com/gravitational/teleport/api/profile" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/api/utils/keypaths" "github.com/gravitational/teleport/lib/cryptosuites" ) @@ -190,7 +190,8 @@ func (c *sshConfigurator) updateSSHConfiguration(ctx context.Context) error { if err != nil { return trace.Wrap(err, "listing profiles") } - hostMatchers := make([]string, 0, len(profileNames)) + // Build a set of unique cluster names for all active clusters. + clusterNames := make(map[string]struct{}) for _, profileName := range profileNames { rootClient, err := c.cfg.clientApplication.GetCachedClient(ctx, profileName, "" /*leafClusterName*/) if err != nil { @@ -199,7 +200,7 @@ func (c *sshConfigurator) updateSSHConfiguration(ctx context.Context) error { "profile", profileName, "error", err) continue } - hostMatchers = append(hostMatchers, hostMatcher(rootClient.RootClusterName())) + clusterNames[rootClient.RootClusterName()] = struct{}{} leafClusters, err := c.cfg.leafClusterCache.getLeafClusters(ctx, rootClient) if err != nil { log.WarnContext(ctx, @@ -208,17 +209,10 @@ func (c *sshConfigurator) updateSSHConfiguration(ctx context.Context) error { continue } for _, leafCluster := range leafClusters { - hostMatchers = append(hostMatchers, hostMatcher(leafCluster)) + clusterNames[leafCluster] = struct{}{} } } - hostMatchers = utils.Deduplicate(hostMatchers) - slices.Sort(hostMatchers) - hostMatchersString := strings.Join(hostMatchers, " ") - return trace.Wrap(writeSSHConfigFile(c.profilePath, hostMatchersString)) -} - -func hostMatcher(clusterName string) string { - return "*." + strings.Trim(clusterName, ".") + return trace.Wrap(writeSSHConfigFile(c.profilePath, clusterNames)) } func deleteSSHConfigFile(profilePath string) error { @@ -233,28 +227,49 @@ func deleteSSHConfigFile(profilePath string) error { return nil } -func writeSSHConfigFile(profilePath, hostMatchers string) error { - t := template.Must(template.New("ssh_config").Parse(configFileTemplate)) +func writeSSHConfigFile(profilePath string, clusterNames map[string]struct{}) error { var b bytes.Buffer - if err := t.Execute(&b, configFileTemplateInput{ - Hosts: hostMatchers, - PrivateKeyPath: strconv.Quote(keypaths.VNetClientSSHKeyPath(profilePath)), - KnownHostsPath: strconv.Quote(keypaths.VNetKnownHostsPath(profilePath)), - }); err != nil { - return trace.Wrap(err, "generating SSH config file") + if len(clusterNames) == 0 { + // Avoid writing the Host block if there are no clusters to match. + b.WriteString("# No active clusters\n") + } else { + hosts := strings.Join(hostMatchers(clusterNames), " ") + if err := configFileTemplate.Execute(&b, configFileTemplateInput{ + Hosts: hosts, + PrivateKeyPath: strconv.Quote(keypaths.VNetClientSSHKeyPath(profilePath)), + KnownHostsPath: strconv.Quote(keypaths.VNetKnownHostsPath(profilePath)), + }); err != nil { + return trace.Wrap(err, "generating SSH config file") + } } p := keypaths.VNetSSHConfigPath(profilePath) err := renameio.WriteFile(p, b.Bytes(), filePerms) return trace.Wrap(trace.ConvertSystemError(err), "writing SSH config file to %s", p) } -const configFileTemplate = `Host {{ .Hosts }} +// hostMatchers returns a sorted list of host matchers for a given set of +// cluster names. +func hostMatchers(clusterNames map[string]struct{}) []string { + sortedClusterNames := slices.Sorted(maps.Keys(clusterNames)) + matchers := make([]string, 0, len(sortedClusterNames)) + for _, clusterName := range sortedClusterNames { + matchers = append(matchers, hostMatcher(clusterName)) + } + return matchers +} + +func hostMatcher(clusterName string) string { + return "*." + strings.Trim(clusterName, ".") +} + +var configFileTemplate = template.Must(template.New("vnet_ssh_config"). + Parse(`Host {{ .Hosts }} IdentityFile {{ .PrivateKeyPath }} GlobalKnownHostsFile {{ .KnownHostsPath }} UserKnownHostsFile /dev/null StrictHostKeyChecking yes IdentitiesOnly yes -` +`)) type configFileTemplateInput struct { Hosts string diff --git a/lib/vnet/opensshconfig_test.go b/lib/vnet/opensshconfig_test.go index 4e933fc3bc915..7e93f163f53a7 100644 --- a/lib/vnet/opensshconfig_test.go +++ b/lib/vnet/opensshconfig_test.go @@ -69,6 +69,9 @@ func TestSSHConfigurator(t *testing.T) { // Intentionally not using the template defined in the production code to // test that it actually produces output that looks like this. expectedConfigFile := func(expectedHosts string) string { + if expectedHosts == "" { + return "# No active clusters\n" + } return fmt.Sprintf(`Host %s IdentityFile "%s/id_vnet" GlobalKnownHostsFile "%s/vnet_known_hosts" @@ -95,20 +98,31 @@ func TestSSHConfigurator(t *testing.T) { // fakeClientApp. assertConfigFile("*.cluster1 *.cluster2 *.leaf1") - // Add a new root and leaf cluster, wait until the configurator is blocked - // in the loop, advance the clock, wait until the configurator is blocked - // again indicating it should have updated the config and made it back into - // the loop, and then assert that the new clusters are in the config file. + // To reliably advance the clock and allow runConfigurationLoop to update + // the config the test waits until the loop is blocked on the clock, then + // advances the clock, then waits until the loop is blocked again. + advance := func() { + fakeClock.BlockUntilContext(ctx, 1) + fakeClock.Advance(sshConfigurationUpdateInterval) + fakeClock.BlockUntilContext(ctx, 1) + } + + // Add a new root and leaf cluster, allow the configuration loop to run, + // and then assert that the new clusters are in the config file. fakeClientApp.cfg.clusters["cluster3"] = testClusterSpec{ leafClusters: map[string]testClusterSpec{ "leaf2": {}, }, } - fakeClock.BlockUntilContext(ctx, 1) - fakeClock.Advance(sshConfigurationUpdateInterval) - fakeClock.BlockUntilContext(ctx, 1) + advance() assertConfigFile("*.cluster1 *.cluster2 *.cluster3 *.leaf1 *.leaf2") + // Delete all clusters as if the user logged out, allow the configuration + // loop to run, and then assert that the config file is well-formed. + fakeClientApp.cfg.clusters = nil + advance() + assertConfigFile("") + // Kill the configurator, wait for it to return, and assert that the config // file was deleted. cancel() From 40bad62e0bef2cf1843a0deda44355a72e22ccfa Mon Sep 17 00:00:00 2001 From: Nic Klaassen Date: Thu, 26 Jun 2025 13:28:40 -0700 Subject: [PATCH 2/2] add more context to comments in vnet_ssh_config --- lib/vnet/opensshconfig.go | 10 +++++++++- lib/vnet/opensshconfig_test.go | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/vnet/opensshconfig.go b/lib/vnet/opensshconfig.go index bee92a7028633..939b9db19681c 100644 --- a/lib/vnet/opensshconfig.go +++ b/lib/vnet/opensshconfig.go @@ -229,9 +229,10 @@ func deleteSSHConfigFile(profilePath string) error { func writeSSHConfigFile(profilePath string, clusterNames map[string]struct{}) error { var b bytes.Buffer + b.WriteString(generatedFileHeader) if len(clusterNames) == 0 { // Avoid writing the Host block if there are no clusters to match. - b.WriteString("# No active clusters\n") + b.WriteString("# VNet currently detects no logged-in clusters, log in to start using VNet\n") } else { hosts := strings.Join(hostMatchers(clusterNames), " ") if err := configFileTemplate.Execute(&b, configFileTemplateInput{ @@ -262,6 +263,13 @@ func hostMatcher(clusterName string) string { return "*." + strings.Trim(clusterName, ".") } +const generatedFileHeader = `# --------------------------------------------------------------------- +# THIS FILE IS AUTOMATICALLY GENERATED BY TELEPORT VNET. DO NOT EDIT. +# Your changes will be overwritten the next time the file is generated. +# --------------------------------------------------------------------- + +` + var configFileTemplate = template.Must(template.New("vnet_ssh_config"). Parse(`Host {{ .Hosts }} IdentityFile {{ .PrivateKeyPath }} diff --git a/lib/vnet/opensshconfig_test.go b/lib/vnet/opensshconfig_test.go index 7e93f163f53a7..e6ed7f25f93ff 100644 --- a/lib/vnet/opensshconfig_test.go +++ b/lib/vnet/opensshconfig_test.go @@ -70,9 +70,9 @@ func TestSSHConfigurator(t *testing.T) { // test that it actually produces output that looks like this. expectedConfigFile := func(expectedHosts string) string { if expectedHosts == "" { - return "# No active clusters\n" + return generatedFileHeader + "# VNet currently detects no logged-in clusters, log in to start using VNet\n" } - return fmt.Sprintf(`Host %s + return generatedFileHeader + fmt.Sprintf(`Host %s IdentityFile "%s/id_vnet" GlobalKnownHostsFile "%s/vnet_known_hosts" UserKnownHostsFile /dev/null