Re: [PATCH v2 2/2] send-email: introduce sendemail.smtpsslcertpath

From: John Keeping <john@keeping.me.uk>
Date: Jul 5, 2013, 18:43 UTC
Message-ID: <20130705184333.GN9161@serenity.lan>
In-Reply-To: <7vehbc7tcc.fsf@alter.siamese.dyndns.org>

On Fri, Jul 05, 2013 at 11:30:11AM -0700, Junio C Hamano wrote:

Show 36 quoted lines

> John Keeping <john@keeping.me.uk> writes:
> 
> > On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
> >> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> >> 
> >> > You've covered the STARTTLS case, but not the SSL one right above it.
> >> > Someone using smtps on port 465 will still see the warning.  You can
> >> > pass SSL_verify_mode to Net::SMTP::SSL->new just like you pass it to
> >> > start_SSL.
> >> 
> >> OK, will a fix-up look like this on top of 1/2 and 2/2?
> >
> > According to IO::Socket::SSL [1], if neither SSL_ca_file nor SSL_ca_path
> > is specified then builtin defaults will be used, so I wonder if we
> > should pass SSL_VERIFY_PEER regardless (possibly with a switch for
> > SSL_VERIFY_NONE if people really need that).
> >
> > [1] http://search.cpan.org/~sullr/IO-Socket-SSL-1.951/lib/IO/Socket/SSL.pm
> 
> Interesting.  That frees us from saying "we assume /etc/ssl/cacerts
> is the default location, and let the users override it".
> 
> To help those "I do not want verification because I know my server
> does not present valid certificate, I know my server is internal and
> trustable, and I do not bother to fix it" people, we can let them
> specify an empty string (or any non-directory) as the CACertPath,
> and structure the code like so?
> 
>         if (defined $smtp_ssl_cert_path && -d $smtp_ssl_cert_path) {
>                 return (SSL_verify_mode => SSL_VERIFY_PEER,
>                         SSL_ca_path => $smtp_ssl_cert_path);
>         } elsif (defined $smtp_ssl_cert_path) {
>                 return (SSL_verify_mode => SSL_VERIFY_NONE);
>         } else {
>                 return (SSL_verify_mode => SSL_VERIFY_PEER);
>         }

I'd rather have '$smtp_ssl_cert_path ne ""' in the first if condition (instead of the '-d $smtp_ssl_cert_path') but that seems reasonable and agrees with my reading of the documentation.

Perhaps a complete solution could allow CA files as well:

	if (defined $smtp_ssl_cert_path) {
		if ($smtp_ssl_cert_path eq "") {
			return (SSL_verify_mode => SSL_VERIFY_NONE);
		} elsif (-f $smtp_ssl_cert_path) {
			return (SSL_verify_mode => SSL_VERIFY_PEER,
				SSL_ca_file => $smtp_ssl_cert_path);
		} else {
			return (SSL_verify_mode => SSL_VERIFY_PEER,
				SSL_ca_path => $smtp_ssl_cert_path);
		}
	} else {
		return (SSL_verify_mode => SSL_VERIFY_PEER);
	}