From 1f1a8b419221e06aec730bf9a856f6040dd64a89 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sun, 7 Aug 2022 20:13:58 -0700 Subject: [PATCH] Handle IP address domains (#285) * Add basic TestRecipientAddress tests * Handle forward-path route spec * Validate IP addr "domains" * Forward-path test cases * Add integration test * Add IPv4 recip swaks test * Special case domain mailbox extraction * add IPv6 swaks test * Formatting Signed-off-by: James Hillyerd * Update changelog --- CHANGELOG.md | 3 + etc/dev-start.sh | 1 + etc/swaks-tests/run-tests.sh | 4 ++ pkg/policy/address.go | 98 ++++++++++++++++++++++------- pkg/policy/address_test.go | 95 ++++++++++++++++++++++++++++ pkg/server/smtp/handler_test.go | 2 + pkg/test/integration_test.go | 62 ++++++++++++++++++ pkg/test/testdata/no-to-ipv4.golden | 12 ++++ pkg/test/testdata/no-to-ipv6.golden | 12 ++++ pkg/test/testdata/no-to.txt | 4 ++ 10 files changed, 269 insertions(+), 24 deletions(-) create mode 100644 pkg/test/testdata/no-to-ipv4.golden create mode 100644 pkg/test/testdata/no-to-ipv6.golden create mode 100644 pkg/test/testdata/no-to.txt 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/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.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('.') 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:", 250}, {`RCPT TO:<"first/last"@host.com`, 250}, + {"RCPT TO:", 250}, + {"RCPT TO:", 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: +To: [] +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: +To: [] +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.