go-guerrilla doesn’t support “RCPT TO: <Postmaster>”.

[issuefiler]

RFC 5321

   Any system that includes an SMTP server supporting mail relaying or
   delivery MUST support the reserved mailbox "postmaster" as a case-
   insensitive local name.  This postmaster address is not strictly
   necessary if the server always returns 554 on connection opening (as
   described in Section 3.1).  The requirement to accept mail for
   postmaster implies that RCPT commands that specify a mailbox for
   postmaster at any of the domains for which the SMTP server provides
   mail service, as well as the special case of "RCPT TO:<Postmaster>"
   (with no domain specification), MUST be supported.

   o  The reserved mailbox name "postmaster" may be used in a RCPT
      command without domain qualification (see Section 4.1.1.3) and
      MUST be accepted if so used.

   Syntax:

      rcpt = "RCPT TO:" ( "<Postmaster@" Domain ">" / "<Postmaster>" /
                  Forward-path ) [SP Rcpt-parameters] CRLF

                  Note that, in a departure from the usual rules for
                  local-parts, the "Postmaster" string shown above is
                  treated as case-insensitive.

---

Test

Preset

"allowed_hosts": ["a.com"]

Expectation

Case # 1

RCPT TO: <[email protected]>
250 2.1.5 OK

Case # 2

RCPT TO: <[email protected]>
454 4.1.1 Error: Relay access denied: not-a.com

Case # 3

RCPT TO: <poSTmAsteR>
250 2.1.5 OK

Case # 4

EDITED

RCPT TO: <"po\ST\mAs\t\eR">
250 2.1.5 OK

Current

Case # 1 ✅

RCPT TO: <[email protected]>
250 2.1.5 OK

Case # 2 ✅

RCPT TO: <[email protected]>
454 4.1.1 Error: Relay access denied: not-a.com

Case # 3 ❌

RCPT TO: <poSTmAsteR>
454 4.1.1 Error: Relay access denied: 

Case # 4 ❌ ✅ ^EDITED

RCPT TO: <"po\ST\mAs\t\eR">
501 5.5.4 Invalid address

Fix (not tested)

1. Validate the syntax per the ABNF. This must be done prior to the step 2, or it would break on the case # 4.
2. If the Host is empty and the User is case-insensitively Postmaster, say OK.

go-guerrilla/server.go

Lines 492 to 503 in 51f7dda

	if !s.allowsHost(to.Host) {
	client.sendResponse(r.ErrorRelayDenied, " ", to.Host)
	} else {
	client.PushRcpt(to)
	rcptError := s.backend().ValidateRcpt(client.Envelope)
	if rcptError != nil {
	client.PopRcpt()
	client.sendResponse(r.FailRcptCmd, " ", rcptError.Error())
	} else {
	client.sendResponse(r.SuccessRcptCmd)
	}
	}

↓

				if s.allowsHost(to.Host) || (to.Host == "" && strings.toLower(to.User) == "postmaster") {
					client.PushRcpt(to)
					rcptError := s.backend().ValidateRcpt(client.Envelope)
					if rcptError != nil {
						client.PopRcpt()
						client.sendResponse(r.FailRcptCmd, " ", rcptError.Error())
					} else {
						client.sendResponse(r.SuccessRcptCmd)
					}
				} else {
					client.sendResponse(r.ErrorRelayDenied, " ", to.Host)
				}

[flashmob]

The ABNF has already been parsed in parse.go

TODO:

• modify the parser to evaluate the quoted local-part values as the expression is being parsed.
• add a new string to the Address struct to store the evaluated result (we want to store both the raw value and the evaluated one)
• modify the if s.allowsHost line as suggested above. However, we should also modify the allowsHost to match the evaluated value. (The case 4 above is a valid address <"po\ST\mAs\t\eR"> - no?)
• add a test case for RCPT TO: <poSTmAsteR> and <"po\ST\mAs\t\eR">

There's a question - what Address.host should we default to? Should we fill any value there? Leave blank, or add a new config option, or just use 'localhost' ?

[issuefiler]

The RFC seems to be a bit unclear for "po\ST\mAs\t\eR" and "po\ST\mAs\t\eR"@example.com inputs, but what you’ve said would be the right path to go, I guess; edited.

And for the Address.Host of <Postmaster>: we could simply leave the Host blank (as is) and modify the stringifying steps for use by processors (#199).

[flashmob]

Hi @issuefiler, Please see PR #202 - it's ready for your acceptance test :-)

It works a little different from what was specified above (#201 (comment))
It simply captures quoted local-parts without the escape characters, and then the String() function knows if it was quoted or not. So if the local-part is "test" " then it will be stored as "test " ".
So, allowsHost didn't need any modification after all.

When testing:

Note that if <postmaster> is used in the RCPT TO (without a host), then new functionality was added to assume that the host is assumed to be the Hostname setting for the Server. Therefore, we must ensure that the Hostname setting also exists in the AllowedHosts config setting. (The server will log a warning if it detects this case)