openssh-rust / openssh

Scriptable SSH through OpenSSH in Rust
Apache License 2.0
232 stars 35 forks source link

Don't attempt to resolve TCP sockets client side #120

Closed benesch closed 1 year ago

benesch commented 1 year ago

It may be important to have the SSH server perform DNS resolution, as the client often does not use the same DNS server as the server.

We ran into this issue at @MaterializeInc, where we use openssh to set up SSH tunnels via bastion hosts. Our customers often want to establish a tunnel to connect to a host whose name is not resolvable on the client side of the tunnel.

jonhoo commented 1 year ago

This change is Reviewable

codecov-commenter commented 1 year ago

Codecov Report

Merging #120 (34404a2) into master (a471aa2) will decrease coverage by 0.16%. The diff coverage is 44.44%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files | [Impacted Files](https://app.codecov.io/gh/openssh-rust/openssh/pull/120?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/port\_forwarding.rs](https://app.codecov.io/gh/openssh-rust/openssh/pull/120?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL3BvcnRfZm9yd2FyZGluZy5ycw==) | `54.54% <44.44%> (-0.46%)` | :arrow_down: |
benesch commented 1 year ago

Thanks for the review, @NobodyXu. Sorry for the terse PR description last night; filed in a rush. Added some more context to the description now.

NobodyXu commented 1 year ago

@benesch Please run cargo fmt locally and fix the format. Thanks!

benesch commented 1 year ago

Whoops, sorry about that! Done.