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

[Junio C Hamano <gitster@pobox.com>]

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 "") {