Marcus Cavanaugh [:mcav] Assignee
	Description • 11 years ago

The e-mail probe doesn't catch invalid e-mail addresses (other than simple syntax validation) during the probing process. This can result in users having an account that cannot send e-mail (unless they delete and recreate the account with a valid address).

As part of the SMTP probe, we should send a "MAIL FROM: <email>" line. Servers might respond with something like "554 5.7.1 <email>: Sender address rejected", in which case we should note the error.

We'll need a new error code for this, so that we can properly report it.

	Marcus Cavanaugh [:mcav] Assignee
	Comment 1 • 11 years ago

Marking as dependent on bug 805501 since that may end up including some SMTP changes that would interfere with a patch for this bug.

	Marcus Cavanaugh [:mcav] Assignee
	Comment 4 • 10 years ago

See smtp/probe.js for most of the relevant changes/implementation
comments. In addition to the prober unit tests, you can manually
verify this by entering an invalid address, then doing manual setup
and entering _valid_ credentials. (Otherwise it'll complain about
bad-user-or-pass before it gets a chance to even try SMTP address
probing.)

The Gaia PR is split in two commits, one with GELAM, one without.

	Andrew Sutherland [:asuth] (he/him)
	Comment 5 • 10 years ago

Code and tests look good and tests pass.  My initial attempts to verify the improved behaviour using postfix were too naive; I still want to test on my postfix server and on a real-world server first before signing off, but I don't expect any changes to be required.  I should be able to do that tomorrow; jet-lag crashing momentarily.

	Andrew Sutherland [:asuth] (he/him)
	Comment 6 • 10 years ago

Comment on attachment 8370218 [details]
Gaia PR: https://github.com/mozilla-b2g/gaia/pull/15951/files

The magic for postfix turns out to be to add "reject_sender_login_mismatch" to the "smtpd_sender_restrictions" directive (http://www.postfix.org/postconf.5.html#smtpd_sender_restrictions).  This also requires using "smtp_sender_login_maps" which turns out to be slightly more confusing and infuriating than the average confusing and infuriating postfix setup stuff.

I think we also want/need to issue a RCPT TO line.  Postfix by default does not issue errors until it sees a RCPT TO.  We would use the same address for RCPT TO that we are using for MAIL FROM.  See http://www.postfix.org/SMTPD_ACCESS_README.html#timing for details.

Postfix is used places like Zimbra, so I think we want this.

Please do not feel the need to try and test against postfix; only one of us needs to go mad.

	Marcus Cavanaugh [:mcav] Assignee
	Comment 7 • 10 years ago

Comment on attachment 8370218 [details]
Gaia PR: https://github.com/mozilla-b2g/gaia/pull/15951/files

I've updated the PRs to attempt "RCPT TO" as well, per your comment above. I updated the unit tests, but since you volunteered to test Postfix, I'll leave that verification to you. By all means holler if there's something more you'd like me to test manually, etc; happy to share the burden.

	Andrew Sutherland [:asuth] (he/him)
	Comment 8 • 10 years ago

Comment on attachment 8370218 [details]
Gaia PR: https://github.com/mozilla-b2g/gaia/pull/15951/files

By me testing postfix, we both get exciting e-mail related victories!  Your changes work great, thanks very much.

So the changes to get this to work for me were as follows.  This was using the system's local auth database via dovecot lmtp sasl auth or whatever our standard (yet horrible) cookbook is.  The map (which should not include a colon; that's for alias formats) needs to map the e-mail to the local username.  If the SASL config or user-names include full domains, normalization may also be required in dovecot's auth config; including that too just in case.

== /etc/postfix/main.cf
smtpd_sender_login_maps = texthash:/etc/postfix/sender-map
smtpd_sender_restrictions = reject_unknown_sender_domain, reject_sender_login_mismatch

== /etc/postfix/sender-map
testy@clicky.visophyte.org testy

== /etc/dovecot/conf.d/10-auth.conf
auth_username_format = %Ln

	Marcus Cavanaugh [:mcav] Assignee
	Comment 9 • 10 years ago

Landed:

https://github.com/mozilla-b2g/gaia/commit/e947bf027c62719415e8c5882b42abb12efcbecb
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/99658402b896529e5538c8f4ce2aa3ee54fdedde

	Brogan Zumwalt [Inactive]
	Comment 11 • 10 years ago

Since this is being duped to bug 996065 I'm adding a 1.3t tracking flag as this is occurring in latest Tarako PVT build.