bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#45162: [PATCH] Fixup Re: bug#45162: 28.0.50; Messages scrambled by s


From: J.P.
Subject: bug#45162: [PATCH] Fixup Re: bug#45162: 28.0.50; Messages scrambled by socks-filter
Date: Thu, 07 Jan 2021 02:35:19 -0800
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Lars and Emacs,

My sincerest apologies. I'm embarrassed/ashamed to admit that my
previous patch,

  Commit: abc8d6b9465fecb989170426756c7ee4b133fd40
  "Append incremental message segments in socks-filter"

was not only incomplete but effectively *broke* socks.el for anyone
who's tried using it since!

In a boneheaded oversight, I neglected to reset the "scratch" work area
after functions performing the SOCKS5 authentication subprotocol are
through with it.

This is a one-line follow-up that hopefully corrects that. Please see
the included test, which passes after this tweak is applied.

Below is a long-winded attempt to explain the awkward placement
of this change for anyone who cares, present or future.

Thanks (and again, so sorry!),
J.P.

P.S. Please let me know if I need to open a new bug report.


Summary of [1] using concrete values for SOCKS 5 and USER/PASS method [2]:

  0502 0002              c: auth offer, socks-ver len-methods methods ...
  0502                   s: auth choice, socks-version method
  0103 666f 6f03 6261 72 c: attempt, version len-user "foo" len-pass "bar"
  0100                   s: verdict, version status

1. (BG) In socks-filter, the WAITING-FOR-AUTH case stashes the latest
   chunk by extending the existing stash rightward. In practice, this is
   the first chunk, which contains the server's selection of one of the
   auth methods offered by the client (Emacs) in the initial request. It
   then transitions to SUBMETHOD-NEGOTIATION.

2. (FG) Once socks-open-connection has detected that transition, it
   calls the handler corresponding to the server's chosen auth method to
   handle the subnegotiation.

3. (FG) That handler, most likely socks-username/password-auth, swaps
   out the primary process filter, socks-filter, for a temporary one.

4. (BG) That temp filter, socks-username/password-auth-filter, parses
   out the status code and transitions to AUTHENTICATED (not sure why
   here, exactly)

5. (FG) The auth handler (3), having detected that transition, returns
   control (and a verdict) to the opener (2), which, on success, ensures
   the state is AUTHENTICATED and restores the primary filter
   (socks-filter).

6. (FG) socks-send-command transitions to WAITING before sending the
   main request (method call) as per the protocol, which is similar to
   SOCKS 4.

>From the point of view of the socks-filter, the AUTHENTICATED case is
effectively invisible because it's preempted explicitly as described
above (as opposed to, e.g., left as a no-op to avoid a race). Thus, we
can't run anything here, however opportune it may seem. Instead, I
tacked the correction onto the end of the opener (which see).

Ironically, the original approach of prepending chunks on the left
papered over any need to zero out the scratch buffer after these opening
negotiations (for the CONNECT method, anyway).

[1]: https://tools.ietf.org/html/rfc1928#section-3 
[2]: https://tools.ietf.org/html/rfc1929

Attachment: 0001-Drop-me-add-test-for-SOCKS5-auth-protocol.patch
Description: Text Data

Attachment: 0002-Clear-socks-protocol-scratch-after-authentication.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]