Bug 214293 - [SOUP] Artificial delay to WebSocket connection to mitigate port scanning attacks
Summary: [SOUP] Artificial delay to WebSocket connection to mitigate port scanning att...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Lauro Moura
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-13 21:59 PDT by Lauro Moura
Modified: 2020-07-15 06:48 PDT (History)
5 users (show)

See Also:


Attachments
Tentative patch (9.78 KB, patch)
2020-07-13 22:38 PDT, Lauro Moura
no flags Details | Formatted Diff | Diff
Updated patch (10.01 KB, patch)
2020-07-14 13:45 PDT, Lauro Moura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lauro Moura 2020-07-13 21:59:24 PDT
r264306/bug213143 added an artificial delay to NetworkSocketStream when the failure to connect to a WebSocket was caused by a closed port.

Soup-based ports follow another path, with the connection failing in WebSocketTaskSoup.cpp with error SOUP_WEBSOCKET_ERROR_NOT_WEBSOCKET, not distinguishing the detail whether it was due to a closed port or not.
Comment 1 Lauro Moura 2020-07-13 22:38:43 PDT
Created attachment 404212 [details]
Tentative patch

This patch adds the randomized delay for SOUP_WEBSOCKET_ERROR_NOT_WEBSOCKET. Initial testing showed no regressions in the hybi suite locally.
Comment 2 Carlos Garcia Campos 2020-07-14 00:13:38 PDT
Comment on attachment 404212 [details]
Tentative patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404212&action=review

> Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp:45
> +    , m_delaySource(RunLoop::main(), this, &WebSocketTask::delayFired)

m_delaySource -> m_delayFailSource or even better m_delayFailTimer

> Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp:68
> +            if (g_error_matches(error.get(), SOUP_WEBSOCKET_ERROR, SOUP_WEBSOCKET_ERROR_NOT_WEBSOCKET)) {

I think we can check the message status code and only do the delay if it is SOUP_STATUS_CANT_CONNECT (and maybe SOUP_STATUS_CANT_CONNECT_PROXY too).

> Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp:69
> +                task->m_errorMessage = error->message;

task->m_errorMessage = String::fromUTF8(error->message);

> Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp:250
> +void WebSocketTask::delayFired()

delayFailTimerFired

> Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp:253
> +    m_delaySource.stop();

You don't need this, it's a one shot, it's already stopped at this point.

> Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.h:70
> +    String m_errorMessage;

m_delayErrorMessage
Comment 3 Lauro Moura 2020-07-14 13:45:33 PDT
Created attachment 404275 [details]
Updated patch
Comment 4 EWS 2020-07-15 06:48:31 PDT
Committed r264394: <https://trac.webkit.org/changeset/264394>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404275 [details].