diff mbox series

send-email: clarify SMTP encryption settings

Message ID 20210409211812.3869-1-sir@cmpwn.com (mailing list archive)
State New
Headers show
Series send-email: clarify SMTP encryption settings | expand

Commit Message

Drew DeVault April 9, 2021, 9:18 p.m. UTC
The present options are misleading; "ssl" enables generic, "modern" SSL
support, which could use either SSL or TLS; and "tls" enables the
SMTP-specific (and deprecated) STARTTLS protocol.

This changes the canonical config options to "ssl/tls" and "starttls",
updates the docs to explain the options in more detail, and updates
git-send-email to accept either form.
---
 Documentation/git-send-email.txt | 11 ++++++++---
 git-send-email.perl              |  4 ++--
 2 files changed, 10 insertions(+), 5 deletions(-)

Comments

Eric Sunshine April 9, 2021, 10:04 p.m. UTC | #1
On Fri, Apr 9, 2021 at 5:18 PM Drew DeVault <sir@cmpwn.com> wrote:
> The present options are misleading; "ssl" enables generic, "modern" SSL
> support, which could use either SSL or TLS; and "tls" enables the
> SMTP-specific (and deprecated) STARTTLS protocol.
>
> This changes the canonical config options to "ssl/tls" and "starttls",
> updates the docs to explain the options in more detail, and updates
> git-send-email to accept either form.
> ---

Missing sign-off.
Georgios Kontaxis April 9, 2021, 11:14 p.m. UTC | #2
> On Apr 9, 2021, at 14:18, Drew DeVault <sir@cmpwn.com> wrote:
> 
> ´╗┐The present options are misleading; "ssl" enables generic, "modern" SSL
> support, which could use either SSL or TLS; and "tls" enables the
> SMTP-specific (and deprecated) STARTTLS protocol.
> 
> This changes the canonical config options to "ssl/tls" and "starttls",
> updates the docs to explain the options in more detail, and updates
> git-send-email to accept either form.
> ---
> Documentation/git-send-email.txt | 11 ++++++++---
> git-send-email.perl              |  4 ++--
> 2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 93708aefea..3597935e41 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -168,9 +168,14 @@ Sending
>    unspecified, choosing the envelope sender is left to your MTA.
> 
> --smtp-encryption=<encryption>::
> -    Specify the encryption to use, either 'ssl' or 'tls'.  Any other
> -    value reverts to plain SMTP.  Default is the value of
> -    `sendemail.smtpEncryption`.
> +    Specify the encryption to use, either 'ssl/tls' or 'starttls', whichever
> +    is recommended by your email service provider.  SSL/TLS is typically
> +    used on port 465 and is preferred if available.  STARTTLS is typically
> +    used on port 25 or 587. Any other value reverts to plain SMTP.  The
Weird that we fail open (no encryption) on typos.
Any chance we can fix that in this patch?

