Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Feature: Quarantine Outgoing Mail #5658

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from

Conversation

SnailShea
Copy link
Contributor

@SnailShea SnailShea commented Jan 21, 2024

I have implemented outbound quarantine scanning for mailcow as an optional feature. The UI and language files have been updated to reflect this feature, and the ability to enable/disable it.

image

The data/conf/rspamd/meta_exporter/pipe.php script will now check if the sender is a mailcow user, and will quarantine a message if the feature is enabled and the action is "reject".

Mail is stored per-recipient, similar to how incoming quarantined mail is processed. If the message being released (not via quick release) from quarantine is from a mailcow sender and a DKIM key exists for that domain, the message will be signed before being released. I have tested this with various email formats (plain text emails, emails with only html, multipart emails with text and html, emails with inline images and attachments) via a few mail clients (SOGo, BetterBird, Evolution), and all released mail appears to leave in its original state. The already included PhpMimeMailParser library was used to load raw message contents, select message body components (text/html/etc). New headers were constructed using the message metadata before signing.

I decided that leveraging the existing PHP tooling in this context made more sense than setting up an additional SMTP port to sign outgoing mail via rspamd being released from quarantine, as the existing quarantine port (590) does not use milters.

Bonus: Headers signed via PHPMailer with some data redacted

@milkmaker
Copy link
Collaborator

Thanks for contributing!

I noticed that you didn't select staging as your base branch. Please change the base branch to staging.
See the attached picture on how to change the base branch to staging:

check_prs_if_on_staging.png

@SnailShea SnailShea changed the base branch from master to staging January 21, 2024 08:35
@SnailShea
Copy link
Contributor Author

I forget to check for staging branch almost every time 🙃

@dragoangel
Copy link
Collaborator

@SnailShea I would add one thing to description: user still would think email was rejected and before releasing email admin should verify that user doesn't send email again.

@dragoangel
Copy link
Collaborator

dragoangel commented Jan 22, 2024

Also I am not sure if is okay to have the same limits per email (meaning we have limits to accept email per mailbox) here they are for opposite and maybe need to be handled separately? What I mean they have strongly increase db size if not be handled specially

@SnailShea
Copy link
Contributor Author

SnailShea commented Jan 23, 2024

These are very good ideas, thank you. I will work on them as soon as I get the chance.

Edit: I think the description about verifying user doesn't send mail again should go at the top of the quarantine page, maybe after "Blacklisted elements are excluded from the quarantine.". Does this seem good? I could put it in the settings as well, but I would either need to create a new text field over the top of settings, or add way too much text I think next to the added checkbox.

@dragoangel
Copy link
Collaborator

@SnailShea If implement special handling for outgoing mail - it can be dedicated section in quarantine settings and this info can be placed there optimally I think

@milkmaker
Copy link
Collaborator

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@milkmaker milkmaker added the stale Please update the issue with current status, unclear if it's still open/needed. label Apr 9, 2024
@milkmaker milkmaker closed this Apr 17, 2024
@FreddleSpl0it FreddleSpl0it reopened this Aug 13, 2024
@milkmaker milkmaker removed the stale Please update the issue with current status, unclear if it's still open/needed. label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants