# [PATCH 2/3] send-email: --smtp-ssl-cert-path takes an argument 5 messages from 2013-12-01 to 2013-12-02. Participants: Thomas Rast, Ramkumar Ramachandra. Thread: https://gitlist.dev/t/35447 ## Thomas Rast, 2013-12-01 22:48 Subject: [PATCH 1/3] send-email: pass Debug to Net::SMTP::SSL::new Message-ID: <3bb0c80c70e1c40236034552bec037cb0c26167c.1385938050.git.tr@thomasrast.ch> URL: https://gitlist.dev/e/3bb0c80c70e1c40236034552bec037cb0c26167c.1385938050.git.tr%40thomasrast.ch ``` We forgot to pass the Debug option through to Net::SMTP::SSL->new -- which is the same as Net::SMTP->new. This meant that with security set to SSL, we would never enable debug output. Pass through the flag. Signed-off-by: Thomas Rast --- git-send-email.perl | 1 + 1 file changed, 1 insertion(+) diff --git a/git-send-email.perl b/git-send-email.perl index 3782c3b..f7468b6 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1217,6 +1217,7 @@ sub send_message { $smtp ||= Net::SMTP::SSL->new($smtp_server, Hello => $smtp_domain, Port => $smtp_server_port, + Debug => $debug_net_smtp, ssl_verify_params()); } else { -- 1.8.5.rc3.5.g2a1fe2f ``` ## Thomas Rast, 2013-12-01 22:48 Subject: [PATCH 2/3] send-email: --smtp-ssl-cert-path takes an argument Message-ID: URL: https://gitlist.dev/e/a8acc2b499f1635be6a7f7414aca8cfd75a7c828.1385938050.git.tr%40thomasrast.ch In-Reply-To: <3bb0c80c70e1c40236034552bec037cb0c26167c.1385938050.git.tr@thomasrast.ch> ``` 35035bb (send-email: be explicit with SSL certificate verification, 2013-07-18) forgot to specify that --smtp-ssl-cert-path takes a string argument. This means that the option could not actually be used as intended. Presumably noone noticed because it's much easier to set it through configs anyway. Add the required "=s". Signed-off-by: Thomas Rast --- git-send-email.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-send-email.perl b/git-send-email.perl index f7468b6..9f31c68 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -291,7 +291,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-ssl-cert-path=s" => \$smtp_ssl_cert_path, "smtp-debug:i" => \$debug_net_smtp, "smtp-domain:s" => \$smtp_domain, "identity=s" => \$identity, -- 1.8.5.rc3.5.g2a1fe2f ``` ## Thomas Rast, 2013-12-01 22:48 Subject: [PATCH 3/3] send-email: set SSL options through IO::Socket::SSL::set_client_defaults Message-ID: URL: https://gitlist.dev/e/c5308d5ffb34b70cbfea5a39e08902904fac1400.1385938050.git.tr%40thomasrast.ch In-Reply-To: <3bb0c80c70e1c40236034552bec037cb0c26167c.1385938050.git.tr@thomasrast.ch> ``` When --smtp-encryption=ssl, we use a Net::SMTP::SSL connection, passing its ->new all the options that would otherwise go to Net::SMTP->new (most options) and IO::Socket::SSL->start_SSL (for the SSL options). However, while Net::SMTP::SSL replaces the underlying socket class with an SSL socket, it does nothing to allow passing options to that socket. So the SSL-relevant options are lost. Fortunately there is an escape hatch: we can directly set the options with IO::Socket::SSL::set_client_defaults. They will then persist within the IO::Socket::SSL module. Signed-off-by: Thomas Rast --- git-send-email.perl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 9f31c68..2016d9c 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1214,11 +1214,14 @@ sub send_message { $smtp_server_port ||= 465; # ssmtp require Net::SMTP::SSL; $smtp_domain ||= maildomain(); + require IO::Socket::SSL; + # Net::SMTP::SSL->new() does not forward any SSL options + IO::Socket::SSL::set_client_defaults( + ssl_verify_params()); $smtp ||= Net::SMTP::SSL->new($smtp_server, Hello => $smtp_domain, Port => $smtp_server_port, - Debug => $debug_net_smtp, - ssl_verify_params()); + Debug => $debug_net_smtp); } else { require Net::SMTP; -- 1.8.5.rc3.5.g2a1fe2f ``` ## Ramkumar Ramachandra, 2013-12-02 10:44 Subject: Re: [PATCH 3/3] send-email: set SSL options through IO::Socket::SSL::set_client_defaults Message-ID: URL: https://gitlist.dev/e/CALkWK0nn867%2B3%2BcToc%3DQMyA0u%2B0oPJkq%2BnmB1T3DP%2Bkiiwb72Q%40mail.gmail.com In-Reply-To: ``` Thomas Rast wrote: > When --smtp-encryption=ssl, we use a Net::SMTP::SSL connection, > passing its ->new all the options that would otherwise go to > Net::SMTP->new (most options) and IO::Socket::SSL->start_SSL (for the > SSL options). > > However, while Net::SMTP::SSL replaces the underlying socket class > with an SSL socket, it does nothing to allow passing options to that > socket. So the SSL-relevant options are lost. Both [1/3] and [2/3] look good. However, I'm curious about this one: Net::SMTP::SSL inherits from IO::Socket::SSL, where new() is defined. In the documentation for IO::Socket::SSL, $ perldoc IO::Socket::SSL I can see examples where SSL_verify_mode and SSL_ca_path are passed to new(). So, I'm not sure what this patch is about. ``` ## Thomas Rast, 2013-12-02 23:23 Subject: Re: [PATCH 3/3] send-email: set SSL options through IO::Socket::SSL::set_client_defaults Message-ID: <87k3fmon0v.fsf@linux-1gf2.Speedport_W723_V_Typ_A_1_00_098> URL: https://gitlist.dev/e/87k3fmon0v.fsf%40linux-1gf2.Speedport_W723_V_Typ_A_1_00_098 In-Reply-To: ``` Ramkumar Ramachandra writes: > Thomas Rast wrote: >> When --smtp-encryption=ssl, we use a Net::SMTP::SSL connection, >> passing its ->new all the options that would otherwise go to >> Net::SMTP->new (most options) and IO::Socket::SSL->start_SSL (for the >> SSL options). >> >> However, while Net::SMTP::SSL replaces the underlying socket class >> with an SSL socket, it does nothing to allow passing options to that >> socket. So the SSL-relevant options are lost. > > Both [1/3] and [2/3] look good. However, I'm curious about this one: > Net::SMTP::SSL inherits from IO::Socket::SSL, where new() is defined. > In the documentation for IO::Socket::SSL, > > $ perldoc IO::Socket::SSL > > I can see examples where SSL_verify_mode and SSL_ca_path are passed to > new(). So, I'm not sure what this patch is about. Net::SMTP::SSL is merely steals all the code from Net::SMTP into a class that has IO::Socket::SSL as its first inheritance line. This works because Net::SMTP (no SSL) inherits from IO::Socket::INET instead, and uses SUPER:: methods to access the latter's features. So by effectively replacing IO::Socket::INET with IO::Socket::SSL, Net::SMTP::SSL can apply all of Net::SMTP's code on an SSL socket. However! That SUPER:: access does not pass anything SSLey. In particular, Net::SMTP::SSL->new (which is just the same as Net::SMTP->new) runs this to initialize its socket: $obj = $type->SUPER::new( PeerAddr => ($host = $h), PeerPort => $arg{Port} || 'smtp(25)', LocalAddr => $arg{LocalAddr}, LocalPort => $arg{LocalPort}, Proto => 'tcp', Timeout => defined $arg{Timeout} ? $arg{Timeout} : 120 ) Note the conspicuous absence of any kind of SSL arguments, or any kind of args-I-don't-know-myself passthrough. If you _do_ specify SSL arguments (i.e. key-value style arguments that would normally be accepted by IO::Socket::SSL->new) to Net::SMTP::SSL->new, they will simply be ignored, because of how the key-value argument passing treats the argument list as a hash. Does that clarify it? This is all assuming I got the details vaguely correct, and the source snippets are from my perl v5.18.1 installed by opensuse 13.1. It turns out the server I was trying to talk to on Sunday had an expired certificate, and despite the code from 35035bb, my efforts to set SSL_VERIFY_NONE were futile. Until I noticed the set_client_defaults() trick. So I'm pretty convinced the patch does *something* right. -- Thomas Rast tr@thomasrast.ch ```