1520153 - FailDelayManager treats different websocket endpoints as one

Status: RESOLVED WORKSFORME

(Core :: Networking: WebSockets, defect)

Description

[gero.posmyk-leinemann]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

When visiting a webpage which uses WebSockets served from multiple subdomains with the same IP (reverse proxy) WebSocket connection errors from Tab A affect Tab B. In extreme cases WebSockets from Tab B cannot connect at all despite the fact that the WebSocket endpoint itself is totally fine (and accessible from a different browser - or after closing Tab A).

This happens especially when you forget about Tab A, the resource there times out and the frontend stubbornly tries to reconnect to that resource. If the retry is even slightly more aggressive than the one FailDelayManager uses we eventually end of in a never ending cooldown.

I think there are two issues here:

1. FailDelayManager maintains identity of WebSocket connections based on <IP>:<port> the connection was established through - which in the proxy case is the same for all subdomains. I think it should be based on the full URL the connection was established with.
2. FailDelayManager.kWSReconnectBaseLifeTime seems oddly big to me. Also I'm not totally sure why it's needed. I wonder if IsExpired could not calculate mLastFailure + toMs(mNextDelay) <= rightNow instead.
   It currently does:
   (mLastFailure + toMs(kWSReconnectBaseLifeTime + mNextDelay)) <= rightNow

Actual results:

WebSockets connections/connection attempts from Tab A on URL a effect those in Tab B to URL b.

Expected results:

WebSockets identity for connection failures should be based on the URL the 'Upgrade' was sent to.

Comment 2

[Michal Novotny [:michal]]

I think there are two issues here:

1. FailDelayManager maintains identity of WebSocket connections based on <IP>:<port> the connection was established through - which in the proxy case is the same for all subdomains. I think it should be based on the full URL the connection was established with.

The mechanism is IMO correctly implemented according to https://tools.ietf.org/html/rfc6455#section-7.2.3. There is no reason to assume that every subdomain has its own server behind a reverse proxy. And since the purpose is to protect the server against too many reconnect attempts after network failure, using IP makes sense. In fact, it's not clear to me why we don't use only IP instead of IP:port.

2. FailDelayManager.kWSReconnectBaseLifeTime seems oddly big to me. Also I'm not totally sure why it's needed. I wonder if IsExpired could not calculate mLastFailure + toMs(mNextDelay) <= rightNow instead.
   It currently does:
   (mLastFailure + toMs(kWSReconnectBaseLifeTime + mNextDelay)) <= rightNow

kWSReconnectBaseLifeTime determines how long we remember the failure. It's not used to calculate mNextDelay. I.e. we will allow next reconnect attempt before the failure is expired.

Comment 3

[gero.posmyk-leinemann]

Hi Michael,

thank you for response!

kWSReconnectBaseLifeTime determines how long we remember the failure. It's not used to calculate mNextDelay. I.e. we will allow next reconnect attempt before the failure is expired.

Agreed. My bad, I overlooked sManager->mFailures.Remove(aChannel->mAddress, aChannel->mPort); which resets the FailDelay on successful connect.

The mechanism is IMO correctly implemented according to https://tools.ietf.org/html/rfc6455#section-7.2.3. There is no reason to assume that every subdomain has its own server behind a reverse proxy. And since the purpose is to protect the server against too many reconnect attempts after network failure, using IP makes sense. In fact, it's not clear to me why we don't use only IP instead of IP:port.

While I'm not 100% firm in the WebSocket specification I thought WebSockets are meant to represent a layer atop TCP, also in terms of addressing model. https://tools.ietf.org/html/rfc6455#section-1.5 (Design Philosophy) seem to support that:

o adds an addressing and protocol naming mechanism to support
multiple services on one port and multiple host names on one IP
address

The section you mentioned https://tools.ietf.org/html/rfc6455#section-7.2.3 (Recovering from Abnormal Closure) actually talks about two concepts: 'server' and 'service'. They seem to be used as synonyms; at least they are not clearly distinguished:

Abnormal closures may be caused by any number of reasons. [...]
Such closures may also be the result of a nontransient
problem, in which case if each deployed client experiences an
abnormal closure and immediately and persistently tries to reconnect,
the server may experience what amounts to a denial-of-service attack
by a large number of clients trying to reconnect. The end result of
such a scenario could be that the service is unable to recover in a
timely manner or recovery is made much more difficult.

(Emphasize mine)

I dug a bit to find definitions for those and found:

1. https://tools.ietf.org/html/rfc6455#section-11.1.1 (Registration of "ws" Scheme)

   A |ws| URI identifies a WebSocket server and resource name.

   Thus a <IP>:<Port> combination identifies a Websocket 'server'. That - combined with https://tools.ietf.org/html/rfc6455#section-1.5 cited above - means that on each 'server', there may be multiple 'services' - who are only distinguishable by their resource name.

2. https://tools.ietf.org/html/rfc6455#section-4.2 (Server-Side Requirements)

   Servers MAY offload the management of the connection to other agents
   on the network, for example, load balancers and reverse proxies. In
   such a situation, the server for the purposes of this specification
   is considered to include all parts of the server-side infrastructure
   from the first device to terminate the TCP connection all the way to
   the server that processes requests and sends responses.

   (Emphasize mine)
   So 'server' also includes everything, up to and including the last unit that handles the request.

So ultimately it's not clear what https://tools.ietf.org/html/rfc6455#section-7.2.3 tries to protect: The whole 'server' or a single 'service'.
One could argue that as the 'server' includes everything up to and including the last unit, and this last unit can only be identified by the resource_name, it MUST be equivalent to a 'service' - but that's a bit too far stretched for my taste.

Still, to me this feels like Firefox is a bit too conservative here. And while it does not explicitly violate the spec I think the solution at least does not fit the philosophy well (see https://tools.ietf.org/html/rfc6455#section-1.5).