x/crypto/ssh: tcpip.go forward and remove compare on ip and port

[gopherbot]

by matt.knopp:

In tcpip.go, forward(...) and remote(...) scan forwardList entries for an entry that
matches laddr on both IP and PORT as part of creating/removing a forward channel. It
looks like it should only be comparing on PORT rather than PORT and IP.

I came across this because some SSH servers (e.g. Apache Mina) (appear?) to pass the
originating interface's IP  when creating a 'forwarded-tcpip' channel rather than the IP
we requested, e.g. 0.0.0.0. The forwarding request is ultimately rejected because
forward() is unable to find a matching IP, PORT pair in forwardList. Removing the IP
check appears to fix the problem.

RFC-4253 states "Implementations MUST reject these messages unless they have
previously requested a remote TCP/IP port forwarding with the given port number". 

Matt

[ianlancetaylor]

Comment 1:

Labels changed: added repo-crypto, release-none.

[hanwen]

while the RFC is not very clear here, I think the current behavior is correct. Since you can setup different port forwards on the same port number for eg. IPv6 and IPv4 (eg forward the former to local port N and latter to M), if we disregard the IP address, we won't know how to route an incoming connection.

However, we could add a bugfix mode to the code, by disregarding the IP check for specific version strings. To do this, we need to know exactly which server version strings exhibit this problem

[mhat]

I believe Apache Mina was the only sshd implementation I ran into this issue with. It just happened to be the one I was working with at the time. My/our experience with Go and the SSH library was positive enough we replaced the service that was using Mina with a Go-Ssh service. Given that it's been two years and I'm the only one that apparently ran into this, it seems fair to assume the issue is pretty narrowly scoped to Mina. My vote would be to close this unless/until someone comes back with the same issue and a list of SSH server version/strings.

Possibly not obvious: I'm the original reporter.

[hanwen]

thanks for the update. Will close this issue for now.