Merge lp:~osomon/webbrowser-app/certificate-error-fixes into lp:webbrowser-app

[Olivier Tilloy]

Record host and error code along with certificate fingerprint when whitelisting a certificate error.

Note to the reviewer: this fixes only points 1 and 2 of bug #1377194. Points 3 and 5 require further design input (I’m in discussions with design, they will be addressed separately), and point 4 is being fixed separately (see https://code.launchpad.net/~osomon/webbrowser-app/reset-cancelled-certificateError/+merge/243522).

[Chris Coulson (chrisccoulson)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:809
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1329/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/521/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/291/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/87/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/87
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/87/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/87/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/453/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/519
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/519/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16504
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/297/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1329/rebuild

review: Needs Fixing (continuous-integration)

[lp:~osomon/webbrowser-app/certificate-error-fixes]

810. By Olivier Tilloy on 2014-12-12

Merge the latest changes from trunk,
and resolve a conflict.

811. By Olivier Tilloy on 2014-12-12

Revert meaningless changes to the translation template,
previously committed by accident.

=== added file 'src/app/UrlUtils.js'
--- src/app/UrlUtils.js	1970-01-01 00:00:00 +0000
+++ src/app/UrlUtils.js	2014-12-12 09:55:56 +0000
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2014 Canonical Ltd.
+ *
+ * This file is part of webbrowser-app.
+ *
+ * webbrowser-app is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * webbrowser-app is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+function extractAuthority(url) {
+    var authority = url.toString()
+    var indexOfScheme = authority.indexOf("://")
+    if (indexOfScheme !== -1) {
+        authority = authority.slice(indexOfScheme + 3)
+    }
+    var indexOfPath = authority.indexOf("/")
+    if (indexOfPath !== -1) {
+        authority = authority.slice(0, indexOfPath)
+    }
+    return authority
+}
+
+function extractHost(url) {
+    var host = extractAuthority(url)
+    var indexOfAt = host.indexOf("@")
+    if (indexOfAt !== -1) {
+        host = host.slice(indexOfAt + 1)
+    }
+    var indexOfColon = host.indexOf(":")
+    if (indexOfColon !== -1) {
+        host = host.slice(0, indexOfColon)
+    }
+    return host
+}

=== modified file 'src/app/WebViewImpl.qml'
--- src/app/WebViewImpl.qml	2014-12-03 11:45:36 +0000
+++ src/app/WebViewImpl.qml	2014-12-12 09:55:56 +0000
@@ -26,9 +26,6 @@
     id: webview
 
     property var currentWebview: webview
-    property var certificateError
-    // Invalid certificates the user has explicitly allowed for this session
-    property var allowedCertificates: []
 
     /*experimental.certificateVerificationDialog: CertificateVerificationDialog {}
     experimental.authenticationDialog: AuthenticationDialog {}
@@ -41,6 +38,7 @@
 
     QtObject {
         id: internal
+
         readonly property var downloadMimeTypesBlacklist: [
             "application/x-shockwave-flash", // http://launchpad.net/bugs/1379806
         ]
@@ -89,13 +87,4 @@
         // TODO: we might want to store the answer to avoid requesting
         //       the permission everytime the user visits this site.
     }
-
-    onCertificateError: {
-        if(webview.allowedCertificates.indexOf(error.certificate.fingerprintSHA1) != -1) {
-            error.allow()
-        } else {
-            certificateError = error
-            error.onCancelled.connect(function() { webview.certificateError = null })
-        }
-    }
 }

=== modified file 'src/app/webbrowser/Browser.qml'
--- src/app/webbrowser/Browser.qml	2014-12-11 17:04:20 +0000
+++ src/app/webbrowser/Browser.qml	2014-12-12 09:55:56 +0000
@@ -23,6 +23,7 @@
 import webbrowsercommon.private 0.1
 import "../actions" as Actions
 import ".."
+import "../UrlUtils.js" as UrlUtils
 
 BrowserView {
     id: browser
@@ -106,14 +107,14 @@
             sourceComponent: InvalidCertificateErrorSheet {
                 visible: currentWebview && currentWebview.certificateError != null
                 certificateError: currentWebview ? currentWebview.certificateError : null
-                onAllowed: { 
+                onAllowed: {
                     // Automatically allow future requests involving this
                     // certificate for the duration of the session.
-                    currentWebview.allowedCertificates.push(currentWebview.certificateError.certificate.fingerprintSHA1)
-                    currentWebview.certificateError = null
+                    internal.allowCertificateError(currentWebview.certificateError)
+                    currentWebview.resetCertificateError()
                 }
                 onDenied: {
-                    currentWebview.certificateError = null
+                    currentWebview.resetCertificateError()
                 }
             }
             asynchronous: true
@@ -422,6 +423,8 @@
         id: webviewComponent
 
         WebViewImpl {
+            id: webviewimpl
+
             currentWebview: browser.currentWebview
 
             anchors.fill: parent
@@ -476,6 +479,24 @@
 
             onGeolocationPermissionRequested: requestGeolocationPermission(request)
 
+            property var certificateError
+            function resetCertificateError() {
+                certificateError = null
+            }
+            onCertificateError: {
+                if (!error.isMainFrame || error.isSubresource) {
+                    // Not a main frame document error, just block the content
+                    // (it’s not overridable anyway).
+                    return
+                }
+                if (internal.isCertificateErrorAllowed(error)) {
+                    error.allow()
+                } else {
+                    certificateError = error
+                    error.onCancelled.connect(webviewimpl.resetCertificateError)
+                }
+            }
+
             Loader {
                 id: newTabViewLoader
                 anchors.fill: parent
@@ -528,6 +549,31 @@
             chrome.forceActiveFocus()
             Qt.inputMethod.show() // work around http://pad.lv/1316057
         }
+
+        // Invalid certificates the user has explicitly allowed for this session
+        property var allowedCertificateErrors: []
+
+        function allowCertificateError(error) {
+            var host = UrlUtils.extractHost(error.url)
+            var code = error.certError
+            var fingerprint = error.certificate.fingerprintSHA1
+            allowedCertificateErrors.push([host, code, fingerprint])
+        }
+
+        function isCertificateErrorAllowed(error) {
+            var host = UrlUtils.extractHost(error.url)
+            var code = error.certError
+            var fingerprint = error.certificate.fingerprintSHA1
+            for (var i in allowedCertificateErrors) {
+                var allowed = allowedCertificateErrors[i]
+                if ((host == allowed[0]) &&
+                    (code == allowed[1]) &&
+                    (fingerprint == allowed[2])) {
+                    return true
+                }
+            }
+            return false
+        }
     }
 
     function openUrlInNewTab(url, setCurrent, load) {

=== modified file 'src/app/webbrowser/Chrome.qml'
--- src/app/webbrowser/Chrome.qml	2014-12-03 17:47:38 +0000
+++ src/app/webbrowser/Chrome.qml	2014-12-12 09:55:56 +0000
@@ -56,7 +56,7 @@
             enabled: chrome.webview ? chrome.webview.canGoBack : false
             onTriggered: {
                 // Workaround for https://launchpad.net/bugs/1377198
-                chrome.webview.certificateError = null
+                chrome.webview.resetCertificateError()
                 chrome.webview.goBack()
             }
         }
@@ -80,7 +80,7 @@
             enabled: chrome.webview ? chrome.webview.canGoForward : false
             onTriggered: {
                 // Workaround for https://launchpad.net/bugs/1377198
-                chrome.webview.certificateError = null
+                chrome.webview.resetCertificateError()
                 chrome.webview.goForward()
             }
         }
@@ -113,12 +113,12 @@
 
             onValidated: {
                 // Workaround for https://launchpad.net/bugs/1377198
-                chrome.webview.certificateError = null
+                chrome.webview.resetCertificateError()
                 chrome.webview.url = requestedUrl
             }
             onRequestReload: {
                 // Workaround for https://launchpad.net/bugs/1377198
-                chrome.webview.certificateError = null
+                chrome.webview.resetCertificateError()
                 chrome.webview.reload()
             }
             onRequestStop: chrome.webview.stop()

=== added file 'tests/unittests/qml/tst_UrlUtils.qml'
--- tests/unittests/qml/tst_UrlUtils.qml	1970-01-01 00:00:00 +0000
+++ tests/unittests/qml/tst_UrlUtils.qml	2014-12-12 09:55:56 +0000
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2014 Canonical Ltd.
+ *
+ * This file is part of webbrowser-app.
+ *
+ * webbrowser-app is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * webbrowser-app is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtTest 1.0
+import "../../../src/app/UrlUtils.js" as UrlUtils
+
+TestCase {
+    name: "UrlUtils"
+
+    function test_extractAuthority_data() {
+        return [
+            {url: "", authority: ""},
+            {url: "http://example.org/", authority: "example.org"},
+            {url: "http://www.example.org/", authority: "www.example.org"},
+            {url: "http://www.example.org/foo/bar", authority: "www.example.org"},
+            {url: "http://example.org:2442/foo", authority: "example.org:2442"},
+            {url: "http://user:[e-mail]/", authority: "user:[e-mail]"},
+            {url: "http://user:[e-mail]:2442/", authority: "user:[e-mail]:2442"},
+            {url: "ftp://user:[e-mail]:21/foo/bar", authority: "user:[e-mail]:21"}
+        ]
+    }
+
+    function test_extractAuthority(data) {
+        compare(UrlUtils.extractAuthority(data.url), data.authority)
+    }
+
+    function test_extractHost_data() {
+        return [
+            {url: "http://example.org/", host: "example.org"},
+            {url: "http://www.example.org/", host: "www.example.org"},
+            {url: "http://www.example.org/foo/bar", host: "www.example.org"},
+            {url: "http://example.org:2442/foo", host: "example.org"},
+            {url: "http://user:[e-mail]/", host: "example.org"},
+            {url: "http://user:[e-mail]:2442/", host: "example.org"},
+            {url: "ftp://user:[e-mail]:21/foo/bar", host: "example.org"}
+        ]
+    }
+
+    function test_extractHost(data) {
+        compare(UrlUtils.extractHost(data.url), data.host)
+    }
+}