1514688 - Websocket unable to connect through Cloudflare

Status: VERIFIED FIXED

(Core :: Networking: WebSockets, defect, P1)

Description

[harald.schilly]

Attachment: Screenshot from 2018-12-16 14-41-39.png

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.80 Safari/537.36

Steps to reproduce:

* download Firefox 65 beta4 on Linux and started it
* Opened this page https://cocalc.com/app?skip_preflight



Actual results:

* No websocket connection (top right has a connection indicator)
* JS console (sorry, german, it says that it can't establish a wss:// connection)

> Firefox kann keine Verbindung zu dem Server unter wss://cocalc.com/hub/?_primuscb=MUxYhFa aufbauen.
> Die Verbindung zu wss://cocalc.com/hub/?_primuscb=MUxYhFa wurde unterbrochen, während die Seite geladen wurde.


Expected results:

Just like with Firefox 64 or Chrome, the connection should be established.

Additionally, trying to connect via the same way directly to the secret IP address works.

Also, this reminds me a lot of https://bugzilla.mozilla.org/show_bug.cgi?id=1453204

Comment 1

[Matthias Versen [:Matti]]

I can confirm that Firefox64 works and Firefox65b4 and Firefox66 are generating the error message

Comment 2

[Dragana Damjanovic [:dragana]]

I cannot reproduce this. can you make http log for me:

https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging

Comment 3

[harald.schilly]

Attachment: issue-1514688.txt-main.20964

network logfile

Comment 4

[harald.schilly]

Attachment: issue-1514688.txt-child.21614

second network log file

Comment 5

[harald.schilly]

Hmm, maybe you have to wait a few seconds? the top-right connection indicator is orange while it tries to connect. When it connects that orange goes away. Anyway, I've attached the logfiles that were generated.

Comment 6

[Matthias Versen [:Matti]]

I used mozregression-gui which gave me a wrong result in the past but this seems to be correct:

>Bug 1434137 - Implement websockets over http/2 - RFC 8441 r=michal,dragana
>
>https://tools.ietf.org/html/rfc8441
>
>This uses our existing http/2 CONNECT infrastructure (modified) to
>enable the new extended CONNECT form defined by 8441, and pretend for
>the websocket's sake that an http/2 stream is actually a socket. From
>the websocket's point of view, this is relatively non-invasive - a few
>things have changed (http response code, absence of some headers) versus
>http/1.1 websockets, but for the most part, the websocket code doesn't
>care.

Comment 7

[Dragana Damjanovic [:dragana]]

Attachment: In case we already have a h2 connection and it does not support websockets, we should not try again to use websockets over h2. r=michal

There are 2 parts of the patch:
1) we should not try to use websockets over h2 if we already know that they are not supported.
2) make sure that we clean up websockets waiting for the settings frame when we close a h2 connection. (the part 1) will fix the issue, this part is only to be 100% that we some how do not retrigger the issue)

Comment 8

[Dragana Damjanovic [:dragana]]

What was happening:

We try to establish a websocket connection, we find a h2 connection that do not support websockets so we open a new one. The new connection have a tls ticket and can use early-data, so we immediately dispatch the websocket transaction to the new connection and the transaction waits in a queue for the settings frame. At some point we decide to close that h2 connection (because we have another one) and the transactions in the queue are just left hanging...

I should also disable 0-rtt for websocket because we will not know if we websockets are supported before connection is established. I will update the patch.

Comment 9

[Dragana Damjanovic [:dragana]]

(In reply to Dragana Damjanovic [:dragana] from comment #8)
> What was happening:
> 
> We try to establish a websocket connection, we find a h2 connection that do
> not support websockets so we open a new one. The new connection have a tls
> ticket and can use early-data, so we immediately dispatch the websocket
> transaction to the new connection and the transaction waits in a queue for
> the settings frame. At some point we decide to close that h2 connection
> (because we have another one) and the transactions in the queue are just
> left hanging...
> 
> I should also disable 0-rtt for websocket because we will not know if we
> websockets are supported before connection is established. I will update the
> patch.

I forgot that we dispatch all transaction to a h2 connection in ealy-data phase even if they cannot do early-data. We queue them if inside h2 session and websocket transactions will be queued inside h2 session so I do not need to do anything.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Hi Michal, can you please prioritize this review as we're running low on time for uplifts to 65 this cycle? Thanks!

Comment 11

[Dragana Damjanovic [:dragana]]

This will need uplift, but I will wait that it lands on m.-c. first. I will need-info myself not to forget about it.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62e3131970c0
In case we already have a h2 connection and it does not support websockets, we should not try again to use websockets over h2. r=michal

Comment 13

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/62e3131970c0

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Dragana, now that this has landed in Nightly66, could you please nominate for uplift to Beta65? Thanks!

Comment 16

[Dragana Damjanovic [:dragana]]

Comment on attachment 9033177 [details]
In case we already have a h2 connection and it does not support websockets, we should not try again to use websockets over h2. r=michal

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1434137

User impact if declined: Websocket do not work if tls early-data are used.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: This is a page that shows the problem: https://cocalc.com/app?skip_preflight .

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): One part of the patch is very simple, therefore low risk. The second part is a copy of a code that is already used and therefore tested and low risk.

String changes made/needed:

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9033177 [details]
In case we already have a h2 connection and it does not support websockets, we should not try again to use websockets over h2. r=michal

[Triage Comment]
Fixes issues with websockets not working if tls early-data are used. Given the lack of automated test coverage here, I do think we should have QA verify the fix on Beta also. Approved for 65.0b11.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/07265912b0c7

Comment 19

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 66.0a1 (BuildId:20181217220100).

This issue is verified fixed using Firefox 66.0a1 (BuildId:20190116093310) and Firefox 65.0b11 (BuildId:20190114172331) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 16.04 64bit. The connection is successfully established.

Comment 20

[Emil Ghitta, QA [:emilghitta]]

Ups, forgot to remove this sneaky flag. Sorry for the noise.

Comment 21

[Mathias Homann]

this seems to be back in FF 71, at least i can't get websockets connections to work if i'm using a http proxy.

affected sites: ravello (virt-viewer in a webbrowser), twitch (chat funktion), possibly others.