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.

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

Jump to bottom

x/crypto/ssh: TCP/IP port forwarding expects IP addresses #37239

Open
tt opened this issue Feb 15, 2020 · 8 comments
Open

x/crypto/ssh: TCP/IP port forwarding expects IP addresses #37239

tt opened this issue Feb 15, 2020 · 8 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@tt
Copy link
Contributor

tt commented Feb 15, 2020 •
edited

(*ssh.Client).ListenTCP expects an IP address (via *net.TCPAddr) and therefore (*ssh.Client).Listen attempts to resolve addresses.

However, section 7.1 of RFC 4254 states:

The 'address to bind' and 'port number to bind' specify the IP
address (or domain name) and port on which connections for forwarding
are to be accepted. Some strings used for 'address to bind' have
special-case semantics.

  • "" means that connections are to be accepted on all protocol
    families supported by the SSH implementation.

  • "0.0.0.0" means to listen on all IPv4 addresses.

  • "::" means to listen on all IPv6 addresses.

  • "localhost" means to listen on all protocol families supported by
    the SSH implementation on loopback addresses only ([RFC3330] and
    [RFC3513]).

  • "127.0.0.1" and "::1" indicate listening on the loopback
    interfaces for IPv4 and IPv6, respectively.

There are two consequences of the current interface:

  1. You can only provide resolvable names. This prohibits two of the strings with special-case semantics from working ("", reported in x/crypto/ssh: listening on remote address blocks indefinitely when address doesn't contain a host part #33227, and "::").

  2. Resolution happens client side. This changes the meaning of the string "localhost" from being "all protocol families supported by the SSH implementation on loopback addresses only" to being only one of those and may provide a different result for other names (AWS hostnames resolving to internal addresses inside a data center comes to mind).

Outside of defining a new public interface, I think the least breaking change would be to extract an unexported listenTCP function taking a string address and call this from Listen which can then drop resolution but of course if you're relying on that behavior, it will still be surprising.

I'm happy to submit a pull request but I'd appreciate some thoughts on how to best evolve the interface into something that both supports the scope of the RFC and doesn't disregard current users.
@gopherbot gopherbot added this to the Unreleased milestone Feb 15, 2020
@tt
Copy link
Contributor Author

tt commented Feb 15, 2020

I should mention that it's of course possible to treat "", "::" and "localhost" separately with even less potential breakage but this doesn't solve the problem of client side resolution generally which was my intention for filing this issue.

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 18, 2020
@toothrot
Copy link
Contributor

/cc @hanwen @FiloSottile

@nya3jp
Copy link
Contributor

nya3jp commented Mar 22, 2021

+1. I'm particularly interested in localhost case so that I don't need to create two listeners for IPv4/IPv6.

@luizfeliperj
Copy link

+1 The address should be resolved by the remote host, as is where the listen() call will happen, not the originating ssh client.
Please fix this!

@luizfeliperj
Copy link

Just adding some more information, supposing it is a multi-homed server, with multiple IP addresses, calling net. Listen () using a hostname needs to resolve the name within the remote server to figure out which interface it should bind. The current implementation doesn't allow this.

@tiger5226
Copy link

tiger5226 commented Nov 9, 2022 •
edited

wow...this is so insurmountable....

SSH Protocol : "channel forward message address is a string"
Golang: "Noooooo we think it should be a Golang IP, which we will convert to a string :)"

All the above, plus I would add that a domain might not be "resolvable" from the client side in instances where a remote forward tcpip-forward is created. "resolvable" should not be a requirement of a domain client side.

@rahmir-fabrice
Copy link

rahmir-fabrice commented Jan 2, 2023 •
edited

+1 This should be resolved on the remote host and treated as a string client side to allow the remote host to preform the lookup on what hostname qualifies to access the connection

@eslym
Copy link

eslym commented May 30, 2023 •
edited

or just simply not to handle forwarded-tcpip channel before any ListenTCP called, so we can implement our own handling for this case

update:
i read the source code, the handleForwards does only called once ListenTCP or ListenUnix is called (at least in the latest version)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
8 participants
@toothrot @tt @nya3jp @tiger5226 @eslym @luizfeliperj @gopherbot @rahmir-fabrice