Re: [PATCH] send-email: clarify SMTP encryption settings

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 10, 2021, 00:52 UTC
Message-ID: <xmqqlf9rklkb.fsf@gitster.g>
In-Reply-To: <20210409211812.3869-1-sir@cmpwn.com>

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, ...

Show 20 quoted lines

>  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.