[Buddy 30860] Added --insecure flag to tbot (#31093)

* Added --insecure flag to tbot; Added test; Added test-setup

* Removed old file

* Review comments

* Updated tests; Rework CAPins & CAPath verification; Split functions

* Cleanup old debug lines

* Cleaned up tests; Remove unnecessary InsecureSkipVerify;

* Add back InsecureSkipVerify to fix Authentication

* renamed DefaultBotConfigOpts parameter; remove some stale debug code; restored wrongfully delted InsecureSkipVerify;

* remove stale newline

* Improved warnings

* Updated tbot usage example

* Fix failing test; Cleanup Makefile target

* Removed unused config option from OnboardingConfig; Fixed import order

* Rename test; Rework if statement; Fix newline

* Updated shell script to comply with shellcheck; added example yaml to gitignore

* Remove example file

* Tidier comments for reg code

* behavior

* Remove unused var from Makefile test-go-unit-tbot

* Further simplify makefile

---------

Co-authored-by: FireDrunk <thijs.cramer@gmail.com>

Makefile

@@ -744,6 +744,14 @@ test-go-unit:
 		| tee $(TEST_LOG_DIR)/unit.json \
 		| gotestsum --raw-command -- cat
 
+# Runs tbot unit tests
+.PHONY: test-go-unit-tbot
+test-go-unit-tbot: FLAGS ?= -race -shuffle on
+test-go-unit-tbot:
+	$(CGOFLAG) go test -cover -json $(FLAGS) $(ADDFLAGS) ./tool/tbot/... ./lib/tbot/... \
+		| tee $(TEST_LOG_DIR)/unit.json \
+		| gotestsum --raw-command -- cat
+
 # rdpclient and libfido2 don't play well together, so we run libfido2 tests
 # separately.
 # TODO(codingllama): Run libfido2 tests along with others once RDP doesn't

integration/proxy/proxy_helpers.go

@@ -51,6 +51,7 @@ import (
 	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/teleport/api/utils/retryutils"
 	"github.com/gravitational/teleport/integration/helpers"
+	"github.com/gravitational/teleport/lib"
 	"github.com/gravitational/teleport/lib/auth"
 	"github.com/gravitational/teleport/lib/client"
 	"github.com/gravitational/teleport/lib/defaults"
@@ -672,6 +673,7 @@ func mustRegisterUsingIAMMethod(t *testing.T, proxyAddr utils.NetAddr, token str
 		JoinMethod:   types.JoinMethodIAM,
 		PublicTLSKey: pubTLS,
 		PublicSSHKey: []byte(fixtures.SSHCAPublicKey),
+		Insecure:     lib.IsInsecureDevMode(),
 	})
 	require.NoError(t, err, trace.DebugReport(err))
 }

lib/auth/register.go

