[Home]

Summary:ASTERISK-28231: res_http_websocket: Not responding to Connection Close Frame (opcode 8)
Reporter:Jeremy Lainé (sharky)Labels:patch pjsip
Date Opened:2019-01-03 10:04:35.000-0600Date Closed:2019-01-22 18:16:40.000-0600
Priority:MajorRegression?
Status:Closed/CompleteComponents:Resources/res_http_websocket
Versions:16.0.1 Frequency of
Occurrence		Constant
Related
Issues:
Environment:Attachments:( 0) test-ws.py
( 1) websocket-close.patch
( 2) websocket-close-v2.patch
Description:Asterisk's WebSocket implementation does not seem to respond to opcode 8, meaning that a client client which tries to close the WebSocket cleanly ends up hanging.

Quoting RFC 6455 section 5.5.1 Close:

{quote}
  If an endpoint receives a Close frame and did not previously send a
  Close frame, the endpoint MUST send a Close frame in response.
{quote}

I can reproduce this 100% against Asterisk 16.0.1 using a trivial Python script.

NOTE: it would probably be good to run the full autobahn test suite to shake out any other quirks: https://github.com/crossbario/autobahn-testsuite
Comments:By: Asterisk Team (asteriskteam) 2019-01-03 10:04:35.719-0600

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur.
By: Jeremy Lainé (sharky) 2019-01-03 10:07:13.508-0600

Running the attached script against Asterisk results in:

DEBUG:asyncio:Using selector: EpollSelector
DEBUG:websockets.protocol:client - state = CONNECTING
DEBUG:websockets.protocol:client - event = connection_made(<_SelectorSocketTransport fd=6 read=idle write=<idle, bufsize=0>>)
DEBUG:websockets.protocol:client - state = OPEN
DEBUG:websockets.protocol:client - state = CLOSING
DEBUG:websockets.protocol:client > Frame(fin=True, opcode=8, data=b'\x03\xe8', rsv1=False, rsv2=False, rsv3=False)

<- LONG WAIT HERE ->

DEBUG:websockets.protocol:client ! timed out waiting for TCP close
DEBUG:websockets.protocol:client x half-closing TCP connection
DEBUG:websockets.protocol:client ! timed out waiting for TCP close
DEBUG:websockets.protocol:client x closing TCP connection
DEBUG:websockets.protocol:client - event = connection_lost(None)
DEBUG:websockets.protocol:client - state = CLOSED
DEBUG:websockets.protocol:client x code = 1006, reason = [no reason]
By: Jeremy Lainé (sharky) 2019-01-04 07:41:29.561-0600

After some further investigation, there are other issues with the way Asterisk handles CLOSE opcodes:

- the payload which is returned to the calling code is garbage, as no unmasking is performed
- the iostream doesn't get closed
By: Jeremy Lainé (sharky) 2019-01-04 07:49:00.678-0600

This patch:

- makes Asterisk responds to CLOSE opcodes received from the remote party
- ensures unmasking is performed on CLOSE opcodes, otherwise the payload is garbage

What it does not do: close the iostream. I think this would be the right thing to do but would appreciated some guidance on this.
By: Jeremy Lainé (sharky) 2019-01-04 11:01:03.793-0600

Version 2 of my patch has some additional fixes:

- payload_len is zeroed out to avoid the client reading from our temporary buffer
- the stream is actually closed

Note:  the call to ast_websocket_close takes care of setting session->closing so we don't need to set it ourselves.
By: Joshua C. Colp (jcolp) 2019-01-07 05:12:03.130-0600

Do you plan on putting this up for review using the code review process for inclusion?
By: Jeremy Lainé (sharky) 2019-01-07 08:30:07.505-0600

Hi Josh, absolutely I've just put the patch up on gerrit. I would however quite like some guidance as to how Asterisk is *designed* to close the stream, as the only code I see which does this is in the "destructor".
By: Joshua C. Colp (jcolp) 2019-01-07 08:35:04.411-0600

The iostream code was contributed by a community member, I do not recall the specifics.
By: Sean Bright (seanbright) 2019-01-18 16:29:01.322-0600

[~sharky], could you test your gerrit patch with this one: https://gerrit.asterisk.org/#/c/asterisk/+/10886/
By: Jeremy Lainé (sharky) 2019-01-22 06:58:42.352-0600

With the patch I put on gerrit + your patch I get the expected behaviour both for chan_sip and chan_pjsip, thanks Sean!
By: Friendly Automation (friendly-automation) 2019-01-22 18:16:41.677-0600

Change 10898 merged by Joshua C. Colp:
res_http_websocket: respond to CLOSE opcode

[https://gerrit.asterisk.org/10898|https://gerrit.asterisk.org/10898]
By: Friendly Automation (friendly-automation) 2019-01-22 18:17:12.731-0600

Change 10856 merged by Joshua C. Colp:
res_http_websocket: respond to CLOSE opcode

[https://gerrit.asterisk.org/10856|https://gerrit.asterisk.org/10856]
By: Friendly Automation (friendly-automation) 2019-01-22 18:17:32.822-0600

Change 10861 merged by Joshua C. Colp:
res_http_websocket: respond to CLOSE opcode

[https://gerrit.asterisk.org/10861|https://gerrit.asterisk.org/10861]