Basic BCP 38 support - #2126
Open
zbalkan wants to merge 120 commits into
Open
Conversation
Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
Member
|
Thanks for the PR. This will require studying things in details which will take a lot of time. Support for DNS cookies is not immediately planned as there are several other things that are planned to be implemented before it. Thus, I am not sure when I can take look at this topic and come back to this PR. |
Contributor
Author
|
Thanks for the information. Since I incidentally built response rate limiting during DNS cookie development, and then split it, they are decoupled. If you believe it's valuable regardless of cookies, I can try to work on it as a totally separate feature, but it will take time. |
This file contains hidden or 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
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.
tl;dr: Moved RRL to a separate PR from #1737. It means this PR depends on it and TechnitiumSoftware/TechnitiumLibrary#56.
UDP DNS Response Rate Limiting
Problem
The Cookie admission limiter is keyed on the source and runs before the query, so it cannot distinguish a cheap
NXDOMAINfrom a large signed answer. A source staying under its admission budget can still pick the responses with the best amplification ratio, and repetition - the actual signature of a reflection flood - is invisible to a limiter that counts requests rather than what they produce.Solution
A second limiter on the finished UDP response, immediately before transmission, keyed on
(client network, response identity). Responses are grouped so repetition shows up even when queries differ:NXDOMAINNxDomainNOERRORErrorNOERROR, no answer,NSin authorityReferralNOERROR, no answerNoDataRRSIGin answerQueryQueryZeroing the type collapses a type-sweep of one name; keying on the delegation owner collapses a random-subdomain flood. Each response is also checked against an
Allbucket, so spreading across classes is still bounded; the stricter result wins.Disabled by default. When enabled: 100 responses/second sustained, 200 instant, slip every 2, 65536 entries, empty bypass list. Clients tracked at /32 and /24, /128, /64 and /56.
Decisions
AddressPrefixandSipHash24. Merging them would couple the two features for no behavioural gain.DnsResponseRrlRequestTrustenum. The limiter files name no Cookie type; the single mapping happens at the call site.BADCOOKIEand QPM truncations are signals the server chose to emit; counting them would let the Cookie bootstrap path exhaust the budget protecting it.TCwould be the amplification this exists to prevent.Volatile.Write; the previous generation survives a failed rebuild, and history is preserved unless the change actually invalidates it.TechnitiumLibrary.Net.master⊂ Cookies ⊂ this PR - with end-of-stream guards, so no migration.No existing API changes: public signatures, properties, HTTP endpoints and settings fields were diffed against
master, none removed or altered.Testing
DnsServerCoreandDnsServerAppbuild clean. The solution build fails onDnsServerSystemTrayAppon Linux, pre-existing and unrelated.There is no test project in the repository, so nothing covers class assignment, slip eligibility, the error leak, the Cookie exemption, or preserve-vs-rebuild on settings changes. That is the main gap for review, along with
BuildResponseIdentity, where wrong grouping is either a bypass or a false positive on legitimate traffic.