On Tue, Jan 25, 2011 at 07:17:27PM +0100, Marc-André Lureau wrote: > Code adapter from RedPeer::ssl_verify_callback() and used by > spice-gtk.
I looked at this one, and was quickly concerned about the amount of security checks we're trying to do on our own. Basically, we let openssl do the certificate verification by itself. However, we try to do 3 additional checks by ourselves as part of the certificate verification: - we try to check if the host we're connecting to (specified through --host) is mentionned in the certificate (maybe to handle certificates stored on vhosts ) - or (mutually exclusive as mentionned in the log) we check the certificate subject matches what was passed on the command line (and there's too much string parsing involved for my taste) - or we check some public key #3 seems to be only used in migration cases, and even there, the pubkey code is just a stub, I couldn't find anything setting the public key data we want to use. It might be better to just drop this part of the code. #1 and #2 probably have their uses (I'd be interested to know what drove adding this code btw), but it's quite some code, and I am no x509 security specialist (far from it, maybe what I'm saying doesn't make anything). Given that proper certificate handling is really tricky (there are regular security bugs in the related code in eg browsers) and that it's a fair amount of code, I have big doubts we can get it 100% right. I'd love to be proven wrong and have someone with deep x509/tls knowledge jump in and tell me he/she wrote this code and she/he is really confident it's right. If not, and if we are not sure why we are doing these additional hostname/subject checks in this code, maybe we should consider disabling these checks and just relying on the builtin openssl verification code. So far, no ACK/NACK on this patch, from a quick glance I couldn't see any big issues, but I need to look at it more closely for the details :) Christophe
pgpbhXngvK1tA.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
