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

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

On Tue, Nov 24, 2015 at 02:58:43PM -0500, Jeff King wrote:
> On Fri, Nov 20, 2015 at 07:46:51PM +0000, John Keeping wrote:
> 
> > > For people who know their systems are broken and want to proceed anyway,
> > > what is the appropriate work-around? Obviously it involves disabling
> > > peer verification, but would we want to include instructions for doing
> > > so (either in the error message, or perhaps mentioning it in the commit
> > > message)?
> > 
> > The documentation already says:
> > 
> > 	Set it to an empty string to disable certificate verification.
> > 
> > It's a bit lost in the middle of a paragraph but I think that is the
> > best place for the detail of how to disable verification.
> > 
> > Having revisted the patch, I do think the message might be a bit terse,
> > but I can't think of a reasonably concise way to point at the
> > --smtp-ssl-cert-path argument as being the culprit.
> 
> Hrm. I was thinking that somebody might not have any clue that
> --smtp-ssl-cert-path exists, and with this patch their setup would
> suddenly go from working (well, insecure but passing mail) to broken.
> They need to know where to look to find that documentation.
> 
> But it looks like this code path only triggers if you have set
> smtp-ssl-cert-path to something bogus. So anybody who does so at least
> knows about the option.
> 
> Which makes me wonder what happens when the cert path isn't defined by
> Git. The code says:
> 
>         if (!defined $smtp_ssl_cert_path) {
>                 # use the OpenSSL defaults
>                 return (SSL_verify_mode => SSL_VERIFY_PEER());
>         }
> 
> What does OpenSSL do when there is no cert? Hopefully it reports a
> verification failure (and in that sense your patch is making our code
> consistent with that, which is a good thing).

I suspect it's not very useful; I originally got here after setting up
git-send-email to talk to a server with a certificate signed by a
corporate CA and had to resort to the perl debugger to figure out where
it was going wrong.  There isn't even any output with --smtp-debug when
the SSL handshake fails.

The error message is (all on one line):

Unable to initialize SMTP properly. Check config and use --smtp-debug.
VALUES: server=<redacted> encryption=ssl hello=<redacted>
port=465 at /usr/libexec/git-core/git-send-email line 1357.

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

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