stalwartlabs / mail-server

Secure & Modern All-in-One Mail Server (IMAP, JMAP, POP3, SMTP)
https://stalw.art
5.25k stars 221 forks source link

[bug]: Sieve scripting fails differently and silently in different stages #611

Closed X-Ryl669 closed 4 months ago

X-Ryl669 commented 4 months ago

What happened?

I'm making a simple alias scenario. If a user send a mail to "f.something@example.com", I want it to be sent to "bob@example.com". I also want to store the message in the "INBOX/something" folder, creating it if it doesn't exist.

I've used the

session.rcpt.rewrite = [
{ if = "is_local_domain('%{DEFAULT_DIRECTORY}%', rcpt_domain) & matches('^f\.([^.]+)@(.+)$', rcpt)", then = "'bob+' + $1 + '@' + $2" },
{ else = false }]

and it's working, but I can't store it anywhere else later on.

So I went to the sieve version. As I understand it, I need to react in different stage of the mail processing pipeline. So I've put a sieve script for the session.rcpt that contains this:

require ["variables", "envelope", "regex", "editheader"];

if allof( envelope :localpart :contains "to" ".",
          envelope :regex "to" "f\.([^.]+)@(.+)$") {
    set "envelope.to" "bob@${2}";
}

This works but I can't store it later on since I've lost the original email address. So I've tried:

require ["variables", "envelope", "regex", "editheader"];

if allof( envelope :localpart :contains "to" ".",
          envelope :regex "to" "f\.([^.]+)@(.+)$") {
    set "envelope.to" "bob@${2}";
    addheader "X-Store-Into" "${1}";
}

This doesn't work and I don't get any error whatsoever in the log files. If I try to use fileinto, directly, it fails with this log:

smtp::scripts::event_loop: context="sieve" event="runtime-error" reason="Unsupported event: FileInto { folder: \"inbox.{$1}\", flags: [], mailbox_id: None, special_use: None, create: false, message_id: 0 }"

Which, AIUI, is because in the RCPT stage, the destination account isn't known yet. In the code, the FileInto event isn't handled for this case.

In my understanding, once the header is present, the final step for sorting the mail in the right folder was to add a session.data containing this script (for the DATA stage):

require ["variables", "envelope", "fileinto", "mailboxid", "mailbox", "copy"];

if header :matches "X-Store-Into" "*" {
    fileinto :copy "inbox";
    fileinto :mailboxid "aliased" :create "inbox/{$1}";
}

But I can't hit that stage yet, since the RCPT fails silently.

How can we reproduce the problem?

See above

Version

v0.8.x

What database are you using?

PostgreSQL

What blob storage are you using?

PostgreSQL

Where is your directory located?

Internal

What operating system are you using?

Linux

Relevant log output

No response

Code of Conduct

X-Ryl669 commented 4 months ago

More information coming here:

In the documentation, it seems that we can use sieve scripts for any stage. While it's partially true, you can't use all of sieve language and extensions in the different stages.

It's quite hard to write and debug sieve scripts with SW (compared to Haraka's JS), so here are few remarks about the issue and technics I've encountered that I think should be incorporated in the documentation to help other users:

  1. In each stage, while it might seem obvious, you don't have access to everything and all sieve extensions. I think Stalwart should choke and report when you're using a sieve extension in a inappropriate place. For example, in RCPT state, you can't addheader (the whole editheader extension should be reported as 'Invalid in this stage'). You can't either fileinto or copy and so on. Please notice that while FileInto triggers a report in the log when it's executed (better than nothing), neither addheader or setting envelope.orcpt does (but it reject the mail silently)

  2. So the documentation listed above should list what is possible in the Sieve scripts for each stage.

  3. I found out it's really hard to debug Sieve script in this state. I'm doing this reject "Dump some message and variable ${1}."; stop; and it helps a bit, since you'll either get a report in the logs or a failed delivery status notification. This should be added in the documentation.

  4. set doesn't work with regex's capture group. If you do set "myvar" "{$1}" it affects "{$1}" to myvar. It works if you do a (useless) replace like this set :replace "AZEZAEEORZFOJZOFJ" "" "myvar" "${1}"; (replace is useless here since the needle is bullshit, but it does accept the regex's capture group. I think this is a bug

  5. envelope.orcpt is documented as:

    ORCPT: Used to provide the original recipient address when it's different from the address in the envelope. This is useful in cases of address rewriting or forwarding.

    and in the RFC as:

      with the TO address used in the SMTP RCPT TO command that resulted
      in this message getting delivered to this user, with xtext
      encoding removed."

    Yet, if you modify envelope.to in RCPT stage like this, envelope.orcpt is also modified in the DATA stage. This makes argument passing between each stage very convoluted when address rewriting. The only trick I've found to pass the original address while rewriting the address is to embed the original address via subadressing (finalaccount+originalaccount@domain). I think this is a hack.

  6. set doesn't work. In fact, even the examples in the documentation don't work. For example, let's look at this simple RCPT stage script: (extracted from documentation):

    if allof( envelope :localpart :contains "to" ".",
          envelope :regex "to" "alias\.([^.]+)@(.+)$") {
    set :replace "alias." "account+" "to" "${0}";
    set "envelope.to" "{$to}";
    reject "Dump: ${envelope.to} ${to}";
    }

    envelope.to is not modified by set. The reject message is therefore smtp::inbound::rcpt: context="sieve" event="reject" address="alias.bob@domain" reason="503 5.5.3 Dump alias.bob@domain account+bob@domain\r\n"

  7. regex special char should be escaped twice in sieve scripts. Try to match ^([^+]+)\+([^+]+)@(.+)$ for any subaddressing "alice+bob@charlie" and it'll fail. You need to use ^([^+]+)\\+([^+]+)@(.+)$ (notice the double \ before +). This isn't obvious that a double escape is required and it's not documented anywhere. In the config.toml rewrite rule, no double escaping is required.

  8. session.data script doesn't work. I've this in my config.toml:

    session.data.script = "'create-mb-for-alias'"

    In the webmail, I have this: image Yet, when run, I get this error in the logs:

    2024-07-10T13:03:19.038819Z  WARN session{instance="smtp" protocol=Smtp remote.ip="1.2.3.4" remote.port=59984}: smtp::scripts::event_loop: context="sieve" event="runtime-error" reason="Unsupported event: FileInto { folder: \"{$2}\", flags: [], mailbox_id: None, special_use: None, create: true, message_id: 0 }"
    2024-07-10T13:03:19.046984Z  INFO session{instance="smtp" protocol=Smtp remote.ip="1.2.3.4" remote.port=59984}: smtp::queue::spool: Message queued for delivery. context="queue" event="scheduled" id=185255275330580481 from="redacted@redacted" nrcpts=1 size=4227

    The delivery fails. The FileInto event error (in the code) happens here.

  9. When using per-user sieve scripts, at least the fileinto is accepted and runs but fileinto extension doesn't support expanding variables. So the above script, when run in user context, will create a folder "{$2}" and not from the subaddress's regex capture group content. I've also tried to set a variable with a :replace trick above to force variable substitution, but even that variable isn't expanded in fileinto expression (if I reject the mail instead, it's correctly expanded/substituted). Looking at the code, I think it the folder variable isn't parsed as a Value::Variable or Value::Regex but a Value::Text and used as-is. I'm not able to debug further, I don't know how.

mdecimus commented 4 months ago

Two things:

X-Ryl669 commented 4 months ago

There are, at least 4 different bugs in this report:

  1. set doesn't expand regex (see my previous post, point 4)
  2. fileinto doesn't expand variable (see my previous post, point 11)
  3. set "envelope.to" doesn't work at all (contrary to what's written in documentation, point 8)
  4. changing envelope.to also modify envelope.orcpt (point 5)

Closing this issue still doesn't solve them.

In the end, however I rotate the problem, I can't do the simple thing of creating a folder for a dynamic/unknown alias and storing the mail into it.

mdecimus commented 4 months ago

Thanks for filing the separate reports. I don't have time to look into this issue at the moment but it will be reviewed as soon as possible. In the meantime I suggest looking at the Sieve test suite for ideas on how to solve the regex and variable expansion issues you're having. Also in the mail-server repository test suite there should be a test that modifies the envelope To address.

X-Ryl669 commented 4 months ago

I won't be available in the next weeks. As soon as I can, I'll try to pull and build the sieve_rs repository and add the missing tests if I can. For the envelope To, I think the issue is that writing to the member in a script isn't visible until the script is finished (an EnvelopeSet event is enqueued), so if the script contains a read to the Envelope.To after it's written (like in the bug report), it'll still return the previous value.

For the other 2 issues, I couldn't spot anything obvious in the code, so I'll have to build the mail server as a debug target and try to break while the script is running to figure out why it doesn't expand variables. I hope I'll be able to locate the issue.