git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zubin Mithra <zubin.mithra@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, "Dhanesh K." <dhanesh1428@gmail.com>
Subject: Re: Certificate validation vulnerability in Git
Date: Mon, 25 Feb 2013 21:12:42 +0530	[thread overview]
Message-ID: <CAA5xPpmBWLRXs_SfwOAkps8fTeFUW3bEnrDQE_FXjXhmBbC4MA@mail.gmail.com> (raw)
In-Reply-To: <20130225031648.GA31988@sigill.intra.peff.net>

On Mon, Feb 25, 2013 at 8:46 AM, Jeff King <peff@peff.net> 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

      reply	other threads:[~2013-02-25 15:43 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
2013-02-25 15:42   ` Zubin Mithra [this message]

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=CAA5xPpmBWLRXs_SfwOAkps8fTeFUW3bEnrDQE_FXjXhmBbC4MA@mail.gmail.com \
    --to=zubin.mithra@gmail.com \
    --cc=dhanesh1428@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).