SSL dialogs improvements

Review Request #101925 - Created July 11, 2011 and submitted

Information
Andrea Diamantini
rekonq
master
Reviewers
rekonq
The patch, available on the remote repo in the branch called "SSL_Dialogs_Improvements",  provides 2 new GUIs, similar to the ones provided by Google Chrome, to notify users about SSL infos.
The patch allows also to export the certificate in binary form. Further upgrades in the SSL management have to be done in kdelibs.

"Side" change to render the same informations Chrom* does is about history: upgraded HISTORY_VERSION and added a firstDateTimeVisit entry in the HistoryTime struct.
Tested against the following sites:
- https://localhost, with an untrusted and expired certificate
- https://encrypted.google.com
- https://paypal.com
- https://sites.nea.org/JoinNea/

In all these sites, rekonq behaves similar to what Chrom* does.
Andrea Diamantini
Andrea Diamantini
Andrea Diamantini
Andrea Diamantini
Review request changed
Richard Moore

   
src/sslinfodialog.cpp (Diff revision 1)
 
 
Errors may contain < or & characters. You need something like:

s = s.replace(QL1S("&"), QL1S("&amp;"));
s = s.replace(QL1S("<"), QL1S("&lt;"));
  1. This escaping can be better implemented by using Qt::escape.
    
    http://doc.qt.nokia.com/4.7/qt.html#escape
src/sslinfodialog.cpp (Diff revision 1)
 
 
All of these fields can contain & or < and need quoting as above.
src/sslinfodialog.cpp (Diff revision 1)
 
 
Using a SHA1 hash is better than MD5 since colliding certificates for MD5 have been created in practice.
src/sslinfodialog.cpp (Diff revision 1)
 
 
The isValid() function is the wrong one to use. That merely checks the certificate start and end dates, and that it has not been blacklisted (as covered in the docs). You should always use the information from the error chain, not the information from the certificate. This is true for many reasons (eg. this code would say any valid certificate is valid for ANY SITE AT ALL).
src/sslinfodialog.cpp (Diff revision 1)
 
 
The common name is untrusted. You need to strip out dangerous characters here (eg. a common name of ../../../.. is totall possible).
src/sslinfodialog.cpp (Diff revision 1)
 
 
Why does this function even exist? The errors are presented by QSslSocket as a list in the first place. Trying to parse them out of a string is asking for security problems.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
This code is wrong. If the cert is null it doesn't mean the site provided a null certificate - there's no such thing. It means that no certificate was presented at all (eg. the server only supports anonymous diffie hellman encryption).
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
This needs to be quoted as above.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
At least tell the user what the problem is!
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
If the certificate is not valid, the security is not low, it's non-existent since a MITM attack may be occurring. Low should be used when the server only supports weak ciphers.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
This is wrong, the connection may be encrypted using anonymous diffie hellman. Most likely however something would be seriously wrong for this to occur and you should steer the user away.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
Is  m_info.supportedChiperBits() a typo?
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
What does the version of the certificate have to do with the version of SSL in use? Answer - Nothing!

To find out the version of SSL in use you need to ask the QSslSocket.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
SSL2 is not medium security. It should be avoided at all costs.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
If this is unknown then you have a serious problem, not medium security.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
This message does not reflect what the code above does.
Pino Toscano

   
src/sslinfo.ui (Diff revision 1)
 
 
Please run `fixuifiles` (part of kdesdk/scripts) on the ui files, before their final push.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
Why the <h4> tag?
If you want to make the text bold, pick the font of the label, set it bold and set it back.
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
missing i18n()?
src/urlbar/sslwidget.cpp (Diff revision 1)
 
 
"It's" -> "It is".
Please no contractions in i18n strings.
src/webpage.h (Diff revision 1)
 
 
make it const
src/webpage.cpp (Diff revision 1)
 
 
 
 
 
 
Just exit at the first invalid certificate:
  for (cert, ...) { if (!cert.isValid()) return false; }
  return true;
  1. Actually this code is totally bogus. As before, this is merely checking the dates and blacklist status.
Loading...