Repository: inbucket/inbucket
Pull request #285 (2022-07-31 21:35:12)

Handle IP address domains
Handle IP address domains for #243.  Also adds support for forward-path routes in RCPT TO.

Given both of these rework how RCPT TO addresses are parsed, this PR has potential for unintended consequences.


jhillyerd: Domain addressing fails from monitor:

```
ERR Error handling request error="Character '[' must be quoted" module=web path=/api/v1/mailbox/[127.0.0.1]
```

Strangely, full addressing does work, ie http://localhost:9000/m/swaks@[127.0.0.1] loads.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1042659..ca98960 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Added
+- Support for IP address as domain in RCPT TO (#285)
+
 
 ## [v3.0.3] - 2022-08-07
 
diff --git a/etc/dev-start.sh b/etc/dev-start.sh
index cec0278..0d8fc89 100755
--- a/etc/dev-start.sh
+++ b/etc/dev-start.sh
@@ -3,6 +3,7 @@
 # description: Developer friendly Inbucket configuration
 
 export INBUCKET_LOGLEVEL="debug"
+#export INBUCKET_MAILBOXNAMING="domain"
 export INBUCKET_SMTP_REJECTDOMAINS="bad-actors.local"
 #export INBUCKET_SMTP_DEFAULTACCEPT="false"
 export INBUCKET_SMTP_ACCEPTDOMAINS="good-actors.local"
diff --git a/pkg/policy/address.go b/pkg/policy/address.go
index 36f516c..cb45f0a 100644
--- a/pkg/policy/address.go
+++ b/pkg/policy/address.go
@@ -3,6 +3,7 @@ package policy
 import (
 	"bytes"
 	"fmt"
+	"net"
 	"net/mail"
 	"strings"
 
@@ -17,44 +18,41 @@ type Addressing struct {
 
 // ExtractMailbox extracts the mailbox name from a partial email address.
 func (a *Addressing) ExtractMailbox(address string) (string, error) {
+	if a.Config.MailboxNaming == config.DomainNaming {
+		return extractDomainMailbox(address)
+	}
+
 	local, domain, err := parseEmailAddress(address)
 	if err != nil {
 		return "", err
 	}
+
 	local, err = parseMailboxName(local)
 	if err != nil {
 		return "", err
 	}
+
 	if a.Config.MailboxNaming == config.LocalNaming {
 		return local, nil
 	}
-	if a.Config.MailboxNaming == config.DomainNaming {
-		// If no domain is specified, assume this is being
-		// used for mailbox lookup via the API.
-		if domain == "" {
-			if ValidateDomainPart(local) == false {
-				return "", fmt.Errorf("Domain part %q in %q failed validation", local, address)
-			}
-			return local, nil
-		}
-		if ValidateDomainPart(domain) == false {
-			return "", fmt.Errorf("Domain part %q in %q failed validation", domain, address)
-		}
-		return domain, nil
-	}
+
 	if a.Config.MailboxNaming != config.FullNaming {
 		return "", fmt.Errorf("Unknown MailboxNaming value: %v", a.Config.MailboxNaming)
 	}
+
 	if domain == "" {
 		return local, nil
 	}
+
 	if !ValidateDomainPart(domain) {
 		return "", fmt.Errorf("Domain part %q in %q failed validation", domain, address)
 	}
+
 	return local + "@" + domain, nil
 }
 
-// NewRecipient parses an address into a Recipient.
+// NewRecipient parses an address into a Recipient. This is used for parsing RCPT TO arguments,
+// not To headers.
 func (a *Addressing) NewRecipient(address string) (*Recipient, error) {
 	local, domain, err := ParseEmailAddress(address)
 	if err != nil {
@@ -64,12 +62,8 @@ func (a *Addressing) NewRecipient(address string) (*Recipient, error) {
 	if err != nil {
 		return nil, err
 	}
-	ar, err := mail.ParseAddress(address)
-	if err != nil {
-		return nil, err
-	}
 	return &Recipient{
-		Address:    *ar,
+		Address:    mail.Address{Address: address},
 		addrPolicy: a,
 		LocalPart:  local,
 		Domain:     domain,
@@ -122,13 +116,24 @@ func ParseEmailAddress(address string) (local string, domain string, err error)
 // ValidateDomainPart returns true if the domain part complies to RFC3696, RFC1035. Used by
 // ParseEmailAddress().
 func ValidateDomainPart(domain string) bool {
-	if len(domain) == 0 {
+	ln := len(domain)
+	if ln == 0 {
 		return false
 	}
-	if len(domain) > 255 {
+	if ln > 255 {
 		return false
 	}
-	if domain[len(domain)-1] != '.' {
+	if ln >= 4 && domain[0] == '[' && domain[ln-1] == ']' {
+		// Bracketed domains must contain an IP address.
+		s := 1
+		if strings.HasPrefix(domain[1:], "IPv6:") {
+			s = 6
+		}
+		ip := net.ParseIP(domain[s : ln-1])
+		return ip != nil
+	}
+
+	if domain[ln-1] != '.' {
 		domain += "."
 	}
 	prev := '.'
@@ -168,6 +173,40 @@ func ValidateDomainPart(domain string) bool {
 	return true
 }
 
+// Extracts the mailbox name when domain addressing is enabled.
+func extractDomainMailbox(address string) (string, error) {
+	var local, domain string
+	var err error
+
+	if address != "" && address[0] == '[' && address[len(address)-1] == ']' {
+		// Likely an IP address in brackets, treat as domain only.
+		domain = address
+	} else {
+		local, domain, err = parseEmailAddress(address)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	if local != "" {
+		local, err = parseMailboxName(local)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	// If no @domain is specified, assume this is being used for mailbox lookup via the API.
+	if domain == "" {
+		domain = local
+	}
+
+	if ValidateDomainPart(domain) == false {
+		return "", fmt.Errorf("Domain part %q in %q failed validation", domain, address)
+	}
+
+	return domain, nil
+}
+
 // parseEmailAddress unescapes an email address, and splits the local part from the domain part.  An
 // error is returned if the local part fails validation following the guidelines in RFC3696. The
 // domain part is optional and not validated.
@@ -178,12 +217,23 @@ func parseEmailAddress(address string) (local string, domain string, err error)
 	if len(address) > 320 {
 		return "", "", fmt.Errorf("address exceeds 320 characters")
 	}
+
+	// Remove forward-path routes.
 	if address[0] == '@' {
-		return "", "", fmt.Errorf("address cannot start with @ symbol")
+		end := strings.IndexRune(address, ':')
+		if end == -1 {
+			return "", "", fmt.Errorf("missing terminating ':' in route specification")
+		}
+		address = address[end+1:]
+		if address == "" {
+			return "", "", fmt.Errorf("Address empty after removing route specification")
+		}
 	}
+
 	if address[0] == '.' {
 		return "", "", fmt.Errorf("address cannot start with a period")
 	}
+
 	// Loop over address parsing out local part.
 	buf := new(bytes.Buffer)
 	prev := byte('.')


Test patch:
diff --git a/etc/swaks-tests/run-tests.sh b/etc/swaks-tests/run-tests.sh
index 38a654c..f1dc571 100755
--- a/etc/swaks-tests/run-tests.sh
+++ b/etc/swaks-tests/run-tests.sh
@@ -59,3 +59,7 @@ swaks $* --data nonmime-html-inlined.raw
 
 # Incorrect charset, malformed final boundary
 swaks $* --data mime-errors.raw
+
+# IP RCPT domain
+swaks $* --to="swaks@[127.0.0.1]" --h-Subject: "IPv4 RCPT Address" --body text.txt
+swaks $* --to="swaks@[IPv6:2001:db8:aaaa:1::100]" --h-Subject: "IPv6 RCPT Address" --body text.txt
diff --git a/pkg/policy/address_test.go b/pkg/policy/address_test.go
index 3a1ae67..9421dc0 100644
--- a/pkg/policy/address_test.go
+++ b/pkg/policy/address_test.go
@@ -259,6 +259,30 @@ func TestExtractMailboxValid(t *testing.T) {
 			full:   "chars|}~@example.co.uk",
 			domain: "example.co.uk",
 		},
+		{
+			input:  "@host:user+label@domain.com",
+			local:  "user",
+			full:   "user@domain.com",
+			domain: "domain.com",
+		},
+		{
+			input:  "@a.com,@b.com:user+label@domain.com",
+			local:  "user",
+			full:   "user@domain.com",
+			domain: "domain.com",
+		},
+		{
+			input:  "u@[127.0.0.1]",
+			local:  "u",
+			full:   "u@[127.0.0.1]",
+			domain: "[127.0.0.1]",
+		},
+		{
+			input:  "u@[IPv6:2001:db8:aaaa:1::100]",
+			local:  "u",
+			full:   "u@[IPv6:2001:db8:aaaa:1::100]",
+			domain: "[IPv6:2001:db8:aaaa:1::100]",
+		},
 	}
 	for _, tc := range testTable {
 		if result, err := localPolicy.ExtractMailbox(tc.input); err != nil {
@@ -285,10 +309,42 @@ func TestExtractMailboxValid(t *testing.T) {
 	}
 }
 
+// Test special cases with domain addressing mode.
+func TestExtractDomainMailboxValid(t *testing.T) {
+	domainPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.DomainNaming}}
+
+	tests := map[string]struct {
+		input  string // Input to test
+		domain string // Expected output when mailbox naming = domain
+	}{
+		"ipv4": {
+			input:  "[127.0.0.1]",
+			domain: "[127.0.0.1]",
+		},
+		"medium ipv6": {
+			input:  "[IPv6:2001:db8:aaaa:1::100]",
+			domain: "[IPv6:2001:db8:aaaa:1::100]",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			if result, err := domainPolicy.ExtractMailbox(tc.input); tc.domain != "" && err != nil {
+				t.Errorf("Error while parsing with domain naming %q: %v", tc.input, err)
+			} else {
+				if result != tc.domain {
+					t.Errorf("Parsing %q, expected %q, got %q", tc.input, tc.domain, result)
+				}
+			}
+		})
+	}
+}
+
 func TestExtractMailboxInvalid(t *testing.T) {
 	localPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.LocalNaming}}
 	fullPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.FullNaming}}
 	domainPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.DomainNaming}}
+
 	// Test local mailbox naming policy.
 	localInvalidTable := []struct {
 		input, msg string
@@ -303,6 +359,7 @@ func TestExtractMailboxInvalid(t *testing.T) {
 			t.Errorf("Didn't get an error while parsing in local mode %q: %v", tt.input, tt.msg)
 		}
 	}
+
 	// Test full mailbox naming policy.
 	fullInvalidTable := []struct {
 		input, msg string
@@ -318,6 +375,7 @@ func TestExtractMailboxInvalid(t *testing.T) {
 			t.Errorf("Didn't get an error while parsing in full mode %q: %v", tt.input, tt.msg)
 		}
 	}
+
 	// Test domain mailbox naming policy.
 	domainInvalidTable := []struct {
 		input, msg string
@@ -365,6 +423,10 @@ func TestValidateDomain(t *testing.T) {
 		{strings.Repeat("a", 256), false, "Max domain length is 255"},
 		{strings.Repeat("a", 63) + ".com", true, "Should allow 63 char domain label"},
 		{strings.Repeat("a", 64) + ".com", false, "Max domain label length is 63"},
+		{"[0.0.0.0]", true, "Single digit octet IP addr is valid"},
+		{"[123.123.123.123]", true, "Multiple digit octet IP addr is valid"},
+		{"[IPv6:2001:0db8:aaaa:0001:0000:0000:0000:0200]", true, "Full IPv6 addr is valid"},
+		{"[IPv6:::1]", true, "Abbr IPv6 addr is valid"},
 	}
 	for _, tt := range testTable {
 		if policy.ValidateDomainPart(tt.input) != tt.expect {
@@ -419,6 +481,9 @@ func TestValidateLocal(t *testing.T) {
 		{"$A12345", true, "RFC3696 test case should be valid"},
 		{"!def!xyz%abc", true, "RFC3696 test case should be valid"},
 		{"_somename", true, "RFC3696 test case should be valid"},
+		{"@host:mailbox", true, "Forward-path routes are valid"},
+		{"@a.com,@b.com:mailbox", true, "Multi-hop forward-path routes are valid"},
+		{"@a.com,mailbox", false, "Unterminated forward-path routes are invalid"},
 	}
 	for _, tt := range testTable {
 		_, _, err := policy.ParseEmailAddress(tt.input + "@domain.com")
@@ -430,3 +495,33 @@ func TestValidateLocal(t *testing.T) {
 		}
 	}
 }
+
+// TestRecipientAddress verifies the Recipient.Address values returned by Addressing.NewRecipient.
+// This function parses a RCPT TO path, not a To header. See rfc5321#section-4.1.2
+func TestRecipientAddress(t *testing.T) {
+	localPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.LocalNaming}}
+
+	tests := map[string]string{
+		"common":          "user@example.com",
+		"with label":      "user+mailbox@example.com",
+		"special chars":   "a!#$%&'*+-/=?^_`{|}~@example.com",
+		"ipv4":            "user@[127.0.0.1]",
+		"ipv6":            "user@[IPv6:::1]",
+		"route host":      "@host:user@example.com",
+		"route domain":    "@route.com:user@example.com",
+		"multi-hop route": "@first.com,@second.com:user@example.com",
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			r, err := localPolicy.NewRecipient(tc)
+			if err != nil {
+				t.Fatalf("Parse of %q failed: %v", tc, err)
+			}
+
+			if got, want := r.Address.Address, tc; got != want {
+				t.Errorf("Got Address: %q, want: %q", got, want)
+			}
+		})
+	}
+}
diff --git a/pkg/server/smtp/handler_test.go b/pkg/server/smtp/handler_test.go
index 582dc42..6cf2cb6 100644
--- a/pkg/server/smtp/handler_test.go
+++ b/pkg/server/smtp/handler_test.go
@@ -266,6 +266,8 @@ func TestMailState(t *testing.T) {
 		{"RSET", 250},
 		{"MAIL FROM:<john@gmail.com>", 250},
 		{`RCPT TO:<"first/last"@host.com`, 250},
+		{"RCPT TO:<u1@[127.0.0.1]>", 250},
+		{"RCPT TO:<u1@[IPv6:2001:db8:aaaa:1::100]>", 250},
 	}
 	if err := playSession(t, server, script); err != nil {
 		t.Error(err)
diff --git a/pkg/test/integration_test.go b/pkg/test/integration_test.go
index ebc0994..704e493 100644
--- a/pkg/test/integration_test.go
+++ b/pkg/test/integration_test.go
@@ -32,6 +32,8 @@ const (
 	smtpHost    = "127.0.0.1:2500"
 )
 
+// TODO: Add suites for domain and full addressing modes.
+
 func TestSuite(t *testing.T) {
 	stopServer, err := startServer()
 	if err != nil {
@@ -46,6 +48,8 @@ func TestSuite(t *testing.T) {
 		{"basic", testBasic},
 		{"fullname", testFullname},
 		{"encodedHeader", testEncodedHeader},
+		{"ipv4Recipient", testIPv4Recipient},
+		{"ipv6Recipient", testIPv6Recipient},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, tc.test)
@@ -139,6 +143,64 @@ func testEncodedHeader(t *testing.T) {
 	goldiff.File(t, got, "testdata", "encodedheader.golden")
 }
 
+func testIPv4Recipient(t *testing.T) {
+	client, err := client.New(restBaseURL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	from := "fromuser@inbucket.org"
+	to := []string{"ip4recipient@[192.168.123.123]"}
+	input := readTestData("no-to.txt")
+
+	// Send mail.
+	err = smtpclient.SendMail(smtpHost, nil, from, to, input)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Confirm receipt.
+	msg, err := client.GetMessage("ip4recipient", "latest")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if msg == nil {
+		t.Errorf("Got nil message, wanted non-nil message.")
+	}
+
+	// Compare to golden.
+	got := formatMessage(msg)
+	goldiff.File(t, got, "testdata", "no-to-ipv4.golden")
+}
+
+func testIPv6Recipient(t *testing.T) {
+	client, err := client.New(restBaseURL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	from := "fromuser@inbucket.org"
+	to := []string{"ip6recipient@[IPv6:2001:0db8:85a3:0000:0000:8a2e:0370:7334]"}
+	input := readTestData("no-to.txt")
+
+	// Send mail.
+	err = smtpclient.SendMail(smtpHost, nil, from, to, input)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Confirm receipt.
+	msg, err := client.GetMessage("ip6recipient", "latest")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if msg == nil {
+		t.Errorf("Got nil message, wanted non-nil message.")
+	}
+
+	// Compare to golden.
+	got := formatMessage(msg)
+	goldiff.File(t, got, "testdata", "no-to-ipv6.golden")
+}
+
 func formatMessage(m *client.Message) []byte {
 	b := &bytes.Buffer{}
 	fmt.Fprintf(b, "Mailbox: %v\n", m.Mailbox)
diff --git a/pkg/test/testdata/no-to-ipv4.golden b/pkg/test/testdata/no-to-ipv4.golden
new file mode 100644
index 0000000..c9342a9
--- /dev/null
+++ b/pkg/test/testdata/no-to-ipv4.golden
@@ -0,0 +1,12 @@
+Mailbox: ip4recipient
+From: <fromuser@inbucket.org>
+To: [<ip4recipient@[192.168.123.123]>]
+Subject: basic subject
+Size: 198
+
+BODY TEXT:
+No-To message.
+
+
+BODY HTML:
+
diff --git a/pkg/test/testdata/no-to-ipv6.golden b/pkg/test/testdata/no-to-ipv6.golden
new file mode 100644
index 0000000..8846de4
--- /dev/null
+++ b/pkg/test/testdata/no-to-ipv6.golden
@@ -0,0 +1,12 @@
+Mailbox: ip6recipient
+From: <fromuser@inbucket.org>
+To: [<ip6recipient@[IPv6:2001:0db8:85a3:0000:0000:8a2e:0370:7334]>]
+Subject: basic subject
+Size: 227
+
+BODY TEXT:
+No-To message.
+
+
+BODY HTML:
+
diff --git a/pkg/test/testdata/no-to.txt b/pkg/test/testdata/no-to.txt
new file mode 100644
index 0000000..e33ed29
--- /dev/null
+++ b/pkg/test/testdata/no-to.txt
@@ -0,0 +1,4 @@
+From: fromuser@inbucket.org
+Subject: basic subject
+
+No-To message.