Re: [PATCH] send-email: If the ca path is not specified, use the defaults

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