Re* [PATCH] send-email: improve SSL certificate verification

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

Torsten Bögershausen <tboegi@web.de> writes:

> I wasn't sure where to apply the patch, so I manually copy/paste it
> on top of pu:
> commit 6b1ca0f4d443ee8716857b871b0513ae85c9f112
> Merge: bce90ab f351fcf
>
> Thanks, t9001 passes on Mac OS X 10.6.
> To be sure I didn't messed it up, please see the diff below.
> When it shows up on pu, I can re-test of course.

As the history of rr/send-email-ssl-verify needs rewriting to squash
this change in, here is a single patch with which I would propose to
replace all the commits accumulated on that branch.

Ramkumar, as you will still be the primary author of the resulting
commit, I tentatively added a line for your sign-off even though you
haven't signed off _this_ version yet, so that I would not forget.
If this version looks good, please tell us you would.

Torsten, I have Tested-by with your name, again so that I would not
forget, but obviously this one hasn't been tested by you yet.  If
this tests OK, please tell us so.

Thanks.

-- >8 --
From: Ramkumar Ramachandra <artagnon@gmail.com>
Subject: send-email: be explicit with SSL certificate verification

When initiating an SSL connection without explicitly specifying the
SSL certificate verification mode, Net::SMTP::SSL defaults to no
verification, but recent versions of the module gives a warning
against this use of the default.

Enable certificate verification by default, using /etc/ssl/certs as
the default path for certificates of certificate authorities.  This
path can be overriden by the --smtp-ssl-cert-path command line
option and the sendemail.smtpSSLCertPath configuration variable.

Passing an empty string as the path for CA certificates path disables
the SSL certificate verification explicitly, which does not trigger
the warning from recent versions of Net::SMTP::SSL.

(PROVISO) Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Helped-by: Brian M. Carlson <sandals@crustytoothpaste.net>
(PROVISO) Tested-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt         |  4 ++++
 Documentation/git-send-email.txt |  6 ++++++
 git-send-email.perl              | 41 +++++++++++++++++++++++++++++++++++++---
 3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..4de154c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2022,6 +2022,10 @@ sendemail.smtpencryption::
 sendemail.smtpssl::
 	Deprecated alias for 'sendemail.smtpencryption = ssl'.
 
+sendemail.smtpsslcertpath::
+	Path to ca-certificates (either a directory or a single file).
+	Set it to an empty string to disable certificate verification.
+
 sendemail.<identity>.*::
 	Identity-specific versions of the 'sendemail.*' parameters
 	found below, taking precedence over those when the this
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 40a9a9a..f0e57a5 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -198,6 +198,12 @@ must be used for each option.
 --smtp-ssl::
 	Legacy alias for '--smtp-encryption ssl'.
 
+--smtp-ssl-cert-path::
+	Path to ca-certificates (either a directory or a single file).
+	Set it to an empty string to disable certificate verification.
+	Defaults to the value set to the 'sendemail.smtpsslcertpath'
+	configuration variable, if set, or `/etc/ssl/certs` otherwise.
+
 --smtp-user=<user>::
 	Username for SMTP-AUTH. Default is the value of 'sendemail.smtpuser';
 	if a username is not specified (with '--smtp-user' or 'sendemail.smtpuser'),
diff --git a/git-send-email.perl b/git-send-email.perl
index bd13cc8..60eaed3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -69,6 +69,9 @@ sub usage {
     --smtp-pass             <str>  * Password for SMTP-AUTH; not necessary.
     --smtp-encryption       <str>  * tls or ssl; anything else disables.
     --smtp-ssl                     * Deprecated. Use '--smtp-encryption ssl'.
+    --smtp-ssl-cert-path    <str>  * Path to ca-certificates (either directory or file).
+                                     Pass an empty string to disable certificate
+                                     verification.
     --smtp-domain           <str>  * The domain name sent to HELO/EHLO handshake
     --smtp-debug            <0|1>  * Disable, enable Net::SMTP debug.
 
@@ -194,7 +197,7 @@ sub do_edit {
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
-my ($smtp_authuser, $smtp_encryption);
+my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -222,6 +225,7 @@ sub do_edit {
     "smtpserveroption" => \@smtp_server_options,
     "smtpuser" => \$smtp_authuser,
     "smtppass" => \$smtp_authpass,
+    "smtpsslcertpath" => \$smtp_ssl_cert_path,
     "smtpdomain" => \$smtp_domain,
     "to" => \@initial_to,
     "tocmd" => \$to_cmd,
@@ -302,6 +306,7 @@ sub signal_handler {
 		    "smtp-pass:s" => \$smtp_authpass,
 		    "smtp-ssl" => sub { $smtp_encryption = 'ssl' },
 		    "smtp-encryption=s" => \$smtp_encryption,
+		    "smtp-ssl-cert-path" => \$smtp_ssl_cert_path,
 		    "smtp-debug:i" => \$debug_net_smtp,
 		    "smtp-domain:s" => \$smtp_domain,
 		    "identity=s" => \$identity,
@@ -1089,6 +1094,34 @@ sub smtp_auth_maybe {
 	return $auth;
 }
 
+sub ssl_verify_params {
+	eval {
+		require IO::Socket::SSL;
+		IO::Socket::SSL->import(qw/SSL_VERIFY_PEER SSL_VERIFY_NONE/);
+	};
+	if ($@) {
+		print STDERR "Not using SSL_VERIFY_PEER due to out-of-date IO::Socket::SSL.\n";
+		return;
+	}
+
+	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 {
+		print STDERR "Not using SSL_VERIFY_PEER because the CA path does not exist.\n";
+		return (SSL_verify_mode => SSL_VERIFY_NONE());
+	}
+}
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1194,7 +1227,8 @@ sub send_message {
 			$smtp_domain ||= maildomain();
 			$smtp ||= Net::SMTP::SSL->new($smtp_server,
 						      Hello => $smtp_domain,
-						      Port => $smtp_server_port);
+						      Port => $smtp_server_port,
+						      ssl_verify_params());
 		}
 		else {
 			require Net::SMTP;
@@ -1207,7 +1241,8 @@ sub send_message {
 				$smtp->command('STARTTLS');
 				$smtp->response();
 				if ($smtp->code == 220) {
-					$smtp = Net::SMTP::SSL->start_SSL($smtp)
+					$smtp = Net::SMTP::SSL->start_SSL($smtp,
+									  ssl_verify_params())
 						or die "STARTTLS failed! ".$smtp->message;
 					$smtp_encryption = '';
 					# Send EHLO again to receive fresh
-- 
1.8.3.3-992-gf0e5e44