Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IPv6 issues. #2357

Closed
wants to merge 1 commit into from
Closed

Fix IPv6 issues. #2357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/3.0/admin-guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ teleport:
# When running in multi-homed or NATed environments Teleport nodes need
# to know which IP it will be reachable at by other nodes
#
# This value can be specified as FQDN e.g. host.example.com
# This value can be specified as FQDN e.g. host.example.com. IPv6 addresses
# should be specified without brackets.
advertise_ip: 10.1.0.5

# list of auth servers in a cluster. you will have more than one auth server
Expand Down
9 changes: 7 additions & 2 deletions lib/client/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ func (tc *TeleportClient) LocalAgent() *LocalKeyAgent {
}

// getTargetNodes returns a list of node addresses this SSH command needs to
// operate on.
// operate on. If no nodes match, the host:port is returned precent encoded.
func (tc *TeleportClient) getTargetNodes(ctx context.Context, proxy *ProxyClient) ([]string, error) {
var (
err error
Expand All @@ -798,7 +798,12 @@ func (tc *TeleportClient) getTargetNodes(ctx context.Context, proxy *ProxyClient
}
}
if len(nodes) == 0 {
retval = append(retval, net.JoinHostPort(tc.Host, strconv.Itoa(tc.HostPort)))
// Precent code the host:port. This is done so the returned address can be
// passed to url.Parse where the host:port is placed within the userinfo
// part of the URL which only allows alpha numberic plus some punctuation.
// See the following for details: https://github.com/golang/go/issues/23392.
escaped := url.QueryEscape(net.JoinHostPort(tc.Host, strconv.Itoa(tc.HostPort)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to encapsulate this logic in the ssh subsystem call library that already deals with parsing @ instead of sprinkling this over the place. Also is escaping is really necessary? can we detect IPV6 address as is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue isn't the SSH subsystem, but that we pack an IPv6 address into the userinfo of a URL then try and call url.Parse() on it. The userinfo part of a URL can't contain [ and ], see the following ticket: golang/go#23392.

The other approach we can take is do some refactoring around how we store node/cluster info in a utils.NetAddr, however that seemed a more invasive fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not sure I understand, where do we treat this address as URL and why do we do it?

retval = append(retval, escaped)
}
return retval, nil
}
Expand Down
14 changes: 12 additions & 2 deletions lib/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"io"
"io/ioutil"
"net"
"net/url"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -329,8 +330,8 @@ func (proxy *ProxyClient) dialAuthServer(ctx context.Context, clusterName string
), nil
}

// ConnectToNode connects to the ssh server via Proxy.
// It returns connected and authenticated NodeClient
// ConnectToNode connects to the SSH server via Proxy. It returns a connected
// and authenticated NodeClient.
func (proxy *ProxyClient) ConnectToNode(ctx context.Context, nodeAddress string, user string, quiet bool) (*NodeClient, error) {
log.Infof("Client=%v connecting to node=%s", proxy.clientAddr, nodeAddress)

Expand All @@ -344,6 +345,15 @@ func (proxy *ProxyClient) ConnectToNode(ctx context.Context, nodeAddress string,
return nil, trace.Wrap(err)
}

// Unescape the node address in-case it is a percent encoded address (IPv6).
// This is because the host:port is placed within the userinfo
// part of the URL which only allows alpha numberic plus some punctuation.
// See the following for details: https://github.com/golang/go/issues/23392.
nodeAddress, err = url.QueryUnescape(nodeAddress)
if err != nil {
return nil, trace.Wrap(err)
}

// after auth but before we create the first session, find out if the proxy
// is in recording mode or not
recordingProxy, err := proxy.isRecordingProxy()
Expand Down