- Revision
- 174394
- Author
- carlo...@webkit.org
- Date
- 2014-10-07 08:49:07 -0700 (Tue, 07 Oct 2014)
Log Message
[SOUP] TLS errors should take precedence over HTTP authentication
https://bugs.webkit.org/show_bug.cgi?id=137300
Reviewed by Sergio Villar Senin.
Source/WebCore:
Handle TLS errors in got-headers callback to make sure it happens
before starting the authentication process.
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::handleUnignoredTLSErrors): Refactored to receive the
soup message.
(WebCore::gotHeadersCallback): Return early and cancel the request in case of TLS errors.
(WebCore::sendRequestCallback): Do not handle TLS errors here, the request should have already been
cancelled at this point in case of TLS errors.
Tools:
Add a test case to check that authenticate signal is not emitted
in case of TLS errors, and the load fails instead.
* TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp:
(webViewAuthenticationCallback):
(testTLSErrorsHTTPAuth):
(httpsServerCallback):
(beforeAll):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (174393 => 174394)
--- trunk/Source/WebCore/ChangeLog 2014-10-07 15:47:15 UTC (rev 174393)
+++ trunk/Source/WebCore/ChangeLog 2014-10-07 15:49:07 UTC (rev 174394)
@@ -1,3 +1,20 @@
+2014-10-07 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [SOUP] TLS errors should take precedence over HTTP authentication
+ https://bugs.webkit.org/show_bug.cgi?id=137300
+
+ Reviewed by Sergio Villar Senin.
+
+ Handle TLS errors in got-headers callback to make sure it happens
+ before starting the authentication process.
+
+ * platform/network/soup/ResourceHandleSoup.cpp:
+ (WebCore::handleUnignoredTLSErrors): Refactored to receive the
+ soup message.
+ (WebCore::gotHeadersCallback): Return early and cancel the request in case of TLS errors.
+ (WebCore::sendRequestCallback): Do not handle TLS errors here, the request should have already been
+ cancelled at this point in case of TLS errors.
+
2014-10-07 Andy Estes <aes...@apple.com>
Fix a warning when passing an NSInteger to abs().
Modified: trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp (174393 => 174394)
--- trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2014-10-07 15:47:15 UTC (rev 174393)
+++ trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2014-10-07 15:49:07 UTC (rev 174394)
@@ -306,12 +306,42 @@
return httpStatusCode == SOUP_STATUS_PROXY_AUTHENTICATION_REQUIRED || httpStatusCode == SOUP_STATUS_UNAUTHORIZED;
}
+static bool handleUnignoredTLSErrors(ResourceHandle* handle, SoupMessage* message)
+{
+ if (gIgnoreSSLErrors)
+ return false;
+
+ GTlsCertificate* certificate = nullptr;
+ GTlsCertificateFlags tlsErrors = static_cast<GTlsCertificateFlags>(0);
+ soup_message_get_https_status(message, &certificate, &tlsErrors);
+ if (!tlsErrors)
+ return false;
+
+ String lowercaseHostURL = handle->firstRequest().url().host().lower();
+ if (allowsAnyHTTPSCertificateHosts().contains(lowercaseHostURL))
+ return false;
+
+ // We aren't ignoring errors globally, but the user may have already decided to accept this certificate.
+ auto it = clientCertificates().find(lowercaseHostURL);
+ if (it != clientCertificates().end() && it->value.contains(certificate))
+ return false;
+
+ ResourceHandleInternal* d = handle->getInternal();
+ handle->client()->didFail(handle, ResourceError::tlsError(d->m_soupRequest.get(), tlsErrors, certificate));
+ return true;
+}
+
static void gotHeadersCallback(SoupMessage* message, gpointer data)
{
ResourceHandle* handle = static_cast<ResourceHandle*>(data);
if (!handle || handle->cancelledOrClientless())
return;
+ if (handleUnignoredTLSErrors(handle, message)) {
+ handle->cancel();
+ return;
+ }
+
ResourceHandleInternal* d = handle->getInternal();
#if PLATFORM(GTK)
@@ -569,27 +599,6 @@
handle->deref();
}
-static bool handleUnignoredTLSErrors(ResourceHandle* handle)
-{
- ResourceHandleInternal* d = handle->getInternal();
- const ResourceResponse& response = d->m_response;
-
- if (!response.soupMessageTLSErrors() || gIgnoreSSLErrors)
- return false;
-
- String lowercaseHostURL = handle->firstRequest().url().host().lower();
- if (allowsAnyHTTPSCertificateHosts().contains(lowercaseHostURL))
- return false;
-
- // We aren't ignoring errors globally, but the user may have already decided to accept this certificate.
- CertificatesMap::iterator i = clientCertificates().find(lowercaseHostURL);
- if (i != clientCertificates().end() && i->value.contains(response.soupMessageCertificate()))
- return false;
-
- handle->client()->didFail(handle, ResourceError::tlsError(d->m_soupRequest.get(), response.soupMessageTLSErrors(), response.soupMessageCertificate()));
- return true;
-}
-
size_t ResourceHandle::currentStreamPosition() const
{
GInputStream* baseStream = d->m_inputStream.get();
@@ -676,11 +685,6 @@
}
d->m_response.updateFromSoupMessage(soupMessage);
- if (handleUnignoredTLSErrors(handle.get())) {
- cleanupSoupRequestOperation(handle.get());
- return;
- }
-
if (SOUP_STATUS_IS_REDIRECTION(soupMessage->status_code) && shouldRedirect(handle.get())) {
d->m_inputStream = inputStream;
g_input_stream_skip_async(d->m_inputStream.get(), gDefaultReadBufferSize, G_PRIORITY_DEFAULT,
Modified: trunk/Tools/ChangeLog (174393 => 174394)
--- trunk/Tools/ChangeLog 2014-10-07 15:47:15 UTC (rev 174393)
+++ trunk/Tools/ChangeLog 2014-10-07 15:49:07 UTC (rev 174394)
@@ -1,3 +1,19 @@
+2014-10-07 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [SOUP] TLS errors should take precedence over HTTP authentication
+ https://bugs.webkit.org/show_bug.cgi?id=137300
+
+ Reviewed by Sergio Villar Senin.
+
+ Add a test case to check that authenticate signal is not emitted
+ in case of TLS errors, and the load fails instead.
+
+ * TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp:
+ (webViewAuthenticationCallback):
+ (testTLSErrorsHTTPAuth):
+ (httpsServerCallback):
+ (beforeAll):
+
2014-10-06 Ryosuke Niwa <rn...@webkit.org>
Make webkit-patch find-users useful
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp (174393 => 174394)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp 2014-10-07 15:47:15 UTC (rev 174393)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp 2014-10-07 15:49:07 UTC (rev 174394)
@@ -144,6 +144,24 @@
g_assert(!test->m_loadEvents.contains(LoadTrackingTest::LoadCommitted));
}
+static gboolean webViewAuthenticationCallback(WebKitWebView*, WebKitAuthenticationRequest* request)
+{
+ g_assert_not_reached();
+ return TRUE;
+}
+
+
+static void testTLSErrorsHTTPAuth(SSLTest* test, gconstpointer)
+{
+ webkit_web_context_set_tls_errors_policy(webkit_web_view_get_context(test->m_webView), WEBKIT_TLS_ERRORS_POLICY_FAIL);
+ g_signal_connect(test->m_webView, "authenticate", G_CALLBACK(webViewAuthenticationCallback), NULL);
+ test->loadURI(kHttpsServer->getURIForPath("/auth").data());
+ test->waitUntilLoadFinished();
+ g_assert(test->m_loadFailed);
+ g_assert(test->m_loadEvents.contains(LoadTrackingTest::ProvisionalLoadFailed));
+ g_assert(!test->m_loadEvents.contains(LoadTrackingTest::LoadCommitted));
+}
+
class TLSErrorsTest: public SSLTest {
public:
MAKE_GLIB_TEST_FIXTURE(TLSErrorsTest);
@@ -234,6 +252,9 @@
} else if (g_str_equal(path, "/redirect")) {
soup_message_set_status(message, SOUP_STATUS_MOVED_PERMANENTLY);
soup_message_headers_append(message->response_headers, "Location", kHttpServer->getURIForPath("/test-image").data());
+ } else if (g_str_equal(path, "/auth")) {
+ soup_message_set_status(message, SOUP_STATUS_UNAUTHORIZED);
+ soup_message_headers_append(message->response_headers, "WWW-Authenticate", "Basic realm=\"HTTPS auth\"");
} else
soup_message_set_status(message, SOUP_STATUS_NOT_FOUND);
}
@@ -282,6 +303,7 @@
// done in the tls-permission-request test.
SSLTest::add("WebKitWebView", "tls-errors-policy", testTLSErrorsPolicy);
SSLTest::add("WebKitWebView", "tls-errors-redirect-to-http", testTLSErrorsRedirect);
+ SSLTest::add("WebKitWebView", "tls-http-auth", testTLSErrorsHTTPAuth);
TLSErrorsTest::add("WebKitWebView", "load-failed-with-tls-errors", testLoadFailedWithTLSErrors);
}
