~sircmpwn/aerc

do not check for STARTTLS support for localhost v1 PROPOSED

Paul Vixie
Paul Vixie: 2
 do not check for STARTTLS support for localhost
 detect loopback by ip address not by string representation of server name

 2 files changed, 42 insertions(+), 26 deletions(-)
I'd recommend the following approach:

- do a `git rebase -i` and squash the commits
- submit a v2 by using `git send-email --annotate -v2 HEAD^`
I am sorry if this seems excessive, but it would be best if we do not
include temporary work in the history and just combine the correction
with the original – imperfect – solution.


NACK. The use of TLS is required for all network connections, localhost
or not. For local mail you should consider using maildir, or install
your own certificate.


Okay, basically, the idea is that we don't want anyone to end up with an
unencrypted connection without explicitly opting into it by typing the
word "insecure" into their config file. This is also important for
avoiding downgrade attacks. I also don't see any reason to make
localhost special, and the complexity introduced in this patch is not
great given the ask.

All of that said, if you want to connect over localhost without
encryption, you should be able to add +insecure to your connection
string. A patch which adds this function to any protocol which does not
already support it would be better than this patch as-proposed.
On Wed Sep 1, 2021 at 6:38 PM CEST, Paul Vixie wrote:
> first, on the source side, i have "imap+insecure://vixie:...@...". i
> do it this way because my maildir is in maildir++ format, which my
> imap server understands, but which aerc does not. would you be willing
> to accept changes that add support for maildir++?
I don't think maildir++ is desirable. Maildir is poorly specified as it
is and we should take a conservative approach here.
Next
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~sircmpwn/aerc/patches/24829/mbox | git am -3
Learn more about email & git
View this thread in the archives

[PATCH 1/2] do not check for STARTTLS support for localhost Export this patch

Paul Vixie
---
 commands/compose/send.go | 53 +++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/commands/compose/send.go b/commands/compose/send.go
index 849182d..34e9fd9 100644
--- a/commands/compose/send.go
+++ b/commands/compose/send.go
@@ -360,7 +360,7 @@ func newSmtpSender(ctx sendCtx) (io.WriteCloser, error) {
func connectSmtp(starttls bool, host string) (*smtp.Client, error) {
	serverName := host
	if !strings.ContainsRune(host, ':') {
		host = host + ":587" // Default to submission port
		host += ":587" // Default to submission port
	} else {
		serverName = host[:strings.IndexRune(host, ':')]
	}
@@ -368,27 +368,36 @@ func connectSmtp(starttls bool, host string) (*smtp.Client, error) {
	if err != nil {
		return nil, errors.Wrap(err, "smtp.Dial")
	}
	if sup, _ := conn.Extension("STARTTLS"); sup {
		if !starttls {
			err := errors.New("STARTTLS is supported by this server, " +
				"but not set in accounts.conf. " +
				"Add smtp-starttls=yes")
			conn.Close()
			return nil, err
		}
		if err = conn.StartTLS(&tls.Config{
			ServerName: serverName,
		}); err != nil {
			conn.Close()
			return nil, errors.Wrap(err, "StartTLS")
		}
	} else {
		if starttls {
			err := errors.New("STARTTLS requested, but not supported " +
				"by this SMTP server. Is someone tampering with your " +
				"connection?")
			conn.Close()
			return nil, err
	localhost := serverName == `localhost` ||
		serverName == `127.0.0.1` ||
		serverName == `127.1` ||
		serverName == `::1`
	if !localhost {
		if sup, _ := conn.Extension("STARTTLS"); sup {
			if !starttls {
				err := errors.New("STARTTLS is supported " +
					"by this non-localhost SMTP server, " +
					"but not requested in accounts.conf. " +
					"Add smtp-starttls=yes")
				conn.Close()
				return nil, err
			}
			if err = conn.StartTLS(&tls.Config{
				ServerName: serverName,
			}); err != nil {
				conn.Close()
				return nil, errors.Wrap(err, "StartTLS")
			}
		} else {
			if starttls {
				err := errors.New("STARTTLS requested " +
					"in accounts.conf, but not supported " +
					"by this non-localhost SMTP server. " +
					"Is someone tampering with your " +
					"connection?")
				conn.Close()
				return nil, err
			}
		}
	}
	return conn, nil
-- 
2.32.0

[PATCH 2/2] detect loopback by ip address not by string representation of server name Export this patch

Paul Vixie
---
 commands/compose/send.go | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/commands/compose/send.go b/commands/compose/send.go
index 34e9fd9..d6fb32c 100644
--- a/commands/compose/send.go
+++ b/commands/compose/send.go
@@ -5,6 +5,7 @@ import (
	"crypto/tls"
	"fmt"
	"io"
	"net"
	"net/url"
	"os/exec"
	"strings"
@@ -368,10 +369,16 @@ func connectSmtp(starttls bool, host string) (*smtp.Client, error) {
	if err != nil {
		return nil, errors.Wrap(err, "smtp.Dial")
	}
	localhost := serverName == `localhost` ||
		serverName == `127.0.0.1` ||
		serverName == `127.1` ||
		serverName == `::1`
	// no way to get the chosen ip out of the smtp.conn; repeat DNS request
	var localhost bool
	if ips, err := net.LookupIP(serverName); err == nil {
		for _, ip := range ips {
			if ip.IsLoopback() {
				localhost = true
				break
			}
		}
	}
	if !localhost {
		if sup, _ := conn.Extension("STARTTLS"); sup {
			if !starttls {
-- 
2.32.0
I'd recommend the following approach:

- do a `git rebase -i` and squash the commits
- submit a v2 by using `git send-email --annotate -v2 HEAD^`

I am sorry if this seems excessive, but it would be best if we do not
include temporary work in the history and just combine the correction
with the original – imperfect – solution.