Skip to content

Commit

Permalink
[v15] Machine ID: Support overriding Proxy address from ProxyPing whe…
Browse files Browse the repository at this point in the history
…n using TLS routing (#48677)

* Machine ID: Support overriding Proxy address from ProxyPing when using TLS routing (#48373)

* Start working on ability to use explicit proxy address

* Convert more call-sites to use new helper

* Convert final callsite

* Simplify implementation

* Fix teests

* Remove additional check

* Fix dialling auth client

* Update env var name

* Machine ID: TBOT_USE_PROXY_ADDR use "yes" vs "1" for consistency (#48667)

* Prefer "yes" over "1" for historical consistency

* Try and retrigger the CLA check with an empty commit
  • Loading branch information
strideynet authored Nov 8, 2024
1 parent 8262e39 commit 05f44b6
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 35 deletions.
5 changes: 4 additions & 1 deletion lib/tbot/service_application_tunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,10 @@ func (s *ApplicationTunnelService) buildLocalProxyConfig(ctx context.Context) (l
if err != nil {
return alpnproxy.LocalProxyConfig{}, trace.Wrap(err, "pinging proxy")
}
proxyAddr := proxyPing.Proxy.SSH.PublicAddr
proxyAddr, err := proxyPing.proxyWebAddr()
if err != nil {
return alpnproxy.LocalProxyConfig{}, trace.Wrap(err, "determining proxy web addr")
}

s.log.DebugContext(ctx, "Issuing initial certificate for local proxy.")
appCert, app, err := s.issueCert(ctx, roles)
Expand Down
5 changes: 4 additions & 1 deletion lib/tbot/service_database_tunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ func (s *DatabaseTunnelService) buildLocalProxyConfig(ctx context.Context) (lpCf
if err != nil {
return alpnproxy.LocalProxyConfig{}, trace.Wrap(err, "pinging proxy")
}
proxyAddr := proxyPing.Proxy.SSH.PublicAddr
proxyAddr, err := proxyPing.proxyWebAddr()
if err != nil {
return alpnproxy.LocalProxyConfig{}, trace.Wrap(err, "determining proxy web address")
}

// Fetch information about the database and then issue the initial
// certificate. We issue the initial certificate to allow us to fail faster.
Expand Down
14 changes: 9 additions & 5 deletions lib/tbot/service_identity_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/gravitational/trace"

"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/client/webclient"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/auth/authclient"
"github.com/gravitational/teleport/lib/config/openssh"
Expand Down Expand Up @@ -251,7 +250,7 @@ type alpnTester interface {
func renderSSHConfig(
ctx context.Context,
log *slog.Logger,
proxyPing *webclient.PingResponse,
proxyPing *proxyPingResponse,
clusterNames []string,
dest bot.Destination,
certAuthGetter certAuthGetter,
Expand All @@ -267,11 +266,16 @@ func renderSSHConfig(
)
defer span.End()

proxyHost, proxyPort, err := utils.SplitHostPort(proxyPing.Proxy.SSH.PublicAddr)
proxyAddr, err := proxyPing.proxyWebAddr()
if err != nil {
return trace.Wrap(err, "determining proxy web addr")
}

proxyHost, proxyPort, err := utils.SplitHostPort(proxyAddr)
if err != nil {
return trace.BadParameter(
"proxy %+v has no usable public address: %v",
proxyPing.Proxy.SSH.PublicAddr, err,
proxyAddr, err,
)
}

Expand Down Expand Up @@ -327,7 +331,7 @@ func renderSSHConfig(
connUpgradeRequired := false
if proxyPing.Proxy.TLSRoutingEnabled {
connUpgradeRequired, err = alpnTester.isUpgradeRequired(
ctx, proxyPing.Proxy.SSH.PublicAddr, botCfg.Insecure,
ctx, proxyAddr, botCfg.Insecure,
)
if err != nil {
return trace.Wrap(err, "determining if ALPN upgrade is required")
Expand Down
14 changes: 8 additions & 6 deletions lib/tbot/service_identity_output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,14 @@ func Test_renderSSHConfig(t *testing.T) {
err := renderSSHConfig(
context.Background(),
utils.NewSlogLoggerForTests(),
&webclient.PingResponse{
ClusterName: mockClusterName,
Proxy: webclient.ProxySettings{
TLSRoutingEnabled: tc.TLSRouting,
SSH: webclient.SSHProxySettings{
PublicAddr: mockProxyAddr,
&proxyPingResponse{
PingResponse: &webclient.PingResponse{
ClusterName: mockClusterName,
Proxy: webclient.ProxySettings{
TLSRoutingEnabled: tc.TLSRouting,
SSH: webclient.SSHProxySettings{
PublicAddr: mockProxyAddr,
},
},
},
},
Expand Down
10 changes: 6 additions & 4 deletions lib/tbot/service_kubernetes_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (

apiclient "github.com/gravitational/teleport/api/client"
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/client/webclient"
"github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/auth/authclient"
Expand Down Expand Up @@ -418,7 +417,7 @@ func getKubeCluster(ctx context.Context, clt *authclient.Client, name string) (t

// selectKubeConnectionMethod determines the address and SNI that should be
// put into the kubeconfig file.
func selectKubeConnectionMethod(proxyPong *webclient.PingResponse) (
func selectKubeConnectionMethod(proxyPong *proxyPingResponse) (
clusterAddr string, sni string, err error,
) {
// First we check for TLS routing. If this is enabled, we use the Proxy's
Expand All @@ -427,8 +426,11 @@ func selectKubeConnectionMethod(proxyPong *webclient.PingResponse) (
// Even if KubePublicAddr is specified, we still use the general
// PublicAddr when using TLS routing.
if proxyPong.Proxy.TLSRoutingEnabled {
addr := proxyPong.Proxy.SSH.PublicAddr
host, _, err := net.SplitHostPort(proxyPong.Proxy.SSH.PublicAddr)
addr, err := proxyPong.proxyWebAddr()
if err != nil {
return "", "", trace.Wrap(err)
}
host, _, err := net.SplitHostPort(addr)
if err != nil {
return "", "", trace.Wrap(err, "parsing proxy public_addr")
}
Expand Down
4 changes: 3 additions & 1 deletion lib/tbot/service_kubernetes_output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,9 @@ func Test_selectKubeConnectionMethod(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
addr, sni, err := selectKubeConnectionMethod(tt.proxyPing)
addr, sni, err := selectKubeConnectionMethod(&proxyPingResponse{
PingResponse: tt.proxyPing,
})
require.NoError(t, err)
require.Equal(t, tt.wantAddr, addr)
require.Equal(t, tt.wantSNI, sni)
Expand Down
8 changes: 6 additions & 2 deletions lib/tbot/service_ssh_multiplexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,15 @@ func (s *SSHMultiplexerService) setup(ctx context.Context) (
if err != nil {
return nil, nil, "", nil, trace.Wrap(err)
}
proxyAddr := proxyPing.Proxy.SSH.PublicAddr
proxyHost, _, err = net.SplitHostPort(proxyPing.Proxy.SSH.PublicAddr)
proxyAddr, err := proxyPing.proxyWebAddr()
if err != nil {
return nil, nil, "", nil, trace.Wrap(err, "determining proxy web addr")
}
proxyHost, _, err = net.SplitHostPort(proxyAddr)
if err != nil {
return nil, nil, "", nil, trace.Wrap(err)
}

connUpgradeRequired := false
if proxyPing.Proxy.TLSRoutingEnabled {
connUpgradeRequired, err = s.alpnUpgradeCache.isUpgradeRequired(
Expand Down
85 changes: 70 additions & 15 deletions lib/tbot/tbot.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,32 @@ func (b *Bot) Run(ctx context.Context) (err error) {
return trace.Wrap(err)
}

addr, _ := b.cfg.Address()
resolver, err := reversetunnelclient.CachingResolver(
ctx,
reversetunnelclient.WebClientResolver(&webclient.Config{
Context: ctx,
ProxyAddr: addr,
Insecure: b.cfg.Insecure,
}),
nil /* clock */)
if err != nil {
return trace.Wrap(err)
addr, addrKind := b.cfg.Address()
var resolver reversetunnelclient.Resolver
if shouldUseProxyAddr() {
if addrKind != config.AddressKindProxy {
return trace.BadParameter("TBOT_USE_PROXY_ADDR requires that a proxy address is set using --proxy-server or proxy_server")
}
// If the user has indicated they want tbot to prefer using the proxy
// address they have configured, we use a static resolver set to this
// address. We also assume that they have TLS routing/multiplexing
// enabled, since otherwise we'd need them to manually configure an
// an entry for each kind of address.
resolver = reversetunnelclient.StaticResolver(
addr, types.ProxyListenerMode_Multiplex,
)
} else {
resolver, err = reversetunnelclient.CachingResolver(
ctx,
reversetunnelclient.WebClientResolver(&webclient.Config{
Context: ctx,
ProxyAddr: addr,
Insecure: b.cfg.Insecure,
}),
nil /* clock */)
if err != nil {
return trace.Wrap(err)
}
}

// Create an error group to manage all the services lifetimes.
Expand Down Expand Up @@ -512,7 +527,8 @@ func (b *Bot) preRunChecks(ctx context.Context) (_ func() error, err error) {
ctx, span := tracer.Start(ctx, "Bot/preRunChecks")
defer func() { apitracing.EndSpan(span, err) }()

switch _, addrKind := b.cfg.Address(); addrKind {
_, addrKind := b.cfg.Address()
switch addrKind {
case config.AddressKindUnspecified:
return nil, trace.BadParameter(
"either a proxy or auth address must be set using --proxy, --auth-server or configuration",
Expand Down Expand Up @@ -716,10 +732,10 @@ type proxyPingCache struct {
log *slog.Logger

mu sync.RWMutex
cachedValue *webclient.PingResponse
cachedValue *proxyPingResponse
}

func (p *proxyPingCache) ping(ctx context.Context) (*webclient.PingResponse, error) {
func (p *proxyPingCache) ping(ctx context.Context) (*proxyPingResponse, error) {
p.mu.Lock()
defer p.mu.Unlock()
if p.cachedValue != nil {
Expand Down Expand Up @@ -753,11 +769,50 @@ func (p *proxyPingCache) ping(ctx context.Context) (*webclient.PingResponse, err
return nil, trace.Wrap(err)
}
p.log.DebugContext(ctx, "Successfully pinged proxy.", "pong", res)
p.cachedValue = res
p.cachedValue = &proxyPingResponse{
PingResponse: res,
configuredProxyAddr: p.botCfg.ProxyServer,
}

return p.cachedValue, nil
}

type proxyPingResponse struct {
*webclient.PingResponse
configuredProxyAddr string
}

// useProxyAddrEnv is an environment variable which can be set to
// force `tbot` to prefer using the proxy address explicitly provided by the
// user over the one fetched from the proxy ping. This is only intended to work
// in cases where TLS routing is enabled, and is intended to support cases where
// the Proxy is accessible from multiple addresses, and the one included in the
// ProxyPing is incorrect.
const useProxyAddrEnv = "TBOT_USE_PROXY_ADDR"

// shouldUseProxyAddr returns true if the TBOT_USE_PROXY_ADDR environment
// variable is set to "yes". More generally, this indicates that the user wishes
// for tbot to prefer using the proxy address that has been explicitly provided
// by the user rather than the one fetched via a discovery process (e.g ping).
func shouldUseProxyAddr() bool {
return os.Getenv(useProxyAddrEnv) == "yes"
}

// proxyWebAddr returns the address to use to connect to the proxy web port.
// In TLS routing mode, this address should be used for most/all connections.
// This function takes into account the TBOT_USE_PROXY_ADDR environment
// variable, which can be used to force the use of the proxy address explicitly
// provided by the user rather than use the one fetched from the proxy ping.
func (p *proxyPingResponse) proxyWebAddr() (string, error) {
if shouldUseProxyAddr() {
if p.configuredProxyAddr == "" {
return "", trace.BadParameter("TBOT_USE_PROXY_ADDR set but no explicit proxy address configured")
}
return p.configuredProxyAddr, nil
}
return p.Proxy.SSH.PublicAddr, nil
}

type alpnProxyConnUpgradeRequiredCache struct {
botCfg *config.BotConfig
log *slog.Logger
Expand Down

0 comments on commit 05f44b6

Please sign in to comment.