bug#11788: url-http does not properly handle https over proxy

bug#11788: url-http does not properly handle https over proxy

[Andreas Schwab]

When url-http is retrieving a https url over a http proxy it should use
the CONNECT method instead of trying to connect the proxy over TLS.  The
TLS handshake needs to start only after the proxy has forwarded the
connection to the remote host, and the request then needs to be
continued as if connected directly.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Ivan Shmakov]

	Example:

(setq url-proxy-services
      '(("https" . "squid.example.net:3128")
        ("http"  . "squid.example.net:3128")))

(url-retrieve "http://example.com/"
              (lambda (&rest args) (message "%S" args)))
; → #<buffer  *http proxy.example.net:3128-668753*>
; the buffer holds the expected HTTP response

(url-retrieve "https://duckduckgo.com/"
              (lambda (&rest args) (message "%S" args)))
; → #<buffer  *http proxy.example.net:3128*-832895>
; the buffer holds an error from the proxy

	A part of the problem is in url-proxy:

    68	(defun url-proxy (url callback &optional cbargs)
    69	  ;; Retrieve URL from a proxy.
    70	  ;; Expects `url-using-proxy' to be bound to the specific proxy to use."
    71	  (setq url-using-proxy (url-generic-parse-url url-using-proxy))
    72	
    73	  (cond
    74	   ((string= (url-type url-using-proxy) "http")
    75	    (url-http url callback cbargs))

	Here, neither url-http (which issues the request in plain) nor
	url-https (which tries to establish a TLS connection right away)
	could be appropriate when requesting an HTTPS URI.

	Instead, a plain connection should be established, followed by a
	CONNECT request to the target HOSTNAME:PORT pair, and only
	thereafter TLS is to be started.

    76	   (t
    77	    (error "Don't know how to use proxy `%s'" url-using-proxy))))

-- 
FSF associate member #7257

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[lo2net]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]


Hi all,
  I've wrote a patch to fix this, it works fine for me with gnutls
support enabled, so I thought it may be useful.

PS: If without gnutls support, it needs to be modified to use external
program with https via proxy support(e.g. openssl s_client post-May 2015 release version:
http://rt.openssl.org/Ticket/Display.html?id=2651) other than just throw an
error. But I think very few people will need this since this bug report
stayed with outstanding status for such a long time.

Here is the patch:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fix-url-https-over-proxy-implement.patch --]
[-- Type: text/x-diff, Size: 8408 bytes --]

From 50ad7c62ac8e34a43b8201702153fd6548d09a10 Mon Sep 17 00:00:00 2001
From: lo2net <fangtao0901@gmail.com>
Date: Fri, 24 Jul 2015 23:19:26 +0800
Subject: [PATCH] fix url https over proxy implement

---
 lisp/url/url-http.el | 106 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 92 insertions(+), 14 deletions(-)

diff --git a/lisp/url/url-http.el b/lisp/url/url-http.el
index 6a7d8e2..2bcd0cf 100644
--- a/lisp/url/url-http.el
+++ b/lisp/url/url-http.el
@@ -207,7 +207,7 @@ request.")
 	;; `url-open-stream' needs a buffer in which to do things
 	;; like authentication.  But we use another buffer afterwards.
 	(unwind-protect
-	    (let ((proc (url-open-stream host buf host port gateway-method)))
+	    (let ((proc (url-open-stream host buf (if url-using-proxy (url-host url-using-proxy) host) (if url-using-proxy (url-port url-using-proxy) port) gateway-method)))
 	      ;; url-open-stream might return nil.
 	      (when (processp proc)
 		;; Drop the temp buffer link before killing the buffer.
@@ -912,6 +912,7 @@ should be shown to the user."
 ;; These unfortunately cannot be macros... please ignore them!
 (defun url-http-idle-sentinel (proc why)
   "Remove (now defunct) process PROC from the list of open connections."
+  (url-http-debug "url-http-idle-sentinel(%s %s)" proc why)
   (maphash (lambda (key val)
 		(if (memq proc val)
 		    (puthash key (delq proc val) url-http-open-connections)))
@@ -922,7 +923,7 @@ should be shown to the user."
   ;; and (ii) closed connection due to reusing a HTTP connection which
   ;; we believed was still alive, but which the server closed on us.
   ;; We handle case (ii) by calling `url-http' again.
