reality: propagate the dest's close to the client in fallback - #6
Open
huangyingw wants to merge 1 commit into
Open
huangyingw wants to merge 1 commit into
huangyingw wants to merge 1 commit into
Conversation
When REALITY falls back to relaying an unauthenticated connection to the dest, it forwards the dest's bytes faithfully but never forwards the close. Teardown sits behind waitGroup.Wait(), which waits on the upload io.Copy, and that copy only returns once the client closes. A client that never closes therefore keeps the connection open indefinitely, long after the real dest has dropped it. That is remotely observable and distinguishes a REALITY server from the site it imitates. Measured against sing-box 1.13.14 (metacubex/utls v1.8.4) with dest www.intel.com: send a 5-byte record header plus 32 random bytes; both the REALITY server and the real dest return the same alert 1503030002020a, but the real dest closes immediately while the REALITY server is still open after 12 seconds. One connection, 37 bytes. Propagate the close with a half-close once the download copy returns, so the client observes the same shutdown the dest performed. Guarded by a type assertion, so it is a no-op where the underlying conn cannot half-close. XTLS/REALITY already does this; see the underlying.CloseWrite() call after the download io.Copy in its tls.go, where underlying is asserted to CloseWriteConn.
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.
When REALITY falls back to relaying an unauthenticated connection to the dest, it forwards the dest's bytes faithfully but never forwards the close.
Teardown sits behind
waitGroup.Wait(), which waits on the uploadio.Copy— and that copy only returns once the client closes. A client that simply never closes keeps the connection open indefinitely, long after the real dest has dropped it.This is remotely observable, and it distinguishes a REALITY server from the site it is imitating.
Measurement
Against sing-box 1.13.14 (which pins
metacubex/utls v1.8.4),dest = www.intel.com.Probe: one connection, send a 5-byte record header plus 32 random bytes, read the alert, then wait.
1503030002020awww.intel.com1503030002020aThe alert itself comes back byte-identical — the relay forwards it faithfully. Only the shutdown is missing. Cost to an attacker: one connection, 37 bytes, wait a second.
Fix
Half-close towards the client once the download copy returns, so the client observes the same shutdown the dest performed. Guarded by a type assertion, so it is a no-op where the underlying conn cannot half-close.
After the patch, same probe, same config:
1503030002020awww.intel.com1503030002020aVerification
go mod edit -replace.xtls-rprx-visionclient through the patched server proxies fine, 4/4 requests.go build ./...clean,gofmtclean.Prior art
XTLS/REALITY already does exactly this — see the
underlying.CloseWrite()call right after the downloadio.Copyin itstls.go, whereunderlyingis asserted toCloseWriteConn, with a comment stating the intent. This change brings the same behaviour here with a minimal, non-invasive assertion rather than changing the type ofunderlying.