{"thread":{"id":"35447","subject":"[PATCH 2/3] send-email: --smtp-ssl-cert-path takes an argument","startedAt":"2013-12-01T22:48:41Z","lastAt":"2013-12-02T23:23:28Z","messageCount":5,"participants":["Thomas Rast","Ramkumar Ramachandra"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"231341","messageId":"3bb0c80c70e1c40236034552bec037cb0c26167c.1385938050.git.tr@thomasrast.ch","threadId":"35447","inReplyTo":null,"subject":"[PATCH 1/3] send-email: pass Debug to Net::SMTP::SSL::new","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2013-12-01T22:48:41Z","receivedAt":"2013-12-01T22:48:41Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"We forgot to pass the Debug option through to Net::SMTP::SSL->new --\nwhich is the same as Net::SMTP->new.  This meant that with security\nset to SSL, we would never enable debug output.\n\nPass through the flag.\n\nSigned-off-by: Thomas Rast <tr@thomasrast.ch>\n---\n git-send-email.perl | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 3782c3b..f7468b6 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1217,6 +1217,7 @@ sub send_message {\n \t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n \t\t\t\t\t\t      Hello => $smtp_domain,\n \t\t\t\t\t\t      Port => $smtp_server_port,\n+\t\t\t\t\t\t      Debug => $debug_net_smtp,\n \t\t\t\t\t\t      ssl_verify_params());\n \t\t}\n \t\telse {\n-- \n1.8.5.rc3.5.g2a1fe2f\n"},{"id":"231340","messageId":"a8acc2b499f1635be6a7f7414aca8cfd75a7c828.1385938050.git.tr@thomasrast.ch","threadId":"35447","inReplyTo":"3bb0c80c70e1c40236034552bec037cb0c26167c.1385938050.git.tr@thomasrast.ch","subject":"[PATCH 2/3] send-email: --smtp-ssl-cert-path takes an argument","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2013-12-01T22:48:42Z","receivedAt":"2013-12-01T22:48:42Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"35035bb (send-email: be explicit with SSL certificate verification,\n2013-07-18) forgot to specify that --smtp-ssl-cert-path takes a string\nargument.  This means that the option could not actually be used as\nintended.  Presumably noone noticed because it's much easier to set it\nthrough configs anyway.\n\nAdd the required \"=s\".\n\nSigned-off-by: Thomas Rast <tr@thomasrast.ch>\n---\n git-send-email.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f7468b6..9f31c68 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -291,7 +291,7 @@ sub signal_handler {\n \t\t    \"smtp-pass:s\" => \\$smtp_authpass,\n \t\t    \"smtp-ssl\" => sub { $smtp_encryption = 'ssl' },\n \t\t    \"smtp-encryption=s\" => \\$smtp_encryption,\n-\t\t    \"smtp-ssl-cert-path\" => \\$smtp_ssl_cert_path,\n+\t\t    \"smtp-ssl-cert-path=s\" => \\$smtp_ssl_cert_path,\n \t\t    \"smtp-debug:i\" => \\$debug_net_smtp,\n \t\t    \"smtp-domain:s\" => \\$smtp_domain,\n \t\t    \"identity=s\" => \\$identity,\n-- \n1.8.5.rc3.5.g2a1fe2f\n"},{"id":"231342","messageId":"c5308d5ffb34b70cbfea5a39e08902904fac1400.1385938050.git.tr@thomasrast.ch","threadId":"35447","inReplyTo":"3bb0c80c70e1c40236034552bec037cb0c26167c.1385938050.git.tr@thomasrast.ch","subject":"[PATCH 3/3] send-email: set SSL options through IO::Socket::SSL::set_client_defaults","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2013-12-01T22:48:43Z","receivedAt":"2013-12-01T22:48:43Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"When --smtp-encryption=ssl, we use a Net::SMTP::SSL connection,\npassing its ->new all the options that would otherwise go to\nNet::SMTP->new (most options) and IO::Socket::SSL->start_SSL (for the\nSSL options).\n\nHowever, while Net::SMTP::SSL replaces the underlying socket class\nwith an SSL socket, it does nothing to allow passing options to that\nsocket.  So the SSL-relevant options are lost.\n\nFortunately there is an escape hatch: we can directly set the options\nwith IO::Socket::SSL::set_client_defaults.  They will then persist\nwithin the IO::Socket::SSL module.\n\nSigned-off-by: Thomas Rast <tr@thomasrast.ch>\n---\n git-send-email.perl | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 9f31c68..2016d9c 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1214,11 +1214,14 @@ sub send_message {\n \t\t\t$smtp_server_port ||= 465; # ssmtp\n \t\t\trequire Net::SMTP::SSL;\n \t\t\t$smtp_domain ||= maildomain();\n+\t\t\trequire IO::Socket::SSL;\n+\t\t\t# Net::SMTP::SSL->new() does not forward any SSL options\n+\t\t\tIO::Socket::SSL::set_client_defaults(\n+\t\t\t\tssl_verify_params());\n \t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server,\n \t\t\t\t\t\t      Hello => $smtp_domain,\n \t\t\t\t\t\t      Port => $smtp_server_port,\n-\t\t\t\t\t\t      Debug => $debug_net_smtp,\n-\t\t\t\t\t\t      ssl_verify_params());\n+\t\t\t\t\t\t      Debug => $debug_net_smtp);\n \t\t}\n \t\telse {\n \t\t\trequire Net::SMTP;\n-- \n1.8.5.rc3.5.g2a1fe2f\n"},{"id":"231361","messageId":"CALkWK0nn867+3+cToc=QMyA0u+0oPJkq+nmB1T3DP+kiiwb72Q@mail.gmail.com","threadId":"35447","inReplyTo":"c5308d5ffb34b70cbfea5a39e08902904fac1400.1385938050.git.tr@thomasrast.ch","subject":"Re: [PATCH 3/3] send-email: set SSL options through IO::Socket::SSL::set_client_defaults","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-12-02T10:44:09Z","receivedAt":"2013-12-02T10:44:09Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Thomas Rast wrote:\n> When --smtp-encryption=ssl, we use a Net::SMTP::SSL connection,\n> passing its ->new all the options that would otherwise go to\n> Net::SMTP->new (most options) and IO::Socket::SSL->start_SSL (for the\n> SSL options).\n>\n> However, while Net::SMTP::SSL replaces the underlying socket class\n> with an SSL socket, it does nothing to allow passing options to that\n> socket.  So the SSL-relevant options are lost.\n\nBoth [1/3] and [2/3] look good. However, I'm curious about this one:\nNet::SMTP::SSL inherits from IO::Socket::SSL, where new() is defined.\nIn the documentation for IO::Socket::SSL,\n\n  $ perldoc IO::Socket::SSL\n\nI can see examples where SSL_verify_mode and SSL_ca_path are passed to\nnew(). So, I'm not sure what this patch is about.\n"},{"id":"231396","messageId":"87k3fmon0v.fsf@linux-1gf2.Speedport_W723_V_Typ_A_1_00_098","threadId":"35447","inReplyTo":"CALkWK0nn867+3+cToc=QMyA0u+0oPJkq+nmB1T3DP+kiiwb72Q@mail.gmail.com","subject":"Re: [PATCH 3/3] send-email: set SSL options through IO::Socket::SSL::set_client_defaults","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2013-12-02T23:23:28Z","receivedAt":"2013-12-02T23:23:28Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Thomas Rast wrote:\n>> When --smtp-encryption=ssl, we use a Net::SMTP::SSL connection,\n>> passing its ->new all the options that would otherwise go to\n>> Net::SMTP->new (most options) and IO::Socket::SSL->start_SSL (for the\n>> SSL options).\n>>\n>> However, while Net::SMTP::SSL replaces the underlying socket class\n>> with an SSL socket, it does nothing to allow passing options to that\n>> socket.  So the SSL-relevant options are lost.\n>\n> Both [1/3] and [2/3] look good. However, I'm curious about this one:\n> Net::SMTP::SSL inherits from IO::Socket::SSL, where new() is defined.\n> In the documentation for IO::Socket::SSL,\n>\n>   $ perldoc IO::Socket::SSL\n>\n> I can see examples where SSL_verify_mode and SSL_ca_path are passed to\n> new(). So, I'm not sure what this patch is about.\n\nNet::SMTP::SSL is merely steals all the code from Net::SMTP into a class\nthat has IO::Socket::SSL as its first inheritance line.\n\nThis works because Net::SMTP (no SSL) inherits from IO::Socket::INET\ninstead, and uses SUPER:: methods to access the latter's features.  So\nby effectively replacing IO::Socket::INET with IO::Socket::SSL,\nNet::SMTP::SSL can apply all of Net::SMTP's code on an SSL socket.\n\nHowever!\n\nThat SUPER:: access does not pass anything SSLey.  In particular,\nNet::SMTP::SSL->new (which is just the same as Net::SMTP->new) runs this\nto initialize its socket:\n\n    $obj = $type->SUPER::new(\n      PeerAddr => ($host = $h),\n      PeerPort => $arg{Port} || 'smtp(25)',\n      LocalAddr => $arg{LocalAddr},\n      LocalPort => $arg{LocalPort},\n      Proto     => 'tcp',\n      Timeout   => defined $arg{Timeout}\n      ? $arg{Timeout}\n      : 120\n      )\n\nNote the conspicuous absence of any kind of SSL arguments, or any kind\nof args-I-don't-know-myself passthrough.\n\nIf you _do_ specify SSL arguments (i.e. key-value style arguments that\nwould normally be accepted by IO::Socket::SSL->new) to\nNet::SMTP::SSL->new, they will simply be ignored, because of how the\nkey-value argument passing treats the argument list as a hash.\n\nDoes that clarify it?\n\nThis is all assuming I got the details vaguely correct, and the source\nsnippets are from my perl v5.18.1 installed by opensuse 13.1.\n\nIt turns out the server I was trying to talk to on Sunday had an expired\ncertificate, and despite the code from 35035bb, my efforts to set\nSSL_VERIFY_NONE were futile.  Until I noticed the set_client_defaults()\ntrick.  So I'm pretty convinced the patch does *something* right.\n\n-- \nThomas Rast\ntr@thomasrast.ch\n"}]}