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

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Andreas Schwab-2
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, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



Reply | Threaded
Open this post in threaded view
|

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



Reply | Threaded
Open this post in threaded view
|

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

Tao Fang

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:


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

0001-fix-url-https-over-proxy-implement.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Lars Ingebrigtsen
lo2net <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: Lars Ingebrigtsen <[hidden email]>
> Date: Fri, 25 Dec 2015 22:31:26 +0100
> Cc: [hidden email], [hidden email], [hidden email]
>
> lo2net <[hidden email]> 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 <[hidden email]>.



Reply | Threaded
Open this post in threaded view
|

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

Tao Fang
Eli Zaretskii <[hidden email]> writes:

>> From: Lars Ingebrigtsen <[hidden email]>
>> Date: Fri, 25 Dec 2015 22:31:26 +0100
>> Cc: [hidden email], [hidden email], [hidden email]
>>
>> lo2net <[hidden email]> 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 <[hidden email]>.

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 [hidden email] now?



Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: lo2net <[hidden email]>
> Cc: Lars Ingebrigtsen <[hidden email]>,  [hidden email],  [hidden email],  [hidden email]
> 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 <[hidden email]>.
>
> 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 [hidden email] now?

Form sent off-list.



Reply | Threaded
Open this post in threaded view
|

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

David Engster
Eli Zaretskii writes:

>> From: lo2net <[hidden email]>
>> Cc: Lars Ingebrigtsen <[hidden email]>, [hidden email],
>> [hidden email], [hidden email]
>
>> 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 <[hidden email]>.
>>
>> 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 [hidden email] 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



Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: David Engster <[hidden email]>
> Cc: lo2net <[hidden email]>,  [hidden email],  [hidden email],  [hidden email],  [hidden email]
> 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 [hidden email] now?
> >
> > Form sent off-list.
>
> Any news on the assignment?

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



Reply | Threaded
Open this post in threaded view
|

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

Tao Fang
Eli Zaretskii <[hidden email]> 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

0001-Fix-url-https-over-proxy-implement.-Bug-11788.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: Tao Fang <[hidden email]>
> Cc: David Engster <[hidden email]>,  [hidden email],  [hidden email],  [hidden email],  [hidden email]
> Date: Tue, 15 Mar 2016 23:47:27 +0800
>
> Eli Zaretskii <[hidden email]> 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.



Reply | Threaded
Open this post in threaded view
|

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

Lars Ingebrigtsen
In reply to this post by Tao Fang
Tao Fang <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

Tao Fang
Lars Magne Ingebrigtsen <[hidden email]> 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:


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

0001-Fix-url-https-over-proxy-implement.-Bug-11788.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Lars Ingebrigtsen
Tao Fang <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

David Engster
Lars Magne Ingebrigtsen writes:
> Tao Fang <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

Lars Ingebrigtsen
David Engster <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

David Engster
Lars Magne Ingebrigtsen writes:
> David Engster <[hidden email]> 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



Reply | Threaded
Open this post in threaded view
|

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

John Wiegley
>>>>> David Engster <[hidden email]> 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