Bug 44769

Summary: [Qt] Web Sockets are insecure with QtWebKit
Product: WebKit Reporter: Simon Hausmann <hausmann>
Component: New BugsAssignee: QtWebKit Unassigned <webkit-qt-unassigned>
Status: RESOLVED FIXED    
Severity: Major CC: ademar, commit-queue, laszlo.gombos, markus, yael
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch. none

Description Simon Hausmann 2010-08-27 08:48:32 PDT
The current Socket Stream Handle implementation, in particular SocketStreamHandlePrivate::socketSslErrors() causes ssl certificate errors to be ignored.

For the QtWebKit 2.1 release we should either

    a) Disable web socket support

or 

    b) Abort the connection on ssl certificate errors.
Comment 1 Yael 2010-09-14 10:41:47 PDT
(In reply to comment #0)
> The current Socket Stream Handle implementation, in particular SocketStreamHandlePrivate::socketSslErrors() causes ssl certificate errors to be ignored.
> 
> For the QtWebKit 2.1 release we should either
> 
>     a) Disable web socket support
> 
> or 
> 
>     b) Abort the connection on ssl certificate errors.

Can we abort the connection in WebKit 2.1, but not in webkit.org ?
I'd like to be able to test with my Apache server, but it does not have a valid certificate :-)
Comment 2 Simon Hausmann 2010-09-16 12:22:41 PDT
(In reply to comment #1)
> (In reply to comment #0)
> > The current Socket Stream Handle implementation, in particular SocketStreamHandlePrivate::socketSslErrors() causes ssl certificate errors to be ignored.
> > 
> > For the QtWebKit 2.1 release we should either
> > 
> >     a) Disable web socket support
> > 
> > or 
> > 
> >     b) Abort the connection on ssl certificate errors.
> 
> Can we abort the connection in WebKit 2.1, but not in webkit.org ?
> I'd like to be able to test with my Apache server, but it does not have a valid certificate :-)

Sure. Another option would be to make this behaviour depend on whether we're running in DRT mode or not.
Comment 3 Yael 2010-09-19 08:09:53 PDT
Created attachment 68025 [details]
Patch.

Throw an error when the websocket server certificate is not valid.
Please note that currently DRT does not test secure websocket connections, so there is no impact to current layout tests.
Once DRT gets back support for secure websocket connections, this patch will cause those tests to fail.
If ok with you, let's apply this patch only to webkit 2.1, but not to trunk. I believe that Chromium is using the same approach.
Comment 4 Markus Goetz 2010-09-22 06:23:28 PDT
The patch looks good to me.

At some point in the future we might want to forward the sslErrors signal to the user (=browser implementor) so he can handle it similar to the sslErrors signal that is coming from the QNetworkAccessManager.
Comment 5 Yael 2010-09-23 16:45:43 PDT
After talking to Laszlo today, I don't mind if this lands in the trunk.
Comment 6 Kenneth Rohde Christiansen 2010-09-23 16:52:38 PDT
Comment on attachment 68025 [details]
Patch.

LGTM, r=me
Comment 7 Yael 2010-09-24 05:31:45 PDT
Comment on attachment 68025 [details]
Patch.

Thanks, Kenneth :-)
Comment 8 WebKit Commit Bot 2010-09-24 05:44:53 PDT
Comment on attachment 68025 [details]
Patch.

Clearing flags on attachment: 68025

Committed r68248: <http://trac.webkit.org/changeset/68248>
Comment 9 WebKit Commit Bot 2010-09-24 05:44:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Ademar Reis 2010-09-24 07:17:37 PDT
Revision r68248 cherry-picked into qtwebkit-2.1 with commit a2fab5a <http://gitorious.org/webkit/qtwebkit/commit/a2fab5a>
Comment 11 Yael 2010-09-24 18:11:39 PDT
*** Bug 36655 has been marked as a duplicate of this bug. ***