After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 754488 - (DWF-2016-89001) Validate TLS certificates
(DWF-2016-89001) Validate TLS certificates
Status: RESOLVED FIXED
Product: shotwell
Classification: Other
Component: general
0.22.x
Other Linux
: Normal critical
: ---
Assigned To: Shotwell Maintainers
Shotwell Maintainers
Depends on: 751709
Blocks: 748991
 
 
Reported: 2015-09-02 20:21 UTC by Michael Catanzaro
Modified: 2016-03-11 15:12 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Michael Catanzaro 2015-09-02 20:21:31 UTC
Seems Shotwell logs into Facebook, etc. without validating TLS certificates. First reported in bug #751709.

Since you use WebKit1 you're responsible not just for all security bugs since security updates ended a year ago, but also for validating TLS certificates on the SoupSession used by your WebKitWebView, before sending any HTTP headers. I've never done this before, but I think the right way is to connect to WebKitWebView:resource-request-starting, grab the WebKitNetworkRequest, get the SoupMessage property from it, then connect to notify::tls-errors and cancel the message immediately in the signal handler (not sure how to do that). I think you also have to somehow tell libsoup to check for TLS errors in the first place; should be easy if you can find a way to get the SoupSession from WebKit (see below), but I'm not sure how to do that (CCing a couple people who might know).

Much better to just upgrade to WebKit2 (bug #751709), which is secure by default.

Also, since you use SoupSessionAsync outside of WebKit, you need to validate them there too, either by upgrading to plain SoupSession or by setting its ssl-use-system-ca-file property to TRUE, regardless of the version of WebKit in use. But that is easy.
Comment 1 Dan Winship 2015-09-02 20:30:13 UTC
(In reply to Michael Catanzaro from comment #0)
> Since you use WebKit1 you're responsible not just for all security bugs
> since security updates ended a year ago, but also for validating TLS
> certificates on the SoupSession used by your WebKitWebView, before sending
> any HTTP headers. I've never done this before, but I think the right way is
> to connect to WebKitWebView:resource-request-starting, grab the
> WebKitNetworkRequest, get the SoupMessage property from it, then connect to
> notify::tls-errors and cancel the message immediately in the signal handler

No, webkit did things complicatedly because it wanted to be able to "accept this certificate anyway". If you just want to kill the connection when the cert is bad, just get the session object and set ssl-use-system-ca-file on it.
Comment 2 Michael Catanzaro 2015-09-02 20:41:23 UTC
That would be enough to satisfy me, but how to we get the session object from WebKit?
Comment 3 Dan Winship 2015-09-02 20:58:51 UTC
In wk1 there's a function that returns it. I don't remember the precise details.
Comment 4 Michael Catanzaro 2015-09-02 22:00:46 UTC
It's webkit_get_default_session. I'm not sure why I could not find it earlier; I thought I had searched the entire API and found no use of the word "session," but apparently not.
Comment 5 Michael Catanzaro 2015-09-02 23:32:50 UTC
Carlos, what is the right way to do this? I have to set ssl-use-system-ca-file again immediately before each call to webkit_web_view_open. That can't be the right way. I downloaded a tarball of 2.4.9 to look at what it's doing, but it's not unsetting it anywhere that I can see.

E.g. with these modified functions in FacebookPublishing.vala:

    private string get_login_url() {
        string facebook_locale = get_system_locale_as_facebook_locale();

        return "https://cacert.org/%s".printf(facebook_locale);
    }

    public void on_pane_installed() {
        WebKit.get_default_session().ssl_use_system_ca_file = true;
        webview.open(get_login_url());
    }

The load gets stopped properly. But if I try to set ssl_use_system_ca_file anywhere else, it doesn't work. Surely we don't have to do this before each webview.open.

P.S. You don't need a Facebook account to test, just click some photo, click Publish on the bottom, and click Log In.
Comment 6 Michael Catanzaro 2015-10-09 21:00:40 UTC
(In reply to Michael Catanzaro from comment #5)
> Carlos, what is the right way to do this? I have to set
> ssl-use-system-ca-file again immediately before each call to
> webkit_web_view_open. That can't be the right way.

I don't plan to spend more time debugging why this setting is getting changed, so this is the approach I will take unless someone has a better idea....
Comment 7 Carlos Garcia Campos 2015-10-12 08:16:48 UTC
I'm not sure I understand the problem. WebKit1, uses the default soup session (SoupNetworkSession::defaultSession().soupSession()) that doesn't set anything about SSL on the session. But, wk1 also calls WebCore::ResourceHandle::setIgnoreSSLErrors(true); for backwards compatibility reasons. That means that WebKit will not handle the SSL errors at all. And we don't change the ssl-use-system-ca-file property anywhere. So, if what we want is that bad certificates are rejected, we should only set the ssl-use-system-ca-file property once, during the app intialization, for example, before anything is loaded. Also ensure that the ssl-strict is TRUE, but it should be by default.
Comment 8 Michael Catanzaro 2015-12-04 17:14:40 UTC
Patches for this in bug #751709
Comment 9 Michael Catanzaro 2015-12-08 16:44:12 UTC
CVE request: http://www.openwall.com/lists/oss-security/2015/12/04/4
Comment 10 Michael Catanzaro 2015-12-20 21:09:52 UTC
(In reply to Michael Catanzaro from comment #9)
> CVE request: http://www.openwall.com/lists/oss-security/2015/12/04/4

It's gone unanswered.
Comment 11 Michael Catanzaro 2015-12-21 01:13:52 UTC
(In reply to Carlos Garcia Campos from comment #7)
> And we don't change the ssl-use-system-ca-file property anywhere.
> So, if what we want is that bad certificates are rejected, we should only
> set the ssl-use-system-ca-file property once, during the app intialization,
> for example, before anything is loaded.

Well that's what I thought too, but for the record, something about that is wrong; the only way I could find to fix this with WK1 was to set ssl-use-system-ca-file once before every load. I never debugged far enough to figure out why, since Ian ported Shotwell to WK2.
Comment 12 Michael Catanzaro 2016-03-11 15:12:58 UTC
(In reply to Michael Catanzaro from comment #10)
> (In reply to Michael Catanzaro from comment #9)
> > CVE request: http://www.openwall.com/lists/oss-security/2015/12/04/4
> 
> It's gone unanswered.

Since MITRE did not respond despite Shotwell clearly falling into its full coverage tier due to its presence in Ubuntu/Debian/RHEL, I requested a DWF, which is a new project aiming to allocate CVE-compatible IDs in response to widespread difficulty of researchers in receiving new CVE assignments. This issue was allocated DWF-2016-89001.