Reviewing merge request #2290: Avoid SSL error poisoning

If a client connects with an invalid certificate that doesn't parse properly, this will generate errors that are reported only through the thread error queue. Unless this queue is drained, the next call to SSL_get_error(), which very likely is from a different QSslSocket, will report these additional errors.

Thus, a malicious client connecting to a central server can cause other clients to be disconnected.

Please note that this is a hack; a stopgap measure until a proper solution can be found.

Commits that would be merged:

Version 1
  • Version 1
  • ede20ec
  • a0b69de

    • Invalid client certificates will cause multiple errors, including

Showing ede20ec-a0b69de

Comments

→ State changed from New to Reviewing

Pending – at current time we cannot accept merge requests for src/network/ssl unfortunately.

avatar Andreas Aardal Hanssen | 2010-05-26 10:18:26 UTC

→ State changed from Reviewing to New

Andreas AH has left. MR needs to be reviewed by new person

avatar Marius Storm-Olsen | 2010-10-06 19:57:30 UTC

→ State changed from New to Reviewing

reviewing…

avatar p--hartmann | 2010-10-18 11:36:24 UTC

Could you include a test case which triggers several errors to appear in the queue?

avatar p--hartmann | 2010-10-18 11:51:11 UTC

→ State changed from Reviewing to Revise and resubmit

–> http://bugreports.qt.nokia.com/browse/QTBUG-14513

avatar p--hartmann | 2010-10-18 12:45:12 UTC

→ State changed from Revise and resubmit to Rejected

rejected, I just pushed a more general fix to the problem to 4.7 (commit db1170458ca4a005f63e6aee9fe9cb346e8f54b6). I was trying to reproduce several errors, but did not succeed so far.

But thanks for pointing out the possible security issue!

avatar p--hartmann | 2010-11-11 10:47:08 UTC

Add a new comment:

Login or create an account to post a comment

How to apply this merge request to your repository