GNU bug report logs - #45162
28.0.50; Messages scrambled by socks-filter

Previous Next

Package: emacs;

Reported by: "J.P." <jp <at> neverwas.me>

Date: Thu, 10 Dec 2020 15:17:01 UTC

Severity: minor

Tags: fixed, patch

Found in version 28.0.50

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45162 in the body.
You can then email your comments to 45162 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Thu, 10 Dec 2020 15:17:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to "J.P." <jp <at> neverwas.me>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 10 Dec 2020 15:17:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Messages scrambled by socks-filter
Date: Thu, 10 Dec 2020 05:02:39 -0800
[Message part 1 (text/plain, inline)]
Severity: minor
Tags: patch

Hi,

If this is indeed a bug, I'm guessing it's a pretty minor one because in
most cases, the bulk of a SOCKS reply doesn't seem to matter, so long as
the status code portion is correctly interpreted. But in rarer cases, for
example when using Tor's RESOLVE command, which is part of a nonstandard
extension suite, a response may need to be preserved intact.

Rather than try to explain, I've included a throwaway before/after test
showing current and expected behavior, as well as a small patch that
seems to fix the issue. If this is all nonsense, please forgive me. I am
no expert, just a grateful end user.

Thanks,
J.P.

In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.23, cairo version 1.16.0)
 of 2020-12-09 built on pc
Repository revision: 4f352ad6f1759ae6dcff6ba43847273491bf9c30
Repository branch: socks
Windowing system distributor 'Fedora Project', version 11.0.12010000
System Description: Fedora 33 (Workstation Edition)

Configured using:
 'configure --with-dbus --with-gif --with-jpeg --with-png --with-rsvg
 --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk3 --with-gpm=no
 --with-xwidgets --with-modules --with-harfbuzz --with-cairo --with-json
 CC=gcc 'CFLAGS=-DMAIL_USE_LOCKF -O0 -g3 -Wall' LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS
LIBSYSTEMD JSON PDUMPER LCMS2

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml easymenu mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads
xwidget-internal dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit
x multi-tty make-network-process emacs)

Memory information:
((conses 16 51710 8248)
 (symbols 48 6739 1)
 (strings 32 18249 1932)
 (string-bytes 1 610727)
 (vectors 16 12311)
 (vector-slots 8 170737 6932)
 (floats 8 21 45)
 (intervals 56 219 0)
 (buffers 984 10))

[0001-DROP-ME-add-test-explaining-existing-bug.patch (text/x-patch, attachment)]
[0002-DROP-ME-update-test-to-reflect-expected-behavior.patch (text/x-patch, attachment)]
[0003-Append-incremental-message-segments-in-socks-filter.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Fri, 11 Dec 2020 15:38:01 GMT) Full text and rfc822 format available.

Message #8 received at 45162 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "J.P." <jp <at> neverwas.me>
Cc: 45162 <at> debbugs.gnu.org
Subject: Re: bug#45162: 28.0.50; Messages scrambled by socks-filter
Date: Fri, 11 Dec 2020 16:37:14 +0100
"J.P." <jp <at> neverwas.me> writes:

> Rather than try to explain, I've included a throwaway before/after test
> showing current and expected behavior, as well as a small patch that
> seems to fix the issue. If this is all nonsense, please forgive me. I am
> no expert, just a grateful end user.

Thanks; the patch makes sense to me, so I've applied it to Emacs 28.

This change was small enough to apply without assigning copyright to the
FSF, but for future patches you want to submit, it might make sense to
get the paperwork started now, so that subsequent patches can be applied
speedily. Would you be willing to sign such paperwork?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Added tag(s) fixed. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Fri, 11 Dec 2020 15:38:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 28.1, send any further explanations to 45162 <at> debbugs.gnu.org and "J.P." <jp <at> neverwas.me> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Fri, 11 Dec 2020 15:38:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Fri, 11 Dec 2020 22:02:02 GMT) Full text and rfc822 format available.

Message #15 received at 45162 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 45162 <at> debbugs.gnu.org
Subject: Re: bug#45162: 28.0.50; Messages scrambled by socks-filter
Date: Fri, 11 Dec 2020 14:01:40 -0800
> Thanks; the patch makes sense to me, so I've applied it to Emacs 28.

Oh no, thank YOU, Lars and Emacs!

> ... Would you be willing to sign such paperwork?

Yes sir, I'd be willing.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Fri, 11 Dec 2020 22:07:01 GMT) Full text and rfc822 format available.

