Repository: inbucket/inbucket
Pull request #375 (2023-07-20 15:44:42)

add reject from origin domain feature
Add a new feature to be able to reject email from specific domains.  
The list of domains to reject is defined in a new environment variable `INBUCKET_SMTP_REJECTORIGINDOMAINS`.
This variable is a coma separated strings list (like `INBUCKET_SMTP_ACCEPTDOMAINS`, `INBUCKET_SMTP_REJECTDOMAINS`, ...).

Signed-off-by: Cyril Dupont <cyd@9bis.com>

jhillyerd: Let me know if there is anything I can do to help with this one, seems like we are pretty close.
cyd01: I'm sorry for not answering. I'm on summer hollydays. I'll be back at work next week.
cyd01: I've tryed to fix the issue with tests compilation (in file `pkg/message/manager_test.go`).  
But now the Github build process does not work (it work on my side with a simple `go build ./cmd/inbucket/` command).  
it seems I need help.

jhillyerd: It looks like you did fix the compilation.  Go doesn't run tests when you compile, but you can run the smtp tests with

`go test ./pkg/server/smtp`

looks like we are getting an unexpected SMTP response code

```
--- FAIL: TestEmptyEnvelope (0.00s)
    writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":17,"message":"Starting SMTP session"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Line received: HELO localhost"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Entering state READY"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Line received: MAIL FROM:<>"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Mail sender is "}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Origin domain is "}
    writer.go:25: {"level":"warn","module":"smtp","remote":"pipe","session":17,"from":"unspecified","error":"Domain part validation failed","message":"Bad address as MAIL arg"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Line received: QUIT"}
    handler_test.go:98: Step 1, sent "MAIL FROM:<>", expected 250, got 501: "Bad origin address syntax"
    writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":18,"message":"Starting SMTP session"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Line received: HELO localhost"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Entering state READY"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Line received: MAIL FROM: <>"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Mail sender is "}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Origin domain is "}
    writer.go:25: {"level":"warn","module":"smtp","remote":"pipe","session":18,"from":"unspecified","error":"Domain part validation failed","message":"Bad address as MAIL arg"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Line received: QUIT"}
    handler_test.go:107: Step 1, sent "MAIL FROM: <>", expected 250, got 501: "Bad origin address syntax"
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Entering state QUIT"}
    writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":18,"message":"Closing connection"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Entering state QUIT"}
    writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":17,"message":"Closing connection"}
--- FAIL: TestReadyStateRejectedDomains (0.00s)
    --- FAIL: TestReadyStateRejectedDomains/MAIL_FROM_john@validdomain.com (0.00s)
        writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":35,"message":"Starting SMTP session"}
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Line received: HELO localhost"}
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Entering state READY"}
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Line received: MAIL FROM john@validdomain.com"}
        writer.go:25: {"level":"warn","module":"smtp","remote":"pipe","session":35,"message":"Bad MAIL argument: \"FROM john@validdomain.com\""}
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Line received: QUIT"}
        handler_test.go:219: Step 1, sent "MAIL FROM john@validdomain.com", expected 250, got 501: "Was expecting MAIL arg syntax of FROM:<address>"
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Entering state QUIT"}
        writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":35,"message":"Closing connection"}
FAIL
FAIL    github.com/inbucket/inbucket/pkg/server/smtp    0.011s
FAIL
```

