Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebSocket error handling frame header fragments #10962

Closed
simplerobot opened this issue Apr 13, 2023 · 7 comments
Closed

WebSocket error handling frame header fragments #10962

simplerobot opened this issue Apr 13, 2023 · 7 comments
Assignees

Comments

@simplerobot
Copy link

I did this
I wrote code that created a WebSocket connection and used it do download data for multiple days.

I expected the following
I expected the WebSocket connection to be stable for extended periods of time. (Yes, I do know this is experimental.)

What actually happened
The connection remained stable for a while, but then received unexpected data, often with an unknown opcode.

  • WS: 347 bytes left to decode (0 frame bytes left)
  • WS:235 received FIN bit 1
  • WS: received OPCODE TEXT
  • WS: received 164 bytes payload (0 left, buflen was 347)
  • WS: 179 bytes left to decode (0 frame bytes left)
  • WS:235 received FIN bit 1
  • WS: received OPCODE TEXT
  • WS: received 172 bytes payload (0 left, buflen was 179)
  • WS: 3 bytes left to decode (0 frame bytes left)
  • WS:235 received FIN bit 1
  • WS: received OPCODE TEXT
  • WS:276 plen == 3, EAGAIN
  • reached easy.c:1231
  • WS: 1983 bytes left to decode (0 frame bytes left)
  • WS:235 received FIN bit 0
  • WS: received OPCODE CLOSE
  • WS: received 49 bytes payload (0 left, buflen was 1983)
  • WS: 1932 bytes left to decode (0 frame bytes left)
  • WS:235 received FIN bit 0
  • WS: received OPCODE CONT
  • WS: received 34 bytes payload (0 left, buflen was 1932)
  • WS: 1896 bytes left to decode (0 frame bytes left)
  • WS:235 received FIN bit 0
  • WS: unknown opcode: 5

Note that after the EAGAIN message, we read a packet header with FIN 0, OPCODE CLOSE where before we read FIN 1, OPCODE TEXT.

