Websocket secure server control frames bug (IDFGH-7209)

[baldhead69]

Hi,

The ping sent by client application and received by server are bug.

Same problem than this bug open by me eight months ago.

I think the patch was not applied to any esp-idf.

#7493

[baldhead69]

Confirmed !!!!!!!

The patch was not applied to esp-idf !!!!!

Please update this !!!!!

[ESP-YJM]

@baldhead69 Yes, it was not applied to any idf version. I think userspace parse the payload in PING control frame is not very common. Our websocket server only need support common case. If you need this, you can apply patch manually.

[baldhead69]

@ESP-YJM ,

There is no payload.

The problem is another.
The patch need to be applied in esp-idf.

Test with client sending ping without payload and you will see de problem.

[ESP-YJM]

@baldhead69 Do you mean this patch(https://github.com/espressif/esp-idf/files/7095983/1.zip)?

[baldhead69]

This patch:

#7493 (comment)

[ansuman87]

@baldhead69 Yes, it was not applied to any idf version. I think userspace parse the payload in PING control frame is not very common. Our websocket server only need support common case. If you need this, you can apply patch manually.

@ESP-YJM By common use-case do you mean client not sending PINGS and only the server doing the same, or you mean server not parsing the payload that comes with the PING?

If it's the latter - I think the PING payload is an optional thing according to the standards. But the library here doesn't support an incoming zero payload PING. I get a warning first as "httpd_ws: httpd_ws_recv_frame: Failed to receive the second byte" and finally the error
"wss_echo_server: httpd_ws_recv_frame failed to get frame len with -1".
I don't think @baldhead69 here is asking for the ability of the user to parse the PING payload. If I am not mistaken, they want the zero payload PING to be allowed and that when there is such a PING request from the client a PONG response to be sent subsequently.

If it's the former, if you think it's not common for clients to send PINGS - well if might be true for browsers, but what about non-browser clients? I am using ESP-32 for my IoT project and want my android client, which is connected locally to the ESP device via a websocket connection, to be able to let the user know of any disconnections from the server side. And to achieve that I need to send PINGS to the server at certain intervals. I don't think this is an unusual use-case.

[baldhead69]

@ansuman87 ,

The patch that i commented above resolve this bug, but espressif team dont want to add this patch to esp idf.

Why ? I dont know.

"I don't think @baldhead69 here is asking for the ability of the user to parse the PING payload. If I am not mistaken, they want the zero payload PING to be allowed and that when there is such a PING request from the client a PONG response to be sent subsequently."
Yes.

I am sending a ping with zero payload from my local android application every 25 seconds.

[ansuman87]

@baldhead69 Thank for the reply. I tried the patch, even though the terminal said patch was unsuccessful, I think it was successful. And coming to my observations, I found that this time the PINGS are accepted by the server, but there are no PONG responses, so my client (OkHttp Websocket) closes the connection. Indeed, when I checked the change in the patch, in the "httpd_ws.c" file, a blob of code corresponding to sending of PONG response is absent in the patch. Are there any other changes in any other part of the file/repo that I missed? Please let me know.
On another note are you/ have you implemented the sending of PONG response yourself? I will try to find the actual problem in the part of the code prior to the patch.

[baldhead69]

I think that when the handle_ws_control_frames = true, the background websocket lib doesn't send pong, maybe if handle_ws_control_frames = false the background websocket lib respond with a pong.

In my case i used handle_ws_control_frames = true and i implemented a pong response when i receive a ping frame inside uri handler.

[baldhead69]

@ESP-YJM ,

The close frame received from client have some problems too.
Sometimes the close frame received from client are Ok.

Could it be because I'm releasing the resources allocated to the socket as soon as the "closed_socket_handler" function is called ?

// typedef void (*httpd_close_func_t)(httpd_handle_t hd, int sockfd);

static void closed_socket_handler(httpd_handle_t hd, int sockfd)
{    
    ESP_LOGI(WSS_SERVER_TAG, LOG_USER("Closed socket handler:"));
    ESP_LOGI(WSS_SERVER_TAG, LOG_USER("hd = %p"), hd);
    ESP_LOGI(WSS_SERVER_TAG, LOG_USER("Closed socket = %d\n"), sockfd);
    
    closes_socket_instantly(hd, sockfd);
}

////////////////////////////////////////////////////////////////////////////////////////////
////////////////////////////////////////////////////////////////////////////////////////////
// Closes the socket instantly, freeing up server resources.

static inline void closes_socket_instantly(httpd_handle_t hd, int sockfd)
{ 
    struct linger linger;
    linger.l_onoff = 1;
    linger.l_linger = 0;

    int ret = setsockopt(sockfd, SOL_SOCKET, SO_LINGER, &linger, sizeof(linger) ); 
    if( ret == 0 )
    {
        ESP_LOGI(WSS_SERVER_TAG, "Set SO_LINGER success");
        close(sockfd); // Now call close will send RST to peer and free pcb immediately.
    }
    
    ESP_LOGI(WSS_SERVER_TAG, "Heap: %d", esp_get_free_heap_size());   
    ESP_LOGI(WSS_SERVER_TAG, "MALLOC_CAP_SPIRAM: %d", heap_caps_get_free_size(MALLOC_CAP_SPIRAM) );
    ESP_LOGI(WSS_SERVER_TAG, "MALLOC_CAP_INTERNAL: %d\n", heap_caps_get_free_size(MALLOC_CAP_INTERNAL) );    
}

Error:

W (16266) httpd_ws: httpd_ws_get_frame_type: Failed to read header byte (socket FD invalid), closing socket now
I (16266) WSS_SERVER: URI HANDLER:
I (16271) WSS_SERVER: req->method = 0

E (16276) esp-tls-mbedtls: read error :-0x004C:
W (16282) httpd_ws: httpd_ws_recv_frame: Failed to receive the second byte
I (16289) WSS_SERVER: httpd_ws_recv_frame failed with: ESP_FAIL

[ansuman87]

I think I found the issue. In the "httpd_ws.c" file, if I remove/comment the following lines, I am able to send the PONG responses.

if(httpd_ws_recv_frame(req, &frame, 126) != ESP_OK) {
ESP_LOGD(TAG, LOG_FMT("Cannot receive the full PING frame"));
return ESP_ERR_INVALID_STATE;
}

Of course in my ws_handler I had to bypass this check(for calculating frame length) - httpd_ws_recv_frame() just for the HTTPD_WS_TYPE_PING type.

I am wondering what is the significance of the above lines in the code? @ESP-YJM Is it for security reasons?

[ESP-YJM]

So sorry for replying so late. @baldhead69 @ansuman87 I think the root reason that show wss_echo_server: httpd_ws_recv_frame failed to get frame len with -1 is that the esp_http_server(websocket) has read the PING frame internal by httpd_ws_get_frame_type, and when you enable handle_ws_control_frames = true, the userspace code will call httpd_ws_recv_frame to read PING frame again. So error occur. I will raise a MR to fix it.