On Mon, Feb 25, 2013 at 8:46 AM, Jeff King <peff@xxxxxxxx> wrote: > On Sun, Feb 24, 2013 at 11:01:50PM +0530, Zubin Mithra wrote: > >> There seems to be a security issue in the way git uses openssl for >> certificate validation. Similar occurrences have been found and >> documented in other open source projects, the research can be found at >> [1]. >> >> -=========] >> - imap-send.c >> >> Line 307 >> >> 307 ret = SSL_connect(sock->ssl); >> 308 if (ret <= 0) { >> 309 socket_perror("SSL_connect", sock, ret); >> 310 return -1; >> 311 } >> 312 >> >> Certificate validation errors are signaled either through return >> values of SSL_connect or by setting internal flags. The internal flags >> need to be checked using the SSL_get_verify_result function. This is >> not performed. > > I'm not sure what you mean. We use SSL_CTX_set_verify to turn on peer > certificate verification, which will cause SSL_connect to return > failure if the certificate signature cannot be traced back to a CA cert > from our local store. > > Is there some case where this does not happen properly? If so, can you > give an example? The paper you referenced says only that there are some > special cases where SSL_connect does not notice the error, but then > gives an example where the application does not turn on SSL_VERIFY_PEER. > But git does. Are there are other cases that SSL_VERIFY_PEER does not > handle? Indeed -- it appears that I was mistaken. I had a quick look at the openssl source code and it does seem that SSL_VERIFY_PEER is equivalent to SSL_get_verify_result. Thank you for your time! - Zubin > > There is a _different_ problem not handled by the code you show above, > which is that SSL_connect does not verify that the hostname we connected > to matches the signed certificate. But that was fixed already by b62fb07 > (imap-send: the subject of SSL certificate must match the host, > 2013-02-15), which is in git v1.8.1.4. > > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html