Skip to content

Commit

Permalink
Adds concurrent default-port selection to tsh
Browse files Browse the repository at this point in the history 
Addresses issue #4924

If a default Web Proxy port is not specified by the user, either via
config or on the command line, `tsh` defaults to `3080`. Unfortunately
`3080` is often blocked by firewalls, leading to an unacceptably long
timeout for the user.

This change adds an RFC8305-like default-port selection algorithm,
that will try multiple ports on the supplied host concurrently and
select the most reponsive address to use for Web Proxy traffic. I
have included the standard HTTPS port (443) in the defaulut set,
and this can be easily expanded if other good candidates come along.

If the port selection fails for any reason, `tsh` reverts to the
legacy behaviour of picking `3080` automatically.
  • Loading branch information
@tcsc
tcsc committed Apr 11, 2021
1 parent 47fa2f9 commit 20eb18f
Show file tree
Hide file tree
Showing 6 changed files with 515 additions and 61 deletions.
85 changes: 56 additions & 29 deletions lib/client/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -789,49 +789,76 @@ func (c *Config) SaveProfile(dir string, makeCurrent bool) error {
return nil
}

// ParseProxyHost parses the proxyHost string and updates the config.
type ParsedProxyHost struct {
Host string
WebProxyAddr string
SSHProxyAddr string
}

