git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] send-email: squelch warning from Net::SMTP::SSL
@ 2013-07-05 10:18 Ramkumar Ramachandra
  2013-07-05 10:45 ` John Keeping
  0 siblings, 1 reply; 7+ messages in thread
From: Ramkumar Ramachandra @ 2013-07-05 10:18 UTC (permalink / raw)
  To: Git List; +Cc: brian m. carlson, Junio C Hamano

Due to a recent change in the Net::SMTP::SSL module, send-email emits
the following ugly warning everytime a email is sent via SSL:

*******************************************************************
 Using the default of SSL_verify_mode of SSL_VERIFY_NONE for client
 is deprecated! Please set SSL_verify_mode to SSL_VERIFY_PEER
 together with SSL_ca_file|SSL_ca_path for verification.
 If you really don't want to verify the certificate and keep the
 connection open to Man-In-The-Middle attacks please set
 SSL_verify_mode explicitly to SSL_VERIFY_NONE in your application.
*******************************************************************

Fix this by explicitly specifying SSL_verify_mode => SSL_VERIFY_NONE in
Net::SMTP::SSL->start_SSL().

Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 git-send-email.perl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index ecbf56f..758100d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1193,10 +1193,12 @@ X-Mailer: git-send-email $gitversion
 						 Debug => $debug_net_smtp);
 			if ($smtp_encryption eq 'tls' && $smtp) {
 				require Net::SMTP::SSL;
+				use IO::Socket::SSL qw(SSL_VERIFY_NONE);
 				$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_mode => SSL_VERIFY_NONE)
 						or die "STARTTLS failed! ".$smtp->message;
 					$smtp_encryption = '';
 					# Send EHLO again to receive fresh
-- 
1.8.3.2.722.g3244e19.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL
  2013-07-05 10:18 [PATCH] send-email: squelch warning from Net::SMTP::SSL Ramkumar Ramachandra
@ 2013-07-05 10:45 ` John Keeping
  2013-07-05 10:52   ` Ramkumar Ramachandra
  0 siblings, 1 reply; 7+ messages in thread
From: John Keeping @ 2013-07-05 10:45 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Git List, brian m. carlson, Junio C Hamano

On Fri, Jul 05, 2013 at 03:48:31PM +0530, Ramkumar Ramachandra wrote:
> Due to a recent change in the Net::SMTP::SSL module, send-email emits
> the following ugly warning everytime a email is sent via SSL:
> 
> *******************************************************************
>  Using the default of SSL_verify_mode of SSL_VERIFY_NONE for client
>  is deprecated! Please set SSL_verify_mode to SSL_VERIFY_PEER
>  together with SSL_ca_file|SSL_ca_path for verification.
>  If you really don't want to verify the certificate and keep the
>  connection open to Man-In-The-Middle attacks please set
>  SSL_verify_mode explicitly to SSL_VERIFY_NONE in your application.
> *******************************************************************
> 
> Fix this by explicitly specifying SSL_verify_mode => SSL_VERIFY_NONE in
> Net::SMTP::SSL->start_SSL().

I don't think this is really "fix", it's more plastering over the
problem.  As the message from OpenSSL says, specifying this means that
we're explicitly saying that we don't want to check the server
certificate which loses half of the security of SSL.

I'd rather leave this as it is (complete with the big scary error
message) and eventually fix it properly by letting the user specify the
ca_file or ca_path.  Perhaps we can even set a sensible default,
although I expect this needs to be platform-specific.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL
  2013-07-05 10:45 ` John Keeping
@ 2013-07-05 10:52   ` Ramkumar Ramachandra
  2013-07-05 11:31     ` Matthieu Moy
  0 siblings, 1 reply; 7+ messages in thread
From: Ramkumar Ramachandra @ 2013-07-05 10:52 UTC (permalink / raw)
  To: John Keeping; +Cc: Git List, brian m. carlson, Junio C Hamano

John Keeping wrote:
> I don't think this is really "fix", it's more plastering over the
> problem.

It defaulted to SSL_VERIFY_NONE before Net::SMTP::SSL was updated, and
the behavior hasn't changed now.  The new version simply asks us to be
explicit about SSL_VERIFY_NONE, so we are aware about it.

> I'd rather leave this as it is (complete with the big scary error
> message) and eventually fix it properly by letting the user specify the
> ca_file or ca_path.  Perhaps we can even set a sensible default,
> although I expect this needs to be platform-specific.

Nothing scary about it: it eats up real estate, and that is annoying.
I personally couldn't care less about ca_file, since all my emails are
to public mailing lists anyway.  Work on specifying a proper ca_file
as a follow-up, if you so desire.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL
  2013-07-05 10:52   ` Ramkumar Ramachandra
@ 2013-07-05 11:31     ` Matthieu Moy
  2013-07-26  9:29       ` Colin Guthrie
  0 siblings, 1 reply; 7+ messages in thread
From: Matthieu Moy @ 2013-07-05 11:31 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: John Keeping, Git List, brian m. carlson, Junio C Hamano

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> John Keeping wrote:
>> I don't think this is really "fix", it's more plastering over the
>> problem.
>
> It defaulted to SSL_VERIFY_NONE before Net::SMTP::SSL was updated, and
> the behavior hasn't changed now.  The new version simply asks us to be
> explicit about SSL_VERIFY_NONE, so we are aware about it.

"We" as "the Git developers", yes. But your change makes sure users are
_not_ aware about it. There's a long history of software ignoring SSL
certificates by default, I don't think we should cast in stone that we
don't want SSL certificate verification.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL
  2013-07-05 11:31     ` Matthieu Moy
