Lmtp ratelimit #5032
Open
Lmtp ratelimit #5032
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brong
force-pushed
the
lmtp-ratelimit
branch
from
3883ac5
to
f4ecbc9
Compare
|
Sadly there's no test cases for maxlogins in general, and it's particularly hard to test for this case, because it only limits while the lmtp delivery itself is happening, so you'd need to hold a lock on the user's conversationsdb! |
rsto
approved these changes
Sep 12, 2024
rsto
reviewed
Sep 12, 2024
brong
force-pushed
the
lmtp-ratelimit
branch
from
f4ecbc9
to
7e28426
Compare
brong
force-pushed
the
lmtp-ratelimit
branch
from
7e28426
to
1fc4495
Compare
|
I rebased and merged in some patch code for the return code - I had originally had it coming from lib/proc.c but we don't have IMAP errors there, and when I removed that bit I forgot to add it to the caller. That's now fixed :) |
brong
force-pushed
the
lmtp-ratelimit
branch
from
1fc4495
to
d94a10c
Compare
brong
force-pushed
the
lmtp-ratelimit
branch
from
d94a10c
to
511585c
Compare
elliefm
reviewed
Sep 13, 2024
This means that somebody with 5 imap connections won't be locked out from pop, but more importantly won't block lmtp delivery once we add that as a limitable service.
The sends a sensible 4xx response back to the user, with an error message saying which limit was hit
brong
force-pushed
the
lmtp-ratelimit
branch
from
f2c2f2f
to
71cf8ef
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We're running into issues in Fastmail production where a single mailbox locked up for a crazy amount of time can cause starvation by having hundreds of lmtp processes waiting on locks.
We could test for locks, but it's OK to have a few waiting - we should just tell the rest to try again later. This re-uses the
proc_checklimit in lmtpd to check the limits for each userid before trying to lock their conversations.db, and returns a useful 451 code to the sender saying that there are too many connections for this user.
We'll probably set something like a 10 process limit for our servers, which allows for all the MXes to send one and a few spares still open.