| Message ID | 20210411125431.28971-4-sir@cmpwn.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-send-email: improve SSL configuration | expand |
On Sun, Apr 11 2021, Drew DeVault wrote: > The name 'tls' is misleading. The 'ssl' option enables a generic > "modern" encryption stack which might very well use TLS; but the 'tls' > option enables STARTTLS support, which works entirely differently. > > This renames the canonical option to 'starttls', to make this > distinction more obvious, and adds 'tls' as an alias for starttls, to > avoid breaking config files. > > Signed-off-by: Drew DeVault <sir@cmpwn.com> > --- > Documentation/git-send-email.txt | 6 ++++-- > git-send-email.perl | 10 +++++++--- > 2 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt > index 520b355e50..f8cea9e1f9 100644 > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -168,12 +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'. 'ssl' enables > - generic SSL/TLS support and is typically used on port 465. 'tls' > + Specify the encryption to use, either 'ssl' or 'starttls'. 'ssl' enables > + generic SSL/TLS support and is typically used on port 465. 'starttls' > enables in-band STARTTLS support and is typically used on port 25 or > 587. Use whichever option is recommended by your mail provider. Leave > empty to disable encryption and use plain SMTP. Default is the value of > `sendemail.smtpEncryption`. > ++ > +'tls' is an alias for 'starttls' for legacy reasons. > > --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 bda5211f0d..3f125bc2b8 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -495,8 +495,12 @@ sub read_config { > > # 'default' encryption is none -- this only prevents a warning > $smtp_encryption = '' unless (defined $smtp_encryption); > -if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "tls") { > - die __("Invalid smtp_encryption configuration: expected 'ssl', 'tls', or nothing.\n"); > +if ($smtp_encryption eq "tls") { > + # "tls" is an alias for starttls for legacy reasons > + $smtp_encryption = "starttls"; > +}; Needless trailing ";". This and the preceding patch would be more readable if it was re-arranged in some way as to not rewrite the newly introduced lines between 2 and 3, maybe: { my $tls_name = "tls"; if (....) } Then you'd only need to change "tls" to "starttls" there. > +if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "starttls") { > + die __("Invalid smtp_encryption configuration: expected 'ssl', 'starttls', or nothing.\n"); > } > > # Set CC suppressions > @@ -1541,7 +1545,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 'starttls' && $smtp) { And this could use the same variable.
On Sun Apr 11, 2021 at 10:17 AM EDT, Ævar Arnfjörð Bjarmason wrote: > > # 'default' encryption is none -- this only prevents a warning > > $smtp_encryption = '' unless (defined $smtp_encryption); > > -if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "tls") { > > - die __("Invalid smtp_encryption configuration: expected 'ssl', 'tls', or nothing.\n"); > > +if ($smtp_encryption eq "tls") { > > + # "tls" is an alias for starttls for legacy reasons > > + $smtp_encryption = "starttls"; > > +}; > > Needless trailing ";". > > This and the preceding patch would be more readable if it was > re-arranged in some way as to not rewrite the newly introduced lines > between 2 and 3, maybe: > > { > my $tls_name = "tls"; > if (....) > } I disagree that this would be an improvement. It would make the patches a bit more readlable on their own, but the resulting code would introduce this bizzare variable which doesn't make sense out of context.
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 520b355e50..f8cea9e1f9 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -168,12 +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'. 'ssl' enables - generic SSL/TLS support and is typically used on port 465. 'tls' + Specify the encryption to use, either 'ssl' or 'starttls'. 'ssl' enables + generic SSL/TLS support and is typically used on port 465. 'starttls' enables in-band STARTTLS support and is typically used on port 25 or 587. Use whichever option is recommended by your mail provider. Leave empty to disable encryption and use plain SMTP. Default is the value of `sendemail.smtpEncryption`. ++ +'tls' is an alias for 'starttls' for legacy reasons. --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 bda5211f0d..3f125bc2b8 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -495,8 +495,12 @@ sub read_config { # 'default' encryption is none -- this only prevents a warning $smtp_encryption = '' unless (defined $smtp_encryption); -if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "tls") { - die __("Invalid smtp_encryption configuration: expected 'ssl', 'tls', or nothing.\n"); +if ($smtp_encryption eq "tls") { + # "tls" is an alias for starttls for legacy reasons + $smtp_encryption = "starttls"; +}; +if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "starttls") { + die __("Invalid smtp_encryption configuration: expected 'ssl', 'starttls', or nothing.\n"); } # Set CC suppressions @@ -1541,7 +1545,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 'starttls' && $smtp) { if ($use_net_smtp_ssl) { $smtp->command('STARTTLS'); $smtp->response();
The name 'tls' is misleading. The 'ssl' option enables a generic "modern" encryption stack which might very well use TLS; but the 'tls' option enables STARTTLS support, which works entirely differently. This renames the canonical option to 'starttls', to make this distinction more obvious, and adds 'tls' as an alias for starttls, to avoid breaking config files. Signed-off-by: Drew DeVault <sir@cmpwn.com> --- Documentation/git-send-email.txt | 6 ++++-- git-send-email.perl | 10 +++++++--- 2 files changed, 11 insertions(+), 5 deletions(-)