Bug 44769 – [Qt] Web Sockets are insecure with QtWebKit

• Component: New Bugs
• Status: RESOLVED FIXED

[Simon Hausmann]

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.

[Yael]

(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 :-)

[Simon Hausmann]

(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.

[Yael]

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.

[Markus Goetz]

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.

[Yael]

After talking to Laszlo today, I don't mind if this lands in the trunk.

[Kenneth Rohde Christiansen]

Comment on attachment 68025 [details]
Patch.

LGTM, r=me

[Yael]

Comment on attachment 68025 [details]
Patch.

Thanks, Kenneth :-)

[WebKit Commit Bot]

Comment on attachment 68025 [details]
Patch.

Clearing flags on attachment: 68025

Committed r68248: <http://trac.webkit.org/changeset/68248>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ademar Reis]

Revision r68248 cherry-picked into qtwebkit-2.1 with commit a2fab5a <http://gitorious.org/webkit/qtwebkit/commit/a2fab5a>

[Yael]

*** Bug 36655 has been marked as a duplicate of this bug. ***