Patch:
diff --git a/pkg/config/config.go b/pkg/config/config.go
index fe56268..b27a428 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -74,21 +74,22 @@ type Lua struct {
 
 // SMTP contains the SMTP server configuration.
 type SMTP struct {
-	Addr            string        `required:"true" default:"0.0.0.0:2500" desc:"SMTP server IP4 host:port"`
-	Domain          string        `required:"true" default:"inbucket" desc:"HELO domain"`
-	MaxRecipients   int           `required:"true" default:"200" desc:"Maximum RCPT TO per message"`
-	MaxMessageBytes int           `required:"true" default:"10240000" desc:"Maximum message size"`
-	DefaultAccept   bool          `required:"true" default:"true" desc:"Accept all mail by default?"`
-	AcceptDomains   []string      `desc:"Domains to accept mail for"`
-	RejectDomains   []string      `desc:"Domains to reject mail for"`
-	DefaultStore    bool          `required:"true" default:"true" desc:"Store all mail by default?"`
-	StoreDomains    []string      `desc:"Domains to store mail for"`
-	DiscardDomains  []string      `desc:"Domains to discard mail for"`
-	Timeout         time.Duration `required:"true" default:"300s" desc:"Idle network timeout"`
-	TLSEnabled      bool          `default:"false" desc:"Enable STARTTLS option"`
-	TLSPrivKey      string        `default:"cert.key" desc:"X509 Private Key file for TLS Support"`
-	TLSCert         string        `default:"cert.crt" desc:"X509 Public Certificate file for TLS Support"`
-	Debug           bool          `ignored:"true"`
+	Addr                string        `required:"true" default:"0.0.0.0:2500" desc:"SMTP server IP4 host:port"`
+	Domain              string        `required:"true" default:"inbucket" desc:"HELO domain"`
+	MaxRecipients       int           `required:"true" default:"200" desc:"Maximum RCPT TO per message"`
+	MaxMessageBytes     int           `required:"true" default:"10240000" desc:"Maximum message size"`
+	DefaultAccept       bool          `required:"true" default:"true" desc:"Accept all mail by default?"`
+	AcceptDomains       []string      `desc:"Domains to accept mail for"`
+	RejectDomains       []string      `desc:"Domains to reject mail for"`
+	DefaultStore        bool          `required:"true" default:"true" desc:"Store all mail by default?"`
+	StoreDomains        []string      `desc:"Domains to store mail for"`
+	DiscardDomains      []string      `desc:"Domains to discard mail for"`
+	RejectOriginDomains []string      `desc:"Domains to reject mail from"`
+	Timeout             time.Duration `required:"true" default:"300s" desc:"Idle network timeout"`
+	TLSEnabled          bool          `default:"false" desc:"Enable STARTTLS option"`
+	TLSPrivKey          string        `default:"cert.key" desc:"X509 Private Key file for TLS Support"`
+	TLSCert             string        `default:"cert.crt" desc:"X509 Public Certificate file for TLS Support"`
+	Debug               bool          `ignored:"true"`
 }
 
 // POP3 contains the POP3 server configuration.
@@ -128,6 +129,7 @@ func Process() (*Root, error) {
 	stringutil.SliceToLower(c.SMTP.RejectDomains)
 	stringutil.SliceToLower(c.SMTP.StoreDomains)
 	stringutil.SliceToLower(c.SMTP.DiscardDomains)
+	stringutil.SliceToLower(c.SMTP.RejectOriginDomains)
 	return c, err
 }
 
diff --git a/pkg/message/manager.go b/pkg/message/manager.go
index 919c919..1249632 100644
--- a/pkg/message/manager.go
+++ b/pkg/message/manager.go
@@ -19,7 +19,7 @@ import (
 type Manager interface {
 	Deliver(
 		to *policy.Recipient,
-		from string,
+		from *policy.Origin,
 		recipients []*policy.Recipient,
 		prefix string,
 		content []byte,
@@ -43,7 +43,7 @@ type StoreManager struct {
 // Deliver submits a new message to the store.
 func (s *StoreManager) Deliver(
 	to *policy.Recipient,
-	from string,
+	from *policy.Origin,
 	recipients []*policy.Recipient,
 	prefix string,
 	source []byte,
@@ -56,7 +56,8 @@ func (s *StoreManager) Deliver(
 	}
 	fromaddr, err := env.AddressList("From")
 	if err != nil || len(fromaddr) == 0 {
-		fromaddr = []*mail.Address{{Address: from}}
+		fromaddr = make([]*mail.Address, 1)
+		fromaddr[0] = &from.Address
 	}
 	toaddr, err := env.AddressList("To")
 	if err != nil {
diff --git a/pkg/policy/address.go b/pkg/policy/address.go
index cb45f0a..824c49e 100644
--- a/pkg/policy/address.go
+++ b/pkg/policy/address.go
@@ -71,6 +71,21 @@ func (a *Addressing) NewRecipient(address string) (*Recipient, error) {
 	}, nil
 }
 
+// ParseOrigin parses an address into a Origin. This is used for parsing MAIL FROM argument,
+// not To headers.
+func (a *Addressing) ParseOrigin(address string) (*Origin, error) {
+	local, domain, err := ParseEmailAddress(address)
+	if err != nil {
+		return nil, err
+	}
+	return &Origin{
+		Address:    mail.Address{Address: address},
+		addrPolicy: a,
+		LocalPart:  local,
+		Domain:     domain,
+	}, nil
+}
+
 // ShouldAcceptDomain indicates if Inbucket accepts mail destined for the specified domain.
 func (a *Addressing) ShouldAcceptDomain(domain string) bool {
 	domain = strings.ToLower(domain)
@@ -99,6 +114,12 @@ func (a *Addressing) ShouldStoreDomain(domain string) bool {
 	return false
 }
 
+// ShouldAcceptOriginDomain indicates if Inbucket accept mail from the specified domain.
+func (a *Addressing) ShouldAcceptOriginDomain(domain string) bool {
+	domain = strings.ToLower(domain)
+	return !stringutil.SliceContains(a.Config.SMTP.RejectOriginDomains, domain) 
+}
+
 // ParseEmailAddress unescapes an email address, and splits the local part from the domain part.
 // An error is returned if the local or domain parts fail validation following the guidelines
 // in RFC3696.
diff --git a/pkg/policy/origin.go b/pkg/policy/origin.go
new file mode 100644
index 0000000..6555ece
--- /dev/null
+++ b/pkg/policy/origin.go
@@ -0,0 +1,20 @@
+package policy
+
+import (
+	"net/mail"
+)
+
+// Origin represents a potential email origin, allows policies for it to be queried.
+type Origin struct {
+	mail.Address
+	addrPolicy *Addressing
+	// LocalPart is the part of the address before @, including +extension.
+	LocalPart string
+	// Domain is the part of the address after @.
+	Domain string
+}
+
+// ShouldAccept returns true if Inbucket should accept mail from this origin.
+func (o *Origin) ShouldAccept() bool {
+	return o.addrPolicy.ShouldAcceptOriginDomain(o.Domain)
+}
diff --git a/pkg/server/smtp/handler.go b/pkg/server/smtp/handler.go
index 01f2fc8..6e8fcf2 100644
--- a/pkg/server/smtp/handler.go
+++ b/pkg/server/smtp/handler.go
@@ -106,7 +106,7 @@ type Session struct {
 	sendError    error               // Last network send error.
 	state        State               // Session state machine.
 	reader       *bufio.Reader       // Buffered reading for TCP conn.
-	from         string              // Sender from MAIL command.
+	from         *policy.Origin      // Sender from MAIL command.
 	recipients   []*policy.Recipient // Recipients from RCPT commands.
 	logger       zerolog.Logger      // Session specific logger.
 	debug        bool                // Print network traffic to stdout.
@@ -384,7 +384,10 @@ func (s *Session) readyHandler(cmd string, arg string) {
 			return
 		}
 		from := m[1]
+		s.logger.Debug().Msgf("Mail sender is %v", from)
 		localpart, domain, err := policy.ParseEmailAddress(from)
+		s.logger.Debug().Msgf("Origin domain is %v", domain)
+
 		if from != "" && err != nil {
 			s.send("501 Bad sender address syntax")
 			s.logger.Warn().Msgf("Bad address as MAIL arg: %q, %s", from, err)
@@ -424,7 +427,19 @@ func (s *Session) readyHandler(cmd string, arg string) {
 
 		if extResult == nil || *extResult {
 			// Permitted by extension, or none had an opinion.
-			s.from = from
+			origin, err := s.addrPolicy.ParseOrigin(from)
+			if err != nil {
+				s.send("501 Bad origin address syntax")
+				s.logger.Warn().Str("from", from).Err(err).Msg("Bad address as MAIL arg")
+				return
+			}
+			s.from = origin
+			if !s.from.ShouldAccept() {
+				s.send("501 Unauthorized domain")
+				s.logger.Warn().Msgf("Bad domain sender %s", domain)
+				return
+			}
+
 			s.logger.Info().Msgf("Mail from: %v", from)
 			s.send(fmt.Sprintf("250 Roger, accepting mail from <%v>", from))
 			s.enterState(MAIL)
@@ -601,6 +616,7 @@ func (s *Session) readLine() (line string, err error) {
 
 func (s *Session) parseCmd(line string) (cmd string, arg string, ok bool) {
 	line = strings.TrimRight(line, "\r\n")
+	s.logger.Debug().Msgf("Line received: %v", line)
 
 	// Find length of command or entire line.
 	hasArg := true
@@ -649,7 +665,7 @@ func (s *Session) parseArgs(arg string) (args map[string]string, ok bool) {
 
 func (s *Session) reset() {
 	s.enterState(READY)
-	s.from = ""
+	s.from = nil
 	s.recipients = nil
 }
 
diff --git a/pkg/webui/root_controller.go b/pkg/webui/root_controller.go
index 3f05ec8..05d3431 100644
--- a/pkg/webui/root_controller.go
+++ b/pkg/webui/root_controller.go
@@ -36,13 +36,14 @@ func RootStatus(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err
 			POP3Listener: root.POP3.Addr,
 			WebListener:  root.Web.Addr,
 			SMTPConfig: jsonSMTPConfig{
-				Addr:           root.SMTP.Addr,
-				DefaultAccept:  root.SMTP.DefaultAccept,
-				AcceptDomains:  root.SMTP.AcceptDomains,
-				RejectDomains:  root.SMTP.RejectDomains,
-				DefaultStore:   root.SMTP.DefaultStore,
-				StoreDomains:   root.SMTP.StoreDomains,
-				DiscardDomains: root.SMTP.DiscardDomains,
+				Addr:                root.SMTP.Addr,
+				DefaultAccept:       root.SMTP.DefaultAccept,
+				AcceptDomains:       root.SMTP.AcceptDomains,
+				RejectDomains:       root.SMTP.RejectDomains,
+				DefaultStore:        root.SMTP.DefaultStore,
+				StoreDomains:        root.SMTP.StoreDomains,
+				DiscardDomains:      root.SMTP.DiscardDomains,
+				RejectOriginDomains: root.SMTP.RejectOriginDomains,
 			},
 			StorageConfig: jsonStorageConfig{
 				MailboxMsgCap:   root.Storage.MailboxMsgCap,
diff --git a/pkg/webui/status_json.go b/pkg/webui/status_json.go
index 51709f1..5511129 100644
--- a/pkg/webui/status_json.go
+++ b/pkg/webui/status_json.go
@@ -10,13 +10,14 @@ type jsonServerConfig struct {
 }
 
 type jsonSMTPConfig struct {
-	Addr           string   `json:"addr"`
-	DefaultAccept  bool     `json:"default-accept"`
-	AcceptDomains  []string `json:"accept-domains"`
-	RejectDomains  []string `json:"reject-domains"`
-	DefaultStore   bool     `json:"default-store"`
-	StoreDomains   []string `json:"store-domains"`
-	DiscardDomains []string `json:"discard-domains"`
+	Addr                string   `json:"addr"`
+	DefaultAccept       bool     `json:"default-accept"`
+	AcceptDomains       []string `json:"accept-domains"`
+	RejectDomains       []string `json:"reject-domains"`
+	DefaultStore        bool     `json:"default-store"`
+	StoreDomains        []string `json:"store-domains"`
+	DiscardDomains      []string `json:"discard-domains"`
+	RejectOriginDomains []string `json:"reject-origin-domains"`
 }
 
 type jsonStorageConfig struct {


Test patch:
diff --git a/pkg/message/manager_test.go b/pkg/message/manager_test.go
index 2111e2d..88519af 100644
--- a/pkg/message/manager_test.go
+++ b/pkg/message/manager_test.go
@@ -22,9 +22,10 @@ func TestManagerEmitsMessageStoredEvent(t *testing.T) {
 	listener := extHost.Events.AfterMessageStored.AsyncTestListener("manager", 1)
 
 	// Attempt to deliver a message to generate event.
+	origin, _ := sm.AddrPolicy.ParseOrigin("from@example.com")
 	if _, err := sm.Deliver(
 		&policy.Recipient{},
-		"from@example.com",
+		origin,
 		[]*policy.Recipient{},
 		"prefix",
 		[]byte("From: from@example.com\n\ntest email"),
diff --git a/pkg/server/smtp/handler_test.go b/pkg/server/smtp/handler_test.go
index 62d794f..7754d33 100644
--- a/pkg/server/smtp/handler_test.go
+++ b/pkg/server/smtp/handler_test.go
@@ -92,7 +92,7 @@ func TestEmptyEnvelope(t *testing.T) {
 	// Test out some empty envelope without blanks
 	script := []scriptStep{
 		{"HELO localhost", 250},
-		{"MAIL FROM:<>", 250},
+		{"MAIL FROM:<>", 501},
 	}
 	if err := playSession(t, server, script); err != nil {
 		t.Error(err)
@@ -101,7 +101,7 @@ func TestEmptyEnvelope(t *testing.T) {
 	// Test out some empty envelope with blanks
 	script = []scriptStep{
 		{"HELO localhost", 250},
-		{"MAIL FROM: <>", 250},
+		{"MAIL FROM: <>", 501},
 	}
 	if err := playSession(t, server, script); err != nil {
 		t.Error(err)
@@ -198,6 +198,31 @@ func TestReadyStateValidCommands(t *testing.T) {
 	}
 }
 
+// Test invalid domains in READY state.
+func TestReadyStateRejectedDomains(t *testing.T) {
+	ds := test.NewStore()
+	server := setupSMTPServer(ds, extension.NewHost())
+
+	tests := []scriptStep{
+		{"MAIL FROM: <john@validdomain.com>", 250},
+		{"MAIL FROM: <john@invalidomain.com>", 501},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.send, func(t *testing.T) {
+			defer server.Drain()
+			script := []scriptStep{
+				{"HELO localhost", 250},
+				tc,
+				{"QUIT", 221}}
+			if err := playSession(t, server, script); err != nil {
+				t.Error(err)
+			}
+		})
+	}
+
+}
+
 // Test invalid commands in READY state.
 func TestReadyStateInvalidCommands(t *testing.T) {
 	ds := test.NewStore()
@@ -557,6 +582,7 @@ func setupSMTPServer(ds storage.Store, extHost *extension.Host) *Server {
 			MaxMessageBytes: 5000,
 			DefaultAccept:   true,
 			RejectDomains:   []string{"deny.com"},
+			RejectOriginDomains: []string{"invalidomain.com"},
 			Timeout:         5,
 		},
 	}