Cases 2.10 and 2.11: don't reward denial-of-service behavior

[wbudd]

RingSocket follows this RFC 6455 quote to the letter:

If an endpoint receives a Ping frame and has not yet sent Pong frame(s) in response to previous Ping frame(s), the endpoint MAY elect to send a Pong frame for only the most recently processed Ping frame.

Sending multiple pings in rapid succession is much more likely to be a form of denial-of-service attack than to be the result of a valid practical scenario. Only responding to the last ping is a pragmatic way to mitigate that potential for abuse, so deviating from RFC 6455 the way autobahn-testsuite does in case 2.10 and 2.11 can IMO only be interpreted as a bug.

Below is what autobahn-testsuite reports as the reason for marking these cases' outcome as a Fail, whereas as I argue that the Observed data listed should be at least as valid/expected as the listed Expected data:

Case Description

Send 10 Pings with payload.

Case Expectation

Pongs for our Pings with all the payloads. Note: This is not required by the Spec .. but we check for this behaviour anyway. Clean close with normal code.

Case Outcome

Actual events differ from any expected.

Expected:
{'OK': [('pong', u'payload-0'), ('pong', u'payload-1'), ('pong', u'payload-2'), ('pong', u'payload-3'), ('pong', u'payload-4'), ('pong', u'payload-5'), ('pong', u'payload-6'), ('pong', u'payload-7'), ('pong', u'payload-8'), ('pong', u'payload-9')]}

Observed:
[('pong', u'payload-9')]

I note that you're aware of this spec deviation, but I believe it would be a good idea to change this behavior sometime.

_{PS: Autobahn-testsuite saved me lots of time testing WebSocket protocol details. Thank you providing this valuable tool!}

[oberstet]

I note that you're aware of this spec deviation, but I believe it would be a good idea to change this behavior sometime.

yes, strictly speaking, it is a testsuite bug.

having said that: virtually all implementations of websocket I've seen reply to every ping .. so they skip the behavior the rfc would allow: only answer the last reply. then of course there is the practical question: as I am allowed to only answer the "latest ping" .. I can just never answer and pretend I would expect another ping yet. IMO, the rfc spec text is insufficient .. anyways;) it is unlikely that I find time to change the testsuite anytime soon ..

[wbudd]

I agree in the sense that I feel that RFC 6455 should have made an attempt at specifying in what cases a peer has sufficient justification for having "not yet sent Pong frame(s) in response to previous Ping frame(s)".

This is what happens in RingSocket's case: it tries to respond to the last ping frame (if any) of every "chop" (i.e., read() call). However, if that chop also contains one or more full or partial text/binary messages, the pong response is postponed until the last message has been fully parsed. Furthermore, if fragmentation causes parsing of said last message to carry over to the next chop, the postponed pong response may subsequently be cancelled in order to instead respond to any ping frame found in that next chop.

[joffrey-bion]

I currently have to ignore these tests because they are not really conform to the spec.
I am building a multiplatform websocket facade and underlying clients behave differently (so I can't change them)
Not all of them emit all pong frames in these tests, and this should be OK according to the RFC6455, even though I have to admit the spec is not very precise as to when it's acceptable for a peer to skip a pong.

I think these test cases should at worst result in NON_STRICT (not FAILED) if only a single pong is sent after all those pings.

[oberstet]

curious, how do you read the spec? as previously discussed, the current text might render an implementation that never answers a ping valid. conforming to that is pointless, both for an implementation and a testsuite. this current testsuite implementation renders any behavior that does not answer every ping immediately by a pong invalid.

[joffrey-bion]

As I said the spec doesn't actually specify what should be accepted by the endpoint that sends the pings.

The way I understand it for the peer that receives them is that it's ok to just send 1 pong for 3 pings. The idea is that you could cancel the job that is replying to previous pings when receiving a new one, and just reply to the last one.

So in my opinion, the test suite should accept receiving 1 pong after many pings on quick succession, without strictly expecting as many pongs as pings.

[oberstet]

As I said the spec doesn't actually specify what should be accepted by the endpoint that sends the pings.

yes, which is why the testsuite tests 1 fully defined behavior (which is stricter than the spec) - but then wrongly flags any deviation from that with "red" (and this "red" instead of "yellow" flagging is a testsuite bug). agreed!

it's ok to just send 1 pong for 3 pings.

"3" is arbitrary and undefined: I just wait for an infinite number of pings pretending to wait for a possibly next ping to be accumulated before sending any pong

receiving 1 pong after many pings on quick succession

"quick" is undefined: I just wait for an infinite amount of time pretending to wait for the next ping in one succession to batch up before responding with any pong

---

anyways, personally I think the spec is unclear and the testsuite is slightly wrong, but that's "ok", as in, good enough;) but if you have a PR, we can change it! the red->yellow flagging I mean ..

[joffrey-bion]

I guess we understand each other, but let me clarify a bit here.

Yes "3" was arbitrary and "quick" is undefined. These were vague examples that were meant to express my understanding of the high level OK behaviour of the peer that receives pings and is expected to send pongs (which is incorrectly considered as FAILED by the current testsuite).

I believe the rationale for that part of the spec is that this peer should send a pong ASAP, but if for some reason sending that pong frame took so long that another ping frame was received, the protocol allows that peer to skip that pong and just send the one corresponding to the last ping.
I don't believe the idea was for that peer to explicitly wait for "potential additional pings", but I think it was really just to allow cancelling some pongs in the event that another ping is received (at least that's my understanding).

"quick" is undefined: I just wait for an infinite amount of time pretending to wait for the next ping in one succession to batch up before responding with any pong

I would argue that even for a single ping, the spec doesn't define how much time a peer has before sending the corresponding pong: "It SHOULD respond with Pong frame as soon as is practical". And nobody really bats an eye for that imprecision 😄 Why should it be different for multi-ping tests?

In a sense, the same could be said for any frame the test server expects from the client. How long does the test server wait for an expected echoed frame from the client under test? Doesn't it just use a timeout before deciding the client didn't respond?

Why not do the same for ping-pong testing? After sending N pings, the test server could simply verify that the last pong (containing the last ping's body) is correctly received - having all N pongs is nice but unnecessary. This could be a way to distinguish OK from NON_STRICT, although I'm not sure what the semantics for NON_STRICT is, because sending only the last pong is actually strictly respecting the spec.

this "red" instead of "yellow" flagging is a testsuite bug. agreed!

Yes, but just to be clear I think it's still possible to actually fail this test - if the peer doesn't send the pong corresponding to the last ping.