Re: [RFC/PATCH] send-email: die if CA path doesn't exist

[John Keeping <john@keeping.me.uk>]

On Tue, Nov 24, 2015 at 05:28:21PM -0500, Jeff King wrote:
> On Tue, Nov 24, 2015 at 10:17:08PM +0000, John Keeping wrote:
> 
> > I wonder if we should do this to help debug SSL issues:
> > 
> > -- >8 --
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index e057051..6d4e0ee 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -1317,6 +1317,10 @@ Message-Id: $message_id
> >  			require Net::SMTP::SSL;
> >  			$smtp_domain ||= maildomain();
> >  			require IO::Socket::SSL;
> > +			if ($debug_net_smtp) {
> > +				no warnings 'once';
> > +				$IO::Socket::SSL::DEBUG = 1;
> > +			}
> >  			# Net::SMTP::SSL->new() does not forward any SSL options
> >  			IO::Socket::SSL::set_client_defaults(
> >  				ssl_verify_params());
> > -- 8< --
> 
> That certainly looks like a reasonable thing to be doing, assuming that
> the output from IO::Socket::SSL is generally helpful.

It's a bit verbose for errors, but it does let you know what went wrong:

DEBUG: .../IO/Socket/SSL.pm:1796: SSL connect attempt failed error:14090086:SSL routines:ssl3_get_server_certificate:certificate verify failed
DEBUG: .../IO/Socket/SSL.pm:673: fatal SSL error: SSL connect attempt failed error:14090086:SSL routines:ssl3_get_server_certificate:certificate verify failed
DEBUG: .../IO/Socket/SSL.pm:1780: IO::Socket::IP configuration failed

It doesn't print anything when the SSL connection is established
successfully, but I don't think that's a problem and if we jump to
level 2 it starts logging things like:

DEBUG: .../IO/Socket/SSL.pm:687: waiting for fd to become ready: SSL wants a read first
DEBUG: .../IO/Socket/SSL.pm:707: socket ready, retrying connect
DEBUG: .../IO/Socket/SSL.pm:677: ssl handshake in progress

without adding anything useful.

> > > > Maybe we shouldn't worry too much about that, but should instead put the
> > > > invalid path into the error message:
> > > > 
> > > > 	die "CA path \"$smtp_ssl_cert_path\" does not exist.";
> > > 
> > > Given what I wrote above, yeah, I'd agree that is sufficient (and I do
> > > think mentioning the path is helpful).
> > 
> > I'll change it to this in a re-roll.
> 
> Thanks.