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

[Feature] external_relay: add ip_map strategy #4537

Merged
merged 2 commits into from Jul 11, 2023

Conversation

moesoha
Copy link
Contributor

@moesoha moesoha commented Jul 11, 2023

Add a strategy named ip_map for finding relays by IP/network map.

Example config:

rules {
  EXTERNAL_RELAY_HOST {
    strategy = "ip_map";
    ip_map = [
      "2001:db8::/32",
      "192.0.2.0/24"
    ];
  }
}

This PR should not be squashed as it contains commits for two goals.

@moesoha moesoha force-pushed the relay/ip_map branch 2 times, most recently from 1787aa0 to b744baf Compare July 11, 2023 08:47
@fatalbanana
Copy link
Member

For testing it may be useful to set a lower priority (like 9?) on all rules in external_relay configuration; probably these tests are just barely hanging together as it is- sorry about it.

In general this LGTM.

@moesoha
Copy link
Contributor Author

moesoha commented Jul 11, 2023

For testing it may be useful to set a lower priority (like 9?) on all rules in external_relay configuration; probably these tests are just barely hanging together as it is- sorry about it.

In general this LGTM.

Oh I also noticed this problem, adjusting rules.

@moesoha moesoha marked this pull request as draft July 11, 2023 09:28
Copy link
Member

@vstakhov vstakhov left a comment

Choose a reason for hiding this comment

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

Looks good in general with some minor nits. Thank you!

end
end

rspamd_logger.errx(task, 'found nothing useful in Received headers')
Copy link
Member

Choose a reason for hiding this comment

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

Should it be a error notice? I think it should be either debug or info maximum. Users generally don't like too verbose logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

everywhere in this module uses errx for logging such message. Of course I can change all such conditions if you want.

for i, rcvd in ipairs(rcvd_hdrs) do
if rcvd.real_ip then
local rcvd_ip = rcvd.real_ip
if rcvd_ip:is_valid() and (not ip_map:get_key(rcvd_ip) or i == num_rcvd) then
Copy link
Member

Choose a reason for hiding this comment

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

This logic is a bit unclear. What is the overall goal of the check i == num_rcvd? It seems that it might break the exisiting logic of passing smtp envelope data to Rspamd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am wondering if a message is originated from a relay host, the real Received header decision will be wrong if we remove this.

@moesoha
Copy link
Contributor Author

moesoha commented Jul 11, 2023

I added a lower priority to EXTERNAL_RELAY_COUNT to fix the test cases. This is in a separate commit so this PR should not be squashed.

@moesoha moesoha marked this pull request as ready for review July 11, 2023 12:31
@vstakhov vstakhov merged commit 1cc0095 into rspamd:master Jul 11, 2023
1 check passed
@moesoha moesoha deleted the relay/ip_map branch July 11, 2023 19:23
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.

None yet

3 participants