git » chasquid » commit 13ee3ba

courier: Use the hostname in SMTP HELO

author Alberto Bertogli
2020-05-13 18:52:08 UTC
committer Alberto Bertogli
2020-05-13 19:27:17 UTC
parent 7b0703eaa0582f4af7a127c756cded8405cf59a8
courier: Use the hostname in SMTP HELO

The SMTP courier, which handles outgoing connections, uses the domain of
the envelope's from as the domain in the HELO/EHLO greeting.

This works fine in practice, but ideally the domain used in the greeting
should match the reverse DNS record. This used to be more relevant but
nowadays it is not really enforced; however, it sometimes comes up in
self checks, and might cause some confusion when troubleshooting.

So this patch makes it use the configured hostname instead, which is
under the users' control and more likely to be compliant. It also
simplifies the code.

The documentation of the hostname configuration option is also updated
to mention this behaviour.

Thanks to Jonas Seydel (thor77) for bringing this up.
chasquid.go +5 -1
docs/man/chasquid.conf.5 +5 -3
docs/man/chasquid.conf.5.pod +3 -1
etc/chasquid/chasquid.conf +3 -1
internal/config/config.pb.go +5 -1
internal/config/config.proto +5 -1
internal/courier/smtp.go +5 -17
internal/courier/smtp_test.go +6 -6 
diff --git a/chasquid.go b/chasquid.go
index 3335fe6..39789c9 100644
--- a/chasquid.go
+++ b/chasquid.go
@@ -160,7 +160,11 @@ func main() {
 		Args:    conf.MailDeliveryAgentArgs,
 		Timeout: 30 * time.Second,
 	}
-	remoteC := &courier.SMTP{Dinfo: dinfo, STSCache: stsCache}
+	remoteC := &courier.SMTP{
+		HelloDomain: conf.Hostname,
+		Dinfo:       dinfo,
+		STSCache:    stsCache,
+	}
 	s.InitQueue(conf.DataDir+"/queue", localC, remoteC)
 
 	// Load the addresses and listeners.
diff --git a/docs/man/chasquid.conf.5 b/docs/man/chasquid.conf.5
index 5a93aac..8da5861 100644
--- a/docs/man/chasquid.conf.5
+++ b/docs/man/chasquid.conf.5
@@ -1,4 +1,4 @@
-.\" Automatically generated by Pod::Man 4.10 (Pod::Simple 3.35)
+.\" Automatically generated by Pod::Man 4.11 (Pod::Simple 3.35)
 .\"
 .\" Standard preamble:
 .\" ========================================================================
@@ -133,7 +133,7 @@
 .\" ========================================================================
 .\"
 .IX Title "chasquid.conf 5"
-.TH chasquid.conf 5 "2019-07-15" "" ""
+.TH chasquid.conf 5 "2020-05-13" "" ""
 .\" For nroff, turn off justification.  Always turn off hyphenation; it makes
 .\" way too many mistakes in technical documents.
 .if n .ad l
@@ -157,7 +157,9 @@ Some values might be repeated, for example the listening addresses.
 .IP "\fBhostname\fR (string):" 8
 .IX Item "hostname (string):"
 Default hostname to use when saying hello. This is used to say hello to
