[v9] UX improvements for tbot (#10833)

tool/tbot/config/config.go

@@ -61,9 +61,9 @@ type CLIConf struct {
 	// Token is a bot join token.
 	Token string
 
-	// RenewInterval is the interval at which certificates are renewed, as a
+	// RenewalInterval is the interval at which certificates are renewed, as a
 	// time.ParseDuration() string. It must be less than the certificate TTL.
-	RenewInterval time.Duration
+	RenewalInterval time.Duration
 
 	// CertificateTTL is the requested TTL of certificates. It should be some
 	// multiple of the renewal interval to allow for failed renewals.
@@ -73,6 +73,9 @@ type CLIConf struct {
 	// initial certificate
 	JoinMethod string
 
+	// Oneshot controls whether the bot quits after a single renewal.
+	Oneshot bool
+
 	// InitDir specifies which destination to initialize if multiple are
 	// configured.
 	InitDir string
@@ -117,17 +120,14 @@ type BotConfig struct {
 	Storage      *StorageConfig       `yaml:"storage,omitempty"`
 	Destinations []*DestinationConfig `yaml:"destinations,omitempty"`
 
-	Debug          bool          `yaml:"debug"`
-	AuthServer     string        `yaml:"auth_server"`
-	CertificateTTL time.Duration `yaml:"certificate_ttl"`
-	RenewInterval  time.Duration `yaml:"renew_interval"`
+	Debug           bool          `yaml:"debug"`
+	AuthServer      string        `yaml:"auth_server"`
+	CertificateTTL  time.Duration `yaml:"certificate_ttl"`
+	RenewalInterval time.Duration `yaml:"renewal_interval"`
+	Oneshot         bool          `yaml:"oneshot"`
 }
 
 func (conf *BotConfig) CheckAndSetDefaults() error {
-	if conf.AuthServer == "" {
-		return trace.BadParameter("an auth server address must be configured")
-	}
-
 	if conf.Storage == nil {
 		conf.Storage = &StorageConfig{}
 	}
@@ -146,8 +146,8 @@ func (conf *BotConfig) CheckAndSetDefaults() error {
 		conf.CertificateTTL = DefaultCertificateTTL
 	}
 
-	if conf.RenewInterval == 0 {
-		conf.RenewInterval = DefaultRenewInterval
+	if conf.RenewalInterval == 0 {
+		conf.RenewalInterval = DefaultRenewInterval
 	}
 
 	return nil
@@ -214,6 +214,10 @@ func FromCLIConf(cf *CLIConf) (*BotConfig, error) {
 		config.Debug = true
 	}
 
+	if cf.Oneshot {
+		config.Oneshot = true
+	}
+
 	if cf.AuthServer != "" {
 		if config.AuthServer != "" {
 			log.Warnf("CLI parameters are overriding auth server configured in %s", cf.ConfigPath)
@@ -228,11 +232,11 @@ func FromCLIConf(cf *CLIConf) (*BotConfig, error) {
 		config.CertificateTTL = cf.CertificateTTL
 	}
 
-	if cf.RenewInterval != 0 {
-		if config.RenewInterval != 0 {
+	if cf.RenewalInterval != 0 {
+		if config.RenewalInterval != 0 {
 			log.Warnf("CLI parameters are overriding renewal interval configured in %s", cf.ConfigPath)
 		}
-		config.RenewInterval = cf.RenewInterval
+		config.RenewalInterval = cf.RenewalInterval
 	}
 
 	// DataDir overrides any previously-configured storage config

tool/tbot/config/config_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/coreos/go-semver/semver"
 	"github.com/gravitational/teleport/tool/tbot/identity"
 	"github.com/stretchr/testify/require"
 )
@@ -30,7 +31,7 @@ func TestConfigDefaults(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Equal(t, DefaultCertificateTTL, cfg.CertificateTTL)
-	require.Equal(t, DefaultRenewInterval, cfg.RenewInterval)
+	require.Equal(t, DefaultRenewInterval, cfg.RenewalInterval)
 
 	storageDest, err := cfg.Storage.GetDestination()
 	require.NoError(t, err)
@@ -93,7 +94,7 @@ func TestConfigFile(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Equal(t, "auth.example.com", cfg.AuthServer)
-	require.Equal(t, time.Minute*5, cfg.RenewInterval)
+	require.Equal(t, time.Minute*5, cfg.RenewalInterval)
 
 	require.NotNil(t, cfg.Onboarding)
 	require.Equal(t, "foo", cfg.Onboarding.Token)
@@ -125,9 +126,53 @@ func TestConfigFile(t *testing.T) {
 	require.Equal(t, "/tmp/foo", destImplReal.Path)
 }
 
+func TestParseSSHVersion(t *testing.T) {
+	tests := []struct {
+		str     string
+		version *semver.Version
+		err     bool
+	}{
+		{
+			str:     "OpenSSH_8.2p1 Ubuntu-4ubuntu0.4, OpenSSL 1.1.1f  31 Mar 2020",
+			version: semver.New("8.2.1"),
+		},
+		{
+			str:     "OpenSSH_8.8p1, OpenSSL 1.1.1m  14 Dec 2021",
+			version: semver.New("8.8.1"),
+		},
+		{
+			str:     "OpenSSH_7.5p1, OpenSSL 1.0.2s-freebsd  28 May 2019",
+			version: semver.New("7.5.1"),
+		},
+		{
+			str:     "OpenSSH_7.9p1 Raspbian-10+deb10u2, OpenSSL 1.1.1d  10 Sep 2019",
+			version: semver.New("7.9.1"),
+		},
+		{
+			// Couldn't find a full example but in theory patch is optional:
+			str:     "OpenSSH_8.1 foo",
+			version: semver.New("8.1.0"),
+		},
+		{
+			str: "Teleport v8.0.0-dev.40 git:v8.0.0-dev.40-0-ge9194c256 go1.17.2",
+			err: true,
+		},
+	}
+
+	for _, test := range tests {
+		version, err := parseSSHVersion(test.str)
+		if test.err {
+			require.Error(t, err)
+		} else {
+			require.NoError(t, err)
+			require.True(t, version.Equal(*test.version), "got version = %v, want = %v", version, test.version)
+		}
+	}
+}
+
 const exampleConfigFile = `
 auth_server: auth.example.com
-renew_interval: 5m
+renewal_interval: 5m
 onboarding:
   token: foo
   ca_pins:

tool/tbot/config/configtemplate_ssh.go

@@ -17,14 +17,18 @@ limitations under the License.
 package config
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 	"text/template"
 
+	"github.com/coreos/go-semver/semver"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/auth"
 	"github.com/gravitational/teleport/lib/defaults"
@@ -40,6 +44,66 @@ type TemplateSSHClient struct {
 	ProxyPort uint16 `yaml:"proxy_port"`
 }
 
+// openSSHVersionRegex is a regex used to parse OpenSSH version strings.
+var openSSHVersionRegex = regexp.MustCompile(`^OpenSSH_(?P<major>\d+)\.(?P<minor>\d+)(?:p(?P<patch>\d+))?`)
+
+// openSSHMinVersionForRSAWorkaround is the OpenSSH version after which the
+// RSA deprecation workaround should be added to generated ssh_config.
+var openSSHMinVersionForRSAWorkaround = semver.New("8.5.0")
+
+// parseSSHVersion attempts to parse
+func parseSSHVersion(versionString string) (*semver.Version, error) {
+	versionTokens := strings.Split(versionString, " ")
+	if len(versionTokens) == 0 {
+		return nil, trace.BadParameter("invalid version string: %s", versionString)
+	}
+
+	versionID := versionTokens[0]
+	matches := openSSHVersionRegex.FindStringSubmatch(versionID)
+	if matches == nil {
+		return nil, trace.BadParameter("cannot parse version string: %q", versionID)
+	}
+
+	major, err := strconv.Atoi(matches[1])
+	if err != nil {
+		return nil, trace.Wrap(err, "invalid major version number: %s", matches[1])
+	}
+
+	minor, err := strconv.Atoi(matches[2])
+	if err != nil {
+		return nil, trace.Wrap(err, "invalid minor version number: %s", matches[2])
+	}
+
+	patch := 0
+	if matches[3] != "" {
+		patch, err = strconv.Atoi(matches[3])
+		if err != nil {
+			return nil, trace.Wrap(err, "invalid patch version number: %s", matches[3])
+		}
+	}
+
+	return &semver.Version{
+		Major: int64(major),
+		Minor: int64(minor),
+		Patch: int64(patch),
+	}, nil
+}
+
+// getSSHVersion attempts to query the system SSH for its current version.
+func getSSHVersion() (*semver.Version, error) {
+	var out bytes.Buffer
+
+	cmd := exec.Command("ssh", "-V")
+	cmd.Stderr = &out
+
+	err := cmd.Run()
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	return parseSSHVersion(out.String())
+}
+
 func (c *TemplateSSHClient) CheckAndSetDefaults() error {
 	if c.ProxyPort == 0 {
 		c.ProxyPort = defaults.SSHProxyListenPort
@@ -111,18 +175,31 @@ func (c *TemplateSSHClient) Render(ctx context.Context, authClient auth.ClientI,
 		return trace.Wrap(err)
 	}
 
+	// Default to including the RSA deprecation workaround.
+	rsaWorkaround := true
+	version, err := getSSHVersion()
+	if err != nil {
+		log.WithError(err).Debugf("Could not determine SSH version, will include RSA workaround.")
+	} else if version.LessThan(*openSSHMinVersionForRSAWorkaround) {
+		log.Debugf("OpenSSH version %s does not require workaround for RSA deprecation", version)
+		rsaWorkaround = false
+	} else {
+		log.Debugf("OpenSSH version %s will use workaround for RSA deprecation", version)
+	}
+
 	var sshConfigBuilder strings.Builder
 	identityFilePath := filepath.Join(dataDir, identity.PrivateKeyKey)
 	certificateFilePath := filepath.Join(dataDir, identity.SSHCertKey)
 	sshConfigPath := filepath.Join(dataDir, "ssh_config")
 	if err := sshConfigTemplate.Execute(&sshConfigBuilder, sshConfigParameters{
-		ClusterName:         clusterName.GetClusterName(),
-		ProxyHost:           proxyHost,
-		ProxyPort:           proxyPort,
-		KnownHostsPath:      knownHostsPath,
-		IdentityFilePath:    identityFilePath,
-		CertificateFilePath: certificateFilePath,
-		SSHConfigPath:       sshConfigPath,
+		ClusterName:          clusterName.GetClusterName(),
+		ProxyHost:            proxyHost,
+		ProxyPort:            proxyPort,
+		KnownHostsPath:       knownHostsPath,
+		IdentityFilePath:     identityFilePath,
+		CertificateFilePath:  certificateFilePath,
+		SSHConfigPath:        sshConfigPath,
+		IncludeRSAWorkaround: rsaWorkaround,
 	}); err != nil {
 		return trace.Wrap(err)
 	}
@@ -142,6 +219,15 @@ type sshConfigParameters struct {
 	ProxyHost           string
 	ProxyPort           string
 	SSHConfigPath       string
+
+	// IncludeRSAWorkaround controls whether the RSA deprecation workaround is
+	// included in the generated configuration. Newer versions of OpenSSH
+	// deprecate RSA certificates and, due to a bug in golang's ssh package,
+	// Teleport wrongly advertises its unaffected certificates as a
+	// now-deprecated certificate type. The workaround includes a config
+	// override to re-enable RSA certs for just Teleport hosts, however it is
+	// only supported on OpenSSH 8.5 and later.
+	IncludeRSAWorkaround bool
 }
 
 var sshConfigTemplate = template.Must(template.New("ssh-config").Parse(`
@@ -152,13 +238,13 @@ Host *.{{ .ClusterName }} {{ .ProxyHost }}
     UserKnownHostsFile "{{ .KnownHostsPath }}"
     IdentityFile "{{ .IdentityFilePath }}"
     CertificateFile "{{ .CertificateFilePath }}"
-    HostKeyAlgorithms ssh-rsa-cert-v01@openssh.com
-    PubkeyAcceptedAlgorithms +ssh-rsa-cert-v01@openssh.com
+    HostKeyAlgorithms ssh-rsa-cert-v01@openssh.com{{- if .IncludeRSAWorkaround }}
+    PubkeyAcceptedAlgorithms +ssh-rsa-cert-v01@openssh.com{{- end }}
 
 # Flags for all {{ .ClusterName }} hosts except the proxy
 Host *.{{ .ClusterName }} !{{ .ProxyHost }}
     Port 3022
-    ProxyCommand ssh -F {{ .SSHConfigPath }} -l %r -p {{ .ProxyPort }} {{ .ProxyHost }} -s proxy:%h:%p@{{ .ClusterName }}
+    ProxyCommand ssh -F {{ .SSHConfigPath }} -l %r -p {{ .ProxyPort }} {{ .ProxyHost }} -s proxy:$(echo %h | cut -d '.' -f 1):%p@{{ .ClusterName }}
 
 # End generated Teleport configuration
 `))