From: "J.P." <jp@neverwas.me>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 46342@debbugs.gnu.org
Subject: bug#46342: 28.0.50; socks-send-command munges IP address bytes to UTF-8
Date: Tue, 09 Feb 2021 07:17:29 -0800 [thread overview]
Message-ID: <874kils22e.fsf@neverwas.me> (raw)
In-Reply-To: <83czxdns61.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 06 Feb 2021 17:15:50 +0200")
[-- Attachment #1: Type: text/plain, Size: 727 bytes --]
Eli,
Apologies for my previous rambling reply. Your time is supremely
valuable, and I should've been more mindful of that. I'm hoping you'll
allow me this mulligan (ahem):
> That depends on what the other end expects. Does it expect latin-1 in
> this case?
Yes.
> Does emitting the single byte \330 produce the correct result in this
> case?
Yes.
> Then by all means please use
>
> (encode-coding-string address 'latin-1)
I have updated my changes to reflect this.
> And those [tor/ssh] expect Latin-1 encoding in this case?
Yes.
When you've got a sec, please instruct me on how best to proceed (even
if that means buggering off 'til I've learned more about Emacs!). :)
Appreciate your patience, as always.
J.P.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-more-auth-related-tests-for-socks.el.patch --]
[-- Type: text/x-patch, Size: 15775 bytes --]
From df68ca06563b9f4d943666c474d978fe66584485 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 5 Feb 2021 05:24:55 -0800
Subject: [PATCH 1/2] Add more auth-related tests for socks.el
* test/lisp/net/socks-tests.el (auth-registration-and-suite-offer)
(filter-response-parsing-v4, filter-response-parsing-v5): Assert
auth-method selection wrangling and socks-filter parsing.
(v5-auth-user-pass, v5-auth-user-pass-blank, v5-auth-none): Show prep
and execution of the SOCKS connect command and proxying of an HTTP
request; simplify fake server.
---
test/lisp/net/socks-tests.el | 263 +++++++++++++++++++++++++++--------
1 file changed, 208 insertions(+), 55 deletions(-)
diff --git a/test/lisp/net/socks-tests.el b/test/lisp/net/socks-tests.el
index b378ed2964e..d2c21094e7f 100644
--- a/test/lisp/net/socks-tests.el
+++ b/test/lisp/net/socks-tests.el
@@ -21,68 +21,149 @@
;;; Code:
+(require 'ert)
(require 'socks)
(require 'url-http)
-(defvar socks-tests-canned-server-port nil)
+(ert-deftest socks-tests-auth-registration-and-suite-offer ()
+ (ert-info ("Default favors user/pass auth")
+ (should (equal socks-authentication-methods
+ '((2 "Username/Password" . socks-username/password-auth)
+ (0 "No authentication" . identity))))
+ (should (equal "\2\0\2" (socks-build-auth-list)))) ; length [offer ...]
+ (let (socks-authentication-methods)
+ (ert-info ("Empty selection/no methods offered")
+ (should (equal "\0" (socks-build-auth-list))))
+ (ert-info ("Simulate library defaults")
+ (socks-register-authentication-method 0 "No authentication"
+ 'identity)
+ (should (equal socks-authentication-methods
+ '((0 "No authentication" . identity))))
+ (should (equal "\1\0" (socks-build-auth-list)))
+ (socks-register-authentication-method 2 "Username/Password"
+ 'socks-username/password-auth)
+ (should (equal socks-authentication-methods
+ '((2 "Username/Password" . socks-username/password-auth)
+ (0 "No authentication" . identity))))
+ (should (equal "\2\0\2" (socks-build-auth-list))))
+ (ert-info ("Removal")
+ (socks-unregister-authentication-method 2)
+ (should (equal socks-authentication-methods
+ '((0 "No authentication" . identity))))
+ (should (equal "\1\0" (socks-build-auth-list)))
+ (socks-unregister-authentication-method 0)
+ (should-not socks-authentication-methods)
+ (should (equal "\0" (socks-build-auth-list))))))
-(defun socks-tests-canned-server-create (verbatim patterns)
- "Create a fake SOCKS server and return the process.
+(ert-deftest socks-tests-filter-response-parsing-v4 ()
+ (let* ((buf (generate-new-buffer "*test-socks-filter*"))
+ (proc (start-process "test-socks-filter" buf "sleep" "1")))
+ (process-put proc 'socks t)
+ (process-put proc 'socks-state socks-state-waiting)
+ (process-put proc 'socks-server-protocol 4)
+ (ert-info ("Receive initial incomplete segment")
+ (socks-filter proc (concat [0 90 0 0 93 184 216]))
+ ;; From example.com: OK status ^ ^ msg start
+ (ert-info ("State still set to waiting")
+ (should (eq (process-get proc 'socks-state) socks-state-waiting)))
+ (ert-info ("Response field is nil because processing incomplete")
+ (should-not (process-get proc 'socks-response)))
+ (ert-info ("Scratch field holds stashed partial payload")
+ (should (string= (concat [0 90 0 0 93 184 216])
+ (process-get proc 'socks-scratch)))))
+ (ert-info ("Last part arrives")
+ (socks-filter proc "\42") ; ?\" 34
+ (ert-info ("State transitions to complete (length check passes)")
+ (should (eq (process-get proc 'socks-state) socks-state-connected)))
+ (ert-info ("Scratch and response fields hold stash w. last chunk")
+ (should (string= (concat [0 90 0 0 93 184 216 34])
+ (process-get proc 'socks-response)))
+ (should (string= (process-get proc 'socks-response)
+ (process-get proc 'socks-scratch)))))
+ (delete-process proc)
+ (kill-buffer buf)))
-`VERBATIM' and `PATTERNS' are dotted alists containing responses.
-Requests are tried in order. On failure, an error is raised."
- (let* ((buf (generate-new-buffer "*canned-socks-server*"))
+(ert-deftest socks-tests-filter-response-parsing-v5 ()
+ (let* ((buf (generate-new-buffer "*test-socks-filter*"))
+ (proc (start-process "test-socks-filter" buf "sleep" "1")))
+ (process-put proc 'socks t)
+ (process-put proc 'socks-state socks-state-waiting)
+ (process-put proc 'socks-server-protocol 5)
+ (ert-info ("Receive initial incomplete segment")
+ ;; From fedora.org: 2605:bc80:3010:600:dead:beef:cafe:fed9
+ ;; 5004 ~~> Version Status (OK) NOOP Addr-Type (4 -> IPv6)
+ (socks-filter proc "\5\0\0\4\x26\x05\xbc\x80\x30\x10\x00\x60") ; unibyte
+ (ert-info ("State still waiting and response emtpy")
+ (should (eq (process-get proc 'socks-state) socks-state-waiting))
+ (should-not (process-get proc 'socks-response)))
+ (ert-info ("Scratch field holds partial payload of pending msg")
+ (should (string= "\5\0\0\4\x26\x05\xbc\x80\x30\x10\x00\x60"
+ (process-get proc 'socks-scratch)))))
+ (ert-info ("Middle chunk arrives")
+ (socks-filter proc "\xde\xad\xbe\xef\xca\xfe\xfe\xd9")
+ (ert-info ("State and response fields still untouched")
+ (should (eq (process-get proc 'socks-state) socks-state-waiting))
+ (should-not (process-get proc 'socks-response)))
+ (ert-info ("Scratch contains new arrival appended (on RHS)")
+ (should (string= (concat "\5\0\0\4"
+ "\x26\x05\xbc\x80\x30\x10\x00\x60"
+ "\xde\xad\xbe\xef\xca\xfe\xfe\xd9")
+ (process-get proc 'socks-scratch)))))
+ (ert-info ("Final part arrives (port number)")
+ (socks-filter proc "\0\0")
+ (ert-info ("State transitions to complete")
+ (should (eq (process-get proc 'socks-state) socks-state-connected)))
+ (ert-info ("Scratch and response fields show last chunk appended")
+ (should (string= (concat "\5\0\0\4"
+ "\x26\x05\xbc\x80\x30\x10\x00\x60"
+ "\xde\xad\xbe\xef\xca\xfe\xfe\xd9"
+ "\0\0")
+ (process-get proc 'socks-scratch)))
+ (should (string= (process-get proc 'socks-response)
+ (process-get proc 'socks-scratch)))))
+ (delete-process proc)
+ (kill-buffer buf)))
+
+(defvar socks-tests-canned-server-patterns nil
+ "Alist containing request/response cons pairs to be tried in order.
+Vectors must match verbatim. Strings are considered regex patterns.")
+
+(defun socks-tests-canned-server-create ()
+ "Create and return a fake SOCKS server."
+ (let* ((port (nth 2 socks-server))
+ (name (format "socks-server:%d" port))
+ (pats socks-tests-canned-server-patterns)
(filt (lambda (proc line)
- (let ((resp (or (assoc-default line verbatim
- (lambda (k s) ; s is line
- (string= (concat k) s)))
- (assoc-default line patterns
- (lambda (p s)
- (string-match-p p s))))))
- (unless resp
+ (pcase-let ((`(,pat . ,resp) (pop pats)))
+ (unless (or (and (vectorp pat) (equal pat (vconcat line)))
+ (string-match-p pat line))
(error "Unknown request: %s" line))
(let ((print-escape-control-characters t))
- (princ (format "<- %s\n" (prin1-to-string line)) buf)
- (princ (format "-> %s\n" (prin1-to-string resp)) buf))
+ (message "[%s] <- %s" name (prin1-to-string line))
+ (message "[%s] -> %s" name (prin1-to-string resp)))
(process-send-string proc (concat resp)))))
- (srv (make-network-process :server 1
- :buffer buf
- :filter filt
- :name "server"
- :family 'ipv4
- :host 'local
- :service socks-tests-canned-server-port)))
- (set-process-query-on-exit-flag srv nil)
- (princ (format "[%s] Listening on localhost:10080\n" srv) buf)
- srv))
-
-;; Add ([5 3 0 1 2] . [5 2]) to the `verbatim' list below to validate
-;; against curl 7.71 with the following options:
-;; $ curl --verbose -U foo:bar --proxy socks5h://127.0.0.1:10080 example.com
-;;
-;; If later implementing version 4a, try these:
-;; [4 1 0 80 0 0 0 1 0 ?e ?x ?a ?m ?p ?l ?e ?. ?c ?o ?m 0] . [0 90 0 0 0 0 0 0]
-;; $ curl --verbose --proxy socks4a://127.0.0.1:10080 example.com
+ (serv (make-network-process :server 1
+ :buffer (get-buffer-create name)
+ :filter filt
+ :name name
+ :family 'ipv4
+ :host 'local
+ :coding 'binary
+ :service port)))
+ (set-process-query-on-exit-flag serv nil)
+ serv))
-(ert-deftest socks-tests-auth-filter-url-http ()
- "Verify correct handling of SOCKS5 user/pass authentication."
- (let* ((socks-server '("server" "127.0.0.1" 10080 5))
- (socks-username "foo")
- (socks-password "bar")
- (url-gateway-method 'socks)
+(defvar socks-tests--hello-world-http-request-pattern
+ (cons "^GET /" (concat "HTTP/1.1 200 OK\r\n"
+ "Content-Type: text/plain\r\n"
+ "Content-Length: 13\r\n\r\n"
+ "Hello World!\n")))
+
+(defun socks-tests-perform-hello-world-http-request ()
+ "Start canned server, validate hello-world response, and finalize."
+ (let* ((url-gateway-method 'socks)
(url (url-generic-parse-url "http://example.com"))
- (verbatim '(([5 2 0 2] . [5 2])
- ([1 3 ?f ?o ?o 3 ?b ?a ?r] . [1 0])
- ([5 1 0 3 11 ?e ?x ?a ?m ?p ?l ?e ?. ?c ?o ?m 0 80]
- . [5 0 0 1 0 0 0 0 0 0])))
- (patterns
- `(("^GET /" . ,(concat "HTTP/1.1 200 OK\r\n"
- "Content-Type: text/plain; charset=UTF-8\r\n"
- "Content-Length: 13\r\n\r\n"
- "Hello World!\n"))))
- (socks-tests-canned-server-port 10080)
- (server (socks-tests-canned-server-create verbatim patterns))
- (tries 10)
+ (server (socks-tests-canned-server-create))
;;
done
;;
@@ -90,14 +171,86 @@ socks-tests-auth-filter-url-http
(goto-char (point-min))
(should (search-forward "Hello World" nil t))
(setq done t)))
- (buf (url-http url cb '(nil))))
- (ert-info ("Connect to HTTP endpoint over SOCKS5 with USER/PASS method")
- (while (and (not done) (< 0 (cl-decf tries))) ; cl-lib via url-http
- (sleep-for 0.1)))
+ (buf (url-http url cb '(nil)))
+ (proc (get-buffer-process buf))
+ (attempts 10))
+ (while (and (not done) (< 0 (cl-decf attempts)))
+ (sleep-for 0.1))
(should done)
(delete-process server)
+ (delete-process proc) ; otherwise seems client proc is sometimes reused
(kill-buffer (process-buffer server))
(kill-buffer buf)
(ignore url-gateway-method)))
+;; Replace first pattern below with ([5 3 0 1 2] . [5 2]) to validate
+;; against curl 7.71 with the following options:
+;; $ curl --verbose -U foo:bar --proxy socks5h://127.0.0.1:10080 example.com
+
+(ert-deftest socks-tests-v5-auth-user-pass ()
+ "Verify correct handling of SOCKS5 user/pass authentication."
+ (should (assq 2 socks-authentication-methods))
+ (let ((socks-server '("server" "127.0.0.1" 10080 5))
+ (socks-username "foo")
+ (socks-password "bar")
+ (url-user-agent "Test/auth-user-pass")
+ (socks-tests-canned-server-patterns
+ `(([5 2 0 2] . [5 2])
+ ([1 3 ?f ?o ?o 3 ?b ?a ?r] . [1 0])
+ ([5 1 0 3 11 ?e ?x ?a ?m ?p ?l ?e ?. ?c ?o ?m 0 80]
+ . [5 0 0 1 0 0 0 0 0 0])
+ ,socks-tests--hello-world-http-request-pattern)))
+ (ert-info ("Make HTTP request over SOCKS5 with USER/PASS auth method")
+ (socks-tests-perform-hello-world-http-request))))
+
+;; Services (like Tor) may be configured without auth but for some
+;; reason still prefer the user/pass method over none when offered both.
+;; Given this library's defaults, the scenario below is possible.
+;;
+;; FYI: RFC 1929 doesn't say that a username or password is required
+;; but notes that the length of both fields should be at least one.
+;; However, both socks.el and curl send zero-length fields (though
+;; curl drops the user part too when the password is empty).
+;;
+;; To verify against curl 7.71, swap out the first two pattern pairs
+;; with ([5 3 0 1 2] . [5 2]) and ([1 0 0] . [1 0]), then run:
+;; $ curl verbose -U "foo:" --proxy socks5h://127.0.0.1:10081 example.com
+
+(ert-deftest socks-tests-v5-auth-user-pass-blank ()
+ "Verify correct SOCKS5 user/pass authentication with empty pass."
+ (should (assq 2 socks-authentication-methods))
+ (let ((socks-server '("server" "127.0.0.1" 10081 5))
+ (socks-username "foo") ; defaults to (user-login-name)
+ (socks-password "") ; simulate user hitting enter when prompted
+ (url-user-agent "Test/auth-user-pass-blank")
+ (socks-tests-canned-server-patterns
+ `(([5 2 0 2] . [5 2])
+ ([1 3 ?f ?o ?o 0] . [1 0])
+ ([5 1 0 3 11 ?e ?x ?a ?m ?p ?l ?e ?. ?c ?o ?m 0 80]
+ . [5 0 0 1 0 0 0 0 0 0])
+ ,socks-tests--hello-world-http-request-pattern)))
+ (ert-info ("Make HTTP request over SOCKS5 with USER/PASS auth method")
+ (socks-tests-perform-hello-world-http-request))))
+
+;; Swap out ([5 2 0 1] . [5 0]) with the first pattern below to validate
+;; against curl 7.71 with the following options:
+;; $ curl --verbose --proxy socks5h://127.0.0.1:10082 example.com
+
+(ert-deftest socks-tests-v5-auth-none ()
+ "Verify correct handling of SOCKS5 when auth method 0 requested."
+ (let ((socks-server '("server" "127.0.0.1" 10082 5))
+ (socks-authentication-methods (append socks-authentication-methods
+ nil))
+ (url-user-agent "Test/auth-none")
+ (socks-tests-canned-server-patterns
+ `(([5 1 0] . [5 0])
+ ([5 1 0 3 11 ?e ?x ?a ?m ?p ?l ?e ?. ?c ?o ?m 0 80]
+ . [5 0 0 1 0 0 0 0 0 0])
+ ,socks-tests--hello-world-http-request-pattern)))
+ (socks-unregister-authentication-method 2)
+ (should-not (assq 2 socks-authentication-methods))
+ (ert-info ("Make HTTP request over SOCKS5 with no auth method")
+ (socks-tests-perform-hello-world-http-request)))
+ (should (assq 2 socks-authentication-methods)))
+
;;; socks-tests.el ends here
--
2.29.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Preserve-numeric-addresses-in-socks-send-command.patch --]
[-- Type: text/x-patch, Size: 2359 bytes --]
From d7c4cbea85f991fe8b2ed3b1edba6e99d3cac98c Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 5 Feb 2021 19:41:04 -0800
Subject: [PATCH 2/2] Preserve numeric addresses in socks-send-command
* lisp/net/socks.el: (socks-send-command):
* test/lisp/net/socks-tests.el: (socks-tests-v4-basic): (Bug#46342).
---
lisp/net/socks.el | 2 +-
test/lisp/net/socks-tests.el | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/lisp/net/socks.el b/lisp/net/socks.el
index 96fafc826b8..019fadd1440 100644
--- a/lisp/net/socks.el
+++ b/lisp/net/socks.el
@@ -393,7 +393,7 @@ socks-send-command
(let ((addr (cond
((or (= atype socks-address-type-v4)
(= atype socks-address-type-v6))
- address)
+ (encode-coding-string address 'latin-1))
((= atype socks-address-type-name)
(format "%c%s" (length address) address))
(t
diff --git a/test/lisp/net/socks-tests.el b/test/lisp/net/socks-tests.el
index d2c21094e7f..7112f42a67c 100644
--- a/test/lisp/net/socks-tests.el
+++ b/test/lisp/net/socks-tests.el
@@ -183,6 +183,26 @@ socks-tests-perform-hello-world-http-request
(kill-buffer buf)
(ignore url-gateway-method)))
+;; Unlike curl, socks.el includes the ID field (but otherwise matches):
+;; $ curl --proxy socks4://127.0.0.1:1080 example.com
+
+(ert-deftest socks-tests-v4-basic ()
+ "Show correct preparation of SOCKS4 connect command."
+ (let ((socks-server '("server" "127.0.0.1" 10079 4))
+ (url-user-agent "Test/4-basic")
+ (socks-tests-canned-server-patterns
+ `(([4 1 0 80 93 184 216 34 ?f ?o ?o 0] . [0 90 0 0 0 0 0 0])
+ ,socks-tests--hello-world-http-request-pattern))
+ socks-nslookup-program)
+ (ert-info ("Make HTTP request over SOCKS4")
+ (cl-letf (((symbol-function 'socks-nslookup-host)
+ (lambda (host)
+ (should (equal host "example.com"))
+ (list 93 184 216 34)))
+ ((symbol-function 'user-full-name)
+ (lambda () "foo")))
+ (socks-tests-perform-hello-world-http-request)))))
+
;; Replace first pattern below with ([5 3 0 1 2] . [5 2]) to validate
;; against curl 7.71 with the following options:
;; $ curl --verbose -U foo:bar --proxy socks5h://127.0.0.1:10080 example.com
--
2.29.2
next prev parent reply other threads:[~2021-02-09 15:17 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-06 11:46 bug#46342: 28.0.50; socks-send-command munges IP address bytes to UTF-8 J.P.
2021-02-06 12:26 ` Eli Zaretskii
2021-02-06 14:19 ` J.P.
2021-02-06 15:15 ` Eli Zaretskii
2021-02-07 14:22 ` J.P.
2021-02-09 15:17 ` J.P. [this message]
2021-02-09 16:14 ` Eli Zaretskii
2021-02-10 13:16 ` J.P.
2021-02-10 16:04 ` Eli Zaretskii
2021-02-11 14:58 ` J.P.
2021-02-11 15:28 ` Eli Zaretskii
2021-02-12 14:30 ` J.P.
2021-02-12 15:04 ` Eli Zaretskii
2021-02-13 15:43 ` J.P.
2021-02-17 14:59 ` J.P.
2021-02-20 9:33 ` Eli Zaretskii
2021-02-20 10:13 ` J.P.
2021-02-20 11:08 ` Eli Zaretskii
2021-02-20 15:08 ` J.P.
2021-02-20 15:19 ` Eli Zaretskii
2021-02-20 10:41 ` J.P.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874kils22e.fsf@neverwas.me \
--to=jp@neverwas.me \
--cc=46342@debbugs.gnu.org \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).