Skip to content

Add a client direct-tcpip channel open - #66

Closed
kvark wants to merge 1 commit into
mkj:mainfrom
navigato-rs:claude/client-direct-tcpip
Closed

Add a client direct-tcpip channel open#66
kvark wants to merge 1 commit into
mkj:mainfrom
navigato-rs:claude/client-direct-tcpip

Conversation

@kvark

@kvark kvark commented Aug 31, 2026

Copy link
Copy Markdown

(This is generated by Claude, and I haven't yet analyzed this code, but it appears to be the only thing that https://github.com/kvark/starcom needs "sunset" to have, ideally published.)

Sunset can decode and encode direct-tcpip channel opens, and the channel-open-confirmation path is already type agnostic, but a client has no way to open one: open_client_session is the only opener, and inbound direct-tcpip is rejected with a TODO.

Add Runner::open_client_tcpip, which is the channel behind ssh -L and ssh -J. Nothing else needs to change to carry its data.

Add Runner::is_channel_finished so a caller waiting on an open can tell a refusal from a slow open. A refused open leaves the channel in PendingDone rather than removing it, and PendingDone is reported neither as eof nor as closed, so without this a rejection is indistinguishable from a slow open until the caller's own deadline expires.

Verified against OpenSSH 9.x: a forward carries data both ways, an unreachable destination is reported as a refusal, and a server with AllowTcpForwarding no is reported as a refusal rather than a timeout.

Sunset can decode and encode direct-tcpip channel opens, and the
channel-open-confirmation path is already type agnostic, but a client
has no way to open one: open_client_session is the only opener, and
inbound direct-tcpip is rejected with a TODO.

Add Runner::open_client_tcpip, which is the channel behind ssh -L and
ssh -J. Nothing else needs to change to carry its data.

Add Runner::is_channel_finished so a caller waiting on an open can tell
a refusal from a slow open. A refused open leaves the channel in
PendingDone rather than removing it, and PendingDone is reported neither
as eof nor as closed, so without this a rejection is indistinguishable
from a slow open until the caller's own deadline expires.

Verified against OpenSSH 9.x: a forward carries data both ways, an
unreachable destination is reported as a refusal, and a server with
AllowTcpForwarding no is reported as a refusal rather than a timeout.
Comment thread Cargo.toml

# Raise config::MAX_CHANNELS from 4 to 16. The channel array is fixed size, so
# this costs memory; it is off by default for `no_std` targets.
many-channels = []

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I wouldn't want this feature just so that I can bump MAX_CHANNELS.
Should we just increase it for everybody?

@mkj mkj closed this Aug 31, 2026
@kvark

kvark commented Sep 3, 2026

Copy link
Copy Markdown
Author

Oof, closing without even leaving a word.

@mkj

mkj commented Sep 3, 2026

Copy link
Copy Markdown
Owner

A claude written PR "I haven't checked this myself" wasn't very appealing.

The code itself looks OK, comments/commit message could improve. If you want to clean it up and review it yourself then it's OK to reopen.

I need to have a think more about channel clean up in general, not sure if is_finished() is the right way (and how it fits with the sunset-async). But it's probably OK for the time being.

For max-channels I guess it will probably end up with features max-channels-1, max-channels-2, 5, 10, 20, 50, ...
Though for std I might end up using something like ManagedSlice

What device are you using for ed25519-sk? I'd thought it was uncommon, most hardware doing ecdsa256-sk.

@kvark

kvark commented Sep 4, 2026

Copy link
Copy Markdown
Author

I can see how "I haven't checked this myself" is not appealing but at least it was honest and was meant to tell you that I am looking at it and refining it - something that should be expected from a draft PR.

I'll consider upstreaming things but for now will be developing a fork in https://github.com/navigato-rs/sunset
Allows me to move fast. I like your library and want to use it more widely. Thank you for making it!
In my fork, currently testing the FTP client implementation, so that I can move https://github.com/navigato-rs/fileman to it.

The device I'm using for ed25519-sk is a Yubikey, which is configured to not require a touch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants