go.crypto/ssh: clean up address parsing in forward code.
LGTM=agl
R=agl, dave, jpsugar
CC=golang-codereviews
https://golang.org/cl/134700043
diff --git a/ssh/client.go b/ssh/client.go
index a8d5235..03c4e77 100644
--- a/ssh/client.go
+++ b/ssh/client.go
@@ -159,25 +159,6 @@
c.mu.Unlock()
}
-// parseTCPAddr parses the originating address from the remote into a *net.TCPAddr.
-// RFC 4254 section 7.2 is mute on what to do if parsing fails but the forwardlist
-// requires a valid *net.TCPAddr to operate, so we enforce that restriction here.
-func parseTCPAddr(b []byte) (*net.TCPAddr, []byte, bool) {
- addr, b, ok := parseString(b)
- if !ok {
- return nil, b, false
- }
- port, b, ok := parseUint32(b)
- if !ok || port == 0 || port > 65535 {
- return nil, b, false
- }
- ip := net.ParseIP(string(addr))
- if ip == nil {
- return nil, b, false
- }
- return &net.TCPAddr{IP: ip, Port: int(port)}, b, true
-}
-
// Dial starts a client connection to the given SSH server. It is a
// convenience function that connects to the given network address,
// initiates the SSH handshake, and then sets up a Client. For access
diff --git a/ssh/tcpip.go b/ssh/tcpip.go
index 5a4fa8b..4ecad0b 100644
--- a/ssh/tcpip.go
+++ b/ssh/tcpip.go
@@ -154,19 +154,47 @@
return f.c
}
+// See RFC 4254, section 7.2
+type forwardedTCPPayload struct {
+ Addr string
+ Port uint32
+ OriginAddr string
+ OriginPort uint32
+}
+
+// parseTCPAddr parses the originating address from the remote into a *net.TCPAddr.
+func parseTCPAddr(addr string, port uint32) (*net.TCPAddr, error) {
+ if port == 0 || port > 65535 {
+ return nil, fmt.Errorf("ssh: port number out of range: %d", port)
+ }
+ ip := net.ParseIP(string(addr))
+ if ip == nil {
+ return nil, fmt.Errorf("ssh: cannot parse IP address %q", addr)
+ }
+ return &net.TCPAddr{IP: ip, Port: int(port)}, nil
+}
+
func (l *forwardList) handleChannels(in <-chan NewChannel) {
for ch := range in {
- laddr, rest, ok := parseTCPAddr(ch.ExtraData())
- if !ok {
- // invalid request
- ch.Reject(ConnectionFailed, "could not parse TCP address")
+ var payload forwardedTCPPayload
+ if err := Unmarshal(ch.ExtraData(), &payload); err != nil {
+ ch.Reject(ConnectionFailed, "could not parse forwarded-tcpip payload: "+err.Error())
continue
}
- raddr, rest, ok := parseTCPAddr(rest)
- if !ok {
- // invalid request
- ch.Reject(ConnectionFailed, "could not parse TCP address")
+ // RFC 4254 section 7.2 specifies that incoming
+ // addresses should list the address, in string
+ // format. It is implied that this should be an IP
+ // address, as it would be impossible to connect to it
+ // otherwise.
+ laddr, err := parseTCPAddr(payload.Addr, payload.Port)
+ if err != nil {
+ ch.Reject(ConnectionFailed, err.Error())
+ continue
+ }
+ raddr, err := parseTCPAddr(payload.OriginAddr, payload.OriginPort)
+ if err != nil {
+ ch.Reject(ConnectionFailed, err.Error())
continue
}