All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Ruben Kerkhof <ruben@rubenkerkhof.com>,
	Ramkumar Ramachandra <artagnon@gmail.com>,
	Igor Gnatenko <i.gnatenko.brain@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] send-email: If the ca path is not specified, use the defaults
Date: Thu, 16 Jan 2014 15:19:14 -0800	[thread overview]
Message-ID: <xmqqppnry0p9.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140115215024.GM18964@google.com> (Jonathan Nieder's message of "Wed, 15 Jan 2014 13:50:24 -0800")

Jonathan Nieder <jrnieder@gmail.com> writes:

> FWIW this should help on Mac OS X, too.  Folks using git on mac
> at $DAYJOB have been using the workaround described at
> http://mercurial.selenic.com/wiki/CACertificates#Mac_OS_X_10.6_and_higher
> so I forgot to report it. :/

Hmph, is that the same issue, though?  That page seems to suggest
using an empty ca file that does not have any useful information as
a workaround.  The issue Fedora folks saw is that we see a directory
/etc/ssl/certs exist on the system, and blindly attempt to use it as
SSL_ca_path when the directory is not suitable to be used as such.

In any case, I tried to summarize the discussion in the updated log
message.  I wanted to say "does not" but stopped at "should not" in
the last paragraph for now.  Maybe Ram can say something before we
merge it to 'next'.

The patch in the meantime will be queued on 'pu'.

-- >8 --
From: Ruben Kerkhof <ruben@rubenkerkhof.com>
Date: Wed, 15 Jan 2014 21:31:11 +0400
Subject: [PATCH] send-email: /etc/ssl/certs/ directory may not be usable as ca_path

When sending patches on Fedora rawhide with
git-1.8.5.2-1.fc21.x86_64 and perl-IO-Socket-SSL-1.962-1.fc21.noarch,
with the following

    [sendemail]
	    smtpencryption = tls
	    smtpserver = smtp.gmail.com
	    smtpuser = ruben@rubenkerkhof.com
	    smtpserverport = 587

git-send-email fails with:

    STARTTLS failed! SSL connect attempt failed with unknown error
    error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate
    verify failed at /usr/libexec/git-core/git-send-email line 1236.

The current code detects the presence of /etc/ssl/certs directory
(it actually is a symlink to another directory, but that does not
matter) and uses SSL_ca_path to point at it when initializing the
connection with IO::Socket::SSL or Net::SMTP::SSL.  However, on the
said platform, it seems that this directory is not designed to be
used as SSL_ca_path.  Using a single file inside that directory
(cert.pem, which is a Mozilla CA bundle) with SSL_ca_file does work,
and also not specifying any SSL_ca_file/SSL_ca_path (and letting the
library use its own default) and asking for peer verification does
work.

By removing the code that blindly defaults $smtp_ssl_cert_path to
"/etc/ssl/certs", we can prevent the codepath that treats any
directory specified with that variable as usable for SSL_ca_path
from incorrectly triggering.

This change could introduce a regression for people on a platform
whose certificate directory is /etc/ssl/certs but its IO::Socket:SSL
somehow fails to use it as SSL_ca_path without being told.  Using
/etc/ssl/certs directory as SSL_ca_path by default like the current
code does would have been hiding such a broken installation without
its user needing to do anything.  These users can still work around
such a platform bug by setting the configuration variable explicitly
to point at /etc/ssl/certs.

This change should not negate what 35035bbf (send-email: be explicit
with SSL certificate verification, 2013-07-18), which was the
original change that introduced the defaulting to /etc/ssl/certs/,
attempted to do, which is to make sure we do not communicate over
insecure connection by default, triggering warning from the library.

Cf. https://bugzilla.redhat.com/show_bug.cgi?id=1043194

Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
Signed-off-by: Ruben Kerkhof <ruben@rubenkerkhof.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-send-email.perl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 3782c3b..689944f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1095,7 +1095,8 @@ sub ssl_verify_params {
 	}
 
 	if (!defined $smtp_ssl_cert_path) {
-		$smtp_ssl_cert_path = "/etc/ssl/certs";
+		# use the OpenSSL defaults
+		return (SSL_verify_mode => SSL_VERIFY_PEER());
 	}
 
 	if ($smtp_ssl_cert_path eq "") {
-- 
1.8.5.3-493-gb139ac2

  reply	other threads:[~2014-01-16 23:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-15 17:31 [PATCH] send-email: If the ca path is not specified, use the defaults Igor Gnatenko
2014-01-15 19:26 ` Junio C Hamano
     [not found]   ` <7AD1C6ED-6177-415D-B342-D1FEA9F810B4@rubenkerkhof.com>
2014-01-15 21:30     ` Junio C Hamano
2014-01-15 21:50       ` Ruben Kerkhof
2014-01-15 21:50       ` Jonathan Nieder
2014-01-16 23:19         ` Junio C Hamano [this message]
2014-01-17  4:21           ` Kyle J. McKay
2014-01-17 18:14             ` Junio C Hamano
2014-01-17 23:34               ` Kyle J. McKay
2014-01-26 17:17           ` Ramkumar Ramachandra
2014-01-27 16:02             ` Junio C Hamano

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=xmqqppnry0p9.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=i.gnatenko.brain@gmail.com \
    --cc=jrnieder@gmail.com \
    --cc=ruben@rubenkerkhof.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.