JMAP: make smtp client protocol errors return correct JMAP EmailSubmission/set errors

[robmueller]

We need the cyrus smtp client to support returning richer errors so we can pass appropriate errors back up to the JMAP stack.

https://jmap.io/spec-mail.html#emailsubmission/set

• tooLarge - The email size is larger than the server supports sending. A maxSize Number property MUST be present on the SetError specifying the maximum size of an email that may be sent, in octets.

https://tools.ietf.org/html/rfc1870

The smtp client should listen for the SIZE response to EHLO and send the SIZE= parameter in the MAIL FROM command if it sees the SIZE extension. If the message size is > the SIZE returned in the EHLO response it should return a tooLarge error at the JMAP level. If the MAIL FROM <...> SIZE=... command fails with a 552 response, it should return a tooLarge error with the SIZE returned in the EHLO response.

• tooManyRecipients - The envelope (supplied or generated) has more recipients than the server allows. A maxRecipients Number property MUST be present on the SetError specifying the maximum number of allowed recipients.

https://tools.ietf.org/html/rfc3463#section-3.6

The smtp client should check if a RCPT TO action returns an enhanced status code of the form X.5.3. If so, it should return a tooManyRecipients error at the JMAP level. It should fill maxRecipients with how ever many RCPT TO actions were able to be correctly sent before the X.5.3 error occurred.

• noRecipients – The envelope (supplied or generated) does not have any rcptTo emails.

Currently the smtp client checks that there are some recipients, but doesn't map to a noRecipients JMAP error if there are none, instead it's a smtpProtocolError.

• invalidRecipients – The rcptTo property of the envelope (supplied or generated) contains at least one rcptTo value which is not a valid email for sending to. An invalidRecipients String[] property MUST be present on the SetError, which is a list of the invalid addresses.

https://tools.ietf.org/html/rfc3463#section-3.2

The smtp client should check if a RCPT TO action returns an enhanced status code of the form X.1.1, X.1.2 or X.1.3. If so, it should collect that address in an error array. At the end, if there are any errors, it should abort the smtp transaction and should return an invalidRecipients error at the JMAP level. It should will in the invalidRecipients result with the collected array of addresses that generated errors.

I think that's the errors that cyrus needs to handle at least. I can make sure the current SMTP handler generates the appropriate errors as needed. The forbiddenMailFrom, forbiddenFrom, forbiddenToSend need to be handled at the perl layer.

[robmueller]

@ksmurchison There was some talk that you updated this commit to get rid of the atof() usage, but I can't find the commit anywhere, and it appears that none of this has been pushed to master yet.

Also, I've been working on the backend part for this, and actually a bunch of things will be easier to return at the DATA command stage rather than each RCPT TO. Specifically there's a bunch of policy related checks (unverified user, exceeding user/bandwidth sending limits, etc) that occur at the DATA command time and I'd like to map those to forbiddenToSend, and the error message is actually returned up the stack as the description shown to the user.

https://jmap.io/spec-mail.html#emailsubmission/set

Looking at possible return codes for this...

https://www.iana.org/assignments/smtp-enhanced-status-codes/smtp-enhanced-status-codes.xhtml

Nothing looks particularly great, but maybe the best is a X.3.0 response? Any text after the XYZ X.3.0 response gets passed up as the description property on the error.

Could you implement this as well please, and then get the whole result pushed to master. Thanks

[ksmurchison]

atof() removed here: https://github.com/cyrusimap/cyrus-imapd/commit/e17dbc4b38ed909d3f75b1c914b60efb1af46058#diff-4e4155fe4f2fe13794ca6ab134e47b72

I will look into passing description up the stack

[ksmurchison]

forbiddenToSend implemented:

https://github.com/cyrusimap/cyrus-imapd/commit/32170cd0f35330686c3fbbbfad7a5e021b995701

[robmueller]

sigh Ok, talking with neil, the forbiddenToSend error is designed as a "this user is locked" sort of error, so the default error message includes stuff about contacting support.

This isn't really quite the same as a rate limiting error, and what we should be returning is the rateLimit error defined in JMAP core for that case.

Right, so I think we should expand what happens with the X.3.0 error a bit and try and allow the SMTP backend to return arbitrary error + description items back up the stack. That'll make it easier in the future if we want to change or refine these a bit, we can just change our smtp sending engine without having to change cyrus each time.

So here's what I propose: if you see a X.3.0 error, look at the next bit of text (skipping whitespace), and if it says jmaperror, then the next bit of text (skipping whitespace) should be the error, and any text after that (skipping whitespace) should be the description.

If you don't see the jmaperror string, just use the remainder of the text as a description and use a forbidden error.

Example 1:

554 5.3.0 jmaperror forbiddenToSend The user is not verified

becomes

{"type":"forbiddenToSend","description":"The user is not verified"}

Example 2

554 5.3.0 jmaperror rateLimit Already reached per-hour limit of 100 messages sent by this user, try again later

becomes

{"type":"rateLimit","description":"Already reached per-hour limit of 100 messages sent by this user, try again later"}

Example 3:

554 5.3.0 Don't do that!

becomes

{"type":"fobidden","description":"Don't do that!"}

If we do that, then I can return a rateLimit error at the DATA stage if the user is exceeding their sending rates.

Assuming you can return a X3.0 error at any stage, later on, I can also generate forbiddenMailFrom at the MAIL FROM and forbiddenFrom at the EOD stages when we get around to validating those addresses in the future.

I think if we do this, then I hope I'll never have to re-open this ticket again...

[elliefm]

I think this will overlap with #2477? In which case, might as well sort that out at the same time?

[ksmurchison]

Rather than just whitespace separating these tokens, can we do something like this (borrowing from IMAP):

554 5.3.0 [jmaperror:forbiddenToSend] The user is not verified

[robmueller]

Yep, that looks good to me as well.

[ksmurchison]

Implemented as above, but chose to use [jmapError:...]" (camel-case rather than lower-case)

[ksmurchison]

@robmueller Has this been tested yet?

[robmueller]

Not yet. I think it only got rolled out to an FM store today. I'll try and do it tomorrow.

[ksmurchison]

@robmueller Can this issue be closed as resolved?

[robmueller]

Yes, we've been using this for ages and all appears to work well.