-clients, for aesthetic purposes. Default: the system's hostname.
+clients (for aesthetic purposes), and as the \s-1HELO/EHLO\s0 domain on outgoing \s-1SMTP\s0
+connections (so ideally it would resolve back to the server, but it isn't a
+big deal if it doesn't). Default: the system's hostname.
 .IP "\fBmax_data_size_mb\fR (int):" 8
 .IX Item "max_data_size_mb (int):"
 Maximum email size, in megabytes. Default: 50.
diff --git a/docs/man/chasquid.conf.5.pod b/docs/man/chasquid.conf.5.pod
index 78b0e8e..2bd2005 100644
--- a/docs/man/chasquid.conf.5.pod
+++ b/docs/man/chasquid.conf.5.pod
@@ -25,7 +25,9 @@ Some values might be repeated, for example the listening addresses.
 =item B<hostname> (string):
 
 Default hostname to use when saying hello. This is used to say hello to
-clients, for aesthetic purposes. Default: the system's hostname.
+clients (for aesthetic purposes), and as the HELO/EHLO domain on outgoing SMTP
+connections (so ideally it would resolve back to the server, but it isn't a
+big deal if it doesn't). Default: the system's hostname.
 
 =item B<max_data_size_mb> (int):
 
diff --git a/etc/chasquid/chasquid.conf b/etc/chasquid/chasquid.conf
index 19e1b2d..5cc23fc 100644
--- a/etc/chasquid/chasquid.conf
+++ b/etc/chasquid/chasquid.conf
@@ -1,6 +1,8 @@
 
 # Default hostname to use when saying hello.
-# This is used to say hello to clients, for aesthetic purposes.
+# This is used to say hello to clients (for aesthetic purposes), and as the
+# HELO/EHLO domain on outgoing SMTP connections (so ideally it would resolve
+# back to the server, but it isn't a big deal if it doesn't).
 # Default: the system's hostname.
 #hostname: "mx.example.com"
 
diff --git a/internal/config/config.pb.go b/internal/config/config.pb.go
index a48a21f..beef41d 100644
--- a/internal/config/config.pb.go
+++ b/internal/config/config.pb.go
@@ -22,7 +22,11 @@ const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package
 
 type Config struct {
 	// Default hostname to use when saying hello.
-	// This is used to say hello to clients, for aesthetic purposes.
+	// This is used:
+	// 1) To say hello to clients, for aesthetic purposes.
+	// 2) As the HELO/EHLO domain on outgoing SMTP connections, so ideally
+	//    it would resolve back to the server. In practice, it's not a big
+	//    deal if it isn't, but it makes troubleshooting easier.
 	// Default: the system's hostname.
 	Hostname string `protobuf:"bytes,1,opt,name=hostname,proto3" json:"hostname,omitempty"`
 	// Maximum email size, in megabytes.
diff --git a/internal/config/config.proto b/internal/config/config.proto
index 2d0f280..1d1dea7 100644
--- a/internal/config/config.proto
+++ b/internal/config/config.proto
@@ -3,7 +3,11 @@ syntax = "proto3";
 
 message Config {
 	// Default hostname to use when saying hello.
-	// This is used to say hello to clients, for aesthetic purposes.
+	// This is used:
+	// 1) To say hello to clients, for aesthetic purposes.
+	// 2) As the HELO/EHLO domain on outgoing SMTP connections, so ideally
+	//    it would resolve back to the server. In practice, it's not a big
+	//    deal if it isn't, but it makes troubleshooting easier.
 	// Default: the system's hostname.
 	string hostname = 1;
 
diff --git a/internal/courier/smtp.go b/internal/courier/smtp.go
index 868e2d9..f09e100 100644
--- a/internal/courier/smtp.go
+++ b/internal/courier/smtp.go
@@ -6,7 +6,6 @@ import (
 	"expvar"
 	"flag"
 	"net"
-	"os"
 	"time"
 
 	"golang.org/x/net/idna"
@@ -45,8 +44,9 @@ var (
 
 // SMTP delivers remote mail via outgoing SMTP.
 type SMTP struct {
-	Dinfo    *domaininfo.DB
-	STSCache *sts.PolicyCache
+	HelloDomain string
+	Dinfo       *domaininfo.DB
+	STSCache    *sts.PolicyCache
 }
 
 // Deliver an email. On failures, returns an error, and whether or not it is
@@ -77,17 +77,6 @@ func (s *SMTP) Deliver(from string, to string, data []byte) (error, bool) {
 		return a.tr.Errorf("Could not find mail server: %v", err), perm
 	}
 
-	// Issue an EHLO with a valid domain; otherwise, some servers like postfix
-	// will complain.
-	a.helloDomain, err = idna.ToASCII(envelope.DomainOf(from))
-	if err != nil {
-		return a.tr.Errorf("Sender domain not IDNA compliant: %v", err), true
-	}
-	if a.helloDomain == "" {
-		// This can happen when sending bounces. Last resort.
-		a.helloDomain, _ = os.Hostname()
-	}
-
 	a.stsPolicy = s.fetchSTSPolicy(a.tr, a.toDomain)
 
 	for _, mx := range mxs {
@@ -118,8 +107,7 @@ type attempt struct {
 	to   string
 	data []byte
 
-	toDomain    string
-	helloDomain string
+	toDomain string
 
 	stsPolicy *sts.Policy
 
@@ -145,7 +133,7 @@ retry:
 		return a.tr.Errorf("Error creating client: %v", err), false
 	}
 
-	if err = c.Hello(a.helloDomain); err != nil {
+	if err = c.Hello(a.courier.HelloDomain); err != nil {
 		return a.tr.Errorf("Error saying hello: %v", err), false
 	}
 
diff --git a/internal/courier/smtp_test.go b/internal/courier/smtp_test.go
index 8af462b..0455023 100644
--- a/internal/courier/smtp_test.go
+++ b/internal/courier/smtp_test.go
@@ -36,7 +36,7 @@ func newSMTP(t *testing.T) (*SMTP, string) {
 		t.Fatal(err)
 	}
 
-	return &SMTP{dinfo, nil}, dir
+	return &SMTP{"hello", dinfo, nil}, dir
 }
 
 // Fake server, to test SMTP out.
@@ -94,7 +94,7 @@ func TestSMTP(t *testing.T) {
 
 	responses := map[string]string{
 		"_welcome":          "220 welcome\n",
-		"EHLO me":           "250 ehlo ok\n",
+		"EHLO hello":        "250 ehlo ok\n",
 		"MAIL FROM:<me@me>": "250 mail ok\n",
 		"RCPT TO:<to@to>":   "250 rcpt ok\n",
 		"DATA":              "354 send data\n",
@@ -140,14 +140,14 @@ func TestSMTPErrors(t *testing.T) {
 		// MAIL FROM not allowed.
 		{
 			"_welcome":          "220 mail from not allowed\n",
-			"EHLO me":           "250 ehlo ok\n",
+			"EHLO hello":        "250 ehlo ok\n",
 			"MAIL FROM:<me@me>": "501 mail error\n",
 		},
 
 		// RCPT TO not allowed.
 		{
 			"_welcome":          "220 rcpt to not allowed\n",
-			"EHLO me":           "250 ehlo ok\n",
+			"EHLO hello":        "250 ehlo ok\n",
 			"MAIL FROM:<me@me>": "250 mail ok\n",
 			"RCPT TO:<to@to>":   "501 rcpt error\n",
 		},
@@ -155,7 +155,7 @@ func TestSMTPErrors(t *testing.T) {
 		// DATA error.
 		{
 			"_welcome":          "220 data error\n",
-			"EHLO me":           "250 ehlo ok\n",
+			"EHLO hello":        "250 ehlo ok\n",
 			"MAIL FROM:<me@me>": "250 mail ok\n",
 			"RCPT TO:<to@to>":   "250 rcpt ok\n",
 			"DATA":              "554 data error\n",
@@ -164,7 +164,7 @@ func TestSMTPErrors(t *testing.T) {
 		// DATA response error.
 		{
 			"_welcome":          "220 data response error\n",
-			"EHLO me":           "250 ehlo ok\n",
+			"EHLO hello":        "250 ehlo ok\n",
 			"MAIL FROM:<me@me>": "250 mail ok\n",
 			"RCPT TO:<to@to>":   "250 rcpt ok\n",
 			"DATA":              "354 send data\n",