Better comments for #15308 and a small vulnerability possible

[Tomasz Wasilczyk]

Hi,

I've been looking at #15308 (the accusation of lobotomy) and wanted to 
tidy up and comment this code, to remove any doubts for spectators (the 
patch is attached).

However, I have a doubt about one check, not directly related to the 
original report. The actual cert verification is done in 
ssl_nss_handshake_cb, *if* gsc->verifier is set. It should be, because 
purple_certificate_find_verifier should return non-NULL value: it's 
always called with ("x509","tls_cached") parameters, which should always 
be valid.

But what, if purple_certificate_find_verifier fails? In such case, cert 
validation is disabled. In the patch I attached, I use default NSS's 
cert verifier then. I see two options:
- purple_certificate_find_verifier never fails, so my change (and the 
"else" branch in ssl_nss_handshake_cb) is dead code. In such case, I 
would insist on removing it and replacing with g_return_if_fail, or 
something like that (just in case of totally unexpected fail);
- purple_certificate_find_verifier can fail in certain conditions: in 
such case, gnutls should be updated in the similiar way the nss plugin was.

I guess, in the first case my patch can go straight into public 
repository, since it's just a code cleanup. In second case, it could be 
embargo-ed, depending on your opinion.

Waiting for the opinion,
Tomek

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4225 bytes
Desc: Kryptograficzna sygnatura S/MIME
URL: <https://pidgin.im/cgi-bin/mailman/private/security/attachments/20131210/9ca70723/attachment.bin>