git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] send-email: If the ca path is not specified, use the defaults
@ 2014-01-15 17:31 Igor Gnatenko
  2014-01-15 19:26 ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Igor Gnatenko @ 2014-01-15 17:31 UTC (permalink / raw
  To: gitster; +Cc: git, Ruben Kerkhof

From: Ruben Kerkhof <ruben@rubenkerkhof.com>

I use gmail for sending patches.
If I have the following defined in my ~/.gitconfig:
[sendemail]
	smtpencryption = tls
	smtpserver = smtp.gmail.com
	smtpuser = ruben@rubenkerkhof.com
	smtpserverport = 587

and try to send a patch, this 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.

Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
Signed-off-by: Ruben Kerkhof <ruben@rubenkerkhof.com>
Reference: https://bugzilla.redhat.com/show_bug.cgi?id=1043194
---
 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.4.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  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>
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2014-01-15 19:26 UTC (permalink / raw
  To: Igor Gnatenko; +Cc: git, Ruben Kerkhof

Igor Gnatenko <i.gnatenko.brain@gmail.com> writes:

> From: Ruben Kerkhof <ruben@rubenkerkhof.com>
>
> I use gmail for sending patches.
> If I have the following defined in my ~/.gitconfig:
> [sendemail]
> 	smtpencryption = tls
> 	smtpserver = smtp.gmail.com
> 	smtpuser = ruben@rubenkerkhof.com
> 	smtpserverport = 587
>
> and try to send a patch, this 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.

... because?  Is it because the cert_path on your platform is
different from /etc/ssl/certs?  What platform was this anyway?

I see in the original discussion in your bugzilla entry that
"/etc/ssl/certs/" _is_ your ssl cert directory, but I wonder why
then this part of the existing codepath does not work for you:

	if (!defined $smtp_ssl_cert_path) {
		$smtp_ssl_cert_path = "/etc/ssl/certs";
	}

	if ($smtp_ssl_cert_path eq "") {
		return (SSL_verify_mode => SSL_VERIFY_NONE());
	} elsif (-d $smtp_ssl_cert_path) {
		return (SSL_verify_mode => SSL_VERIFY_PEER(),
			SSL_ca_path => $smtp_ssl_cert_path);
	} elsif (-f $smtp_ssl_cert_path) {
		return (SSL_verify_mode => SSL_VERIFY_PEER(),
			SSL_ca_file => $smtp_ssl_cert_path);
	} else ...

We set cert_path to "/etc/ssl/certs" and return SSL_VERIFY_PEER() as
the SSL_verify_mode, and also return that directory as SSL_ca_path,
which means the callsites of the ssl_verify_params sub, which are
Net::SMTP:SSL->start_SSL() or IO::Socket::SSL::set_client_defaults(),
will be told with SSL_ca_path (not SSL_ca_file) that
"/etc/ssl/certs" is the directory to find the certificates in.

Now, http://search.cpan.org/~sullr/IO-Socket-SSL-1.964/lib/IO/Socket/SSL.pm
says:

  SSL_ca_file | SSL_ca_path

    Usually you want to verify that the peer certificate has been
    signed by a trusted certificate authority. In this case you
    should use this option to specify the file (SSL_ca_file) or
    directory (SSL_ca_path) containing the certificate(s) of the
    trusted certificate authorities.

So I have to wonder why isn't this working.  Without knowing why
using SSL_ca_path for the certificate directory is not working on
your platform, the patch looks more like a band-aid that works
around an undiagnosed malfunction of IO::Socket::SSL on your
platform than a real solution to the breakage, no?

Puzzled...

By the way, please do not tell the readers of proposed log message
to refer to your custom "Reference:" footer to answer the
"... because?" question at the beginning of this message.  Your
proposed log message should have allowed anybody who comments on
your patch to make the above observation without referring to
external website.

Having said all that.

Does this patch affect people on other distros, who never set the
cert_path in their configuration and have been relying on the
hardwired default?  If so in what way?

My knee-jerk answer to that question is that it would negatively
affect folks only if their platform does have the certs in
/etc/ssl/certs/, but their Perl IO::Socket::SSL somehow defaults to
a wrong location, which is already a broken installation.  In that
sense, I suspect that the potential downside of this patch may be
small, but I'd prefer to see evidence that the patch author thought
through ramifications of the change in the proposed log message ;-)

Also, if the above observation is correct, i.e. we are calling
IO::Socket::SSL with SSL_ca_path set to a correct directory but
somehow your platform is misbehaving and not detecting it as a
proper SSL_ca_path, then it could be argued that the code before
this patch catered people on platforms with one class of breakage,
namely "IO::Socket::SSL does not work with default configuration
without SSL_ca_file nor SSL_ca_path", and the code with this patch
caters people on platforms with another class of breakage, namely
"IO::Socket::SSL does not work when told with SSL_ca_path where the
certificate directory is" (or it could be "/etc/ssl/certs is a
directory that ought to be usable as SSL_ca_path, but it lacks
necessary hash symlinks").  Sort of like robbing Peter to pay Paul,
which is not very nice in principle.

> Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com>
> Signed-off-by: Ruben Kerkhof <ruben@rubenkerkhof.com>
> Reference: https://bugzilla.redhat.com/show_bug.cgi?id=1043194
> ---
>  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 "") {

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
       [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
  0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2014-01-15 21:30 UTC (permalink / raw
  To: Ruben Kerkhof, Ramkumar Ramachandra; +Cc: Igor Gnatenko, git

Ruben Kerkhof <ruben@rubenkerkhof.com> writes:

>> ... because?  Is it because the cert_path on your platform is
>> different from /etc/ssl/certs?  What platform was this anyway?
>
> This is Fedora rawhide, git-1.8.5.2-1.fc21.x86_64, perl-IO-Socket-SSL-1.962-1.fc21.noarch
>> 
>> I see in the original discussion in your bugzilla entry that
>> "/etc/ssl/certs/" _is_ your ssl cert directory, but I wonder why
>> then this part of the existing codepath does not work for you:
>
> Actually, it's not a directory but a symlink to a directory:
>
> [ruben@vm ~]$ ls -l /etc/ssl/certs
> lrwxrwxrwx. 1 root root 16 Jan 11 12:04 /etc/ssl/certs -> ../pki/tls/certs
>
> Just to rule that out I set smtpsslcertpath = /etc/pki/tls/certs,
> but that doesn't work either.

Yeah, I suspect as much, as "-d" test would dereference symlinks and
would see /etc/ssl/certs is a symlink pointing at a directory.

> ...  I posted the patch to Fedora's bugzilla, since this was
> biting me on Fedora, and Igor took the liberty to forward it.  Not
> that I mind of course, but please don't take this patch as a
> proper fix. I don't pretend to fully understand the code and the
> implications, much less the impact on other platforms.

That is fine, and thanks for starting discussion for a proper fix
(the "thanks" go to both of you).

>> Also, if the above observation is correct, i.e. we are calling
>> IO::Socket::SSL with SSL_ca_path set to a correct directory but
>> somehow your platform is misbehaving and not detecting it as a
>> proper SSL_ca_path, then it could be argued that the code before
>> this patch catered people on platforms with one class of breakage,
>> namely "IO::Socket::SSL does not work with default configuration
>> without SSL_ca_file nor SSL_ca_path", and the code with this patch
>> caters people on platforms with another class of breakage, namely
>> "IO::Socket::SSL does not work when told with SSL_ca_path where the
>> certificate directory is" (or it could be "/etc/ssl/certs is a
>> directory that ought to be usable as SSL_ca_path, but it lacks
>> necessary hash symlinks").  Sort of like robbing Peter to pay Paul,
>> which is not very nice in principle.
>
> I suspect that's exactly the case,...

Actually, there is another possibility.  Perhaps on your system,
even though the current code thinks /etc/ssl/certs/ is usable as
SSL_ca_path, the directory is not meant to be usable as such, and
the default OpenSSL uses the equivalent of SSL_ca_file and uses the
single certificate bundle file without consulting other stuff in the
directory.

And that is not a broken installation at all, which is sort of
consistent with your observation here: 

> ...
> As a last check, I set smtpsslcertpath = /etc/pki/tls/cert.pem in
> ~/.gitconfig and git-send-email works fine now.

Which would mean that the existing code, by blindly defaulting to
/etc/ssl/certs/ and misdiagnosing that the directory is meant to be
used as SSL_ca_path, breaks a set-up that otherwise _should_ work.

I see that the original change that introduced the defaulting to
/etc/ssl/certs/, which is 35035bbf (send-email: be explicit with SSL
certificate verification, 2013-07-18), primarily wanted to avoid
letting Net::SMTP::SSL defaulting to no verification and causing it
to emit warnings of the use of the insecure default.  And I think
the same outcome will result with your patch.  By default, we still
insist on using SSL_VERIFY_PEER(), not SSL_VERIFY_NONE(), which
would avoid defaulting to insecure communication and triggering the
warning.  The way to disable the security by setting ssl_cert_path
to an empty string is unchanged.

Ram (who did 35035bbf), with the patch from Ruben in the thread, do
you get either the warning or SSL failure?  Conceptually, the
resulting code is much better, I think, without blindly defaulting
/etc/ssl/certs and instead of relying on the underlying platform
just working out of the box with its own default, and I would be
happy if we can apply the change without regression to existing
users.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  2014-01-15 21:30     ` Junio C Hamano
@ 2014-01-15 21:50       ` Ruben Kerkhof
  2014-01-15 21:50       ` Jonathan Nieder
  1 sibling, 0 replies; 11+ messages in thread
From: Ruben Kerkhof @ 2014-01-15 21:50 UTC (permalink / raw
  To: Junio C Hamano; +Cc: Ramkumar Ramachandra, Igor Gnatenko, git


On 15 jan. 2014, at 22:30, Junio C Hamano <gitster@pobox.com> wrote:

> Ruben Kerkhof <ruben@rubenkerkhof.com> writes:
> 
>>> ... because?  Is it because the cert_path on your platform is
>>> different from /etc/ssl/certs?  What platform was this anyway?
>> 
>> This is Fedora rawhide, git-1.8.5.2-1.fc21.x86_64, perl-IO-Socket-SSL-1.962-1.fc21.noarch
>>> 
>>> I see in the original discussion in your bugzilla entry that
>>> "/etc/ssl/certs/" _is_ your ssl cert directory, but I wonder why
>>> then this part of the existing codepath does not work for you:
>> 
>> Actually, it's not a directory but a symlink to a directory:
>> 
>> [ruben@vm ~]$ ls -l /etc/ssl/certs
>> lrwxrwxrwx. 1 root root 16 Jan 11 12:04 /etc/ssl/certs -> ../pki/tls/certs
>> 
>> Just to rule that out I set smtpsslcertpath = /etc/pki/tls/certs,
>> but that doesn't work either.
> 
> Yeah, I suspect as much, as "-d" test would dereference symlinks and
> would see /etc/ssl/certs is a symlink pointing at a directory.
> 
>> ...  I posted the patch to Fedora's bugzilla, since this was
>> biting me on Fedora, and Igor took the liberty to forward it.  Not
>> that I mind of course, but please don't take this patch as a
>> proper fix. I don't pretend to fully understand the code and the
>> implications, much less the impact on other platforms.
> 
> That is fine, and thanks for starting discussion for a proper fix
> (the "thanks" go to both of you).
> 
>>> Also, if the above observation is correct, i.e. we are calling
>>> IO::Socket::SSL with SSL_ca_path set to a correct directory but
>>> somehow your platform is misbehaving and not detecting it as a
>>> proper SSL_ca_path, then it could be argued that the code before
>>> this patch catered people on platforms with one class of breakage,
>>> namely "IO::Socket::SSL does not work with default configuration
>>> without SSL_ca_file nor SSL_ca_path", and the code with this patch
>>> caters people on platforms with another class of breakage, namely
>>> "IO::Socket::SSL does not work when told with SSL_ca_path where the
>>> certificate directory is" (or it could be "/etc/ssl/certs is a
>>> directory that ought to be usable as SSL_ca_path, but it lacks
>>> necessary hash symlinks").  Sort of like robbing Peter to pay Paul,
>>> which is not very nice in principle.
>> 
>> I suspect that's exactly the case,...
> 
> Actually, there is another possibility.  Perhaps on your system,
> even though the current code thinks /etc/ssl/certs/ is usable as
> SSL_ca_path, the directory is not meant to be usable as such, and
> the default OpenSSL uses the equivalent of SSL_ca_file and uses the
> single certificate bundle file without consulting other stuff in the
> directory.
> 
> And that is not a broken installation at all, which is sort of
> consistent with your observation here: 
> 
>> ...
>> As a last check, I set smtpsslcertpath = /etc/pki/tls/cert.pem in
>> ~/.gitconfig and git-send-email works fine now.
> 
> Which would mean that the existing code, by blindly defaulting to
> /etc/ssl/certs/ and misdiagnosing that the directory is meant to be
> used as SSL_ca_path, breaks a set-up that otherwise _should_ work.

Exactly. IO::Socket::SSL calls Net::SSLeay::CTX_load_verify_locations( $ctx, $arg_hash->{SSL_ca_file} || '',$arg_hash->{SSL_ca_path} || ''),
which in turn calls OpenSSL's SSL_CTX_load_verify_locations

According to the comments at http://www.openssl.org/docs/ssl/SSL_CTX_load_verify_locations.html, the CApath should be a directory with hashes pointing to
to certs, which is not the case in Fedora.
> 
> I see that the original change that introduced the defaulting to
> /etc/ssl/certs/, which is 35035bbf (send-email: be explicit with SSL
> certificate verification, 2013-07-18), primarily wanted to avoid
> letting Net::SMTP::SSL defaulting to no verification and causing it
> to emit warnings of the use of the insecure default.  And I think
> the same outcome will result with your patch.  By default, we still
> insist on using SSL_VERIFY_PEER(), not SSL_VERIFY_NONE(), which
> would avoid defaulting to insecure communication and triggering the
> warning.  The way to disable the security by setting ssl_cert_path
> to an empty string is unchanged.
> 
> Ram (who did 35035bbf), with the patch from Ruben in the thread, do
> you get either the warning or SSL failure?  Conceptually, the
> resulting code is much better, I think, without blindly defaulting
> /etc/ssl/certs and instead of relying on the underlying platform
> just working out of the box with its own default, and I would be
> happy if we can apply the change without regression to existing
> users.


/etc/pki/tls/cert.pem is a symlink to the Mozilla CA bundle,
/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem, which as far as I can tell is the one that should be used for this distro.

Kind regards,

Ruben

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  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
  1 sibling, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2014-01-15 21:50 UTC (permalink / raw
  To: Junio C Hamano; +Cc: Ruben Kerkhof, Ramkumar Ramachandra, Igor Gnatenko, git

Junio C Hamano wrote:
> Ruben Kerkhof <ruben@rubenkerkhof.com> writes:

>> As a last check, I set smtpsslcertpath = /etc/pki/tls/cert.pem in
>> ~/.gitconfig and git-send-email works fine now.
>
> Which would mean that the existing code, by blindly defaulting to
> /etc/ssl/certs/ and misdiagnosing that the directory is meant to be
> used as SSL_ca_path, breaks a set-up that otherwise _should_ work.
[...]
> Ram (who did 35035bbf), with the patch from Ruben in the thread, do
> you get either the warning or SSL failure?  Conceptually, the
> resulting code is much better, I think, without blindly defaulting
> /etc/ssl/certs and instead of relying on the underlying platform
> just working out of the box with its own default,

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. :/

Thanks,
Jonathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  2014-01-15 21:50       ` Jonathan Nieder
@ 2014-01-16 23:19         ` Junio C Hamano
  2014-01-17  4:21           ` Kyle J. McKay
  2014-01-26 17:17           ` Ramkumar Ramachandra
  0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2014-01-16 23:19 UTC (permalink / raw
  To: Jonathan Nieder; +Cc: Ruben Kerkhof, Ramkumar Ramachandra, Igor Gnatenko, git

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  2014-01-16 23:19         ` Junio C Hamano
@ 2014-01-17  4:21           ` Kyle J. McKay
  2014-01-17 18:14             ` Junio C Hamano
  2014-01-26 17:17           ` Ramkumar Ramachandra
  1 sibling, 1 reply; 11+ messages in thread
From: Kyle J. McKay @ 2014-01-17  4:21 UTC (permalink / raw
  To: Junio C Hamano, Jonathan Nieder
  Cc: Ruben Kerkhof, Ramkumar Ramachandra, Igor Gnatenko,
	Git Mailing List

On Jan 16, 2014, at 15:19, Junio C Hamano wrote:
> 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.

I had written up this draft email that describes the situation on OS X  
and decided it might not be exactly on topic and wasn't going to send  
it, but, since you asked... ;)

OpenSSL's default location can be found with:

   openssl version -d

On my Ubuntu system I get this:

   $ openssl version -d
   OPENSSLDIR: "/usr/lib/ssl"

Then if I look in there like so:

   $ ls -l /usr/lib/ssl
   total 8
   lrwxrwxrwx 1 root root   14 certs -> /etc/ssl/certs
   drwxr-xr-x 2 root root 4096 engines
   drwxr-xr-x 2 root root 4096 misc
   lrwxrwxrwx 1 root root   20 openssl.cnf -> /etc/ssl/openssl.cnf
   lrwxrwxrwx 1 root root   16 private -> /etc/ssl/private

And that's how it ends up using /etc/ssl/certs.  From the  
SSL_CTX_load_verify_locations man page:

   "When looking up CA certificates, the OpenSSL library will first  
search the certificates in CAfile, then those in CApath."

The default CAfile is "cert.pem" and the default CApath is "certs"  
both located in the openssl version -d directory.  See the crypto/ 
cryptlib.h header [1].

On my OS X platform depending on which version of OpenSSL I'm using,  
the OPENSSLDIR path would be one of these:

   /System/Library/OpenSSL
   /opt/local/etc/openssl

And neither of those uses a "certs" directory, they both use a  
"cert.pem" bundle instead:

   $ ls -l /System/Library/OpenSSL
   total 32
   lrwxrwxrwx  1 root  wheel    42 cert.pem -> ../../../usr/share/curl/ 
curl-ca-bundle.crt
   drwxr-xr-x  2 root  wheel    68 certs
   drwxr-xr-x  8 root  wheel   272 misc
   -rw-r--r--  1 root  wheel  9381 openssl.cnf
   drwxr-xr-x  2 root  wheel    68 private
   # the certs directory is empty

   $ ls -l /opt/local/etc/openssl
   total 32
   lrwxrwxrwx   1 root  admin    35 cert.pem@ -> ../../share/curl/curl- 
ca-bundle.crt
   drwxr-xr-x   9 root  admin   306 misc/
   -rw-r--r--   1 root  admin 10835 openssl.cnf

Notice neither of those refers to /etc/ssl/certs at all.

So the short answer is, yes, hard-coding /etc/ssl/certs as the path on  
OS X is incorrect and if setting /etc/ssl/certs as the path has the  
effect of replacing the default locations the verification will  
fail.   Of course Apple patches the included version of OpenSSL  
starting with OS X 10.6 to look in Apple's keychain, but if you're  
using a MacPorts-built version that won't happen and still /etc/ssl/ 
certs will be wrong in both cases.

As for the hint in that wiki/CACertificates link above, that does seem  
odd to me as well.

A much better solution would be (if python simply MUST have a file) to  
export the system's list of trusted root certificates like so:

   security export -k \
     /System/Library/Keychains/SystemRootCertificates.keychain \
     -t certs -f pemseq > rootcerts.pem

   # the generated rootcerts.pem file is suitable for use with the
   # openssl verify -CAfile option (root certs)

   # Substituting SystemCACertificates for SystemRootCertificates
   # produces a file suitable for use with the
   # openssl verify -untrusted option (intermediate certs)

and then point python to that rootcerts.pem file.  This file [2] may  
be helpful in understanding what's in SystemRootCertificates.keychain  
and SystemCACertificates.keychain.  The intermediate certs may also  
need to be exported to a file and pointed to as well (a quick glance  
at the hgrc docs did not turn up an option for this).

--Kyle

[1] http://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=crypto/cryptlib.h#l84
[2] <http://www.opensource.apple.com/source/security_certificates/security_certificates-32641/CertificateInstructions.rtf 
 >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  2014-01-17  4:21           ` Kyle J. McKay
@ 2014-01-17 18:14             ` Junio C Hamano
  2014-01-17 23:34               ` Kyle J. McKay
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2014-01-17 18:14 UTC (permalink / raw
  To: Kyle J. McKay
  Cc: Jonathan Nieder, Ruben Kerkhof, Ramkumar Ramachandra,
	Igor Gnatenko, Git Mailing List

"Kyle J. McKay" <mackyle@gmail.com> writes:

> On my OS X platform depending on which version of OpenSSL I'm using,
> the OPENSSLDIR path would be one of these:
>
>   /System/Library/OpenSSL
>   /opt/local/etc/openssl
>
> And neither of those uses a "certs" directory, they both use a
> "cert.pem" bundle instead:
>
>   $ ls -l /System/Library/OpenSSL
>   total 32
>   lrwxrwxrwx  1 root  wheel    42 cert.pem -> ../../../usr/share/curl/
> curl-ca-bundle.crt
>   drwxr-xr-x  2 root  wheel    68 certs
>   drwxr-xr-x  8 root  wheel   272 misc
>   -rw-r--r--  1 root  wheel  9381 openssl.cnf
>   drwxr-xr-x  2 root  wheel    68 private
>   # the certs directory is empty
>
>   $ ls -l /opt/local/etc/openssl
>   total 32
>   lrwxrwxrwx   1 root  admin    35 cert.pem@ -> ../../share/curl/curl-
> ca-bundle.crt
>   drwxr-xr-x   9 root  admin   306 misc/
>   -rw-r--r--   1 root  admin 10835 openssl.cnf
>
> Notice neither of those refers to /etc/ssl/certs at all.
>
> So the short answer is, yes, hard-coding /etc/ssl/certs as the path on
> OS X is incorrect and if setting /etc/ssl/certs as the path has the
> effect of replacing the default locations the verification will fail.

The current code says "if nothing is specified, let's pretend
/etc/ssl/certs was specified.  Then if it is a directory, use it
with SSL_ca_path, if it is a file, use it with SSL_ca_file, if it
does not exist, do not even attempt verification."

And that "let's pretend" breaks Fedora, where "/etc/ssl/certs" is a
directory but is not meant to be used with SSL_ca_path---we try to
use /etc/ssl/certs with SSL_ca_path and verification fails miserably.

If I am reading the code correctly, if /etc/ssl/certs does not exist
on the filesystem at all, it wouldn't even attempt verification, so
I take your "the verification will fail" to mean that you forgot to
also mention "And on OS X, /etc/ssl/certs directory still exists,
even though OpenSSL does not use it."  If that is the case, then our
current code indeed is broken in exactly the same way for OS X as
for Fedora.

The proposed change in this thread would stop the defaulting
altogether, and still ask verification to the library using its own
default, so I can see how that would make the setting you described
used on OS X work properly.

In short, I agree with you on both counts (the current code is wrong
for OS X, and the proposed change will fix it).  I just want to make
sure that my understanding of the current breakage is in line with
the reality ;-)

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  2014-01-17 18:14             ` Junio C Hamano
@ 2014-01-17 23:34               ` Kyle J. McKay
  0 siblings, 0 replies; 11+ messages in thread
From: Kyle J. McKay @ 2014-01-17 23:34 UTC (permalink / raw
  To: Junio C Hamano
  Cc: Jonathan Nieder, Ruben Kerkhof, Ramkumar Ramachandra,
	Igor Gnatenko, Git Mailing List

On Jan 17, 2014, at 10:14, Junio C Hamano wrote:
> If I am reading the code correctly, if /etc/ssl/certs does not exist
> on the filesystem at all, it wouldn't even attempt verification, so
> I take your "the verification will fail" to mean that you forgot to
> also mention "And on OS X, /etc/ssl/certs directory still exists,
> even though OpenSSL does not use it."  If that is the case, then our
> current code indeed is broken in exactly the same way for OS X as
> for Fedora.

My bad.  That directory does not normally exist, but, if some errant  
installer were to create an empty one...

> In short, I agree with you on both counts (the current code is wrong
> for OS X, and the proposed change will fix it).  I just want to make
> sure that my understanding of the current breakage is in line with
> the reality ;-)

Yes, you're right.  :)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  2014-01-16 23:19         ` Junio C Hamano
  2014-01-17  4:21           ` Kyle J. McKay
@ 2014-01-26 17:17           ` Ramkumar Ramachandra
  2014-01-27 16:02             ` Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Ramkumar Ramachandra @ 2014-01-26 17:17 UTC (permalink / raw
  To: Junio C Hamano; +Cc: Jonathan Nieder, Ruben Kerkhof, Igor Gnatenko, Git List

Junio C Hamano wrote:
> 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.

I can confirm that my git-send-email doesn't regress to the
pre-35035bbf state; my certificate directory is /etc/ssl/certs. I'm
somewhat surprised that IO::Socket::SSL picks the right file/
directory on every platform without being told explicitly. This change
definitely looks like the right fix.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] send-email: If the ca path is not specified, use the defaults
  2014-01-26 17:17           ` Ramkumar Ramachandra
@ 2014-01-27 16:02             ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2014-01-27 16:02 UTC (permalink / raw
  To: Ramkumar Ramachandra
  Cc: Jonathan Nieder, Ruben Kerkhof, Igor Gnatenko, Git List

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>> 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.
>
> I can confirm that my git-send-email doesn't regress to the
> pre-35035bbf state; my certificate directory is /etc/ssl/certs. I'm
> somewhat surprised that IO::Socket::SSL picks the right file/
> directory on every platform without being told explicitly. This change
> definitely looks like the right fix.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-01-27 16:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).