// ParseProxyHost parses a ProxyHost string of the format <hostname>:<proxy_web_port>,<proxy_ssh_port>
// and returns the parsed components.
//
// Format of proxyHost string:
// proxy_web_addr:<proxy_web_port>,<proxy_ssh_port>
func (c *Config) ParseProxyHost(proxyHost string) error {
// If a definitive answer is not possible (e.g. no proxy port is specified in
// the supplied string), the empty string is returned
func ParseProxyHost(proxyHost string) (ParsedProxyHost, error) {
host, port, err := net.SplitHostPort(proxyHost)
if err != nil {
host = proxyHost
port = ""
}

result := ParsedProxyHost{Host: host}

// Split on comma.
parts := strings.Split(port, ",")

webPort := ""
sshPort := strconv.Itoa(defaults.SSHProxyListenPort)

switch {
// Default ports for both the SSH and Web proxy.
case len(parts) == 0:
c.WebProxyAddr = net.JoinHostPort(host, strconv.Itoa(defaults.HTTPListenPort))
c.SSHProxyAddr = net.JoinHostPort(host, strconv.Itoa(defaults.SSHProxyListenPort))
break

// User defined HTTP proxy port, default SSH proxy port.
case len(parts) == 1:
webPort := parts[0]
if webPort == "" {
webPort = strconv.Itoa(defaults.HTTPListenPort)
}
c.WebProxyAddr = net.JoinHostPort(host, webPort)
c.SSHProxyAddr = net.JoinHostPort(host, strconv.Itoa(defaults.SSHProxyListenPort))
webPort = parts[0]

// User defined HTTP and SSH proxy ports.
case len(parts) == 2:
webPort := parts[0]
if webPort == "" {
webPort = strconv.Itoa(defaults.HTTPListenPort)
}
sshPort := parts[1]
if sshPort == "" {
sshPort = strconv.Itoa(defaults.SSHProxyListenPort)
}
c.WebProxyAddr = net.JoinHostPort(host, webPort)
c.SSHProxyAddr = net.JoinHostPort(host, sshPort)
webPort, sshPort = parts[0], parts[1]

default:
return trace.BadParameter("unable to parse port: %v", port)
return result, trace.BadParameter("unable to parse port: %v", port)
}

if webPort != "" {
result.WebProxyAddr = net.JoinHostPort(host, webPort)
}
result.SSHProxyAddr = net.JoinHostPort(host, sshPort)

return result, nil
}

// ParseProxyHost parses the proxyHost string and updates the config.
//
// Format of proxyHost string:
// proxy_web_addr:<proxy_web_port>,<proxy_ssh_port>
func (c *Config) ParseProxyHost(proxyHost string) error {
parsedAddrs, err := ParseProxyHost(proxyHost)
if err != nil {
return err
}

// if the parser wasn't able to unambiguously provide a WebProxyAddr,
// then we just pick the standard teleport one and be done with it.
if parsedAddrs.WebProxyAddr == "" {
parsedAddrs.WebProxyAddr = net.JoinHostPort(
parsedAddrs.Host, strconv.Itoa(defaults.HTTPListenPort))
}

c.WebProxyAddr = parsedAddrs.WebProxyAddr
c.SSHProxyAddr = parsedAddrs.SSHProxyAddr
return nil
}

Expand Down Expand Up @@ -2264,7 +2291,7 @@ func (tc *TeleportClient) Ping(ctx context.Context) (*client.PingResponse, error
ctx,
tc.WebProxyAddr,
tc.InsecureSkipVerify,
loopbackPool(tc.WebProxyAddr),
LoopbackPool(tc.WebProxyAddr),
tc.AuthConnector)
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -2520,7 +2547,7 @@ func (tc *TeleportClient) directLogin(ctx context.Context, secondFactorType cons
PubKey: pub,
TTL: tc.KeyTTL,
Insecure: tc.InsecureSkipVerify,
Pool: loopbackPool(tc.WebProxyAddr),
Pool: LoopbackPool(tc.WebProxyAddr),
Compatibility: tc.CertificateFormat,
RouteToCluster: tc.SiteName,
KubernetesCluster: tc.KubernetesCluster,
Expand Down Expand Up @@ -2549,7 +2576,7 @@ func (tc *TeleportClient) ssoLogin(ctx context.Context, connectorID string, pub
PubKey: pub,
TTL: tc.KeyTTL,
Insecure: tc.InsecureSkipVerify,
Pool: loopbackPool(tc.WebProxyAddr),
Pool: LoopbackPool(tc.WebProxyAddr),
Compatibility: tc.CertificateFormat,
RouteToCluster: tc.SiteName,
KubernetesCluster: tc.KubernetesCluster,
Expand All @@ -2575,7 +2602,7 @@ func (tc *TeleportClient) mfaLocalLogin(ctx context.Context, pub []byte) (*auth.
PubKey: pub,
TTL: tc.KeyTTL,
Insecure: tc.InsecureSkipVerify,
Pool: loopbackPool(tc.WebProxyAddr),
Pool: LoopbackPool(tc.WebProxyAddr),
Compatibility: tc.CertificateFormat,
RouteToCluster: tc.SiteName,
KubernetesCluster: tc.KubernetesCluster,
Expand Down Expand Up @@ -2605,9 +2632,9 @@ func (tc *TeleportClient) EventsChannel() <-chan events.EventFields {
return tc.eventsCh
}

// loopbackPool reads trusted CAs if it finds it in a predefined location
// LoopbackPool reads trusted CAs if it finds it in a predefined location
// and will work only if target proxy address is loopback
func loopbackPool(proxyAddr string) *x509.CertPool {
func LoopbackPool(proxyAddr string) *x509.CertPool {
if !utils.IsLoopback(proxyAddr) {
log.Debugf("not using loopback pool for remote proxy addr: %v", proxyAddr)
return nil
Expand Down
82 changes: 56 additions & 26 deletions lib/client/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

"github.com/gravitational/teleport/lib/utils"
"github.com/stretchr/testify/require"

"gopkg.in/check.v1"
)
Expand All @@ -39,34 +40,63 @@ func TestClientAPI(t *testing.T) { check.TestingT(t) }

var _ = check.Suite(&APITestSuite{})

func (s *APITestSuite) TestConfig(c *check.C) {
var conf Config
c.Assert(conf.ProxySpecified(), check.Equals, false)
err := conf.ParseProxyHost("example.org")
c.Assert(err, check.IsNil)
c.Assert(conf.ProxySpecified(), check.Equals, true)
c.Assert(conf.SSHProxyAddr, check.Equals, "example.org:3023")
c.Assert(conf.WebProxyAddr, check.Equals, "example.org:3080")

conf.WebProxyAddr = "example.org:100"
conf.SSHProxyAddr = "example.org:200"
c.Assert(conf.WebProxyAddr, check.Equals, "example.org:100")
c.Assert(conf.SSHProxyAddr, check.Equals, "example.org:200")

err = conf.ParseProxyHost("example.org:200")
c.Assert(err, check.IsNil)
c.Assert(conf.WebProxyAddr, check.Equals, "example.org:200")
c.Assert(conf.SSHProxyAddr, check.Equals, "example.org:3023")
func TestParseProxyHostString(t *testing.T) {
testCases := []struct {
name string
input string
assertErr require.ErrorAssertionFunc
expect ParsedProxyHost
}{
{
name: "Empty port string",
input: "example.org",
assertErr: require.NoError,
expect: ParsedProxyHost{
Host: "example.org",
WebProxyAddr: "",
SSHProxyAddr: "example.org:3023",
},
}, {
name: "Proxy port only",
input: "example.org:1234",
assertErr: require.NoError,
expect: ParsedProxyHost{
Host: "example.org",
WebProxyAddr: "example.org:1234",
SSHProxyAddr: "example.org:3023",
},
}, {
name: "SSH port only",
input: "example.org:,200",
assertErr: require.NoError,
expect: ParsedProxyHost{
Host: "example.org",
WebProxyAddr: "",
SSHProxyAddr: "example.org:200",
},
}, {
name: "Both ports specified",
input: "example.org:100,200",
assertErr: require.NoError,
expect: ParsedProxyHost{
Host: "example.org",
WebProxyAddr: "example.org:100",
SSHProxyAddr: "example.org:200",
},
},
}

err = conf.ParseProxyHost("example.org:,200")
c.Assert(err, check.IsNil)
c.Assert(conf.SSHProxyAddr, check.Equals, "example.org:200")
c.Assert(conf.WebProxyAddr, check.Equals, "example.org:3080")
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
expected := testCase.expect
actual, err := ParseProxyHost(testCase.input)
testCase.assertErr(t, err)

conf.WebProxyAddr = "example.org:100"
conf.SSHProxyAddr = "example.org:200"
c.Assert(conf.WebProxyAddr, check.Equals, "example.org:100")
c.Assert(conf.SSHProxyAddr, check.Equals, "example.org:200")
require.Equal(t, expected.Host, actual.Host)
require.Equal(t, expected.WebProxyAddr, actual.WebProxyAddr)
require.Equal(t, expected.SSHProxyAddr, actual.SSHProxyAddr)
})
}
}

func (s *APITestSuite) TestNew(c *check.C) {
Expand Down
4 changes: 4 additions & 0 deletions lib/defaults/defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ const (

// NodeJoinTokenTTL is when a token for nodes expires.
NodeJoinTokenTTL = 4 * time.Hour

// StandardHTTPSListenPort is the standard port used by HTTPS servers, and
// is the implied port in an HTTP url if no port is specified.
StandardHTTPSListenPort = 443
)

var (
Expand Down
Loading

0 comments on commit 20eb18f

Please sign in to comment.