-  (url-http-debug "url-http-end-of-document-sentinel in buffer (%s)"
+  (url-http-debug "url-http-end-of-document-sentinel(%s %s) in buffer (%s)" proc why
 		  (process-buffer proc))
   (url-http-idle-sentinel proc why)
   (when (buffer-name (process-buffer proc))
@@ -936,7 +937,11 @@ should be shown to the user."
 	       (erase-buffer)
                (let ((url-request-method url-http-method)
                      (url-request-extra-headers url-http-extra-headers)
-                     (url-request-data url-http-data))
+                     (url-request-data url-http-data)
+                     (url-using-proxy (url-find-proxy-for-url url-current-object (url-host url-current-object)))
+                     )
+                 (when url-using-proxy
+                     (setq url-using-proxy (url-generic-parse-url url-using-proxy)))
                  (url-http url-current-object url-callback-function
                            url-callback-arguments (current-buffer)))))
 	    ((url-http-parse-headers)
@@ -1217,16 +1222,16 @@ overriding the value of `url-gateway-method'."
 	 (nsm-noninteractive (or url-request-noninteractive
 				 (and (boundp 'url-http-noninteractive)
 				      url-http-noninteractive)))
-	 (connection (url-http-find-free-connection host port gateway-method))
+     (connection (url-http-find-free-connection (url-host url) (url-port url) gateway-method))
 	 (buffer (or retry-buffer
 		     (generate-new-buffer
-                      (format " *http %s:%d*" host port)))))
+                      (format " *http %s:%d*" (url-host url) (url-port url))))))
     (if (not connection)
 	;; Failed to open the connection for some reason
 	(progn
 	  (kill-buffer buffer)
 	  (setq buffer nil)
-	  (error "Could not create connection to %s:%d" host port))
+	  (error "Could not create connection to %s:%d" (url-host url) (url-port url)))
       (with-current-buffer buffer
 	(mm-disable-multibyte)
 	(setq url-current-object url
@@ -1274,22 +1279,91 @@ overriding the value of `url-gateway-method'."
 
 	(set-process-buffer connection buffer)
 	(set-process-filter connection 'url-http-generic-filter)
+
+    (url-http-debug "url-http() status: %s" (process-status connection))
+
 	(pcase (process-status connection)
           (`connect
            ;; Asynchronous connection
            (set-process-sentinel connection 'url-http-async-sentinel))
           (`failed
            ;; Asynchronous connection failed
-           (error "Could not create connection to %s:%d" host port))
+           (error "Could not create connection to %s:%d" (url-host url) (url-port url)))
           (_
+           (if (and url-http-proxy (string= "https" (url-type url-current-object)))
+               (url-https-proxy-connect connection)
+
            (set-process-sentinel connection
                                  'url-http-end-of-document-sentinel)
-           (process-send-string connection (url-http-create-request))))))
-    buffer))
+           (process-send-string connection (url-http-create-request))
+             )
+           )
+          )
+    ))
+
+    buffer)
+  )
+(defun url-https-proxy-connect (connection)
+  ;; https proxy
+  (setq url-http-after-change-function 'url-https-proxy-after-change-function)
+  (process-send-string connection (format (concat "CONNECT %s:%d HTTP/1.1\r\n"
+                                                  "Host: %s\r\n"
+                                                  "\r\n")
+                                          (url-host url-current-object) (or (url-port url-current-object) 443) (url-host url-current-object)))
+  )
+(defun url-https-proxy-after-change-function (st nd length)
+  (let* ((process-buffer (current-buffer))
+         (proc (get-buffer-process process-buffer))
+         )
+    (goto-char (point-min))
+    (when (re-search-forward "^\r?\n" nil t)
+      (backward-char 1)
+      ;; Saw the end of the headers
+      (setq url-http-end-of-headers (set-marker (make-marker) (point)))
+      (url-http-parse-response)
+      (cond
+       ((null url-http-response-status)
+        ;; We got back a headerless malformed response from the
+        ;; server.
+        (url-http-activate-callback)
+        (error "Malformed response from proxy, fail!"))
+       ((= url-http-response-status 200)
+        (if (gnutls-available-p)
+          (condition-case e
+              (let ((tls-connection (gnutls-negotiate
+                                     :process proc
+                                     :hostname (url-host url-current-object)
+                                     :verify-error nil)))
+                (with-current-buffer process-buffer
+                  (erase-buffer))
+                (set-process-buffer tls-connection process-buffer)
+                (setq url-http-after-change-function 'url-http-wait-for-headers-change-function)
+                (set-process-filter tls-connection 'url-http-generic-filter)
+                (process-send-string tls-connection (url-http-create-request))
+                )
+            (gnutls-error
+             (url-http-activate-callback)
+             (error "gnutls-error: %s" e))
+            (error
+             (url-http-activate-callback)
+             (error "error: %s" e))
+            )
+          (error "error: gnutls support needed!")
+          )
+        )
+       (t
+        ;; (setq url-http-connection-opened nil)
+        (url-http-activate-callback)
+        (error "error response: %d\n" url-http-response-status))
+       )
+      )
+    )
+  )
 
 (defun url-http-async-sentinel (proc why)
   ;; We are performing an asynchronous connection, and a status change
   ;; has occurred.
+  (url-http-debug "url-http-async-sentinel(%s %s)" proc why)
   (when (buffer-name (process-buffer proc))
     (with-current-buffer (process-buffer proc)
       (cond
@@ -1298,11 +1372,15 @@ overriding the value of `url-gateway-method'."
 	(url-http-end-of-document-sentinel proc why))
        ((string= (substring why 0 4) "open")
 	(setq url-http-connection-opened t)
-	(condition-case error
-	    (process-send-string proc (url-http-create-request))
-	  (file-error
-	   (setq url-http-connection-opened nil)
-	   (message "HTTP error: %s" error))))
+        (if (and url-http-proxy (string= "https" (url-type url-current-object)))
+            (url-https-proxy-connect proc)
+          (condition-case error
+              (process-send-string proc (url-http-create-request))
+            (file-error
+             (setq url-http-connection-opened nil)
+             (message "HTTP error: %s" error)))
+          )
+        )
        (t
 	(setf (car url-callback-arguments)
 	      (nconc (list :error (list 'error 'connection-failed why
-- 
2.4.6


[-- Attachment #3: Type: text/plain, Size: 53 bytes --]


If anything goes wrong, please let me know, thanks!

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Lars Ingebrigtsen]

lo2net <fangtao0901@gmail.com> writes:

> PS: If without gnutls support, it needs to be modified to use external
> program with https via proxy support(e.g. openssl s_client post-May 2015 release version:
> http://rt.openssl.org/Ticket/Display.html?id=2651) other than just throw an
> error. But I think very few people will need this since this bug report
> stayed with outstanding status for such a long time.
>
> Here is the patch:

I don't use proxies, so I can't test this, but looking at the code
quickly, it looks good.  (But see comments below on style.)

Do you have FSF copyright assignments for Emacs on file?

> -	    (let ((proc (url-open-stream host buf host port gateway-method)))
> +	    (let ((proc (url-open-stream host buf (if url-using-proxy (url-host url-using-proxy) host) (if url-using-proxy (url-port url-using-proxy) port) gateway-method)))

Lines should preferably not be longer than 80 characters.

> +                     (url-request-data url-http-data)
> +                     (url-using-proxy (url-find-proxy-for-url url-current-object (url-host url-current-object)))
> +                     )

Don't put closing parentheses on separate lines -- they should be on the
previous line.

> -    buffer))
> +           (process-send-string connection (url-http-create-request))
> +             )
> +           )
> +          )
> +    ))

And ditto.  :-)

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

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 25 Dec 2015 22:31:26 +0100
> Cc: schwab@linux-m68k.org, ivan@siamics.net, 11788@debbugs.gnu.org
> 
> lo2net <fangtao0901@gmail.com> writes:
> 
> > PS: If without gnutls support, it needs to be modified to use external
> > program with https via proxy support(e.g. openssl s_client post-May 2015 release version:
> > http://rt.openssl.org/Ticket/Display.html?id=2651) other than just throw an
> > error. But I think very few people will need this since this bug report
> > stayed with outstanding status for such a long time.
> >
> > Here is the patch:
> 
> I don't use proxies, so I can't test this, but looking at the code
> quickly, it looks good.  (But see comments below on style.)
> 
> Do you have FSF copyright assignments for Emacs on file?

There's no assignment on file under the name lo2net <fangtao0901@gmail.com>.

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[lo2net]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Date: Fri, 25 Dec 2015 22:31:26 +0100
>> Cc: schwab@linux-m68k.org, ivan@siamics.net, 11788@debbugs.gnu.org
>> 
>> lo2net <fangtao0901@gmail.com> writes:
>> 
>> > PS: If without gnutls support, it needs to be modified to use external
>> > program with https via proxy support(e.g. openssl s_client post-May 2015 release version:
>> > http://rt.openssl.org/Ticket/Display.html?id=2651) other than just throw an
>> > error. But I think very few people will need this since this bug report
>> > stayed with outstanding status for such a long time.
>> >
>> > Here is the patch:
>> 
>> I don't use proxies, so I can't test this, but looking at the code
>> quickly, it looks good.  (But see comments below on style.)

Sorry about bad coding style, I'll fix that.

>> 
>> Do you have FSF copyright assignments for Emacs on file?
>
> There's no assignment on file under the name lo2net <fangtao0901@gmail.com>.

What should I do next so this bug can be fixed ASAP? Although I've just
read http://www.gnu.org/software/emacs/CONTRIBUTE, but I still can't figure
out. Should I email request-assign.future to assign@gnu.org now?

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Eli Zaretskii]

> From: lo2net <fangtao0901@gmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  schwab@linux-m68k.org,  ivan@siamics.net,  11788@debbugs.gnu.org
> Date: Thu, 31 Dec 2015 00:16:03 +0800
> 
> >> Do you have FSF copyright assignments for Emacs on file?
> >
> > There's no assignment on file under the name lo2net <fangtao0901@gmail.com>.
> 
> What should I do next so this bug can be fixed ASAP? Although I've just
> read http://www.gnu.org/software/emacs/CONTRIBUTE, but I still can't figure
> out. Should I email request-assign.future to assign@gnu.org now?

Form sent off-list.

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[David Engster]

Eli Zaretskii writes:
>> From: lo2net <fangtao0901@gmail.com>
>> Cc: Lars Ingebrigtsen <larsi@gnus.org>, schwab@linux-m68k.org,
>> ivan@siamics.net, 11788@debbugs.gnu.org
>
>> Date: Thu, 31 Dec 2015 00:16:03 +0800
>> 
>> >> Do you have FSF copyright assignments for Emacs on file?
>> >
>> > There's no assignment on file under the name lo2net <fangtao0901@gmail.com>.
>> 
>> What should I do next so this bug can be fixed ASAP? Although I've just
>> read http://www.gnu.org/software/emacs/CONTRIBUTE, but I still can't figure
>> out. Should I email request-assign.future to assign@gnu.org now?
>
> Form sent off-list.

Any news on the assignment?

I've stumbled upon this bug today, and IMHO this is actually pretty
serious. It should definitely be fixed for Emacs 25.1.

It would be OK if https over a proxy simply fails; what I've seen
however is that the proxy connects to the requested host via Port 80
instead (meaning plain http). When a site publishes the same content
over https as well as http, the user is led to believe that she
communicates over an secure channel, when in fact everything is
communicated over plain http. For instance, when I do

M-x eww RET https://www.google.de RET

Emacs will connect to the configured proxy and use a GET request:

 GET https://www.google.de/ HTTP/1.1
 ...

At least the two proxies I tested with (CYAN, tinyproxy) will ignore the
'https' part and send a GET request to www.google.de on Port 80
instead. In effect, Eww will succesfully display the Google web site,
showing 'https://www.google.de' in its URL bar, while in fact everything
I now enter is send over plain http without encryption.

-David

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: lo2net <fangtao0901@gmail.com>,  larsi@gnus.org,  schwab@linux-m68k.org,  ivan@siamics.net,  11788@debbugs.gnu.org
> Date: Tue, 08 Mar 2016 20:41:23 +0100
> 
> >> What should I do next so this bug can be fixed ASAP? Although I've just
> >> read http://www.gnu.org/software/emacs/CONTRIBUTE, but I still can't figure
> >> out. Should I email request-assign.future to assign@gnu.org now?
> >
> > Form sent off-list.
> 
> Any news on the assignment?

No, I still don't see it on file.

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Tao Fang]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

> No, I still don't see it on file.

I've received notice email of the assignment/disclaimer process with the
FSF yesterday, and currently it's complete, please check the file to see
if it's all okay?

And I've re-format the previous attached patch file and maybe somebody
could helping review, modify and apply it to the repo?

Thanks!

-- 
Emacs/Gnus

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch file --]
[-- Type: text/x-diff, Size: 7295 bytes --]

From 2b09b6d1dff2afebc73ce4476eb866c6fd40fba2 Mon Sep 17 00:00:00 2001
From: Tao Fang <fangtao0901@gmail.com>
Date: Tue, 15 Mar 2016 23:04:25 +0800
Subject: [PATCH] Fix url https over proxy implement. (Bug#11788)

---
 lisp/url/url-http.el | 86 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 14 deletions(-)

diff --git a/lisp/url/url-http.el b/lisp/url/url-http.el
index 33f6d11..6855ece 100644
--- a/lisp/url/url-http.el
+++ b/lisp/url/url-http.el
@@ -197,7 +197,10 @@ request.")
 	;; `url-open-stream' needs a buffer in which to do things
 	;; like authentication.  But we use another buffer afterwards.
 	(unwind-protect
-	    (let ((proc (url-open-stream host buf host port gateway-method)))
+	    (let ((proc (url-open-stream host buf
+                                         (if url-using-proxy (url-host url-using-proxy) host)
+                                         (if url-using-proxy (url-port url-using-proxy) port)
+                                         gateway-method)))
 	      ;; url-open-stream might return nil.
 	      (when (processp proc)
 		;; Drop the temp buffer link before killing the buffer.
@@ -925,7 +928,11 @@ should be shown to the user."
 	       (erase-buffer)
                (let ((url-request-method url-http-method)
                      (url-request-extra-headers url-http-extra-headers)
-                     (url-request-data url-http-data))
+                     (url-request-data url-http-data)
+                     (url-using-proxy (url-find-proxy-for-url url-current-object
+                                                              (url-host url-current-object))))
+                 (when url-using-proxy
+                   (setq url-using-proxy (url-generic-parse-url url-using-proxy)))
                  (url-http url-current-object url-callback-function
                            url-callback-arguments (current-buffer)))))
 	    ((url-http-parse-headers)
@@ -1209,17 +1216,17 @@ The return value of this function is the retrieval buffer."
 	 (nsm-noninteractive (or url-request-noninteractive
 				 (and (boundp 'url-http-noninteractive)
 				      url-http-noninteractive)))
-	 (connection (url-http-find-free-connection host port gateway-method))
+	 (connection (url-http-find-free-connection (url-host url) (url-port url) gateway-method))
          (mime-accept-string url-mime-accept-string)
 	 (buffer (or retry-buffer
 		     (generate-new-buffer
-                      (format " *http %s:%d*" host port)))))
+                      (format " *http %s:%d*" (url-host url) (url-port url))))))
     (if (not connection)
 	;; Failed to open the connection for some reason
 	(progn
 	  (kill-buffer buffer)
 	  (setq buffer nil)
-	  (error "Could not create connection to %s:%d" host port))
+	  (error "Could not create connection to %s:%d" (url-host url) (url-port url)))
       (with-current-buffer buffer
 	(mm-disable-multibyte)
 	(setq url-current-object url
@@ -1275,13 +1282,62 @@ The return value of this function is the retrieval buffer."
            (set-process-sentinel connection 'url-http-async-sentinel))
           (`failed
            ;; Asynchronous connection failed
-           (error "Could not create connection to %s:%d" host port))
+           (error "Could not create connection to %s:%d" (url-host url) (url-port url)))
           (_
-           (set-process-sentinel connection
-                                 'url-http-end-of-document-sentinel)
-           (process-send-string connection (url-http-create-request))))))
+           (if (and url-http-proxy (string= "https" (url-type url-current-object)))
+               (url-https-proxy-connect connection)
+             (set-process-sentinel connection 'url-http-end-of-document-sentinel)
+             (process-send-string connection (url-http-create-request)))))))
     buffer))
 
+(defun url-https-proxy-connect (connection)
+  (setq url-http-after-change-function 'url-https-proxy-after-change-function)
+  (process-send-string connection (format (concat "CONNECT %s:%d HTTP/1.1\r\n"
+                                                  "Host: %s\r\n"
+                                                  "\r\n")
+                                          (url-host url-current-object)
+                                          (or (url-port url-current-object) 443)
+                                          (url-host url-current-object))))
+
+(defun url-https-proxy-after-change-function (st nd length)
+  (let* ((process-buffer (current-buffer))
+         (proc (get-buffer-process process-buffer)))
+    (goto-char (point-min))
+    (when (re-search-forward "^\r?\n" nil t)
+      (backward-char 1)
+      ;; Saw the end of the headers
+      (setq url-http-end-of-headers (set-marker (make-marker) (point)))
+      (url-http-parse-response)
+      (cond
+       ((null url-http-response-status)
+        ;; We got back a headerless malformed response from the
+        ;; server.
+        (url-http-activate-callback)
+        (error "Malformed response from proxy, fail!"))
+       ((= url-http-response-status 200)
+        (if (gnutls-available-p)
+            (condition-case e
+                (let ((tls-connection (gnutls-negotiate
+                                       :process proc
+                                       :hostname (url-host url-current-object)
+                                       :verify-error nil)))
+                  (with-current-buffer process-buffer (erase-buffer))
+                  (set-process-buffer tls-connection process-buffer)
+                  (setq url-http-after-change-function 'url-http-wait-for-headers-change-function)
+                  (set-process-filter tls-connection 'url-http-generic-filter)
+                  (process-send-string tls-connection (url-http-create-request)))
+              (gnutls-error
+               (url-http-activate-callback)
+               (error "gnutls-error: %s" e))
+              (error
+               (url-http-activate-callback)
+               (error "error: %s" e)))
+          (error "error: gnutls support needed!")))
+       (t
+        ;; (setq url-http-connection-opened nil)
+        (url-http-activate-callback)
+        (error "error response: %d\n" url-http-response-status))))))
+
 (defun url-http-async-sentinel (proc why)
   ;; We are performing an asynchronous connection, and a status change
   ;; has occurred.
@@ -1293,11 +1349,13 @@ The return value of this function is the retrieval buffer."
 	(url-http-end-of-document-sentinel proc why))
        ((string= (substring why 0 4) "open")
 	(setq url-http-connection-opened t)
-	(condition-case error
-	    (process-send-string proc (url-http-create-request))
-	  (file-error
-	   (setq url-http-connection-opened nil)
-	   (message "HTTP error: %s" error))))
+        (if (and url-http-proxy (string= "https" (url-type url-current-object)))
+            (url-https-proxy-connect proc)
+          (condition-case error
+              (process-send-string proc (url-http-create-request))
+            (file-error
+             (setq url-http-connection-opened nil)
+             (message "HTTP error: %s" error)))))
        (t
 	(setf (car url-callback-arguments)
 	      (nconc (list :error (list 'error 'connection-failed why
-- 
2.7.3

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Eli Zaretskii]

> From: Tao Fang <fangtao0901@gmail.com>
> Cc: David Engster <deng@randomsample.de>,  larsi@gnus.org,  schwab@linux-m68k.org,  ivan@siamics.net,  11788@debbugs.gnu.org
> Date: Tue, 15 Mar 2016 23:47:27 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > No, I still don't see it on file.
> 
> I've received notice email of the assignment/disclaimer process with the
> FSF yesterday, and currently it's complete, please check the file to see
> if it's all okay?

Not yet, probably in a couple of days.

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Lars Magne Ingebrigtsen]

Tao Fang <fangtao0901@gmail.com> writes:

> I've received notice email of the assignment/disclaimer process with the
> FSF yesterday, and currently it's complete, please check the file to see
> if it's all okay?

Your assignment is now on file...

> And I've re-format the previous attached patch file and maybe somebody
> could helping review, modify and apply it to the repo?

Looks basically good, but a few notes:

> -	    (let ((proc (url-open-stream host buf host port gateway-method)))
> +	    (let ((proc (url-open-stream host buf
> +                                         (if url-using-proxy (url-host url-using-proxy) host)
> +                                         (if url-using-proxy (url-port url-using-proxy) port)
> +                                         gateway-method)))

Throughout the code, the lines seem to be too long.  They should
preferably not be more than 80 characters long (unless there's an
absolute need).

[...]

> +                (let ((tls-connection (gnutls-negotiate
> +                                       :process proc
> +                                       :hostname (url-host url-current-object)
> +                                       :verify-error nil)))

After negotiation, you should probably call `nsm-verify-connection'.

Uhm...  and that's it.  Oh, and a NEWS entry saying that url now
supports HTTPS proxies would be nice, and a ChangeLog style commit
message.

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

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Tao Fang]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Throughout the code, the lines seem to be too long.  They should
> preferably not be more than 80 characters long (unless there's an
> absolute need).

> After negotiation, you should probably call `nsm-verify-connection'.

> Uhm...  and that's it.  Oh, and a NEWS entry saying that url now
> supports HTTPS proxies would be nice, and a ChangeLog style commit
> message.

Done with it.

Here is the patch file:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-url-https-over-proxy-implement.-Bug-11788.patch --]
[-- Type: text/x-diff, Size: 8609 bytes --]

From 172363d31b3ad5f45da44aa09652d0e0779ef5f2 Mon Sep 17 00:00:00 2001
From: Tao Fang <fangtao0901@gmail.com>
Date: Tue, 22 Mar 2016 22:39:51 +0800
Subject: [PATCH] Fix url https over proxy implement. (Bug#11788)

* lisp/url/url-http.el: Fix url https over proxy implement. (Bug#11788)

* etc/NEWS: Mention this.
---
 etc/NEWS             |   3 ++
 lisp/url/url-http.el | 105 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 94 insertions(+), 14 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 4414625..7d2cc92 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1193,6 +1193,9 @@ plist will contain a :peer element that has the output of
 programmatically delete all cookies, or cookies from a specific
 domain.
 
++++
+*** The URL package now support https over proxy.
+
 ** Tramp
 
 +++
diff --git a/lisp/url/url-http.el b/lisp/url/url-http.el
index 33f6d11..4f180ed 100644
--- a/lisp/url/url-http.el
+++ b/lisp/url/url-http.el
@@ -197,7 +197,14 @@ request.")
 	;; `url-open-stream' needs a buffer in which to do things
 	;; like authentication.  But we use another buffer afterwards.
 	(unwind-protect
-	    (let ((proc (url-open-stream host buf host port gateway-method)))
+            (let ((proc (url-open-stream host buf
+                                         (if url-using-proxy
+                                             (url-host url-using-proxy)
+                                           host)
+                                         (if url-using-proxy
+                                             (url-port url-using-proxy)
+                                           port)
+                                         gateway-method)))
 	      ;; url-open-stream might return nil.
 	      (when (processp proc)
 		;; Drop the temp buffer link before killing the buffer.
@@ -925,7 +932,13 @@ should be shown to the user."
 	       (erase-buffer)
                (let ((url-request-method url-http-method)
                      (url-request-extra-headers url-http-extra-headers)
-                     (url-request-data url-http-data))
+                     (url-request-data url-http-data)
+                     (url-using-proxy (url-find-proxy-for-url
+                                       url-current-object
+                                       (url-host url-current-object))))
+                 (when url-using-proxy
+                   (setq url-using-proxy
+                         (url-generic-parse-url url-using-proxy)))
                  (url-http url-current-object url-callback-function
                            url-callback-arguments (current-buffer)))))
 	    ((url-http-parse-headers)
@@ -1209,17 +1222,20 @@ The return value of this function is the retrieval buffer."
 	 (nsm-noninteractive (or url-request-noninteractive
 				 (and (boundp 'url-http-noninteractive)
 				      url-http-noninteractive)))
-	 (connection (url-http-find-free-connection host port gateway-method))
+         (connection (url-http-find-free-connection (url-host url)
+                                                    (url-port url)
+                                                    gateway-method))
          (mime-accept-string url-mime-accept-string)
 	 (buffer (or retry-buffer
 		     (generate-new-buffer
-                      (format " *http %s:%d*" host port)))))
+                      (format " *http %s:%d*" (url-host url) (url-port url))))))
     (if (not connection)
 	;; Failed to open the connection for some reason
 	(progn
 	  (kill-buffer buffer)
 	  (setq buffer nil)
-	  (error "Could not create connection to %s:%d" host port))
+          (error "Could not create connection to %s:%d" (url-host url)
+                 (url-port url)))
       (with-current-buffer buffer
 	(mm-disable-multibyte)
 	(setq url-current-object url
@@ -1275,13 +1291,72 @@ The return value of this function is the retrieval buffer."
            (set-process-sentinel connection 'url-http-async-sentinel))
           (`failed
            ;; Asynchronous connection failed
-           (error "Could not create connection to %s:%d" host port))
+           (error "Could not create connection to %s:%d" (url-host url)
+                  (url-port url)))
           (_
-           (set-process-sentinel connection
-                                 'url-http-end-of-document-sentinel)
-           (process-send-string connection (url-http-create-request))))))
+           (if (and url-http-proxy (string= "https"
+                                            (url-type url-current-object)))
+               (url-https-proxy-connect connection)
+             (set-process-sentinel connection
+                                   'url-http-end-of-document-sentinel)
+             (process-send-string connection (url-http-create-request)))))))
     buffer))
 
+(defun url-https-proxy-connect (connection)
+  (setq url-http-after-change-function 'url-https-proxy-after-change-function)
+  (process-send-string connection (format (concat "CONNECT %s:%d HTTP/1.1\r\n"
+                                                  "Host: %s\r\n"
+                                                  "\r\n")
+                                          (url-host url-current-object)
+                                          (or (url-port url-current-object)
+                                              url-https-default-port)
+                                          (url-host url-current-object))))
+
+(defun url-https-proxy-after-change-function (st nd length)
+  (let* ((process-buffer (current-buffer))
+         (proc (get-buffer-process process-buffer)))
+    (goto-char (point-min))
+    (when (re-search-forward "^\r?\n" nil t)
+      (backward-char 1)
+      ;; Saw the end of the headers
+      (setq url-http-end-of-headers (set-marker (make-marker) (point)))
+      (url-http-parse-response)
+      (cond
+       ((null url-http-response-status)
+        ;; We got back a headerless malformed response from the
+        ;; server.
+        (url-http-activate-callback)
+        (error "Malformed response from proxy, fail!"))
+       ((= url-http-response-status 200)
+        (if (gnutls-available-p)
+            (condition-case e
+                (let ((tls-connection (gnutls-negotiate
+                                       :process proc
+                                       :hostname (url-host url-current-object)
+                                       :verify-error nil)))
+                  ;; check certificate validity
+                  (setq tls-connection
+                        (nsm-verify-connection tls-connection
+                                               (url-host url-current-object)
+                                               (url-port url-current-object)))
+                  (with-current-buffer process-buffer (erase-buffer))
+                  (set-process-buffer tls-connection process-buffer)
+                  (setq url-http-after-change-function
+                        'url-http-wait-for-headers-change-function)
+                  (set-process-filter tls-connection 'url-http-generic-filter)
+                  (process-send-string tls-connection
+                                       (url-http-create-request)))
+              (gnutls-error
+               (url-http-activate-callback)
+               (error "gnutls-error: %s" e))
+              (error
+               (url-http-activate-callback)
+               (error "error: %s" e)))
+          (error "error: gnutls support needed!")))
+       (t
+        (url-http-activate-callback)
+        (message "error response: %d" url-http-response-status))))))
+
 (defun url-http-async-sentinel (proc why)
   ;; We are performing an asynchronous connection, and a status change
   ;; has occurred.
@@ -1293,11 +1368,13 @@ The return value of this function is the retrieval buffer."
 	(url-http-end-of-document-sentinel proc why))
        ((string= (substring why 0 4) "open")
 	(setq url-http-connection-opened t)
-	(condition-case error
-	    (process-send-string proc (url-http-create-request))
-	  (file-error
-	   (setq url-http-connection-opened nil)
-	   (message "HTTP error: %s" error))))
+        (if (and url-http-proxy (string= "https" (url-type url-current-object)))
+            (url-https-proxy-connect proc)
+          (condition-case error
+              (process-send-string proc (url-http-create-request))
+            (file-error
+             (setq url-http-connection-opened nil)
+             (message "HTTP error: %s" error)))))
        (t
 	(setf (car url-callback-arguments)
 	      (nconc (list :error (list 'error 'connection-failed why
-- 
2.7.4


[-- Attachment #3: Type: text/plain, Size: 194 bytes --]


Feel free to modify the patch, I'm not very familiar with that :)
But I hope the bug can be fixed ASAP since I don't want to modify it
every time when udpate emacs daily build.

-- 
Emacs/Gnus

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Lars Magne Ingebrigtsen]

Tao Fang <fangtao0901@gmail.com> writes:

> Here is the patch file:

Thanks; applied to the trunk.

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

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[David Engster]

Lars Magne Ingebrigtsen writes:
> Tao Fang <fangtao0901@gmail.com> writes:
>
>> Here is the patch file:
>
> Thanks; applied to the trunk.

As I've written in

http://article.gmane.org/gmane.emacs.bugs/114598

I think this fixes a pretty serious bug and should hence land in
emacs-25.

-David

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[Lars Magne Ingebrigtsen]

David Engster <deng@randomsample.de> writes:

> I think this fixes a pretty serious bug and should hence land in
> emacs-25.

I've now backported it.

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

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[David Engster]

Lars Magne Ingebrigtsen writes:
> David Engster <deng@randomsample.de> writes:
>
>> I think this fixes a pretty serious bug and should hence land in
>> emacs-25.
>
> I've now backported it.

I see you reverted it.

I can understand your reasoning, but IMHO it is unacceptable that people
are led to believe they communicate over https when in fact they
don't. It is not uncommon that sites still accept credentials over http
as well as https.

At least let's properly fail when people try to use https over a proxy.

-David

bug#11788: [babc40c4] still fails to implement HTTPS over HTTP proxy properly

[John Wiegley]

>>>>> David Engster <deng@randomsample.de> writes:

> At least let's properly fail when people try to use https over a proxy.

That sounds like a reasonable behavior for emacs-25, with the new support
going to master.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2