NLnetLabs / domain

A DNS library for Rust.
https://nlnetlabs.nl/projects/domain/about/
BSD 3-Clause "New" or "Revised" License
332 stars 56 forks source link

Add support for netblocks to `CookiesMiddlewareProcessor`. #340

Open ximon18 opened 1 month ago

ximon18 commented 1 month ago

This PR adds support (via new dependency ipnetwork) for net blocks in the deny list of the cookie middleware processor, ala Unbound, otherwise the deny list is difficult to use beyond a few IP addresses.

partim commented 1 month ago

You might want to consider using inetnum rather than ipnetwork – and add things that are potentially missing.

ximon18 commented 1 month ago

You might want to consider using inetnum rather than ipnetwork – and add things that are potentially missing.

Nice idea, but inetnum seems to lack all of the needed functionality.

ximon18 commented 1 month ago

You might want to consider using inetnum rather than ipnetwork – and add things that are potentially missing.

Nice idea, but inetnum seems to lack all of the needed functionality.

My apoloiges @partim, of course it has what is needed, I just didn't see it as it is called Prefix rather than network or netblock etc.

I have updated this PR to use inetnum. I did note one regression compared to using ipnetwork which is that FromStr for Prefix is less accepting than that of ipnetwork as it will reject 127.0.0.1/24 as it has non-zero host bits. For end users however while stricter it still provides the functionality that's actually needed.

ximon18 commented 1 month ago

You might want to consider using inetnum rather than ipnetwork – and add things that are potentially missing.

Nice idea, but inetnum seems to lack all of the needed functionality.

My apoloiges @partim, of course it has what is needed, I just didn't see it as it is called Prefix rather than network or netblock etc.

I have updated this PR to use inetnum. I did note one regression compared to using ipnetwork which is that FromStr for Prefix is less accepting than that of ipnetwork as it will reject 127.0.0.1/24 as it has non-zero host bits. For end users however while stricter it still provides the functionality that's actually needed.

I've updated the PR again to use its own FromStr impl that handles the bare IP address case which FromStr in inetnum rejects as lacking a prefix len. @partim and I briefly discussed extending the support in inetnum but that snowballed into a larger discussion which is parked for the moment, instead this is the pragmatic step forward for now.