Conversation
|
|
||
| /** Carries an SSH connection through one or more direct-tcpip channels. */ | ||
| public final class ProxyJump implements Proxy { | ||
| private static final ThreadLocal<Set<String>> CONNECTING = ThreadLocal.withInitial(HashSet::new); |
There was a problem hiding this comment.
Doesn't using a ThreadLocal mean that a single thread can't open multiple independent JSch sessions that happen to go thru the same proxy host?
Or am I misunderstanding something?
There was a problem hiding this comment.
I am not super deep into the mechanics of Java, this is all ChatGPT 6.0-Sol/ medium assisted to scratch my itch and be able to use the library with epiccastle/bbssh or epiccastle/clojuressh for some configuration management work.
My understanding is the ThreadLocal tracks the destination alias target.org_host, not the jump host. Each connect() removes its entry in finally, so one thread can connect multiple sessions sequentially and keep them all open, including sessions using the same jump host.
The edge case is a nested, re-entrant connect() to the same destination alias before the first call returns. It's currently rejected as a possible ProxyJump cycle, even if the nested connection is independent.
There was a problem hiding this comment.
Be aware that with using ThreadLocals you potentially leak memory, especially when you don't cleanup correctly.
There was a problem hiding this comment.
I don't think using a ThreadLocal is the correct mechanism to use for this.
As @theit points out it can be a memory leak.
Additionally, even if org_host tracks destination hosts, I think it would also prevent the same thread from opening multiple independent sessions to the same destination host.
The chain now shares one connect deadline: the largest ConnectTimeout of the target and its hops, instead of the full timeout per hop. A silent hop with ConnectTimeout no longer hangs a target connect(0). The timed hop stream waits on the pipe monitor and wakes a writer after reading. PipedInputStream only wakes a writer when a read finds the pipe empty, so downloads with any read timeout set were limited to one 32 KiB pipe per second. Hop channels use a 2 MiB window like OpenSSH. An explicit StrictHostKeyChecking on the target is only carried to a hop when it is stricter, so "no" never disables a hop's own checking. Hops inherit the target's daemon flag, thread factory and logger. ProxyJump parsing follows OpenSSH's URI rules, rejects ssh:// passwords and redacts user:password values in errors.
A static ThreadLocal tracked the aliases being connected on the current thread. Besides the class-loader and thread-pool leak risk of static ThreadLocals, it rejected an independent connect to the same alias started on the connecting thread, e.g. from a UserInfo or SocketFactory callback, as a cycle. Chains only recurse through the first hop's own ProxyJump, which createHop() constructs, so pass the aliases that lead to it down explicitly. A loop is now rejected when that hop is created, before any socket is opened, and no thread or static state remains.
Cycle detection no longer keeps state on the proxies. Chains only nest through the first hop's own ProxyJump setting, which its Session takes from the config repository, so connect() follows those settings from alias to alias before creating any session. A loop is reported with the aliases that form it. An explicit ProxyJump set on a session is not part of the walk, so jumping through a host whose config jumps back to an alias without a ProxyJump is correctly allowed. The hop channel now writes into a bounded buffer that ProxyJump owns instead of the JDK pipe. PipedInputStream ties itself to the threads that last used it and reports "Read end dead" when one exits, which the handshake's hand-off from the connecting thread to the session thread exposes, and its writer only wakes on a one-second timer. The buffer starts at 32 KiB, grows to the channel window before it throttles the hop, and bounds reads by the session's timeout. ProxyJump.through(Session) tunnels a session through a hop the caller has already connected, so several sessions can share one hop like OpenSSH's ControlMaster. The caller owns the hop; the proxy only closes its own channel.
ProxyJump.share(factory) returns a SharedHop that behaves like OpenSSH's ControlMaster without ControlPersist: the first session to connect through one of its proxies opens the hop, every further session reuses it, and the last session to disconnect closes it. A later session opens a fresh hop, and a hop that died is replaced on the next connect, so the hop Session never has to be reconnected. close() shuts it at once. Tunnels now take their hop from a HopSource and give it back on close, which is where the reference count lives. through(Session) keeps its meaning: the application owns that hop.
Hops received no UserInfo, so a bastion that authenticates by password or keyboard-interactive could not be used, and a hop with an unknown host key could not be confirmed. Like ssh -J, hops now share the target's UserInfo; every prompt names the hop it is for. A password set with Session.setPassword still applies to the target only. Verified against a Windows bastion authenticating by password in front of a key-authenticated Linux target, a password bastion in front of a password target, and a wrong bastion password, which fails at the hop without leaking threads. The shared-hop concurrency test now completes all acquires before any release; a release racing a queued acquire may close and reopen the hop, which is the documented behaviour rather than a defect.
Wait on the tunnel buffer's monitor from inside the loops that hold it, so the condition is rechecked after every wake-up and the monitor is visibly held. The channel's side of the buffer is a named inner class that owns its write and grow logic. The hop address parser is split into bracketed and plain forms. Session names its host-key config keys once and declares the exceptions setReadTimeout can throw. Tests no longer sleep: the concurrent shared-hop test releases the first connect through a latch once every thread has started, and the blocked writer test yields while it waits for the writer to park.
Hops no longer receive the target's UserInfo. Prompts from hops go to the ProxyJump UserInfo set with Session.setProxyJumpUserInfo, so a UserInfo that answers every prompt with one stored password cannot send it to a bastion by accident. OpenSSH asks for each hop in turn; this is the same, but the application chooses which prompts hops may reach. An independent concurrency review of TunnelBuffer and SharedHop found: SharedHop held its monitor while connecting the hop, so close() and other users waited on a slow bastion or an unanswered prompt; a hop that was closing could return a null channel outside the cleanup, stranding a reference; a timed read rounded to whole milliseconds and could time out early; a zero-sized buffer would spin under the monitor. The hop now connects outside the monitor and close() abandons a connect in progress, channel setup is inside the cleanup, the deadline is compared in nanoseconds, and the buffer size is validated. ProxyJumpIT replaces the property-gated live test. It runs against the existing sshd image, which serves as hop and as target reached through the hop, and covers config chains, the shared hop and through().
Caught exceptions are kept as causes instead of being dropped, the integration test follows JUnit 5 visibility and naming rules and reads the command output to EOF instead of polling, an unneeded throws clause is gone, and the local variable touched in Session follows the naming rule. The remaining suggestions to use unnamed catch parameters and instanceof patterns need Java 22 and 16; the main code targets Java 8.
The two constants introduced for the duplicated-literal rule did not match the file's convention; the same string in a few places is not a problem.
|



ProxyJump: reach hosts behind a bastion the way
ssh -JdoesThis adds OpenSSH-compatible
ProxyJumpto JSch. The typical setup it iswritten for: an internal network where only the bastion's SSH port is
reachable, and everything behind it is a pinhole away. With this change a
JSch application reaches those hosts with the same
ssh_configanadministrator already uses with OpenSSH, and no code changes beyond
loading that config.
What is supported
ProxyJump host,user@host:port, comma-separated chains(
ProxyJump a,b,c), thessh://user@host:portform including bracketedIPv6, and
ProxyJump none.Sessionbuilt from its ownHostblock, so ahop's
HostName,User,IdentityFile,UserKnownHostsFile,StrictHostKeyCheckingandConnectTimeoutall apply. As withssh -J,only the first hop honours a
ProxyJumpof its own; later hops arereached through the previous one.
direct-tcpipchannel with a2 MiB window. Data passes through a bounded buffer owned by the proxy,
not through a
PipedInputStream, so read timeouts, EOF and back-pressurebehave like a socket. SFTP through a hop runs at wire speed on a LAN.
Security properties
constraints an application sets explicitly on the target session (a
HostKeyRepository, a stricterStrictHostKeyChecking, orserver_host_key) are also applied to the hops, and a hop's ownStrictHostKeyCheckingis never weakened by the target's.UserInfoor password. Prompts from ahop, for a bastion password or an unknown host key, go to a separate
Session.setProxyJumpUserInfo(...)if the application sets one, and everyprompt names the hop it is for. A stored password therefore cannot reach
a bastion by accident.
ssh://user:password@hosthop is rejected, andit is redacted in the error message.
ajumps viab,bviaa) are detected bywalking the config before any socket is opened, and the message names the
loop. No thread-local or static state is involved.
ConnectTimeouton the path, so a silent hop cannot stretch the connect to the sum of all
timeouts.
Programmatic use and hop sharing
ProxyJump.through(Session hop)tunnels a session through a hop theapplication has connected itself; the hop stays open when the tunnelled
session closes.
ProxyJump.share(factory)returns aSharedHopthat behaves likeOpenSSH's
ControlMasterwithoutControlPersist: the first session toconnect opens the bastion, later sessions reuse it, the last one to
disconnect closes it, and a later session opens a fresh one.
close()shuts it at once. It is safe to use from several threads; the hop is
connected outside the lock so
close()never waits on a slow bastion.Testing
credential isolation, the connect budget, the transport buffer
(timeouts, EOF ordering, growth, a blocked writer released by close) and
the shared hop's reference counting under concurrent use.
ProxyJumpITruns against the repository's sshd image withtestcontainers: one- and two-hop chains from config, the shared hop
opening and closing with its first and last session, and
through().as bastion in front of OpenSSH for Windows 9.5, a Windows host as a
password-authenticated bastion, three-hop chains, 64 MB SFTP transfers
through a hop, 20 concurrent sessions through one bastion, and soak runs
of several hundred connects with no leaked threads, sockets or heap.
review; its findings are folded into the last commits.
Notes for reviewers
ThreadLocalfor loop detection is gone, which addressesthe concerns raised in the review threads.
ConnectTimeout; this implementationbounds the whole chain by the largest one. The Javadoc says so.
Sessionobject afterdisconnect()is notsupported by JSch today;
SharedHoptherefore creates a fresh hopsession when it reopens.
IncludeandMatchsupport forssh_configis in a separate PR.instanceofpatterns; themain code targets Java 8.
Development
Followup PRs
Include%dfor the home directory in paths in the configuration