Message #18 received at 45162 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "J.P." <jp <at> neverwas.me>
Cc: 45162 <at> debbugs.gnu.org
Subject: Re: bug#45162: 28.0.50; Messages scrambled by socks-filter
Date: Fri, 11 Dec 2020 23:06:04 +0100
"J.P." <jp <at> neverwas.me> writes:

>> ... Would you be willing to sign such paperwork?
>
> Yes sir, I'd be willing.

Great!  Here's the paperwork to get started:



Please email the following information to assign <at> gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]

[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]

[For the copyright registration, what country are you a citizen of?]

[What year were you born?]

[Please write your email address here.]

[Please write your postal address here.]

[Which files have you changed so far, and which new files have you written
so far?]




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Thu, 07 Jan 2021 10:36:02 GMT) Full text and rfc822 format available.

Message #21 received at 45162 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 45162 <at> debbugs.gnu.org
Subject: [PATCH] Fixup Re: bug#45162: 28.0.50; Messages scrambled by
 socks-filter
Date: Thu, 07 Jan 2021 02:35:19 -0800
[Message part 1 (text/plain, inline)]
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
[0001-Drop-me-add-test-for-SOCKS5-auth-protocol.patch (text/x-patch, attachment)]
[0002-Clear-socks-protocol-scratch-after-authentication.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Thu, 07 Jan 2021 12:46:02 GMT) Full text and rfc822 format available.

Message #24 received at 45162 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "J.P." <jp <at> neverwas.me>
Cc: 45162 <at> debbugs.gnu.org
Subject: Re: [PATCH] Fixup Re: bug#45162: 28.0.50; Messages scrambled by
 socks-filter
Date: Thu, 07 Jan 2021 13:44:59 +0100
"J.P." <jp <at> neverwas.me> writes:

> 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.

Thanks; I've now applied the fix to Emacs 28, but not the test (because
the amount of code would require a copyright assignment).

I forgot whether I've asked before -- would you be willing to sign such
paperwork?  If so, reworking that test into an ert-deftest would be
nice.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Thu, 07 Jan 2021 22:52:02 GMT) Full text and rfc822 format available.

Message #27 received at 45162 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 45162 <at> debbugs.gnu.org
Subject: Re: [PATCH] Fixup Re: bug#45162: 28.0.50; Messages scrambled by
 socks-filter
Date: Thu, 07 Jan 2021 14:51:00 -0800
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I forgot whether I've asked before -- would you be willing to sign such
> paperwork?  If so, reworking that test into an ert-deftest would be
> nice.

I did indeed begin the paperwork process and sent the signed documents
on December 28. I'd be happy to rework the test portion to conform to
your standards and, when I hear back, resubmit it (in this thread?).

BTW, in case you didn't notice, the one test in the last attachment is
already an ert-deftest. But I'm sure I failed to adequately follow the
coding conventions. I'll definitely review those and revise accordingly.

Thanks so much, Lars, for being so gracious and tolerating my
sloppiness. I pledge to be more responsible in the future.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Sat, 09 Jan 2021 14:17:02 GMT) Full text and rfc822 format available.

Message #30 received at 45162 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 45162 <at> debbugs.gnu.org
Subject: Re: [PATCH] Fixup Re: bug#45162: 28.0.50; Messages scrambled by
 socks-filter
Date: Sat, 09 Jan 2021 06:16:39 -0800
[Message part 1 (text/plain, inline)]
Just got my paperwork sorted, yay! (Famous last words.)

I've attempted to rework the test as suggested. It now runs in process
instead of spawning another Emacs. I wasn't sure whether having url-http
as a dependency made sense but left it as is. Happy to change whatever
needs changing.

Thanks again,
J.P.
[0001-Create-new-test-file-for-socks.el.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45162; Package emacs. (Sun, 10 Jan 2021 11:42:01 GMT) Full text and rfc822 format available.

Message #33 received at 45162 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "J.P." <jp <at> neverwas.me>
Cc: 45162 <at> debbugs.gnu.org
Subject: Re: [PATCH] Fixup Re: bug#45162: 28.0.50; Messages scrambled by
 socks-filter
Date: Sun, 10 Jan 2021 12:41:21 +0100
"J.P." <jp <at> neverwas.me> writes:

> Just got my paperwork sorted, yay! (Famous last words.)

Great timing.  :-)

> I've attempted to rework the test as suggested. It now runs in process
> instead of spawning another Emacs. I wasn't sure whether having url-http
> as a dependency made sense but left it as is. Happy to change whatever
> needs changing.

Looks good to me; pushed to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 07 Feb 2021 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 50 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.