@@ -39,7 +39,6 @@ import (
 	"github.com/gravitational/teleport/api/metadata"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/api/utils/aws"
-	"github.com/gravitational/teleport/lib"
 	"github.com/gravitational/teleport/lib/auth/native"
 	"github.com/gravitational/teleport/lib/circleci"
 	"github.com/gravitational/teleport/lib/cloud/azure"
@@ -153,6 +152,8 @@ type RegisterParams struct {
 	// certificates that are returned by registering should expire at.
 	// It should not be specified for non-bot registrations.
 	Expires *time.Time
+	// Insecure trusts the certificates from the Auth Server or Proxy during registration without verification.
+	Insecure bool
 }
 
 func (r *RegisterParams) checkAndSetDefaults() error {
@@ -308,10 +309,11 @@ func proxyServerIsAuth(server utils.NetAddr) bool {
 // registerThroughProxy is used to register through the proxy server.
 func registerThroughProxy(token string, params RegisterParams) (*proto.Certs, error) {
 	var certs *proto.Certs
+
 	switch params.JoinMethod {
 	case types.JoinMethodIAM, types.JoinMethodAzure:
 		// IAM and Azure join methods require gRPC client
-		conn, err := proxyJoinServiceConn(params, lib.IsInsecureDevMode())
+		conn, err := proxyJoinServiceConn(params, params.Insecure)
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}
@@ -333,7 +335,7 @@ func registerThroughProxy(token string, params RegisterParams) (*proto.Certs, er
 		var err error
 		certs, err = params.GetHostCredentials(context.Background(),
 			getHostAddresses(params)[0],
-			lib.IsInsecureDevMode(),
+			params.Insecure,
 			types.RegisterUsingTokenRequest{
 				Token:                token,
 				HostID:               params.ID.HostUUID,
@@ -367,13 +369,22 @@ func registerThroughAuth(token string, params RegisterParams) (*proto.Certs, err
 	var client *Client
 	var err error
 
-	// Build a client to the Auth Server. If a CA pin is specified require the
-	// Auth Server is validated. Otherwise attempt to use the CA file on disk
-	// but if it's not available connect without validating the Auth Server CA.
+	// Build a client for the Auth Server with different certificate validation
+	// depending on the configured values for Insecure, CAPins and CAPath.
 	switch {
+	case params.Insecure:
+		log.Warnf("Insecure mode enabled. Auth Server cert will not be validated and CAPins and CAPath value will be ignored.")
+		client, err = insecureRegisterClient(params)
 	case len(params.CAPins) != 0:
+		// CAPins takes precedence over CAPath
 		client, err = pinRegisterClient(params)
+	case params.CAPath != "":
+		client, err = caPathRegisterClient(params)
 	default:
+		// We fall back to insecure mode here - this is a little odd but is
+		// necessary to preserve the behavior of registration. At a later date,
+		// we may consider making this an error asking the user to provide
+		// Insecure, CAPins or CAPath.
 		client, err = insecureRegisterClient(params)
 	}
 	if err != nil {
@@ -481,31 +492,15 @@ func verifyALPNUpgradedConn(clock clockwork.Clock) func(tls.ConnectionState) err
 // CA on disk. If no CA is found on disk, Teleport will not verify the Auth
 // Server it is connecting to.
 func insecureRegisterClient(params RegisterParams) (*Client, error) {
-	tlsConfig := utils.TLSConfig(params.CipherSuites)
-	tlsConfig.Time = params.Clock.Now
-
-	cert, err := readCA(params.CAPath)
-	if err != nil && !trace.IsNotFound(err) {
-		return nil, trace.Wrap(err)
-	}
-
-	// If no CA was found, then create a insecure connection to the Auth Server,
-	// otherwise use the CA on disk to validate the Auth Server.
-	if trace.IsNotFound(err) {
-		tlsConfig.InsecureSkipVerify = true
-
 	log.Warnf("Joining cluster without validating the identity of the Auth " +
 		"Server. This may open you up to a Man-In-The-Middle (MITM) attack if an " +
 		"attacker can gain privileged network access. To remedy this, use the CA pin " +
 		"value provided when join token was generated to validate the identity of " +
-			"the Auth Server.")
-	} else {
-		certPool := x509.NewCertPool()
-		certPool.AddCert(cert)
-		tlsConfig.RootCAs = certPool
+		"the Auth Server or point to a valid Certificate via the CA Path option.")
 
-		log.Infof("Joining remote cluster %v, validating connection with certificate on disk.", cert.Subject.CommonName)
-	}
+	tlsConfig := utils.TLSConfig(params.CipherSuites)
+	tlsConfig.Time = params.Clock.Now
+	tlsConfig.InsecureSkipVerify = true
 
 	client, err := NewClient(client.Config{
 		Addrs: getHostAddresses(params),
@@ -611,6 +606,44 @@ func pinRegisterClient(params RegisterParams) (*Client, error) {
 	return authClient, nil
 }
 
+func caPathRegisterClient(params RegisterParams) (*Client, error) {
+	tlsConfig := utils.TLSConfig(params.CipherSuites)
+	tlsConfig.Time = params.Clock.Now
+
+	cert, err := readCA(params.CAPath)
+	if err != nil && !trace.IsNotFound(err) {
+		return nil, trace.Wrap(err)
+	}
+
+	// If we're unable to read the file at CAPath, we fall back to insecure
+	// registration. This preserves the existing behavior. At a later date,
+	// we may wish to consider changing this to return an error - but this is a
+	// breaking change.
+	if trace.IsNotFound(err) {
+		log.Warnf("Falling back to insecurely joining because a missing or empty CA Path was provided.")
+		return insecureRegisterClient(params)
+	}
+
+	certPool := x509.NewCertPool()
+	certPool.AddCert(cert)
+	tlsConfig.RootCAs = certPool
+
+	log.Infof("Joining remote cluster %v, validating connection with certificate on disk.", cert.Subject.CommonName)
+
+	client, err := NewClient(client.Config{
+		Addrs: getHostAddresses(params),
+		Credentials: []client.Credentials{
+			client.LoadTLS(tlsConfig),
+		},
+		CircuitBreakerConfig: params.CircuitBreakerConfig,
+	})
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	return client, nil
+}
+
 type joinServiceClient interface {
 	RegisterUsingIAMMethod(ctx context.Context, challengeResponse client.RegisterIAMChallengeResponseFunc) (*proto.Certs, error)
 	RegisterUsingAzureMethod(ctx context.Context, challengeResponse client.RegisterAzureChallengeResponseFunc) (*proto.Certs, error)

lib/service/connect.go

@@ -491,6 +491,7 @@ func (process *TeleportProcess) firstTimeConnect(role types.SystemRole) (*Connec
 			JoinMethod:           process.Config.JoinMethod,
 			CircuitBreakerConfig: process.Config.CircuitBreakerConfig,
 			FIPS:                 process.Config.FIPS,
+			Insecure:             lib.IsInsecureDevMode(),
 		}
 		if registerParams.JoinMethod == types.JoinMethodAzure {
 			registerParams.AzureParams = auth.AzureParams{

lib/tbot/bot_identity.go

@@ -222,7 +222,9 @@ func botIdentityFromToken(log logrus.FieldLogger, cfg *config.BotConfig) (*ident
 		Expires:            &expires,
 		FIPS:               cfg.FIPS,
 		CipherSuites:       cfg.CipherSuites(),
+		Insecure:           cfg.Insecure,
 	}
+
 	if params.JoinMethod == types.JoinMethodAzure {
 		params.AzureParams = auth.AzureParams{
 			ClientID: cfg.Onboarding.Azure.ClientID,

lib/tbot/ca_rotation_test.go

@@ -285,7 +285,12 @@ func TestBot_Run_CARotation(t *testing.T) {
 
 	// Make and join a new bot instance.
 	botParams := testhelpers.MakeBot(t, client, "test", "access")
-	botConfig := testhelpers.DefaultBotConfig(t, fc, botParams, nil)
+	botConfig := testhelpers.DefaultBotConfig(t, fc, botParams, nil,
+		testhelpers.DefaultBotConfigOpts{
+			UseAuthServer: true,
+			Insecure:      true,
+		},
+	)
 	b := New(botConfig, log)
 
 	wg.Add(1)

lib/tbot/config/config.go

@@ -176,6 +176,9 @@ type CLIConf struct {
 	// DiagAddr is the address the diagnostics http service should listen on.
 	// If not set, no diagnostics listener is created.
 	DiagAddr string
+
+	// Insecure instructs `tbot` to trust the Auth Server without verifying the CA.
+	Insecure bool
 }
 
 // AzureOnboardingConfig holds configuration relevant to the "azure" join method.
@@ -275,9 +278,9 @@ type BotConfig struct {
 	// renewal.
 	ReloadCh <-chan struct{} `yaml:"-"`
 
-	// Insecure configures the bot to blindly trust the certificates offered by
-	// the auth server. Used for tests.
-	Insecure bool `yaml:"-"`
+	// Insecure configures the bot to trust the certificates from the Auth Server or Proxy on first connect without verification.
+	// Do not use in production.
+	Insecure bool `yaml:"insecure,omitempty"`
 }
 
 func (conf *BotConfig) CipherSuites() []uint16 {
@@ -346,6 +349,21 @@ func (conf *BotConfig) CheckAndSetDefaults() error {
 		}
 	}
 
+	// Validate Insecure and CA Settings
+	if conf.Insecure {
+		if len(conf.Onboarding.CAPins) > 0 {
+			return trace.BadParameter("the option ca-pin is mutually exclusive with --insecure")
+		}
+
+		if conf.Onboarding.CAPath != "" {
+			return trace.BadParameter("the option ca-path is mutually exclusive with --insecure")
+		}
+	} else {
+		if len(conf.Onboarding.CAPins) > 0 && conf.Onboarding.CAPath != "" {
+			return trace.BadParameter("the options ca-pin and ca-path are mutually exclusive")
+		}
+	}
+
 	return nil
 }
 
@@ -635,6 +653,10 @@ func FromCLIConf(cf *CLIConf) (*BotConfig, error) {
 		return nil, trace.Wrap(err, "validating merged bot config")
 	}
 
+	if cf.Insecure {
+		config.Insecure = true
+	}
+
 	return config, nil
 }
 

lib/tbot/config/config_test.go

@@ -300,3 +300,37 @@ func testYAML[T any](t *testing.T, tests []testYAMLCase[T]) {
 		})
 	}
 }
+
+func TestBotConfig_InsecureWithCAPins(t *testing.T) {
+	cfg := &BotConfig{
+		Insecure: true,
+		Onboarding: OnboardingConfig{
+			CAPins: []string{"123"},
+		},
+	}
+
+	require.ErrorContains(t, cfg.CheckAndSetDefaults(), "ca-pin")
+}
+
+func TestBotConfig_InsecureWithCAPath(t *testing.T) {
+	cfg := &BotConfig{
+		Insecure: true,
+		Onboarding: OnboardingConfig{
+			CAPath: "/tmp/invalid-path/some.crt",
+		},
+	}
+
+	require.ErrorContains(t, cfg.CheckAndSetDefaults(), "ca-path")
+}
+
+func TestBotConfig_WithCAPathAndCAPins(t *testing.T) {
+	cfg := &BotConfig{
+		Insecure: false,
+		Onboarding: OnboardingConfig{
+			CAPath: "/tmp/invalid-path/some.crt",
+			CAPins: []string{"123"},
+		},
+	}
+
+	require.ErrorContains(t, cfg.CheckAndSetDefaults(), "mutually exclusive")
+}

lib/tbot/tbot.go

@@ -439,6 +439,8 @@ func (b *Bot) AuthenticatedUserClientFromIdentity(ctx context.Context, id *ident
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	// Pass through the Insecure mode flag so that the webapi/find call works with a self-signed certificate as expected.
+	tlsConfig.InsecureSkipVerify = b.cfg.Insecure
 
 	sshConfig, err := id.SSHClientConfig(b.cfg.FIPS)
 	if err != nil {

lib/tbot/tbot_test.go

@@ -218,6 +218,10 @@ func TestBot(t *testing.T) {
 			sshHostOutput,
 			kubeOutput,
 		},
+		testhelpers.DefaultBotConfigOpts{
+			UseAuthServer: true,
+			Insecure:      true,
+		},
 	)
 	b := New(botConfig, log)
 	require.NoError(t, b.Run(ctx))
@@ -369,7 +373,13 @@ func TestBot_ResumeFromStorage(t *testing.T) {
 	// Create bot user and join token
 	botParams := testhelpers.MakeBot(t, rootClient, "test", "access")
 
-	botConfig := testhelpers.DefaultBotConfig(t, fc, botParams, []config.Output{})
+	botConfig := testhelpers.DefaultBotConfig(t, fc, botParams, []config.Output{},
+		testhelpers.DefaultBotConfigOpts{
+			UseAuthServer: true,
+			Insecure:      true,
+		},
+	)
+
 	// Use a destination directory to ensure locking behaves correctly and
 	// the bot isn't left in a locked state.
 	directoryDest := &config.DestinationDirectory{
@@ -394,3 +404,36 @@ func TestBot_ResumeFromStorage(t *testing.T) {
 	thirdBot := New(botConfig, log)
 	require.NoError(t, thirdBot.Run(ctx))
 }
+
+func TestBot_InsecureViaProxy(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	log := utils.NewLoggerForTests()
+
+	// Make a new auth server.
+	fc, fds := testhelpers.DefaultConfig(t)
+	_ = testhelpers.MakeAndRunTestAuthServer(t, log, fc, fds)
+	rootClient := testhelpers.MakeDefaultAuthClient(t, log, fc)
+
+	// Create bot user and join token
+	botParams := testhelpers.MakeBot(t, rootClient, "test", "access")
+
+	botConfig := testhelpers.DefaultBotConfig(t, fc, botParams, []config.Output{},
+		testhelpers.DefaultBotConfigOpts{
+			UseAuthServer: false,
+			Insecure:      true,
+		},
+	)
+	// Use a destination directory to ensure locking behaves correctly and
+	// the bot isn't left in a locked state.
+	directoryDest := &config.DestinationDirectory{
+		Path:     t.TempDir(),
+		Symlinks: botfs.SymlinksInsecure,
+		ACLs:     botfs.ACLOff,
+	}
+	botConfig.Storage.Destination = directoryDest
+
+	// Run the bot a first time
+	firstBot := New(botConfig, log)
+	require.NoError(t, firstBot.Run(ctx))
+}

lib/tbot/testhelpers/srv.go

@@ -37,6 +37,14 @@ import (
 	"github.com/gravitational/teleport/tool/teleport/testenv"
 )
 
+type DefaultBotConfigOpts struct {
+	// Makes the bot connect via the Auth Server instead of the Proxy server.
+	UseAuthServer bool
+
+	// Makes the bot accept an Insecure auth or proxy server
+	Insecure bool
+}
+
 // DefaultConfig returns a FileConfig to be used in tests, with random listen
 // addresses that are tied to the listeners returned in the FileDescriptor
 // slice, which should be passed as exported file descriptors to NewTeleport;
@@ -167,7 +175,7 @@ func MakeBot(t *testing.T, client auth.ClientI, name string, roles ...string) *p
 // - Uses a memory storage destination
 // - Does not verify Proxy WebAPI certificates
 func DefaultBotConfig(
-	t *testing.T, fc *config.FileConfig, botParams *proto.CreateBotResponse, outputs []botconfig.Output,
+	t *testing.T, fc *config.FileConfig, botParams *proto.CreateBotResponse, outputs []botconfig.Output, opts DefaultBotConfigOpts,
 ) *botconfig.BotConfig {
 	t.Helper()
 
@@ -175,8 +183,13 @@ func DefaultBotConfig(
 	err := config.ApplyFileConfig(fc, authCfg)
 	require.NoError(t, err)
 
+	var authServer = authCfg.Proxy.WebAddr.String()
+	if opts.UseAuthServer {
+		authServer = authCfg.AuthServerAddresses()[0].String()
+	}
+
 	cfg := &botconfig.BotConfig{
-		AuthServer: authCfg.AuthServerAddresses()[0].String(),
+		AuthServer: authServer,
 		Onboarding: botconfig.OnboardingConfig{
 			JoinMethod: botParams.JoinMethod,
 		},
@@ -187,7 +200,7 @@ func DefaultBotConfig(
 		Outputs: outputs,
 		// Set Insecure so the bot will trust the Proxy's webapi default signed
 		// certs.
-		Insecure: true,
+		Insecure: opts.Insecure,
 	}
 
 	cfg.Onboarding.SetToken(botParams.TokenID)

tool/tbot/main.go

@@ -84,6 +84,7 @@ func Run(args []string, stdout io.Writer) error {
 	startCmd.Flag("destination-dir", "Directory to write short-lived machine certificates.").StringVar(&cf.DestinationDir)
 	startCmd.Flag("certificate-ttl", "TTL of short-lived machine certificates.").DurationVar(&cf.CertificateTTL)
 	startCmd.Flag("renewal-interval", "Interval at which short-lived certificates are renewed; must be less than the certificate TTL.").DurationVar(&cf.RenewalInterval)
+	startCmd.Flag("insecure", "Insecure configures the bot to trust the certificates from the Auth Server or Proxy on first connect without verification. Do not use in production.").BoolVar(&cf.Insecure)
 	startCmd.Flag("join-method", "Method to use to join the cluster. "+joinMethodList).EnumVar(&cf.JoinMethod, config.SupportedJoinMethods...)
 	startCmd.Flag("oneshot", "If set, quit after the first renewal.").BoolVar(&cf.Oneshot)
 	startCmd.Flag("diag-addr", "If set and the bot is in debug mode, a diagnostics service will listen on specified address.").StringVar(&cf.DiagAddr)
@@ -107,6 +108,7 @@ func Run(args []string, stdout io.Writer) error {
 	configureCmd.Flag("ca-pin", "CA pin to validate the Teleport Auth Server; used on first connect.").StringsVar(&cf.CAPins)
 	configureCmd.Flag("certificate-ttl", "TTL of short-lived machine certificates.").Default("60m").DurationVar(&cf.CertificateTTL)
 	configureCmd.Flag("data-dir", "Directory to store internal bot data. Access to this directory should be limited.").StringVar(&cf.DataDir)
+	configureCmd.Flag("insecure", "Insecure configures the bot to trust the certificates from the Auth Server or Proxy on first connect without verification. Do not use in production.").BoolVar(&cf.Insecure)
 	configureCmd.Flag("join-method", "Method to use to join the cluster. "+joinMethodList).EnumVar(&cf.JoinMethod, config.SupportedJoinMethods...)
 	configureCmd.Flag("oneshot", "If set, quit after the first renewal.").BoolVar(&cf.Oneshot)
 	configureCmd.Flag("renewal-interval", "Interval at which short-lived certificates are renewed; must be less than the certificate TTL.").DurationVar(&cf.RenewalInterval)