@ 2013-07-26  9:29       ` Colin Guthrie
  2013-07-29 16:01         ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Colin Guthrie @ 2013-07-26  9:29 UTC (permalink / raw)
  To: git

'Twas brillig, and Matthieu Moy at 05/07/13 12:31 did gyre and gimble:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
> 
>> John Keeping wrote:
>>> I don't think this is really "fix", it's more plastering over the
>>> problem.
>>
>> It defaulted to SSL_VERIFY_NONE before Net::SMTP::SSL was updated, and
>> the behavior hasn't changed now.  The new version simply asks us to be
>> explicit about SSL_VERIFY_NONE, so we are aware about it.
> 
> "We" as "the Git developers", yes. But your change makes sure users are
> _not_ aware about it. There's a long history of software ignoring SSL
> certificates by default, I don't think we should cast in stone that we
> don't want SSL certificate verification.

For what it's worth, after upgrading here, I got this error at the
server side:

Jul 26 10:15:41 foo.example.com postfix/smtpd[7736]: warning: TLS
library problem: 7736:error:14094418:SSL routines:SSL3_READ_BYTES:tlsv1
alert unknown ca:s3_pkt.c:1256:SSL alert number 48:


This is because my postfix doesn't have a ca bundle configured but all
other mail clients have been fine before.

With the original patch here I could continue.

I'd really love to see an option to set this to none in the .gitconfig,
but agree with the principle that it should be one by default and the
setting should over ride that.

All the best

Col

PS I'm mainly posting this such that people searching the intertubes for
the postfix error above and git-send-email will match at least this
message and find the fix/workaround :)

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL
  2013-07-26  9:29       ` Colin Guthrie
@ 2013-07-29 16:01         ` Junio C Hamano
  2013-07-29 17:12           ` Colin Guthrie
  0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2013-07-29 16:01 UTC (permalink / raw)
  To: Colin Guthrie; +Cc: git

Colin Guthrie <gmane@colin.guthr.ie> writes:

> For what it's worth, after upgrading here, I got this error at the
> server side:
>
> Jul 26 10:15:41 foo.example.com postfix/smtpd[7736]: warning: TLS
> library problem: 7736:error:14094418:SSL routines:SSL3_READ_BYTES:tlsv1
> alert unknown ca:s3_pkt.c:1256:SSL alert number 48:
>
>
> This is because my postfix doesn't have a ca bundle configured but all
> other mail clients have been fine before.
>
> With the original patch here I could continue.
>
> I'd really love to see an option to set this to none in the .gitconfig,

Isn't that what the final patch committed under Ram's name
implements?

    sendemail.smtpsslcertpath::
           Path to ca-certificates (either a directory or a single file).
           Set it to an empty string to disable certificate verification.

or have we missed your use case?

> but agree with the principle that it should be one by default and the
> setting should over ride that.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL
  2013-07-29 16:01         ` Junio C Hamano
@ 2013-07-29 17:12           ` Colin Guthrie
  0 siblings, 0 replies; 7+ messages in thread
From: Colin Guthrie @ 2013-07-29 17:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

'Twas brillig, and Junio C Hamano at 29/07/13 17:01 did gyre and gimble:
> Colin Guthrie <gmane@colin.guthr.ie> writes:
> 
>> For what it's worth, after upgrading here, I got this error at the
>> server side:
>>
>> Jul 26 10:15:41 foo.example.com postfix/smtpd[7736]: warning: TLS
>> library problem: 7736:error:14094418:SSL routines:SSL3_READ_BYTES:tlsv1
>> alert unknown ca:s3_pkt.c:1256:SSL alert number 48:
>>
>>
>> This is because my postfix doesn't have a ca bundle configured but all
>> other mail clients have been fine before.
>>
>> With the original patch here I could continue.
>>
>> I'd really love to see an option to set this to none in the .gitconfig,
> 
> Isn't that what the final patch committed under Ram's name
> implements?
> 
>     sendemail.smtpsslcertpath::
>            Path to ca-certificates (either a directory or a single file).
>            Set it to an empty string to disable certificate verification.
> 
> or have we missed your use case?

Yeah that patch works fine if I set the certpath = "" in the config, and
I even added that path to our packages.

I read the mailing list after following the google trail and replied to
that specific message without realising a more recent patch was
available and then didn't post a followup saying the final patch worked
- my bad!

Sorry about that, and thanks to everyone for the final patch :)

Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-07-29 17:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-05 10:18 [PATCH] send-email: squelch warning from Net::SMTP::SSL Ramkumar Ramachandra
2013-07-05 10:45 ` John Keeping
2013-07-05 10:52   ` Ramkumar Ramachandra
2013-07-05 11:31     ` Matthieu Moy
2013-07-26  9:29       ` Colin Guthrie
2013-07-29 16:01         ` Junio C Hamano
2013-07-29 17:12           ` Colin Guthrie

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).