Changeset 230886 in webkit


Ignore:
Timestamp:
Apr 20, 2018 11:36:12 PM (6 years ago)
Author:
Carlos Garcia Campos
Message:

REGRESSION(r228088): [SOUP] Check TLS errors for WebSockets on GTlsConnection::accept-certificate
​https://bugs.webkit.org/show_bug.cgi?id=184804

Source/WebCore:

Reviewed by Michael Catanzaro.

  • platform/network/soup/SocketStreamHandleImpl.h: Add a public url getter.
  • platform/network/soup/SocketStreamHandleImplSoup.cpp:

(WebCore::acceptCertificateCallback): Call SoupNetworkSession::checkTLSErrors() to decide whether to accept the
certificate or not.
(WebCore::connectProgressCallback): Receive the SocketStreamHandle and pass it to acceptCertificateCallback callback.
(WebCore::socketClientEventCallback): Ditto.
(WebCore::SocketStreamHandleImpl::create): Always connect to network events.
(WebCore::wssConnectionAcceptCertificateCallback): Deleted.
(WebCore::wssSocketClientEventCallback): Deleted.

Tools:

Patch by Michael Catanzaro <Michael Catanzaro> on 2018-04-20
Reviewed by Carlos Garcia Campos.

  • TestWebKitAPI/Tests/WebKitGLib/TestSSL.cpp:

(WebSocketTest::WebSocketTest):
(WebSocketTest::~WebSocketTest):
(WebSocketTest::serverWebSocketCallback):
(WebSocketTest::webSocketTestResultCallback):
(WebSocketTest::connectToServerAndWaitForEvents):
(testWebSocketTLSErrors):
(beforeAll):

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r230885 r230886  
     12018-04-20  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        REGRESSION(r228088): [SOUP] Check TLS errors for WebSockets on GTlsConnection::accept-certificate
     4        https://bugs.webkit.org/show_bug.cgi?id=184804
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        * platform/network/soup/SocketStreamHandleImpl.h: Add a public url getter.
     9        * platform/network/soup/SocketStreamHandleImplSoup.cpp:
     10        (WebCore::acceptCertificateCallback): Call SoupNetworkSession::checkTLSErrors() to decide whether to accept the
     11        certificate or not.
     12        (WebCore::connectProgressCallback): Receive the SocketStreamHandle and pass it to acceptCertificateCallback callback.
     13        (WebCore::socketClientEventCallback): Ditto.
     14        (WebCore::SocketStreamHandleImpl::create): Always connect to network events.
     15        (WebCore::wssConnectionAcceptCertificateCallback): Deleted.
     16        (WebCore::wssSocketClientEventCallback): Deleted.
     17
    1182018-04-20  Carlos Garcia Campos  <cgarcia@igalia.com>
    219

  • trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImpl.h

    r230875 r230886  
    5252    virtual ~SocketStreamHandleImpl();
    5353
     54    const URL& url() const { return m_url; }
     55
    5456    void platformSend(const uint8_t* data, size_t length, Function<void(bool)>&&) final;
    5557    void platformSendHandshake(const uint8_t* data, size_t length, const std::optional<CookieRequestHeaderFieldProxy>&, Function<void(bool, bool)>&&) final;

  • trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImplSoup.cpp

    r230875 r230886  
    5151namespace WebCore {
    5252
    53 static gboolean wssConnectionAcceptCertificateCallback(GTlsConnection*, GTlsCertificate*, GTlsCertificateFlags)
    54 {
    55     return TRUE;
     53static gboolean acceptCertificateCallback(GTlsConnection*, GTlsCertificate* certificate, GTlsCertificateFlags errors, SocketStreamHandleImpl* handle)
     54{
     55    // FIXME: Using DeprecatedGlobalSettings from here is a layering violation.
     56    if (DeprecatedGlobalSettings::allowsAnySSLCertificate())
     57        return TRUE;
     58
     59    return !SoupNetworkSession::checkTLSErrors(handle->url(), certificate, errors);
    5660}
    5761
    5862#if SOUP_CHECK_VERSION(2, 61, 90)
    59 static void wssSocketClientEventCallback(SoupSession*, GSocketClientEvent event, GIOStream* connection)
     63static void connectProgressCallback(SoupSession*, GSocketClientEvent event, GIOStream* connection, SocketStreamHandleImpl* handle)
    6064{
    6165    if (event != G_SOCKET_CLIENT_TLS_HANDSHAKING)
    6266        return;
    6367
    64     g_signal_connect(connection, "accept-certificate", G_CALLBACK(wssConnectionAcceptCertificateCallback), nullptr);
     68    g_signal_connect(connection, "accept-certificate", G_CALLBACK(acceptCertificateCallback), handle);
    6569}
    6670#else
    67 static void wssSocketClientEventCallback(GSocketClient*, GSocketClientEvent event, GSocketConnectable*, GIOStream* connection)
     71static void socketClientEventCallback(GSocketClient*, GSocketClientEvent event, GSocketConnectable*, GIOStream* connection, SocketStreamHandleImpl* handle)
    6872{
    6973    if (event != G_SOCKET_CLIENT_TLS_HANDSHAKING)
    7074        return;
    7175
    72     g_signal_connect(connection, "accept-certificate", G_CALLBACK(wssConnectionAcceptCertificateCallback), nullptr);
     76    g_signal_connect(connection, "accept-certificate", G_CALLBACK(acceptCertificateCallback), handle);
    7377}
    7478#endif
    … …  
    7781{
    7882    Ref<SocketStreamHandleImpl> socket = adoptRef(*new SocketStreamHandleImpl(url, client));
    79 
    80     // FIXME: Using DeprecatedGlobalSettings from here is a layering violation.
    81     bool allowsAnySSLCertificate = url.protocolIs("wss") && DeprecatedGlobalSettings::allowsAnySSLCertificate();
    8283
    8384#if SOUP_CHECK_VERSION(2, 61, 90)
    … …  
    8990    Ref<SocketStreamHandle> protectedSocketStreamHandle = socket.copyRef();
    9091    soup_session_connect_async(networkStorageSession->getOrCreateSoupNetworkSession().soupSession(), uri.get(), socket->m_cancellable.get(),
    91         allowsAnySSLCertificate ? reinterpret_cast<SoupSessionConnectProgressCallback>(wssSocketClientEventCallback) : nullptr,
     92        url.protocolIs("wss") ? reinterpret_cast<SoupSessionConnectProgressCallback>(connectProgressCallback) : nullptr,
    9293        reinterpret_cast<GAsyncReadyCallback>(connectedCallback), &protectedSocketStreamHandle.leakRef());
    9394#else
    … …  
    9798    if (url.protocolIs("wss")) {
    9899        g_socket_client_set_tls(socketClient.get(), TRUE);
    99         if (allowsAnySSLCertificate)
    100             g_signal_connect(socketClient.get(), "event", G_CALLBACK(wssSocketClientEventCallback), nullptr);
     100        g_signal_connect(socketClient.get(), "event", G_CALLBACK(socketClientEventCallback), socket.ptr());
    101101    }
    102102    Ref<SocketStreamHandle> protectedSocketStreamHandle = socket.copyRef();

  • trunk/Tools/ChangeLog

    r230883 r230886  
     12018-04-20  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        REGRESSION(r228088): [SOUP] Check TLS errors for WebSockets on GTlsConnection::accept-certificate
     4        https://bugs.webkit.org/show_bug.cgi?id=184804
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        * TestWebKitAPI/Tests/WebKitGLib/TestSSL.cpp:
     9        (WebSocketTest::WebSocketTest):
     10        (WebSocketTest::~WebSocketTest):
     11        (WebSocketTest::serverWebSocketCallback):
     12        (WebSocketTest::webSocketTestResultCallback):
     13        (WebSocketTest::connectToServerAndWaitForEvents):
     14        (testWebSocketTLSErrors):
     15        (beforeAll):
     16
    1172018-04-20  Chris Dumez  <cdumez@apple.com>
    218

  • trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestSSL.cpp

    r218686 r230886  
    339339}
    340340
     341#if SOUP_CHECK_VERSION(2, 50, 0)
     342class WebSocketTest : public WebViewTest {
     343public:
     344    MAKE_GLIB_TEST_FIXTURE(WebSocketTest);
     345
     346    enum EventFlags {
     347        None = 0,
     348        DidServerCompleteHandshake = 1 << 0,
     349        DidOpen = 1 << 1,
     350        DidClose = 1 << 2
     351    };
     352
     353    WebSocketTest()
     354    {
     355        webkit_user_content_manager_register_script_message_handler(m_userContentManager.get(), "event");
     356        g_signal_connect(m_userContentManager.get(), "script-message-received::event", G_CALLBACK(webSocketTestResultCallback), this);
     357    }
     358
     359    virtual ~WebSocketTest()
     360    {
     361        webkit_user_content_manager_unregister_script_message_handler(m_userContentManager.get(), "event");
     362        g_signal_handlers_disconnect_by_data(m_userContentManager.get(), this);
     363    }
     364
     365    static constexpr const char* webSocketTestJSFormat =
     366        "var socket = new WebSocket('%s');"
     367        "socket.addEventListener('open', onOpen);"
     368        "socket.addEventListener('close', onClose);"
     369        "function onOpen() {"
     370        "    window.webkit.messageHandlers.event.postMessage('open');"
     371        "    socket.removeEventListener('close', onClose);"
     372        "}"
     373        "function onClose() {"
     374        "    window.webkit.messageHandlers.event.postMessage('close');"
     375        "    socket.removeEventListener('open', onOpen);"
     376        "}";
     377
     378    static void serverWebSocketCallback(SoupServer*, SoupWebsocketConnection*, const char*, SoupClientContext*, gpointer userData)
     379    {
     380        static_cast<WebSocketTest*>(userData)->m_events |= WebSocketTest::EventFlags::DidServerCompleteHandshake;
     381    }
     382
     383    static void webSocketTestResultCallback(WebKitUserContentManager*, WebKitJavascriptResult* javascriptResult, WebSocketTest* test)
     384    {
     385        GUniquePtr<char> event(WebViewTest::javascriptResultToCString(javascriptResult));
     386        if (!g_strcmp0(event.get(), "open"))
     387            test->m_events |= WebSocketTest::EventFlags::DidOpen;
     388        else if (!g_strcmp0(event.get(), "close"))
     389            test->m_events |= WebSocketTest::EventFlags::DidClose;
     390        else
     391            g_assert_not_reached();
     392        test->quitMainLoop();
     393    }
     394
     395    unsigned connectToServerAndWaitForEvents(WebKitTestServer* server)
     396    {
     397        m_events = 0;
     398
     399        server->addWebSocketHandler(serverWebSocketCallback, this);
     400        GUniquePtr<char> createWebSocketJS(g_strdup_printf(webSocketTestJSFormat, server->getWebSocketURIForPath("/foo").data()));
     401        webkit_web_view_run_javascript(m_webView, createWebSocketJS.get(), nullptr, nullptr, nullptr);
     402        g_main_loop_run(m_mainLoop);
     403        server->removeWebSocketHandler();
     404
     405        return m_events;
     406    }
     407
     408    unsigned m_events { 0 };
     409};
     410
     411static void testWebSocketTLSErrors(WebSocketTest* test, gconstpointer)
     412{
     413    WebKitWebContext* context = webkit_web_view_get_context(test->m_webView);
     414    WebKitTLSErrorsPolicy originalPolicy = webkit_web_context_get_tls_errors_policy(context);
     415    webkit_web_context_set_tls_errors_policy(context, WEBKIT_TLS_ERRORS_POLICY_FAIL);
     416
     417    // First, check that insecure ws:// web sockets work fine.
     418    unsigned events = test->connectToServerAndWaitForEvents(kHttpServer);
     419    g_assert_true(events);
     420    g_assert_true(events & WebSocketTest::EventFlags::DidServerCompleteHandshake);
     421    g_assert_true(events & WebSocketTest::EventFlags::DidOpen);
     422    g_assert_false(events & WebSocketTest::EventFlags::DidClose);
     423
     424    // Try again using wss:// this time. It should be blocked because the
     425    // server certificate is self-signed.
     426    events = test->connectToServerAndWaitForEvents(kHttpsServer);
     427    g_assert_true(events);
     428    g_assert_false(events & WebSocketTest::EventFlags::DidServerCompleteHandshake);
     429    g_assert_false(events & WebSocketTest::EventFlags::DidOpen);
     430    g_assert_true(events & WebSocketTest::EventFlags::DidClose);
     431
     432    // Now try wss:// again, this time ignoring TLS errors.
     433    webkit_web_context_set_tls_errors_policy(context, WEBKIT_TLS_ERRORS_POLICY_IGNORE);
     434    events = test->connectToServerAndWaitForEvents(kHttpsServer);
     435    g_assert_true(events & WebSocketTest::EventFlags::DidServerCompleteHandshake);
     436    g_assert_true(events & WebSocketTest::EventFlags::DidOpen);
     437    g_assert_false(events & WebSocketTest::EventFlags::DidClose);
     438
     439    webkit_web_context_set_tls_errors_policy(context, originalPolicy);
     440}
     441#endif
     442
    341443static void httpsServerCallback(SoupServer* server, SoupMessage* message, const char* path, GHashTable*, SoupClientContext*, gpointer)
    342444{
    … …  
    425527    TLSSubresourceTest::add("WebKitWebView", "tls-subresource", testSubresourceLoadFailedWithTLSErrors);
    426528    TLSErrorsTest::add("WebKitWebView", "load-failed-with-tls-errors", testLoadFailedWithTLSErrors);
     529#if SOUP_CHECK_VERSION(2, 50, 0)
     530    WebSocketTest::add("WebKitWebView", "web-socket-tls-errors", testWebSocketTLSErrors);
     531#endif
    427532}
    428533
Note: See TracChangeset for help on using the changeset viewer.