Diff
Modified: trunk/Source/WebCore/ChangeLog (230885 => 230886)
--- trunk/Source/WebCore/ChangeLog 2018-04-21 06:22:15 UTC (rev 230885)
+++ trunk/Source/WebCore/ChangeLog 2018-04-21 06:36:12 UTC (rev 230886)
@@ -1,5 +1,22 @@
2018-04-20 Carlos Garcia Campos <cgar...@igalia.com>
+ REGRESSION(r228088): [SOUP] Check TLS errors for WebSockets on GTlsConnection::accept-certificate
+ https://bugs.webkit.org/show_bug.cgi?id=184804
+
+ 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.
+
+2018-04-20 Carlos Garcia Campos <cgar...@igalia.com>
+
[SOUP] Do TLS error checking on GTlsConnection::accept-certificate
https://bugs.webkit.org/show_bug.cgi?id=184480
Modified: trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImpl.h (230885 => 230886)
--- trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImpl.h 2018-04-21 06:22:15 UTC (rev 230885)
+++ trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImpl.h 2018-04-21 06:36:12 UTC (rev 230886)
@@ -51,6 +51,8 @@
static Ref<SocketStreamHandleImpl> create(const URL&, SocketStreamHandleClient&, PAL::SessionID, const String&, SourceApplicationAuditToken&&);
virtual ~SocketStreamHandleImpl();
+ const URL& url() const { return m_url; }
+
void platformSend(const uint8_t* data, size_t length, Function<void(bool)>&&) final;
void platformSendHandshake(const uint8_t* data, size_t length, const std::optional<CookieRequestHeaderFieldProxy>&, Function<void(bool, bool)>&&) final;
void platformClose() final;
Modified: trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImplSoup.cpp (230885 => 230886)
--- trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImplSoup.cpp 2018-04-21 06:22:15 UTC (rev 230885)
+++ trunk/Source/WebCore/platform/network/soup/SocketStreamHandleImplSoup.cpp 2018-04-21 06:36:12 UTC (rev 230886)
@@ -50,26 +50,30 @@
namespace WebCore {
-static gboolean wssConnectionAcceptCertificateCallback(GTlsConnection*, GTlsCertificate*, GTlsCertificateFlags)
+static gboolean acceptCertificateCallback(GTlsConnection*, GTlsCertificate* certificate, GTlsCertificateFlags errors, SocketStreamHandleImpl* handle)
{
- return TRUE;
+ // FIXME: Using DeprecatedGlobalSettings from here is a layering violation.
+ if (DeprecatedGlobalSettings::allowsAnySSLCertificate())
+ return TRUE;
+
+ return !SoupNetworkSession::checkTLSErrors(handle->url(), certificate, errors);
}
#if SOUP_CHECK_VERSION(2, 61, 90)
-static void wssSocketClientEventCallback(SoupSession*, GSocketClientEvent event, GIOStream* connection)
+static void connectProgressCallback(SoupSession*, GSocketClientEvent event, GIOStream* connection, SocketStreamHandleImpl* handle)
{
if (event != G_SOCKET_CLIENT_TLS_HANDSHAKING)
return;
- g_signal_connect(connection, "accept-certificate", G_CALLBACK(wssConnectionAcceptCertificateCallback), nullptr);
+ g_signal_connect(connection, "accept-certificate", G_CALLBACK(acceptCertificateCallback), handle);
}
#else
-static void wssSocketClientEventCallback(GSocketClient*, GSocketClientEvent event, GSocketConnectable*, GIOStream* connection)
+static void socketClientEventCallback(GSocketClient*, GSocketClientEvent event, GSocketConnectable*, GIOStream* connection, SocketStreamHandleImpl* handle)
{
if (event != G_SOCKET_CLIENT_TLS_HANDSHAKING)
return;
- g_signal_connect(connection, "accept-certificate", G_CALLBACK(wssConnectionAcceptCertificateCallback), nullptr);
+ g_signal_connect(connection, "accept-certificate", G_CALLBACK(acceptCertificateCallback), handle);
}
#endif
@@ -77,9 +81,6 @@
{
Ref<SocketStreamHandleImpl> socket = adoptRef(*new SocketStreamHandleImpl(url, client));
- // FIXME: Using DeprecatedGlobalSettings from here is a layering violation.
- bool allowsAnySSLCertificate = url.protocolIs("wss") && DeprecatedGlobalSettings::allowsAnySSLCertificate();
-
#if SOUP_CHECK_VERSION(2, 61, 90)
auto* networkStorageSession = NetworkStorageSession::storageSession(sessionID);
if (!networkStorageSession)
@@ -88,7 +89,7 @@
auto uri = url.createSoupURI();
Ref<SocketStreamHandle> protectedSocketStreamHandle = socket.copyRef();
soup_session_connect_async(networkStorageSession->getOrCreateSoupNetworkSession().soupSession(), uri.get(), socket->m_cancellable.get(),
- allowsAnySSLCertificate ? reinterpret_cast<SoupSessionConnectProgressCallback>(wssSocketClientEventCallback) : nullptr,
+ url.protocolIs("wss") ? reinterpret_cast<SoupSessionConnectProgressCallback>(connectProgressCallback) : nullptr,
reinterpret_cast<GAsyncReadyCallback>(connectedCallback), &protectedSocketStreamHandle.leakRef());
#else
UNUSED_PARAM(sessionID);
@@ -96,8 +97,7 @@
GRefPtr<GSocketClient> socketClient = adoptGRef(g_socket_client_new());
if (url.protocolIs("wss")) {
g_socket_client_set_tls(socketClient.get(), TRUE);
- if (allowsAnySSLCertificate)
- g_signal_connect(socketClient.get(), "event", G_CALLBACK(wssSocketClientEventCallback), nullptr);
+ g_signal_connect(socketClient.get(), "event", G_CALLBACK(socketClientEventCallback), socket.ptr());
}
Ref<SocketStreamHandle> protectedSocketStreamHandle = socket.copyRef();
g_socket_client_connect_to_host_async(socketClient.get(), url.host().utf8().data(), port, socket->m_cancellable.get(),
Modified: trunk/Tools/ChangeLog (230885 => 230886)
--- trunk/Tools/ChangeLog 2018-04-21 06:22:15 UTC (rev 230885)
+++ trunk/Tools/ChangeLog 2018-04-21 06:36:12 UTC (rev 230886)
@@ -1,3 +1,19 @@
+2018-04-20 Michael Catanzaro <mcatanz...@igalia.com>
+
+ REGRESSION(r228088): [SOUP] Check TLS errors for WebSockets on GTlsConnection::accept-certificate
+ https://bugs.webkit.org/show_bug.cgi?id=184804
+
+ Reviewed by Carlos Garcia Campos.
+
+ * TestWebKitAPI/Tests/WebKitGLib/TestSSL.cpp:
+ (WebSocketTest::WebSocketTest):
+ (WebSocketTest::~WebSocketTest):
+ (WebSocketTest::serverWebSocketCallback):
+ (WebSocketTest::webSocketTestResultCallback):
+ (WebSocketTest::connectToServerAndWaitForEvents):
+ (testWebSocketTLSErrors):
+ (beforeAll):
+
2018-04-20 Chris Dumez <cdu...@apple.com>
Unreviewed, update 2 more API tests after r230876.
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestSSL.cpp (230885 => 230886)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestSSL.cpp 2018-04-21 06:22:15 UTC (rev 230885)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestSSL.cpp 2018-04-21 06:36:12 UTC (rev 230886)
@@ -338,6 +338,108 @@
assertIfSSLRequestProcessed = false;
}
+#if SOUP_CHECK_VERSION(2, 50, 0)
+class WebSocketTest : public WebViewTest {
+public:
+ MAKE_GLIB_TEST_FIXTURE(WebSocketTest);
+
+ enum EventFlags {
+ None = 0,
+ DidServerCompleteHandshake = 1 << 0,
+ DidOpen = 1 << 1,
+ DidClose = 1 << 2
+ };
+
+ WebSocketTest()
+ {
+ webkit_user_content_manager_register_script_message_handler(m_userContentManager.get(), "event");
+ g_signal_connect(m_userContentManager.get(), "script-message-received::event", G_CALLBACK(webSocketTestResultCallback), this);
+ }
+
+ virtual ~WebSocketTest()
+ {
+ webkit_user_content_manager_unregister_script_message_handler(m_userContentManager.get(), "event");
+ g_signal_handlers_disconnect_by_data(m_userContentManager.get(), this);
+ }
+
+ static constexpr const char* webSocketTestJSFormat =
+ "var socket = new WebSocket('%s');"
+ "socket.addEventListener('open', onOpen);"
+ "socket.addEventListener('close', onClose);"
+ "function onOpen() {"
+ " window.webkit.messageHandlers.event.postMessage('open');"
+ " socket.removeEventListener('close', onClose);"
+ "}"
+ "function onClose() {"
+ " window.webkit.messageHandlers.event.postMessage('close');"
+ " socket.removeEventListener('open', onOpen);"
+ "}";
+
+ static void serverWebSocketCallback(SoupServer*, SoupWebsocketConnection*, const char*, SoupClientContext*, gpointer userData)
+ {
+ static_cast<WebSocketTest*>(userData)->m_events |= WebSocketTest::EventFlags::DidServerCompleteHandshake;
+ }
+
+ static void webSocketTestResultCallback(WebKitUserContentManager*, WebKitJavascriptResult* _javascript_Result, WebSocketTest* test)
+ {
+ GUniquePtr<char> event(WebViewTest::_javascript_ResultToCString(_javascript_Result));
+ if (!g_strcmp0(event.get(), "open"))
+ test->m_events |= WebSocketTest::EventFlags::DidOpen;
+ else if (!g_strcmp0(event.get(), "close"))
+ test->m_events |= WebSocketTest::EventFlags::DidClose;
+ else
+ g_assert_not_reached();
+ test->quitMainLoop();
+ }
+
+ unsigned connectToServerAndWaitForEvents(WebKitTestServer* server)
+ {
+ m_events = 0;
+
+ server->addWebSocketHandler(serverWebSocketCallback, this);
+ GUniquePtr<char> createWebSocketJS(g_strdup_printf(webSocketTestJSFormat, server->getWebSocketURIForPath("/foo").data()));
+ webkit_web_view_run_javascript(m_webView, createWebSocketJS.get(), nullptr, nullptr, nullptr);
+ g_main_loop_run(m_mainLoop);
+ server->removeWebSocketHandler();
+
+ return m_events;
+ }
+
+ unsigned m_events { 0 };
+};
+
+static void testWebSocketTLSErrors(WebSocketTest* test, gconstpointer)
+{
+ WebKitWebContext* context = webkit_web_view_get_context(test->m_webView);
+ WebKitTLSErrorsPolicy originalPolicy = webkit_web_context_get_tls_errors_policy(context);
+ webkit_web_context_set_tls_errors_policy(context, WEBKIT_TLS_ERRORS_POLICY_FAIL);
+
+ // First, check that insecure ws:// web sockets work fine.
+ unsigned events = test->connectToServerAndWaitForEvents(kHttpServer);
+ g_assert_true(events);
+ g_assert_true(events & WebSocketTest::EventFlags::DidServerCompleteHandshake);
+ g_assert_true(events & WebSocketTest::EventFlags::DidOpen);
+ g_assert_false(events & WebSocketTest::EventFlags::DidClose);
+
+ // Try again using wss:// this time. It should be blocked because the
+ // server certificate is self-signed.
+ events = test->connectToServerAndWaitForEvents(kHttpsServer);
+ g_assert_true(events);
+ g_assert_false(events & WebSocketTest::EventFlags::DidServerCompleteHandshake);
+ g_assert_false(events & WebSocketTest::EventFlags::DidOpen);
+ g_assert_true(events & WebSocketTest::EventFlags::DidClose);
+
+ // Now try wss:// again, this time ignoring TLS errors.
+ webkit_web_context_set_tls_errors_policy(context, WEBKIT_TLS_ERRORS_POLICY_IGNORE);
+ events = test->connectToServerAndWaitForEvents(kHttpsServer);
+ g_assert_true(events & WebSocketTest::EventFlags::DidServerCompleteHandshake);
+ g_assert_true(events & WebSocketTest::EventFlags::DidOpen);
+ g_assert_false(events & WebSocketTest::EventFlags::DidClose);
+
+ webkit_web_context_set_tls_errors_policy(context, originalPolicy);
+}
+#endif
+
static void httpsServerCallback(SoupServer* server, SoupMessage* message, const char* path, GHashTable*, SoupClientContext*, gpointer)
{
if (message->method != SOUP_METHOD_GET) {
@@ -424,6 +526,9 @@
SSLTest::add("WebKitWebView", "tls-http-auth", testTLSErrorsHTTPAuth);
TLSSubresourceTest::add("WebKitWebView", "tls-subresource", testSubresourceLoadFailedWithTLSErrors);
TLSErrorsTest::add("WebKitWebView", "load-failed-with-tls-errors", testLoadFailedWithTLSErrors);
+#if SOUP_CHECK_VERSION(2, 50, 0)
+ WebSocketTest::add("WebKitWebView", "web-socket-tls-errors", testWebSocketTLSErrors);
+#endif
}
void afterAll()