I dug into this behavior and believe I have found the source of the problem in ws.c. I believe I introduced this defect in my prior suggestion. When ws_decode returns EAGAIN (https://github.com/curl/curl/blob/master/lib/ws.c#L451), the next read happens at the beginning of buffer, even if the previous fragment is not located there. I suggest updating these lines to:

if(result == CURLE_AGAIN) {
/* a packet fragment only, loop and try reading more */
memmove(data->state.buffer, wsp->stillb, wsp->stillblen);
continue;
}

curl/libcurl version
curl 7.88.1 (x86_64-pc-linux-gnu) libcurl/7.88.1 OpenSSL/3.0.2 zlib/1.2.11
Release-Date: 2023-02-20
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp ws wss
Features: alt-svc AsynchDNS HSTS HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL threadsafe TLS-SRP UnixSockets

operating system
Linux virt-linux 5.19.0-35-generic #36 SMP PREEMPT_DYNAMIC Fri Feb 17 15:17:25 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

@calvin2021y
Copy link

maybe upgrade to 8.0.1 or master for the test.

@icing icing self-assigned this Apr 17, 2023
@icing
Copy link
Contributor

icing commented Apr 21, 2023

@simplerobot I made a re-implementation of websocket frame handling in #10999. If you find the time to take this out for some tests, I'd be happy to hear how it fares.

@calvin2021y
Copy link

@icing

I get one very rare error with 8.0.1, not sure related to this.

Program received signal SIGPIPE, Broken pipe.
0x00007ffff7e6a44c in __libc_send (fd=14, buf=0x60c0000a204b, len=24, flags=0) at ../sysdeps/unix/sysv/linux/send.c:28
28      ../sysdeps/unix/sysv/linux/send.c: No such file or directory.
(gdb) bt
#0  0x00007ffff7e6a44c in __libc_send (fd=14, buf=0x60c0000a204b, len=24, flags=0) at ../sysdeps/unix/sysv/linux/send.c:28
#1  0x0000555555846591 in send ()
#2  0x000055555603a69e in wolfIO_Send (sd=14, buf=0x60c0000a204b "\027\003\003", sz=24, wrFlags=-135879604) at wssl/src/wolfio.c:849
#3  EmbedSend (ssl=ssl@entry=0x621000208100, buf=buf@entry=0x60c0000a204b "\027\003\003", sz=sz@entry=24, ctx=<optimized out>) at wssl/src/wolfio.c:306
#4  0x0000555556052a37 in SendBuffered (ssl=ssl@entry=0x621000208100) at wssl/src/internal.c:9991
#5  0x000055555609bfef in SendAlert_ex (ssl=0x621000208100, severity=<optimized out>, type=<optimized out>) at wssl/src/internal.c:23008
#6  0x000055555604712e in SendAlert (ssl=0xe, ssl@entry=0x621000208100, severity=severity@entry=1, type=type@entry=0) at wssl/src/internal.c:23052
#7  0x0000555555d8ddc9 in wolfSSL_shutdown (ssl=0x621000208100) at wssl/src/ssl.c:4419
#8  0x0000555555c87a0a in wolfssl_close (cf=<optimized out>, data=<optimized out>) at wcurl/lib/vtls/wolfssl.c:1006
#9  0x0000555555c83760 in cf_close (cf=0x60400003f450, data=0x62200003c100) at wcurl/lib/vtls/vtls.c:1423
#10 ssl_cf_close (cf=0x60400003f450, data=0x62200003c100) at wcurl/lib/vtls/vtls.c:1495
#11 0x0000555555bb1c3e in cf_setup_close (cf=<optimized out>, data=0x62200003c100) at wcurl/lib/connect.c:1293
#12 0x0000555555c5ffe3 in conn_shutdown (data=0x62200003c100) at wcurl/lib/url.c:692
#13 Curl_disconnect (data=0x62200003c100, conn=0x619000613a80, dead_connection=<optimized out>) at wcurl/lib/url.c:803
#14 0x0000555555c68ff3 in prune_dead_connections (data=0x62200003c100) at wcurl/lib/url.c:1016
#15 0x0000555555c65793 in create_conn (data=0x62200003c100, async=0x7ffff5729230, in_connect=<optimized out>) at wcurl/lib/url.c:3668
#16 Curl_connect (data=<optimized out>, asyncp=0x7ffff5729230, protocol_done=<optimized out>) at wcurl/lib/url.c:3901
#17 0x0000555555c27cf6 in multi_runsingle (multi=<optimized out>, nowp=<optimized out>, data=<optimized out>) at wcurl/lib/multi.c:1965
#18 0x0000555555c2ef4a in multi_socket (multi=<optimized out>, checkall=<optimized out>, s=<optimized out>, ev_bitmask=<optimized out>, running_handles=<optimized out>) at wcurl/lib/multi.c:3230
#19 0x0000555555c2f610 in curl_multi_socket_action (multi=0x614000000240, s=-1, ev_bitmask=0, running_handles=0x7ffff59404c0) at wcurl/lib/multi.c:3351

Can I apply https://github.com/curl/curl/pull/10999.patch without other change into 8.0.1 to test ?

@bagder
Copy link
Member

bagder commented Apr 25, 2023

Program received signal SIGPIPE, Broken pipe.

This sounds totally separate. This is curl (the TLS layer) sending data over the socket when the remote has closed the connection - and curl does not ignore this signal (by choice?)

@bagder bagder closed this as completed in 930c00c Apr 25, 2023
@calvin2021y
Copy link

hi @bagder

Thanks for the tips, now I add extra check before call curl_ws_send, will let your know if the problem still exists.

@calvin2021y
Copy link

hi @bagder

I add check before curl_ws_send, if the data callback function return error then skip send.

after long time run, still get this error.

@bagder
Copy link
Member

bagder commented Apr 27, 2023

@calvin2021y you're talking about a problem that seems to be another one than this issue, which btw is closed because we believe it is fixed. It would be helpful if you worked on a way to reproduce your problem and file a new issue with all the details there!

bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
- state is fully kept at connection, since curl_ws_send() and
  curl_ws_rec() have lifetime beyond usual transfers
- no more limit on frame sizes

Reported-by: simplerobot on github
Fixes curl#10962
Closes curl#10999
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants