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

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:

quoted

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.

quoted

quoted

quoted

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.