* [PATCH v2 0/3] git-send-email: improve SSL configuration @ 2021-04-11 12:54 Drew DeVault 2021-04-11 12:54 ` [PATCH v2 1/3] git-send-email(1): improve smtp-encryption docs Drew DeVault ` (3 more replies) 0 siblings, 4 replies; 24+ messages in thread From: Drew DeVault @ 2021-04-11 12:54 UTC (permalink / raw) To: git; +Cc: Drew DeVault Following feedback on v1, this splits the changes up into 3 commits, including a new change which makes any value other than 'ssl', 'starttls', or nothing into an error. This also removes 'ssl/tls' in favor of 'ssl' to avoid confusion, and avoids taking a stance on the matter of the deprecation of either approach. Drew DeVault (3): git-send-email(1): improve smtp-encryption docs git-send-email: die on invalid smtp_encryption git-send-email: rename 'tls' to 'starttls' Documentation/git-send-email.txt | 9 +++++++-- git-send-email.perl | 9 ++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 1/3] git-send-email(1): improve smtp-encryption docs 2021-04-11 12:54 [PATCH v2 0/3] git-send-email: improve SSL configuration Drew DeVault @ 2021-04-11 12:54 ` Drew DeVault 2021-04-11 14:11 ` Ævar Arnfjörð Bjarmason 2021-04-11 12:54 ` [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption Drew DeVault ` (2 subsequent siblings) 3 siblings, 1 reply; 24+ messages in thread From: Drew DeVault @ 2021-04-11 12:54 UTC (permalink / raw) To: git; +Cc: Drew DeVault This clarifies the meaning of the 'ssl' and 'tls' values for this option, which respectively enable SSL/TLS, i.e. a standard "modern" SSL approach; and STARTTLS, i.e. opportunistic in-band TLS. Signed-off-by: Drew DeVault <sir@cmpwn.com> --- Documentation/git-send-email.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 93708aefea..c17c3b400a 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -168,8 +168,11 @@ 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 + Specify the encryption to use, either 'ssl' or 'tls'. 'ssl' enables + generic SSL/TLS support and is typically used on port 465. 'tls' + enables in-band STARTTLS support and is typically used on port 25 or + 587. Use whichever option is recommended by your mail provider. Any + other value reverts to plain SMTP. Default is the value of `sendemail.smtpEncryption`. --smtp-domain=<FQDN>:: -- 2.31.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/3] git-send-email(1): improve smtp-encryption docs 2021-04-11 12:54 ` [PATCH v2 1/3] git-send-email(1): improve smtp-encryption docs Drew DeVault @ 2021-04-11 14:11 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 14:11 UTC (permalink / raw) To: Drew DeVault; +Cc: git On Sun, Apr 11 2021, Drew DeVault wrote: Subject nit: 1/3 is "git-send-email(1):", the rest "git-send-email:". I'd suggest just "send-email:", we usually omit "git-" from the subcommand, and don't use man sections to refer to our own software. > This clarifies the meaning of the 'ssl' and 'tls' values for this > option, which respectively enable SSL/TLS, i.e. a standard "modern" SSL > approach; and STARTTLS, i.e. opportunistic in-band TLS. > > Signed-off-by: Drew DeVault <sir@cmpwn.com> > --- > Documentation/git-send-email.txt | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt > index 93708aefea..c17c3b400a 100644 > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -168,8 +168,11 @@ 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 > + Specify the encryption to use, either 'ssl' or 'tls'. 'ssl' enables Starting a sentance with a quoted lower-case word makes for hard reading. Maybe: When set to 'ssl' ... Or something? > + generic SSL/TLS support and is typically used on port 465. 'tls' > + enables in-band STARTTLS support and is typically used on port 25 or > + 587. Use whichever option is recommended by your mail provider. Any > + other value reverts to plain SMTP. Default is the value of > `sendemail.smtpEncryption`. > > --smtp-domain=<FQDN>:: ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-11 12:54 [PATCH v2 0/3] git-send-email: improve SSL configuration Drew DeVault 2021-04-11 12:54 ` [PATCH v2 1/3] git-send-email(1): improve smtp-encryption docs Drew DeVault @ 2021-04-11 12:54 ` Drew DeVault 2021-04-11 14:20 ` Ævar Arnfjörð Bjarmason 2021-04-11 12:54 ` [PATCH v2 3/3] git-send-email: rename 'tls' to 'starttls' Drew DeVault 2021-04-11 14:43 ` [PATCH 0/2] send-email: simplify smtp.{smtpssl,smtpencryption} parsing Ævar Arnfjörð Bjarmason 3 siblings, 1 reply; 24+ messages in thread From: Drew DeVault @ 2021-04-11 12:54 UTC (permalink / raw) To: git; +Cc: Drew DeVault Signed-off-by: Drew DeVault <sir@cmpwn.com> --- Documentation/git-send-email.txt | 4 ++-- git-send-email.perl | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index c17c3b400a..520b355e50 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -171,8 +171,8 @@ Sending Specify the encryption to use, either 'ssl' or 'tls'. 'ssl' enables generic SSL/TLS support and is typically used on port 465. 'tls' enables in-band STARTTLS support and is typically used on port 25 or - 587. Use whichever option is recommended by your mail provider. Any - other value reverts to plain SMTP. Default is the value of + 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`. --smtp-domain=<FQDN>:: diff --git a/git-send-email.perl b/git-send-email.perl index f5bbf1647e..bda5211f0d 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -495,6 +495,9 @@ 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"); +} # Set CC suppressions my(%suppress_cc); -- 2.31.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-11 12:54 ` [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption Drew DeVault @ 2021-04-11 14:20 ` Ævar Arnfjörð Bjarmason 2021-04-11 14:21 ` Drew DeVault 0 siblings, 1 reply; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 14:20 UTC (permalink / raw) To: Drew DeVault; +Cc: git On Sun, Apr 11 2021, Drew DeVault wrote: > Signed-off-by: Drew DeVault <sir@cmpwn.com> > --- > Documentation/git-send-email.txt | 4 ++-- > git-send-email.perl | 3 +++ > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt > index c17c3b400a..520b355e50 100644 > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -171,8 +171,8 @@ Sending > Specify the encryption to use, either 'ssl' or 'tls'. 'ssl' enables > generic SSL/TLS support and is typically used on port 465. 'tls' > enables in-band STARTTLS support and is typically used on port 25 or > - 587. Use whichever option is recommended by your mail provider. Any > - other value reverts to plain SMTP. Default is the value of > + 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`. > > --smtp-domain=<FQDN>:: > diff --git a/git-send-email.perl b/git-send-email.perl > index f5bbf1647e..bda5211f0d 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -495,6 +495,9 @@ 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"); > +} Having not tested this but just eyeballed the code, I'm fairly sure you're adding a logic error here, or is $smtp_encryption guaranteed to be defined at this point? We start with it as undef, then read the config, then the CLI options. If we've got neither it'll still be undef here, no? Thus the string comparison will emit a warning. Maybe I'm overly used to regexen in Perl, but I'd also find this more readable as: $smtp_encryption !~ /^(?:|ssl|tls)$/s Or something like: my @valid_smtp_encryption = ('', qw(ssl tls)); if (!grep { $_ eq $smtp_encryption } @valid_smtp_encryption) { .... } ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-11 14:20 ` Ævar Arnfjörð Bjarmason @ 2021-04-11 14:21 ` Drew DeVault 2021-04-11 14:30 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 24+ messages in thread From: Drew DeVault @ 2021-04-11 14:21 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git On Sun Apr 11, 2021 at 10:20 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"); > > +} > > Having not tested this but just eyeballed the code, I'm fairly sure > you're adding a logic error here, or is $smtp_encryption guaranteed to > be defined at this point? I will admit to being ignorant of much of Perl's semantics, but I had assumed that the line prior to my additions addresses this: $smtp_encryption = '' unless (defined $smtp_encryption); > $smtp_encryption !~ /^(?:|ssl|tls)$/s Yeah, that would probably be better. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-11 14:21 ` Drew DeVault @ 2021-04-11 14:30 ` Ævar Arnfjörð Bjarmason 2021-04-11 15:06 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 14:30 UTC (permalink / raw) To: Drew DeVault; +Cc: git On Sun, Apr 11 2021, Drew DeVault wrote: > On Sun Apr 11, 2021 at 10:20 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"); >> > +} >> >> Having not tested this but just eyeballed the code, I'm fairly sure >> you're adding a logic error here, or is $smtp_encryption guaranteed to >> be defined at this point? > > I will admit to being ignorant of much of Perl's semantics, but I had > assumed that the line prior to my additions addresses this: > > $smtp_encryption = '' unless (defined $smtp_encryption); You're right, I just misread the diff. Nevermind. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-11 14:30 ` Ævar Arnfjörð Bjarmason @ 2021-04-11 15:06 ` Ævar Arnfjörð Bjarmason 2021-04-11 15:18 ` Drew DeVault 0 siblings, 1 reply; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 15:06 UTC (permalink / raw) To: Drew DeVault; +Cc: git On Sun, Apr 11 2021, Ævar Arnfjörð Bjarmason wrote: > On Sun, Apr 11 2021, Drew DeVault wrote: > >> On Sun Apr 11, 2021 at 10:20 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"); >>> > +} >>> >>> Having not tested this but just eyeballed the code, I'm fairly sure >>> you're adding a logic error here, or is $smtp_encryption guaranteed to >>> be defined at this point? >> >> I will admit to being ignorant of much of Perl's semantics, but I had >> assumed that the line prior to my additions addresses this: >> >> $smtp_encryption = '' unless (defined $smtp_encryption); > > You're right, I just misread the diff. Nevermind. So on a second reading. So first, I've been sitting on some fairly extensive send-email patches locally, but have been trying to focus on re-rolling some of my outstanding stuff. But I just sent two patches directly relevant to this series as https://lore.kernel.org/git/cover-0.2-00000000000-20210411T144128Z-avarab@gmail.com/ Something felt a bit wrong about the approach in your series, I wasn't quite sure what initially, but here it is; So, the only reason we have that "encryption is none -- this only prevents a warning" so late in the file (as opposed to setting it to '' when we declare the variable) was because of the smtp.{smtpssl,smtpencryption} interaction, i.e. we relied on it being undef to see if we needed to parse the secondary variable. But with it gone with my small series (it already didn't work) we can get rid of that special case. But, on the specifics of the "felt funny": 1. Your 2/3 changes a long standing existing "any other value = no encryption" to "die on unrecognized". I happen to think this is probably a good idea, but let's be explicit in the commit message, e.g.: We don't think it's a good idea to silently degrade to non-encrypted as we've been promising just because your version doesn't support something, let's die instead. 2. If we're breaking the "any other value" we should not be documenting the "or nothing", the distinction between "" and undef on the Perl-level was just a leaky implementation detail. But let's not conflate that with how we present something to the user. It's not the same to not set a variable v.s. setting it to the empty string. With my 2-part series it's even more trivial to detect that, but even on top of master you just move your check above the "set to empty unless defined". 3. While I'm very much leaning to #1 being a good idea, I'm very much leaning towards introducing this "starttls" alias being a bad idea for the same reason. I.e. let's not create a new 'starttls' if we can avoid it explicitly because we used to have the long-standing "anything unrecognized is empty == no encryption" behavior. A lot of users read documentation for the latest version online, but may have an older version installed. To the extent that anyone cares about the transport security of git-send-email (I'm kind of "meh" on it, but if we're making sendemail.smtpEncryption parsing strict you probably disagree), then such silent downgrading seems worse than just not accepting starttls at all. I.e. to have a new behavior of something like: if (defined $smtp_encryption) { die "we call 'starttls' 'tls' for historical reasons, sorry!" if $smtp_encryption eq 'starttls'; die "unknown mode '$smtp_encryption'" unless $smtp_encryption =~ /^(?:ssl|tls)$/s; } else { $smtp_encryption = ''; } I.e. I get that it's confusing, but isn't it enough to address the TLS v.s. STARTTLS confusion in the docs, as opposed to supporting it in the config format, which as noted will silently downgrade on older versions? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-11 15:06 ` Ævar Arnfjörð Bjarmason @ 2021-04-11 15:18 ` Drew DeVault 2021-04-11 19:56 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 24+ messages in thread From: Drew DeVault @ 2021-04-11 15:18 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git On Sun Apr 11, 2021 at 11:06 AM EDT, Ævar Arnfjörð Bjarmason wrote: > 3. While I'm very much leaning to #1 being a good idea, I'm very much > leaning towards introducing this "starttls" alias being a bad idea > for the same reason. > > i.e. let's not create a new 'starttls' if we can avoid it explicitly > because we used to have the long-standing "anything unrecognized is > empty == no encryption" behavior. > > A lot of users read documentation for the latest version online, but > may have an older version installed. I feel quite strongly that the options here are a grave failure of usability, and that it needs to be corrected. I help people troubleshoot git send-email problems quite often, and this is a recurring error. However, you make a good point in that someone might see some online documentation which does not match their git version and end up with a surprisingly unencrypted connection. As a compromise, let's consider making this a gradual change. We can start by clarifying the docs and forbiding the use of any value other than 'ssl' or 'tls'. If an unknown value is set, the user is not getting the encryption they expected anyway, and this should cause an error. Then we can leave the issue aside for some agreed upon period of time to allow the change to proliferate in the ecosystem, and then revisit this at some point in the future to rename the options to make more sense. Does this seem like a reasonable compromise? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-11 15:18 ` Drew DeVault @ 2021-04-11 19:56 ` Ævar Arnfjörð Bjarmason 2021-04-12 12:33 ` Drew DeVault 0 siblings, 1 reply; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 19:56 UTC (permalink / raw) To: Drew DeVault; +Cc: git On Sun, Apr 11 2021, Drew DeVault wrote: > On Sun Apr 11, 2021 at 11:06 AM EDT, Ævar Arnfjörð Bjarmason wrote: >> 3. While I'm very much leaning to #1 being a good idea, I'm very much >> leaning towards introducing this "starttls" alias being a bad idea >> for the same reason. >> >> i.e. let's not create a new 'starttls' if we can avoid it explicitly >> because we used to have the long-standing "anything unrecognized is >> empty == no encryption" behavior. >> >> A lot of users read documentation for the latest version online, but >> may have an older version installed. > > I feel quite strongly that the options here are a grave failure of > usability, and that it needs to be corrected. I help people troubleshoot > git send-email problems quite often, and this is a recurring error. > However, you make a good point in that someone might see some online > documentation which does not match their git version and end up with a > surprisingly unencrypted connection. > > As a compromise, let's consider making this a gradual change. We can > start by clarifying the docs and forbiding the use of any value other > than 'ssl' or 'tls'. If an unknown value is set, the user is not getting > the encryption they expected anyway, and this should cause an error. > > Then we can leave the issue aside for some agreed upon period of time to > allow the change to proliferate in the ecosystem, and then revisit this > at some point in the future to rename the options to make more sense. > > Does this seem like a reasonable compromise? I suggest we don't compromise and just go with whatever you're OK with :) I really don't care enough about #1 and #3 in my E-Mail to in any way push for it, sorry if it came off that way. I just wanted to check your assumptions when reviewing the series. I do think that it would make sense to more prominently note something to the effect of "this was documented to do X all along, now we do Y, but that's OK because ABC", and to note why the new starttls = plaintext on older versions is OK, maybe it's just fine. I really don't know. Isn't it pretty common in any case that SMTP servers in the wild just refuse plaintext these days when dealing with auth'd connections? I don't know. I do think it makes sense to fixup for my suggested #2, i.e. not leaking the internal detail of the "empty string". ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-11 19:56 ` Ævar Arnfjörð Bjarmason @ 2021-04-12 12:33 ` Drew DeVault 2021-04-12 13:16 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 24+ messages in thread From: Drew DeVault @ 2021-04-12 12:33 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git On Sun Apr 11, 2021 at 3:56 PM EDT, Ævar Arnfjörð Bjarmason wrote: > I suggest we don't compromise and just go with whatever you're OK with :) Well, if you're giving me an opportunity to not drag this out into a multi-phase rollout, then I'll take it :) Another option is to forbid an unknown value (which is almost certainly (1) wrong and (2) causing users to unexpectedly use plaintext when they expected encryption), file a CVE, and pitch it as a security fix - then we can expect a reasonably quick rollout of the change to the ecosystem at large. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-12 12:33 ` Drew DeVault @ 2021-04-12 13:16 ` Ævar Arnfjörð Bjarmason 2021-04-13 12:12 ` Drew DeVault 0 siblings, 1 reply; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-12 13:16 UTC (permalink / raw) To: Drew DeVault; +Cc: git On Mon, Apr 12 2021, Drew DeVault wrote: > On Sun Apr 11, 2021 at 3:56 PM EDT, Ævar Arnfjörð Bjarmason wrote: >> I suggest we don't compromise and just go with whatever you're OK with :) > > Well, if you're giving me an opportunity to not drag this out into a > multi-phase rollout, then I'll take it :) Just to be clear even if I was insisting on that I'm still just one guy on the ML reviewing your patch. As a first approximation the opinion of regular contributors counts for more when the topic is some tricky interaction of code they wrote/are familiar with. In this case we're just discussing the general interaction of security, optional switches, software versioning and how SMTP servers in the wild work. I'd think someone who e.g. needs to regularly deal with SMTP servers in the wild would have a much better idea of those trade-offs than someone (like me) who happens to have some existing patches in git.git to git-send-email.perl. > Another option is to forbid an unknown value (which is almost certainly > (1) wrong and (2) causing users to unexpectedly use plaintext when they > expected encryption), file a CVE, and pitch it as a security fix - then > we can expect a reasonably quick rollout of the change to the ecosystem > at large. I think anyone would agree that in retrospect "unknown is plaintext" for the "what encryption do you want" option is at best a something approaching a shotgun to your foot of a UI pattern. But I think it falls far short of a CVE. We *do* prominently document it, a potential CVE would be if we had silent degration to plaintext (well, in a mode whose inherent workings aren't to be vulnerable to that attack, as STARTTLS is...). FWIW since my upthread <87zgy4egtp.fsf@evledraar.gmail.com> I tried sending mail through GMail's plain-text smtp gateway as an authenticated user. Testing with: nc smtp.gmail.com 25 openssl s_client -connect smtp.gmail.com:465 It will emit a 530 if you try to AUTH in plain-text (telling you to use STARTTLS), it will also only say "AUTH" in the EHLO response to the latter. And indeed Net::SMTP picks up on this, and doesn't even send your user/password: https://metacpan.org/release/libnet/source/lib/Net/SMTP.pm#L169 So this hypothetical degradation of the connection and sending auth over plain-text I suggested in upthread #3 seems to mostly/entirely be a non-issue as far as e.g. accidentally sending your password on some open WiFi network goes due to a local misconfiguration. As long as the SMTP server is functional enough to say it doesn't support AUTH on plain-text you'll be OK. I'm assuming that these days with the push for "SSL everywhere" most/all big providers/MTAs have moved away from supporing plain-text auth by default. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-12 13:16 ` Ævar Arnfjörð Bjarmason @ 2021-04-13 12:12 ` Drew DeVault 2021-04-13 14:22 ` Ævar Arnfjörð Bjarmason 2021-04-13 21:39 ` Junio C Hamano 0 siblings, 2 replies; 24+ messages in thread From: Drew DeVault @ 2021-04-13 12:12 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git Can I get one of the maintainers to chime in on this thread and explain, in their opinion, what this patchset needs before it is acceptable? I'm not sure where I should go from this discussion. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-13 12:12 ` Drew DeVault @ 2021-04-13 14:22 ` Ævar Arnfjörð Bjarmason 2021-04-13 21:39 ` Junio C Hamano 1 sibling, 0 replies; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-13 14:22 UTC (permalink / raw) To: Drew DeVault; +Cc: git On Tue, Apr 13 2021, Drew DeVault wrote: > Can I get one of the maintainers to chime in on this thread and explain, > in their opinion, what this patchset needs before it is acceptable? I'm > not sure where I should go from this discussion. Git just has the one maintainer, Junio C Hamano. Ultimately getting your patch in is up to his whimsy. Maybe he'll reply, but in an attempt to save him some time (which I understand I'm taking too much of these days): Getting your patch in is ultimately up to you. So you submitted a patch, got some feedback/review. Through some combination of this thread (mainly <875z0sg8t9.fsf@evledraar.gmail.com> and <87o8ejej8m.fsf@evledraar.gmail.com>) I suggested making some changes in a v3. I.e. cleaning up some of the semantics (config docs/handling leaking implementation details) and improving the commit message(s) to summarize the trade-offs, why this approach is safe/isn't safe/why it's OK in the cases it's not etc. So, whatever Junio or anyone else thinks of my opinion I think it's fair to say that at this point he's most likely to skim this thread and see that there's some outstanding feedback that hasn't been addressed. "Addressed" means either re-rolling into a v3, or deciding that nothing (or only part of the feedback) needs to be changed and/or addressed. Both/some of those/that are perfectly acceptable approaches, but in either case making things easily digestable to Junio will help your series along. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption 2021-04-13 12:12 ` Drew DeVault 2021-04-13 14:22 ` Ævar Arnfjörð Bjarmason @ 2021-04-13 21:39 ` Junio C Hamano 1 sibling, 0 replies; 24+ messages in thread From: Junio C Hamano @ 2021-04-13 21:39 UTC (permalink / raw) To: Drew DeVault; +Cc: Ævar Arnfjörð Bjarmason, git "Drew DeVault" <sir@cmpwn.com> writes: > Can I get one of the maintainers to chime in on this thread and explain, > in their opinion, what this patchset needs before it is acceptable? I'm > not sure where I should go from this discussion. What you called "compromise" in the upthread didn't even smelled like a compromise but the safest way forward. My reasoning goes (thinking aloud, so that you and Ævar can correct me if I am talking nonsense and discount my input based on it): - If we were designing this from scratch, we would have called the smtps:// tunnelled transport SSL/TLS and in-place upgrade transport STARTTLS, like e-mail providers and client programs do, but unfortunately we didn't. We ended up with 'ssl' vs 'tls'. - We could introduce and advertise STARTTLS as a synonym to 'tls', but then those whose send-email does not understand STARTTLS but read about the new way of spelling would end up having no encryption due to another earlier mistake we made, i.e. an unrecognised option value silently turns into no encryption. - To avoid the above problem, the first phase is not to change the status quo that 'ssl' vs 'tls' are the only two choices. What we do is to make the program error out if we see an unrecognised value given to the option. We release this to the wild, and wait for the current versions of send-email that turns unrecognised words into no-encryption die out. It may take several years, though. - After waiting, we add 'starttls' as a synonym to 'tls'. We may also add 'ssl/tls' as a synonym to 'ssl'. Unfortunately 'tls' alone cannot be repurposed as a synonym for smtps:// without another deprecation dance, and it is not in scope of the transtion. Am I on the same page as you two? ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 3/3] git-send-email: rename 'tls' to 'starttls' 2021-04-11 12:54 [PATCH v2 0/3] git-send-email: improve SSL configuration Drew DeVault 2021-04-11 12:54 ` [PATCH v2 1/3] git-send-email(1): improve smtp-encryption docs Drew DeVault 2021-04-11 12:54 ` [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption Drew DeVault @ 2021-04-11 12:54 ` Drew DeVault 2021-04-11 14:17 ` Ævar Arnfjörð Bjarmason 2021-04-11 14:43 ` [PATCH 0/2] send-email: simplify smtp.{smtpssl,smtpencryption} parsing Ævar Arnfjörð Bjarmason 3 siblings, 1 reply; 24+ messages in thread From: Drew DeVault @ 2021-04-11 12:54 UTC (permalink / raw) To: git; +Cc: Drew DeVault 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"; +}; +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(); -- 2.31.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/3] git-send-email: rename 'tls' to 'starttls' 2021-04-11 12:54 ` [PATCH v2 3/3] git-send-email: rename 'tls' to 'starttls' Drew DeVault @ 2021-04-11 14:17 ` Ævar Arnfjörð Bjarmason 2021-04-11 14:22 ` Drew DeVault 0 siblings, 1 reply; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 14:17 UTC (permalink / raw) To: Drew DeVault; +Cc: git 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/3] git-send-email: rename 'tls' to 'starttls' 2021-04-11 14:17 ` Ævar Arnfjörð Bjarmason @ 2021-04-11 14:22 ` Drew DeVault 0 siblings, 0 replies; 24+ messages in thread From: Drew DeVault @ 2021-04-11 14:22 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/2] send-email: simplify smtp.{smtpssl,smtpencryption} parsing 2021-04-11 12:54 [PATCH v2 0/3] git-send-email: improve SSL configuration Drew DeVault ` (2 preceding siblings ...) 2021-04-11 12:54 ` [PATCH v2 3/3] git-send-email: rename 'tls' to 'starttls' Drew DeVault @ 2021-04-11 14:43 ` Ævar Arnfjörð Bjarmason 2021-04-11 14:43 ` [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" Ævar Arnfjörð Bjarmason 2021-04-11 14:43 ` [PATCH 2/2] send-email: refactor sendemail.smtpencryption config parsing Ævar Arnfjörð Bjarmason 3 siblings, 2 replies; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 14:43 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Drew DeVault, Ævar Arnfjörð Bjarmason As noted in 1/2 I unintentionally broke the deprecated sendemail.smtpssl configuration parsing a while ago. Since nothing actually uses it let's remove it. This doesn't conflict with Drew's http://lore.kernel.org/git/20210411125431.28971-1-sir@cmpwn.com series, but as I'll reply to there knowing that we can do this might simplify some things for it, if it were to be based on top of this. Ævar Arnfjörð Bjarmason (2): send-email: remove non-working support for "sendemail.smtpssl" send-email: refactor sendemail.smtpencryption config parsing Documentation/config/sendemail.txt | 3 --- git-send-email.perl | 13 +------------ 2 files changed, 1 insertion(+), 15 deletions(-) -- 2.31.1.623.g88b15a793d ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" 2021-04-11 14:43 ` [PATCH 0/2] send-email: simplify smtp.{smtpssl,smtpencryption} parsing Ævar Arnfjörð Bjarmason @ 2021-04-11 14:43 ` Ævar Arnfjörð Bjarmason 2021-04-11 19:08 ` Junio C Hamano 2021-04-11 14:43 ` [PATCH 2/2] send-email: refactor sendemail.smtpencryption config parsing Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 14:43 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Drew DeVault, Ævar Arnfjörð Bjarmason Remove the already dead code to support "sendemail.smtssl" by finally removing the dead code supporting the configuration option. In f6bebd121ac (git-send-email: add support for TLS via Net::SMTP::SSL, 2008-06-25) the --smtp-ssl command-line option was documented as deprecated, later in 65180c66186 (List send-email config options in config.txt., 2009-07-22) the "sendemail.smtpssl" configuration option was also documented as such. Then in in 3ff15040e22 (send-email: fix regression in sendemail.identity parsing, 2019-05-17) I unintentionally removed support for it by introducing a bug in read_config(). As can be seen from the diff context we've already returned unless $enc i defined, so it's not possible for us to reach the "elsif" branch here. This code was therefore already dead since Git v2.23.0. So let's just remove it instead of fixing the bug, clearly nobody's cared enough to complain. The --smtp-ssl option is still deprecated, if someone cares they can follow-up and remove that too, but unlike the config option that one could still be in use in the wild. I'm just removing this code that's provably unused already. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Documentation/config/sendemail.txt | 3 --- git-send-email.perl | 6 +----- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt index cbc5af42fdf..50baa5d6bfb 100644 --- a/Documentation/config/sendemail.txt +++ b/Documentation/config/sendemail.txt @@ -8,9 +8,6 @@ sendemail.smtpEncryption:: See linkgit:git-send-email[1] for description. Note that this setting is not subject to the 'identity' mechanism. -sendemail.smtpssl (deprecated):: - Deprecated alias for 'sendemail.smtpEncryption = ssl'. - sendemail.smtpsslcertpath:: Path to ca-certificates (either a directory or a single file). Set it to an empty string to disable certificate verification. diff --git a/git-send-email.perl b/git-send-email.perl index f5bbf1647e3..877c7dd1a21 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -374,11 +374,7 @@ sub read_config { my $enc = Git::config(@repo, $setting); return unless defined $enc; return if $configured->{$setting}++; - if (defined $enc) { - $smtp_encryption = $enc; - } elsif (Git::config_bool(@repo, "$prefix.smtpssl")) { - $smtp_encryption = 'ssl'; - } + $smtp_encryption = $enc; } } -- 2.31.1.623.g88b15a793d ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" 2021-04-11 14:43 ` [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" Ævar Arnfjörð Bjarmason @ 2021-04-11 19:08 ` Junio C Hamano 2021-04-11 19:51 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 24+ messages in thread From: Junio C Hamano @ 2021-04-11 19:08 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Drew DeVault Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > So let's just remove it instead of fixing the bug, clearly nobody's > cared enough to complain. Hmph, is that a safe assumption? They may have just assumed that you did not break it and kept using plaintext without knowing? If we do not give a warning when sending over an unencrypted channel in red flashing letters, that is more likely explanation than nobody caring that we saw no breakage reports, no? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" 2021-04-11 19:08 ` Junio C Hamano @ 2021-04-11 19:51 ` Ævar Arnfjörð Bjarmason 2021-05-01 9:15 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 19:51 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Drew DeVault On Sun, Apr 11 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> So let's just remove it instead of fixing the bug, clearly nobody's >> cared enough to complain. > > Hmph, is that a safe assumption? They may have just assumed that > you did not break it and kept using plaintext without knowing? If > we do not give a warning when sending over an unencrypted channel in > red flashing letters, that is more likely explanation than nobody > caring that we saw no breakage reports, no? Maybe, I think in either case this patch series makes senes. We were already 11 years into a stated deprecation period of that variable, now it's 13. If we're going to e.g. emit some notice about it I think the parsing simplification this series gives us makes sense, we can always add a trivial patch on top to make it die if it sees the old variable. I don't think that's needed, do you? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" 2021-04-11 19:51 ` Ævar Arnfjörð Bjarmason @ 2021-05-01 9:15 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-05-01 9:15 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Drew DeVault On Sun, Apr 11 2021, Ævar Arnfjörð Bjarmason wrote: > On Sun, Apr 11 2021, Junio C Hamano wrote: > >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>> So let's just remove it instead of fixing the bug, clearly nobody's >>> cared enough to complain. >> >> Hmph, is that a safe assumption? They may have just assumed that >> you did not break it and kept using plaintext without knowing? If >> we do not give a warning when sending over an unencrypted channel in >> red flashing letters, that is more likely explanation than nobody >> caring that we saw no breakage reports, no? > > Maybe, I think in either case this patch series makes senes. We were > already 11 years into a stated deprecation period of that variable, now > it's 13. > > If we're going to e.g. emit some notice about it I think the parsing > simplification this series gives us makes sense, we can always add a > trivial patch on top to make it die if it sees the old variable. > > I don't think that's needed, do you? Junio: *Poke*. Was going thorugh my outstanding patches, I still think it makes sense to just pick this up. Especially with the related discussion later about how common in-the-wild service providers would just not support AUTH non-encrypted, so in practice I think it's even less likely that anyone saw silent breakage as a result of this already-deprecated variable being ignored. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/2] send-email: refactor sendemail.smtpencryption config parsing 2021-04-11 14:43 ` [PATCH 0/2] send-email: simplify smtp.{smtpssl,smtpencryption} parsing Ævar Arnfjörð Bjarmason 2021-04-11 14:43 ` [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" Ævar Arnfjörð Bjarmason @ 2021-04-11 14:43 ` Ævar Arnfjörð Bjarmason 1 sibling, 0 replies; 24+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-04-11 14:43 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Drew DeVault, Ævar Arnfjörð Bjarmason With the removal of the support for sendemail.smtpssl in the preceding commit the parsing of sendemail.smtpencryption is no longer special, and can by moved to %config_settings. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- git-send-email.perl | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 877c7dd1a21..da28c6e8b4b 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -268,6 +268,7 @@ sub do_edit { ); my %config_settings = ( + "smtpencryption" => \$smtp_encryption, "smtpserver" => \$smtp_server, "smtpserverport" => \$smtp_server_port, "smtpserveroption" => \@smtp_server_options, @@ -368,14 +369,6 @@ sub read_config { $$target = $v; } } - - if (!defined $smtp_encryption) { - my $setting = "$prefix.smtpencryption"; - my $enc = Git::config(@repo, $setting); - return unless defined $enc; - return if $configured->{$setting}++; - $smtp_encryption = $enc; - } } # sendemail.identity yields to --identity. We must parse this -- 2.31.1.623.g88b15a793d ^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-05-01 9:17 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-11 12:54 [PATCH v2 0/3] git-send-email: improve SSL configuration Drew DeVault 2021-04-11 12:54 ` [PATCH v2 1/3] git-send-email(1): improve smtp-encryption docs Drew DeVault 2021-04-11 14:11 ` Ævar Arnfjörð Bjarmason 2021-04-11 12:54 ` [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption Drew DeVault 2021-04-11 14:20 ` Ævar Arnfjörð Bjarmason 2021-04-11 14:21 ` Drew DeVault 2021-04-11 14:30 ` Ævar Arnfjörð Bjarmason 2021-04-11 15:06 ` Ævar Arnfjörð Bjarmason 2021-04-11 15:18 ` Drew DeVault 2021-04-11 19:56 ` Ævar Arnfjörð Bjarmason 2021-04-12 12:33 ` Drew DeVault 2021-04-12 13:16 ` Ævar Arnfjörð Bjarmason 2021-04-13 12:12 ` Drew DeVault 2021-04-13 14:22 ` Ævar Arnfjörð Bjarmason 2021-04-13 21:39 ` Junio C Hamano 2021-04-11 12:54 ` [PATCH v2 3/3] git-send-email: rename 'tls' to 'starttls' Drew DeVault 2021-04-11 14:17 ` Ævar Arnfjörð Bjarmason 2021-04-11 14:22 ` Drew DeVault 2021-04-11 14:43 ` [PATCH 0/2] send-email: simplify smtp.{smtpssl,smtpencryption} parsing Ævar Arnfjörð Bjarmason 2021-04-11 14:43 ` [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" Ævar Arnfjörð Bjarmason 2021-04-11 19:08 ` Junio C Hamano 2021-04-11 19:51 ` Ævar Arnfjörð Bjarmason 2021-05-01 9:15 ` Ævar Arnfjörð Bjarmason 2021-04-11 14:43 ` [PATCH 2/2] send-email: refactor sendemail.smtpencryption config parsing Ævar Arnfjörð Bjarmason
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).