git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Zubin Mithra <zubin.mithra@gmail.com>
Cc: git@vger.kernel.org, "Dhanesh K." <dhanesh1428@gmail.com>
Subject: Re: Certificate validation vulnerability in Git
Date: Sun, 24 Feb 2013 22:16:48 -0500	[thread overview]
Message-ID: <20130225031648.GA31988@sigill.intra.peff.net> (raw)
In-Reply-To: <CAA5xPpmmZuMK7q3-pTOx4L6DxFtyw5HWYdH7kHEsK=96KM5kAQ@mail.gmail.com>

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?

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

  parent reply	other threads:[~2013-02-25  3:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-24 17:31 Certificate validation vulnerability in Git Zubin Mithra
2013-02-24 18:46 ` Andreas Ericsson
2013-02-25  2:28   ` Zubin Mithra
2013-02-25  3:18   ` Jeff King
2013-02-25  5:35     ` Junio C Hamano
2013-02-25  3:16 ` Jeff King [this message]
2013-02-25 15:42   ` Zubin Mithra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130225031648.GA31988@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dhanesh1428@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=zubin.mithra@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).