Re: Certificate validation vulnerability in Git

2013-02-25 Thread Zubin Mithra 
On Mon, Feb 25, 2013 at 8:46 AM, Jeff King p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Certificate validation vulnerability in Git

2013-02-24 Thread Andreas Ericsson 
On 02/24/2013 06:31 PM, Zubin Mithra wrote:
 Hello,
 
 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.
 
 Kindly fix these issues, file a CVE and credit it to Dhanesh K. and
 Zubin Mithra. Thanks.
 

The lack of certificate authority verification presents no attack vector
for git imap-send. As such, it doesn't warrant a CVE. I'm sure you'll
be credited with a reported-by line in the commit message if someone
decides to fix it though. Personally, I'm not fussed.

 We are not subscribed to this list, so we'd appreciate it if you could
 CC us in the replies.
 

That's standard on this list. Please follow the same convention if/when
you reply. Thanks.

-- 
Andreas Ericsson   andreas.erics...@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Certificate validation vulnerability in Git

2013-02-24 Thread Zubin Mithra 
Hello,

On Mon, Feb 25, 2013 at 12:16 AM, Andreas Ericsson a...@op5.se wrote:
 On 02/24/2013 06:31 PM, Zubin Mithra wrote:
 Hello,

 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.

 Kindly fix these issues, file a CVE and credit it to Dhanesh K. and
 Zubin Mithra. Thanks.


 The lack of certificate authority verification presents no attack vector
 for git imap-send. As such, it doesn't warrant a CVE. I'm sure you'll
 be credited with a reported-by line in the commit message if someone
 decides to fix it though. Personally, I'm not fussed.

I'd like to add in a few points -- generally SSL/TLS would be used in
cases where the authenticity of the server and confidentiality of the
messages transferred would be required. In this particular case, the
threat scenarios would be :-

- Usage of an invalid attacker certificate could result in the
attacker gaining access to authentication information sent over the
wire.
- If the code repository were private, the patches thus generated are
also assumed to be kept private. An invalid certificate check at the
client side would enable an attacker to gain access to those patches.


Is there anything I'm missing? I believe this is a valid security issue.



Thanks,
Zubin



 We are not subscribed to this list, so we'd appreciate it if you could
 CC us in the replies.


 That's standard on this list. Please follow the same convention if/when
 you reply. Thanks.

 --
 Andreas Ericsson   andreas.erics...@op5.se
 OP5 AB www.op5.se
 Tel: +46 8-230225  Fax: +46 8-230231

 Considering the successes of the wars on alcohol, poverty, drugs and
 terror, I think we should give some serious thought to declaring war
 on peace.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Certificate validation vulnerability in Git

2013-02-24 Thread Jeff King 
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Certificate validation vulnerability in Git

2013-02-24 Thread Jeff King 
On Sun, Feb 24, 2013 at 07:46:51PM +0100, Andreas Ericsson wrote:

 The lack of certificate authority verification presents no attack vector
 for git imap-send. As such, it doesn't warrant a CVE. I'm sure you'll
 be credited with a reported-by line in the commit message if someone
 decides to fix it though. Personally, I'm not fussed.

Sure it presents an attack vector. I can man-in-the-middle your
imap-send client and read your otherwise secret patches. Or your
otherwise secret imap password.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Certificate validation vulnerability in Git

2013-02-24 Thread Junio C Hamano 
Jeff King p...@peff.net writes:

 On Sun, Feb 24, 2013 at 07:46:51PM +0100, Andreas Ericsson wrote:

 The lack of certificate authority verification presents no attack vector
 for git imap-send. As such, it doesn't warrant a CVE. I'm sure you'll
 be credited with a reported-by line in the commit message if someone
 decides to fix it though. Personally, I'm not fussed.

 Sure it presents an attack vector. I can man-in-the-middle your
 imap-send client and read your otherwise secret patches. Or your
 otherwise secret imap password.

Yes, the lack of verification alone will not hurt the victim; you
would need to also be able to insert yourself in the middle, perhaps
by poisoning the victim's DNS.  But one of the points of using
SSL/TLS is to resist such an attack, and it certainly is an attack
surfce, even though it may be of a lessor kind than other kinds of
attacks.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html