unosquare / embedio

A tiny, cross-platform, module based web server for .NET
http://unosquare.github.io/embedio
Other
1.47k stars 176 forks source link

Refactor IPBanningModule #438

Open rdeago opened 4 years ago

rdeago commented 4 years ago

Leaving aside some implementation problems, some of which already addressed in recent commits, IPBanningModule could greatly benefit from refactoring, extending the scope of banning criteria so that a client may be banned, for example, based on their identity, or user agent, or whatever one fancies.

This implies a number of breaking changes, removing "IP" from the name of some classes, as well as changing the criterion interface to accept an HTTP context instead of an IP address.

Then there are (or better, there aren't) white and black lists. Currently, IPBanningConfiguration defines BlackList as the list of currently banned IP addresses. This is not what a black list is: a black list is a list of IP addresses that are always banned, as well as a white list is a list of addresses that shall not be banned. These features are currently missing, albeit being very useful especially in the same environments where IP banning works best (namely, private networks).

Currently, black and white lists may be implemented (sort of) using regular expressions with IPBanningRegexCriterion, but addressing IP ranges with regexes can become messy (not to mention very slow) quite quickly.

A better implementation of white and black lists (as well as IP banning criteria) should ideally use IP address ranges, a concept that .NET has no built-in support for. Instead of reinventing the wheel, we could use this library by Junichi Sakamoto, but it would add the burden of an additional dependency to all EmbedIO users. I propose that the refactored banning module be moved to EmbedIO.Extras and given its own NuGet package.

I'd also like to leave the wheel-reinventing option on the table, since I'm not exactly in love with the above-mentioned library, my main concern with it being that it represents address ranges with mutable objects. We could incorporate a modified version of it in SWAN, as the license (MPL 2.0) seems to allow for it.

rdeago commented 4 years ago

After a good look at the IPAddressRange library I've decided to roll my own, partially based on existing CidrAddress and CidrSubnet classes I wrote some time ago.

Needless to say, it will come as a PR to Swan.

Without the additional dependency, even the need to move the banning module to Extras is gone.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.