Skip to content

Rspamd: improve SPF, DKIM and DMARC Symbol Weights#3913

Merged
georglauterbach merged 8 commits intomasterfrom
rspamd/spf-dkim-dmarc-symbols
Mar 2, 2024
Merged

Rspamd: improve SPF, DKIM and DMARC Symbol Weights#3913
georglauterbach merged 8 commits intomasterfrom
rspamd/spf-dkim-dmarc-symbols

Conversation

@georglauterbach
Copy link
Copy Markdown
Member

@georglauterbach georglauterbach commented Feb 29, 2024 •
edited
Loading

Description

I updated the symbol weights according to my new insights in #3690 to fix a bug pointed out by @2GetApp and to improve the logic itself.

Previously, I reasoned about combinations of symbols that cannot exist, e.g., SPF allow, DKIM allow, DMARC reject. Removing these symbols and then reasoning about the rest is more appropriate.

Moreover, I added DMARC_POLICY_NA and DMARC_POLICY_SOFTFAIL to the whole calculation.

The issue description of #3690 I updated. I also added the Rust code I
used to do and verify the calculations.

Please review commit by commit.

Successor of: #3726
Fixes #3690

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change that does improve existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary, I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have added information about changes made in this PR to CHANGELOG.md

@georglauterbach
move symbols to better places and add comments
32b9bd8 
The order was chosen because it will reflect an ascending symbol weight with upcoming commits. I also added comments to explain when a symbol is applied.
@georglauterbach
add comment on RFC7489
2167ae9
@georglauterbach
update symbols weights according to updates in #3690
11e6094 
I updated the symbol weights according to my new insights in #3690 to
fix a bug pointed out by @2GetApp and to improve the logic itself.
Previously, I reasoned about combinations of symbols that cannot exists,
e.g., SPF allow, DKIM allow, DMARC reject. Removing these symbols and
then reasoning about the rest is more appropriate.

Moreover, I added `DMARC_POLICY_NA` and `DMARC_POLICY_SOFTFAIL` to the
whole calculation.

The issue description of #3690 I updated. I also added the Rust code I
used to do and verify the calculations.
@georglauterbach
add entry to CHANGELOG
05b5336
@georglauterbach georglauterbach self-assigned this Feb 29, 2024
@georglauterbach georglauterbach added kind/improvement Improve an existing feature, configuration file or the documentation area/configuration (file) service/security/rspamd kind/bug/fix A fix (PR) for a confirmed bug labels Feb 29, 2024
@georglauterbach georglauterbach added this to the v14.0.0 milestone Feb 29, 2024
@georglauterbach georglauterbach force-pushed the rspamd/spf-dkim-dmarc-symbols branch from 11fef33 to 9d828c5 Compare February 29, 2024 15:40
polarathene
polarathene requested changes Feb 29, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't comment on the actual config changes themselves, but here are some suggestion for possible improvements 👍

I noticed the changelog entries omitting the # prefix a few times, should probably correct that.

Comment thread docs/content/config/security/rspamd.md Outdated
Comment thread target/rspamd/scores.d/policies_group.conf Outdated
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
@georglauterbach
Copy link
Copy Markdown
Member Author

georglauterbach commented Mar 1, 2024

I have slightly adjusted the suggestion on the warning admonition you posted @polarathene. I hope you like it :)

@georglauterbach
Copy link
Copy Markdown
Member Author

georglauterbach commented Mar 1, 2024 •
edited
Loading

Oh my, what in the world happened to the links on the Rspamd page???

EDIT: I corrected this mishap in f3bfe32. Better merge fast, this is ludicrous.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 1, 2024

Documentation preview for this PR is ready! 🎉

Built with commit: d327f5c

@polarathene
Copy link
Copy Markdown
Member

polarathene commented Mar 1, 2024

Oh my, what in the world happened to the links on the Rspamd page???

EDIT: I corrected this mishap in f3bfe32. Better merge fast, this is ludicrous.

That was due to my review suggestion being applied from a prior PR, but getting approved without the corresponding change through the rest of the document 😓

My bad, I should really try find time to revive my old work on integrating lychee into the CI, although in this case it seems we'd need a markdown lint like this one to catch this sort of mistake slipping through review 🤔

@georglauterbach
Copy link
Copy Markdown
Member Author

georglauterbach commented Mar 2, 2024

I actually found the issue because of the line I use in VS Code (that you also linked to @polarathene), but I'm afraid this would cause a bunch of work when implemented as your MD may not conform very well with the standards..

@georglauterbach georglauterbach merged commit 12f5101 into master Mar 2, 2024
@georglauterbach georglauterbach deleted the rspamd/spf-dkim-dmarc-symbols branch March 2, 2024 01:42
@georglauterbach georglauterbach mentioned this pull request Mar 4, 2024
Merged
8 tasks
AlMaVizca added a commit to AlMaVizca/docker-mailserver-helm that referenced this pull request Jun 20, 2024
@AlMaVizca
Add RSPAMD_NEURAL env
956ecfc 
PR - docker-mailserver/docker-mailserver#3833
Also mentioned in
docker-mailserver/docker-mailserver#3913

Signed-off-by: Aldo Maria Vizcaino <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@casperklein casperklein casperklein approved these changes

@polarathene polarathene polarathene approved these changes

Assignees

@georglauterbach georglauterbach

Labels

area/configuration (file) kind/bug/fix A fix (PR) for a confirmed bug kind/improvement Improve an existing feature, configuration file or the documentation priority/high service/security/rspamd

Projects

Rspamd Integration
Archived in project

Milestone

v14.0.0

Development

Successfully merging this pull request may close these issues.

Rspamd: provide RFC7489 compliance for SPF, DKIM & DMARC

3 participants

@georglauterbach @polarathene @casperklein