Mailing List Archive

[Bug 428] Greatly expand ratelimit features
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428





------- Comment #1 from graeme@graemef.net 2007-06-19 00:39 -------
Created an attachment (id=198)
--> (http://www.exim.org/bugzilla/attachment.cgi?id=198&action=view)
Attempt #1 to implement "/ noupdate" for ratelimit ACL tests

Attempt #1 to implement "/ noupdate" for ratelimit ACL tests.

Marc's rabid insistence over all things "great" made me go and read the code,
and it doesn't appear so difficult to modify I thought I'd have a crack at it.

This patch *almost* works (against 4.67).

Usage:

warn ratelimit = 10 / 1d / leaky|strict [/ noupdate] [/ key]

The key, if defined, must still be the last item on the line. Note that if the
lookup key (by default "$sender_rate_period / strict|leaky [/
sender_host_address]" *does not exist* in the ratelimit DB then the test will
return false. Something *must* set it for "/ noupdate" to look it up.

Sadly, if a "noupdate" ratelimit lookup/test is the first one in the config
file for a given key, it seems to hold true for all subsequent ones. If it's
the second one, then the first does an update (as expected) and the second
simply looks up the result, as per desired requirement.

I'm buggered if I can get the thing to work both ways. I appear to be in a
logic hole, and my code is probably so poor that Tom and Phil will chuckle at
it and throw it away :)

Anyway, it's a start. Maybe someone else (Marc?) can pick it up and run with
it.

Graeme

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428


graeme@graemef.net changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #198|Attempt #1 to implement "/ |Obsolete patch
description|noupdate" for ratelimit ACL |
|tests |
Attachment #198 is|0 |1
obsolete| |



--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428





------- Comment #2 from graeme@graemef.net 2007-06-19 10:07 -------
Created an attachment (id=199)
--> (http://www.exim.org/bugzilla/attachment.cgi?id=199&action=view)
Working patch for "/ noupdate" feature

Working patch. Usage examples to follow.

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428


graeme@graemef.net changed:

What |Removed |Added
----------------------------------------------------------------------------
Version|4.63 |4.67




------- Comment #3 from graeme@graemef.net 2007-06-19 10:54 -------
Usage for ratelimit "/ noupdate" option:

In relevant ACLs, it's possible to lookup the existence of a specified (or
auto-generated) ratelimit key without incrementing the ratelimit counter for
that key.

Syntax:

warn ratelimit = rate / period / strict|leaky [/ noupdate] [/ options] [/ key]

In order to make use of it, another ACL entry *must* set the rate somewhere (or
the noupdate option is useless). In this case, the "/ per_cmd", "/ per_mail" or
"/ per_rcpt" options must be used or the previously precomputed rate from the
"/ noupdate" line will be used and no update will be done.

Example:

acl_check_connect:

# Read the rate; if it doesn't exist or is below the maximum
# we update it below
deny ratelimit = 100 / 5m / strict / noupdate
log_message = RATE: $sender_rate / $sender_rate_period (max
$sender_rate_limit)

[... some other logic and tests...]

warn ratelimit = 100 / 5m / strict / per_cmd
log_message = RATE UPDATE: $sender_rate / $sender_rate_period (max
$sender_rate_limit)
condition = ${if le{$sender_rate}{$sender_rate_limit}}

accept


In order to make proper use of this, it is important to set an appropriate key
as the last part of the ratelimit function. The precomputed key is of the form:

$ratelimit_period / strict|leaky / $sender_host_address

This is useful for the SMTP "connect" ACL, and perhaps others; to make use of
(for example) per-recipient ratelimits then it will be worth creating a
specific key which includes the relevant detail:

deny ratelimit = 100 / 5m / strict / per_rcpt / $local_part@$domain
message = This recipient is receiving messages too quickly.\
Please try again later.

===============================================================================

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428


graeme@graemef.net changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |graeme@graemef.net




------- Comment #4 from graeme@graemef.net 2007-06-19 10:56 -------
If interested parties would like to try this patch out to see if it works, that
would be very helpful. It works for me, but I have no real usage for it on any
systems over which I have control.

Graeme

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428


graeme@graemef.net changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #199|text/x-patch |text/plain
mime type| |
Attachment #199 is|0 |1
patch| |



--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428





------- Comment #5 from marc@perkel.com 2007-06-19 23:28 -------
Thanks for taking on this project Graeme. I think it will be a very useful
feature.

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428