> +    default is the value of `sendemail.smtpEncryption`.
> ++
> +For legacy reasons, 'ssl' is accepted for 'ssl/tls' and 'tls' is accepted for
> +'starttls'.
> 
> --smtp-domain=<FQDN>::
>    Specifies the Fully Qualified Domain Name (FQDN) used in the
> diff --git a/git-send-email.perl b/git-send-email.perl
> index f5bbf1647e..34fdf587bd 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1503,7 +1503,7 @@ sub send_message {
>        my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34");
>        $smtp_domain ||= maildomain();
> 
> -        if ($smtp_encryption eq 'ssl') {
> +        if ($smtp_encryption eq 'ssl' || $smtp_encryption eq 'ssl/tls') {
>            $smtp_server_port ||= 465; # ssmtp
>            require IO::Socket::SSL;
> 
> @@ -1538,7 +1538,7 @@ sub send_message {
>                         Hello => $smtp_domain,
>                         Debug => $debug_net_smtp,
>                         Port => $smtp_server_port);
> -            if ($smtp_encryption eq 'tls' && $smtp) {
> +            if (($smtp_encryption eq 'tls' || $smtp_encryption eq 'starttls') && $smtp) {
>                if ($use_net_smtp_ssl) {
>                    $smtp->command('STARTTLS');
>                    $smtp->response();
> -- 
> 2.31.1
>
Drew DeVault April 9, 2021, 11:39 p.m. UTC | #3
On Fri Apr 9, 2021 at 7:14 PM EDT, Georgios Kontaxis wrote:
> Weird that we fail open (no encryption) on typos.
> Any chance we can fix that in this patch?

That would technically be a breaking change.
Junio C Hamano April 10, 2021, 12:52 a.m. UTC | #4
Drew DeVault <sir@cmpwn.com> writes:

> The present options are misleading; "ssl" enables generic, "modern" SSL
> support, which could use either SSL or TLS; and "tls" enables the
> SMTP-specific (and deprecated) STARTTLS protocol.

Hmph.

Isn't SMTPS (running SMTP over SSL encrypted connection) the one
that was once deprecated until it got resurrected)?

STARTTLS is not all that SMTP specific---POP and IMAP can also start
in cleartext and upgrade with STARTTLS the same way, no?

I couldn't find a justification for our log message to call
STARTTLS-style explicit TLS "deprecated".  When you send an updated
version, please give a reference.

> This changes the canonical config options to "ssl/tls" and "starttls",
> updates the docs to explain the options in more detail, and updates
> git-send-email to accept either form.
> ---

Missing Sign-off, ...

>  Documentation/git-send-email.txt | 11 ++++++++---
>  git-send-email.perl              |  4 ++--
>  2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 93708aefea..3597935e41 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -168,9 +168,14 @@ Sending
>  	unspecified, choosing the envelope sender is left to your MTA.
>  
>  --smtp-encryption=<encryption>::
> -	Specify the encryption to use, either 'ssl' or 'tls'.  Any other
> -	value reverts to plain SMTP.  Default is the value of
> -	`sendemail.smtpEncryption`.
> +	Specify the encryption to use, either 'ssl/tls' or 'starttls', whichever
> +	is recommended by your email service provider.  SSL/TLS is typically
> +	used on port 465 and is preferred if available.  STARTTLS is typically
> +	used on port 25 or 587. Any other value reverts to plain SMTP.  The
> +	default is the value of `sendemail.smtpEncryption`.

I think it is a vast improvement to describe what existing 'ssl' and
'tls' does, like the above does.  It is a documentation update that
deserves its own commit (i.e. [PATCH 1/3]), and it should be done
before adding the new ssl/tls and starttls synonyms.

Making it an error to give unrecognised string (i.e. other than
'ssl' and 'tls'), or at least warning, would be a good follow-up
change (i.e. [PATCH 2/3]), but that is optional.

And then, it may make sense to introduce the synonyms, but please
make it a separate patch that builds on top of the other two steps
(i.e. [PATCH 3/3]).

Honestly I am ambivalent about these two synonyms this patch added.

In the ideal world, it would have been nice if we could make 'tls'
as the name of the choice that has been known as 'ssl' (i.e. the
underlying transport protocol to run SMTP or any other higher layer
protocol on top, there used to be SSL but these days TLS is used as
an improved alternative---SSL 2.0/3.0 have been deprecated for some
time), but because we used 'tls' to mean the STARTTLS-style "start
SMTP as plain and then upgrade to encrypted channel", we can't reuse
the 'tls' for that purpose.

I do not have any qualm about the fully spelled out "starttls"
synonym for the latter.  In fact, if we can go back in time and redo
the history with hindsight, that is the name we should have used
from the beginning.  But I find it unfortunate that we need to say
'ssl/tls', i.e. prefixing the name of the choice with the name of a
deprecated thing, for the former.  Another reason I am hesitant
about 'ssl/tls' is because the description of it in documentation
naturally invites errors.  I.e. "You can set it to 'ssl/tls'..."
sounds as if the manual is telling me to use one of 'ssl' or 'tls',
which is not what it is sayng---it literally wants me to say
'ssl/tls' with a slash in it.

Thanks.
Drew DeVault April 10, 2021, 12:57 a.m. UTC | #5
On Fri Apr 9, 2021 at 8:52 PM EDT, Junio C Hamano wrote:
> Hmph.
>
> Isn't SMTPS (running SMTP over SSL encrypted connection) the one
> that was once deprecated until it got resurrected)?

Kind of, back in the 90's, but that's water under the bridge now. SMTP
over SSL/TLS is the de-facto standard.

> STARTTLS is not all that SMTP specific---POP and IMAP can also start
> in cleartext and upgrade with STARTTLS the same way, no?

Well, email-specific, at least. Sorry for the confusion.

> I couldn't find a justification for our log message to call
> STARTTLS-style explicit TLS "deprecated". When you send an updated
> version, please give a reference.

The main concern with STARTTLS is downgrade attacks. I'll note this in
the commit message for v2.

> I think it is a vast improvement to describe what existing 'ssl' and
> 'tls' does, like the above does. It is a documentation update that
> deserves its own commit (i.e. [PATCH 1/3]), and it should be done
> before adding the new ssl/tls and starttls synonyms.
>
> Making it an error to give unrecognised string (i.e. other than
> 'ssl' and 'tls'), or at least warning, would be a good follow-up
> change (i.e. [PATCH 2/3]), but that is optional.
>
> And then, it may make sense to introduce the synonyms, but please
> make it a separate patch that builds on top of the other two steps
> (i.e. [PATCH 3/3]).

Ack, can do.

> In the ideal world, it would have been nice if we could make 'tls'
> as the name of the choice that has been known as 'ssl' (i.e. the
> underlying transport protocol to run SMTP or any other higher layer
> protocol on top, there used to be SSL but these days TLS is used as
> an improved alternative---SSL 2.0/3.0 have been deprecated for some
> time), but because we used 'tls' to mean the STARTTLS-style "start
> SMTP as plain and then upgrade to encrypted channel", we can't reuse
> the 'tls' for that purpose.
>
> I do not have any qualm about the fully spelled out "starttls"
> synonym for the latter. In fact, if we can go back in time and redo
> the history with hindsight, that is the name we should have used
> from the beginning. But I find it unfortunate that we need to say
> 'ssl/tls', i.e. prefixing the name of the choice with the name of a
> deprecated thing, for the former. Another reason I am hesitant
> about 'ssl/tls' is because the description of it in documentation
> naturally invites errors. I.e. "You can set it to 'ssl/tls'..."
> sounds as if the manual is telling me to use one of 'ssl' or 'tls',
> which is not what it is sayng---it literally wants me to say
> 'ssl/tls' with a slash in it.

If I may propose a bold alternative: what I added as "ssl/tls", i.e.
"modern" SSL, should be "yes", no encryption should be "no", and if you
specifically need starttls: "starttls".
Junio C Hamano April 10, 2021, 1:09 a.m. UTC | #6
"Drew DeVault" <sir@cmpwn.com> writes:

>> I couldn't find a justification for our log message to call
>> STARTTLS-style explicit TLS "deprecated". When you send an updated
>> version, please give a reference.
>
> The main concern with STARTTLS is downgrade attacks. I'll note this in
> the commit message for v2.
> ...
> If I may propose a bold alternative: what I added as "ssl/tls", i.e.
> "modern" SSL, should be "yes", no encryption should be "no", and if you
> specifically need starttls: "starttls".

Well, "is starttls deprecated" given to search engine gives me

    SMTPS (implicit SSL) has been deprecated/obsolete since
    SMTP+STARTTLS (explicit SSL) was defined in RFC2487.

as the "featured snippet", and there are debates like "SMTPS has
been deprecated since forever (late 90's or thereabouts)"
https://news.ycombinator.com/item?id=10556797

I strongly prefer to keep our documentation out of that mess by not
taking sides.  To me, both are valid options to make the world safer
over cleartext, and we won't have to make recommendations when both
are available.

Thanks.
brian m. carlson April 10, 2021, 1:39 a.m. UTC | #7
On 2021-04-09 at 21:18:12, Drew DeVault wrote:
> The present options are misleading; "ssl" enables generic, "modern" SSL
> support, which could use either SSL or TLS; and "tls" enables the
> SMTP-specific (and deprecated) STARTTLS protocol.
> 
> This changes the canonical config options to "ssl/tls" and "starttls",
> updates the docs to explain the options in more detail, and updates
> git-send-email to accept either form.
> ---
>  Documentation/git-send-email.txt | 11 ++++++++---
>  git-send-email.perl              |  4 ++--
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 93708aefea..3597935e41 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -168,9 +168,14 @@ Sending
>  	unspecified, choosing the envelope sender is left to your MTA.
>  
>  --smtp-encryption=<encryption>::
> -	Specify the encryption to use, either 'ssl' or 'tls'.  Any other
> -	value reverts to plain SMTP.  Default is the value of
> -	`sendemail.smtpEncryption`.
> +	Specify the encryption to use, either 'ssl/tls' or 'starttls', whichever
> +	is recommended by your email service provider.  SSL/TLS is typically
> +	used on port 465 and is preferred if available.  STARTTLS is typically
> +	used on port 25 or 587. Any other value reverts to plain SMTP.  The
> +	default is the value of `sendemail.smtpEncryption`.
> ++
> +For legacy reasons, 'ssl' is accepted for 'ssl/tls' and 'tls' is accepted for
> +'starttls'.

I definitely approve of describing the two options.  Even just saying
that one option is tunneled and one is actually STARTTLS would be an
improvement here without the additional options.  Apparently I managed
to figure it out, but I'm not sure if that's because I use STARTTLS or
because I would logically prefer the more modern TLS over SSL just by
looking at the names.

Since I agree that "ssl/tls" may be a bit confusing, maybe we could call
that option "wrapped" or "tunneled"?  Other names are possible, of
course.
Drew DeVault April 10, 2021, 1:42 a.m. UTC | #8
On Fri Apr 9, 2021 at 9:39 PM EDT, brian m. carlson wrote:
> Since I agree that "ssl/tls" may be a bit confusing, maybe we could call
> that option "wrapped" or "tunneled"? Other names are possible, of
> course.

I would prefer to name the options after the terms we can expect the
user to find in their mail service provider's documentation, hence
SSL/TLS and STARTTLS. Though I can see the confusion in including the
slash, I'll figure something else out.
Junio C Hamano April 10, 2021, 4:43 p.m. UTC | #9
"Drew DeVault" <sir@cmpwn.com> writes:

> On Fri Apr 9, 2021 at 9:39 PM EDT, brian m. carlson wrote:
>> Since I agree that "ssl/tls" may be a bit confusing, maybe we could call
>> that option "wrapped" or "tunneled"? Other names are possible, of
>> course.
>
> I would prefer to name the options after the terms we can expect the
> user to find in their mail service provider's documentation, hence
> SSL/TLS and STARTTLS. Though I can see the confusion in including the
> slash, I'll figure something else out.

OK.  

 * My e-mail provider [*] seems to label these two as SSL/TLS
   (sometimes just TLS) and STARTTLS, but that is aligning how the
   popular client programs call these two methods, so it may not be
   a good datapoint.

 * GMail help page [*] seems to use 'SSL' vs 'STARTTLS' (they seem
   to support both).

 * Outlook.live.com/ help page [*] says they want you to use
   'STARTTLS' for SMTP, but they use 'SSL/TLS' to describe their
   IMAP and POP offerings.

With the above limited samples, I agree that the choices between
'SSL/TLS' and 'STARTTLS' would appear familiar to our end-users.



[Reference]

* https://helpspot.pobox.com/index.php?pg=kb.page&id=118
  https://helpspot.pobox.com/index.php?pg=kb.page&id=125
  https://helpspot.pobox.com/index.php?pg=kb.page&id=399

* https://support.google.com/mail/answer/7126229?hl=en

* https://support.microsoft.com/en-us/office/pop-imap-and-stmp-settings-8361e398-8af4-4e97-b147-6c6c4ac95353
Bagas Sanjaya April 11, 2021, 5:48 a.m. UTC | #10
On 10/04/21 07.52, Junio C Hamano wrote:
> Isn't SMTPS (running SMTP over SSL encrypted connection) the one
> that was once deprecated until it got resurrected)?
> 
> STARTTLS is not all that SMTP specific---POP and IMAP can also start
> in cleartext and upgrade with STARTTLS the same way, no?

Wikipedia entry on Opportunistic TLS [1] said that STARTTLS is not specific
to SMTP, but also to various protocols:
> The STARTTLS command for IMAP and POP3 is defined in RFC 2595, for SMTP in RFC 3207, for XMPP in RFC 6120 and for NNTP in RFC 4642. For IRC, the IRCv3 Working Group has defined the STARTTLS extension. FTP uses the command "AUTH TLS" defined in RFC 4217 and LDAP defines a protocol extension OID in RFC 2830. HTTP uses upgrade header. 

[1]: https://en.wikipedia.org/wiki/Opportunistic_TLS
diff mbox series

Patch

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 93708aefea..3597935e41 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -168,9 +168,14 @@  Sending
 	unspecified, choosing the envelope sender is left to your MTA.
 
 --smtp-encryption=<encryption>::
-	Specify the encryption to use, either 'ssl' or 'tls'.  Any other
-	value reverts to plain SMTP.  Default is the value of
-	`sendemail.smtpEncryption`.
+	Specify the encryption to use, either 'ssl/tls' or 'starttls', whichever
+	is recommended by your email service provider.  SSL/TLS is typically
+	used on port 465 and is preferred if available.  STARTTLS is typically
+	used on port 25 or 587. Any other value reverts to plain SMTP.  The
+	default is the value of `sendemail.smtpEncryption`.
++
+For legacy reasons, 'ssl' is accepted for 'ssl/tls' and 'tls' is accepted for
+'starttls'.
 
 --smtp-domain=<FQDN>::
 	Specifies the Fully Qualified Domain Name (FQDN) used in the
diff --git a/git-send-email.perl b/git-send-email.perl
index f5bbf1647e..34fdf587bd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1503,7 +1503,7 @@  sub send_message {
 		my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34");
 		$smtp_domain ||= maildomain();
 
-		if ($smtp_encryption eq 'ssl') {
+		if ($smtp_encryption eq 'ssl' || $smtp_encryption eq 'ssl/tls') {
 			$smtp_server_port ||= 465; # ssmtp
 			require IO::Socket::SSL;
 
@@ -1538,7 +1538,7 @@  sub send_message {
 						 Hello => $smtp_domain,
 						 Debug => $debug_net_smtp,
 						 Port => $smtp_server_port);
-			if ($smtp_encryption eq 'tls' && $smtp) {
+			if (($smtp_encryption eq 'tls' || $smtp_encryption eq 'starttls') && $smtp) {
 				if ($use_net_smtp_ssl) {
 					$smtp->command('STARTTLS');
 					$smtp->response();