feat: expose SSH_MSG_USERAUTH_BANNER messages via Connection.getBanners()#494
Open
pdecat wants to merge 1 commit into
Open
feat: expose SSH_MSG_USERAUTH_BANNER messages via Connection.getBanners()#494pdecat wants to merge 1 commit into
pdecat wants to merge 1 commit into
Conversation
…rs() Servers can send banner messages at any point before authentication completes — typically a login notice, or for managed-SSH offerings like Tailscale, a web URL the user must visit to finish authenticating. Today these banners are received and stored, but the field is package- private on `AuthenticationManager` and inaccessible from outside the library, leaving no way for callers to surface them to users. Mirror the upstream trilead-ssh2 change (trilead/trilead-ssh2#206): - `AuthenticationManager`: accumulate banners into a `List<String>` instead of clobbering the previous value, and expose `getBanners()` returning a defensive snapshot. - `Connection`: add `getBanners()` that delegates to the auth manager, or returns an empty list when authentication hasn't been attempted yet. Threading note: neither accessor synchronizes on the `Connection` or `AuthenticationManager` monitor. The blocking `authenticateWith*` entry points already hold the `Connection` monitor for the entire duration of the auth call, so a caller that wants to surface banners live (Tailscale's web-login URL arrives in a banner BEFORE USERAUTH_SUCCESS) must be able to read them from another thread while that call is still in flight. Synchronization is scoped to the `banners` list itself.
|
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
Today
SSH_MSG_USERAUTH_BANNERmessages are read byAuthenticationManagerbut stored in a package-private field(
String banner) that no caller outside the library can reach. Thatmakes it impossible to surface banners to users — most painfully when
the server is a Tailscale-managed sshd
that delivers a web-login URL in a banner and then waits for the user
to finish the web login before sending
USERAUTH_SUCCESS. TheauthenticateWithNone()call blocks the whole time and the user hasno way to see the URL they need to visit.
This change mirrors the upstream trilead-ssh2 patch in
jenkinsci/trilead-ssh2#206:
AuthenticationManager: store banners in aList<String>somultiple banners aren't clobbered, and expose a public
getBanners()that returns a defensive snapshot.Connection: add a publicgetBanners()that delegates to theauth manager, returning an empty list when authentication hasn't
been attempted yet.
Threading
Neither accessor synchronizes on the
ConnectionorAuthenticationManagermonitor. The blockingauthenticateWith*entry points already hold the
Connectionmonitor for the fullduration of the auth call, so a caller that wants to surface banners
live — Tailscale's URL arrives in a banner before
USERAUTH_SUCCESS— must be able to read them from another thread while the auth call
is still in flight. Synchronization is scoped to the
bannerslistitself: the auth thread holds the list monitor while appending; the
accessor holds it briefly while copying out a snapshot.
Why now
ConnectBot has issue #1150
open for the Tailscale-banner case. In
connectbot#2211
the workaround uses reflection on the package-private
am/bannerfields. Once this API ships in a sshlib release, that reflection block
can be replaced with
connection.getBanners().I've tested this exact change end-to-end against a Tailscale-managed
sshd by publishing this branch as
2.2.47-SNAPSHOTto mavenLocal,pointing a connectbot debug build at it, and swapping the reflection
block for a polling loop over
connection.banners: thehttps://login.tailscale.com/a/...URL now appears in the terminalduring the auth wait and is also picked up by URL Scan. (See screenshot
on the linked ConnectBot PR.)
Compatibility
String bannerfield is replaced with a package-privateList<String> banners. That field was never part of the public API.Test plan
ConnectionTest#testGetBannersReturnsEmptyBeforeAuthenticationverifying
getBanners()returns a non-null, empty list whenamhasn't been instantiated yet../gradlew compileJavapasses (pre-existingAccessControllerdeprecation warning only, unrelated).
./gradlew test --tests "com.trilead.ssh2.ConnectionTest"passes../gradlew spotlessCheckpasses.to a local SNAPSHOT of this branch (see Why now).