graeme@graemef.net changed:

What |Removed |Added
----------------------------------------------------------------------------
AssignedTo|ph10@hermes.cam.ac.uk |graeme@graemef.net



--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428


graeme@graemef.net changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED




------- Comment #6 from graeme@graemef.net 2007-06-20 09:55 -------
(In reply to comment #0)
> Ratelimiting is great. It should be expanded to the point where greylist logic
> can be written using just Exim ACLs. It needs:
>
> The ability to increment a counter without returning a true or false result to
> the calling ACL.

You can already do this by simply using a "warn" acl verb preceding the
"ratelimit" command. The returned result is irrelevant - all it dictates is
what happens to the following modifier(s); if the result is "true" then the
modifiers are actioned, if it is "false" then they are skipped. In both cases
control passes to the next ACL statement.

> The ability to test a count without incrementing the count.
>
> This would allow one ACL to count and a different ACL to test. Would be a huge
> improvement.

Please test the patch.

Graeme

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428





------- Comment #7 from ph10@hermes.cam.ac.uk 2007-06-20 12:06 -------
There seem to be some anomalies, both in the patch and in the original code.
The original includes /per_xxx in the key, but only if it is explicitly given.
So the default is lacking it. Also, although per_cmd and per_rcpt are supposed
to be synonymous, they are used as specified in the config, so could create two
different keys. A bug in the new code makes it not quite compatible with the
old: you insert "/ strict /" for example in the key, but the old code, since it
uses what's given by the user, might have "/strict /" or even "/strict/". I
propose to clean all this up and generate standardised keys. It will mean that
old behaviour might be forgotten on an Exim upgrade, but I don't think this is
a huge problem.

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428





------- Comment #8 from graeme@graemef.net 2007-06-20 12:11 -------
On Wed, 2007-06-20 at 12:06 +0100, ph10@hermes.cam.ac.uk wrote:
> ------- Comment #7 from ph10@hermes.cam.ac.uk 2007-06-20 12:06 -------
> There seem to be some anomalies, both in the patch and in the original code.
> The original includes /per_xxx in the key, but only if it is explicitly given.
> So the default is lacking it. Also, although per_cmd and per_rcpt are supposed
> to be synonymous, they are used as specified in the config, so could create two
> different keys. A bug in the new code makes it not quite compatible with the
> old: you insert "/ strict /" for example in the key, but the old code, since it
> uses what's given by the user, might have "/strict /" or even "/strict/". I
> propose to clean all this up and generate standardised keys. It will mean that
> old behaviour might be forgotten on an Exim upgrade, but I don't think this is
> a huge problem.

Thanks for looking, Phil.

Please note Bug #496 - spaces in the key cause exim_tidydb not to
process the ratelimit DB properly.

I don't believe that "forgetting" the ratelimit DB on an upgrade will
matter greatly, either *unless* people start to use it for greylisting
in which case the docs will have to caveat/note the changes greatly.

Graeme

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428


ph10@hermes.cam.ac.uk changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution| |FIXED




------- Comment #9 from ph10@hermes.cam.ac.uk 2007-06-20 15:19 -------
(In reply to comment #8)
> Please note Bug #496 - spaces in the key cause exim_tidydb not to
> process the ratelimit DB properly.

My new keys don't have spaces (though I guess the user's bit might have.)

> I don't believe that "forgetting" the ratelimit DB on an upgrade will
> matter greatly, either *unless* people start to use it for greylisting
> in which case the docs will have to caveat/note the changes greatly.

Good. I've committed my version of the patch (and done some simple tests). I'll
also upload the patch here. One issue: the code keeps the rate in main memory
for re-use in the same process. It was always updating this copy; I've made it
not do so for /noupdate. But it still does it even when it doesn't update the
database because it's leaky. I wonder if it should only update the in-memory
copy when it updates the database?

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
[Bug 428] Greatly expand ratelimit features [ In reply to ]
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

http://www.exim.org/bugzilla/show_bug.cgi?id=428


ph10@hermes.cam.ac.uk changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #199 is|0 |1
obsolete| |




------- Comment #10 from ph10@hermes.cam.ac.uk 2007-06-20 15:21 -------
Created an attachment (id=200)
--> (http://www.exim.org/bugzilla/attachment.cgi?id=200&action=view)
Modified patch for /noupdate

--
Configure bugmail: http://www.exim.org/bugzilla/userprefs.cgi?tab=email

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##