feat: Add whitelist functionality (#31) #35
Conversation
Adds a whitelist of IPs which bypasses the CIDR checks. This commit introduces a new `whitelist` directive to the configuration, allowing specific IPs to be excluded from blocking. The whitelist is checked before any CIDR matching. WIP: Still need to integrate with other middleware.
The middleware logic was inverted, causing requests from blocked ranges to be allowed and requests from allowed ranges to be blocked. This commit corrects the logic to ensure that requests from blocked ranges are blocked and requests from allowed ranges or whitelisted IPs are allowed.
…on (#31) * Fix inverted middleware logic that incorrectly handled blocked/allowed IP ranges * Refactor whitelist package to use netip.Addr for more efficient IP handling * Add comprehensive test coverage for whitelist functionality * Move whitelist code to dedicated package * Update config validation to handle default ranges properly * Rename WhiteList field to Whitelist for consistency
|
@aleho Would you mind checking this over pre-merge? |
Corrected the indentation of the `whitelist` directive within the `defender` block in the Caddyfile examples. This improves readability and consistency.
| @@ -112,10 +119,6 @@ func (m *Defender) Validate() error { | |||
| if m.responder == nil { | |||
| return fmt.Errorf("responder not configured") | |||
| } | |||
| if len(m.Ranges) == 0 { | |||
There was a problem hiding this comment.
If I understand correctly, the new behavior would be to use all ranges, right?
This should be documented in the README.
There was a problem hiding this comment.
Always been the behavior you can look at the provision step to see where it's set.
There was a problem hiding this comment.
I'll make a PR to document that behavior.
| @@ -112,10 +119,6 @@ func (m *Defender) Validate() error { | |||
| if m.responder == nil { | |||
| return fmt.Errorf("responder not configured") | |||
| } | |||
| if len(m.Ranges) == 0 { | |||
There was a problem hiding this comment.
By using all predefined ranges, wouldn't this include private and essentially block all access for most if ranges is omitted?
There was a problem hiding this comment.
Yes. It's not a perfect solution. If you have any suggestions for an alternative let me know
There was a problem hiding this comment.
Yes. It's not a perfect solution. If you have any suggestions for an alternative let me know
| @@ -112,10 +119,6 @@ func (m *Defender) Validate() error { | |||
| if m.responder == nil { | |||
| return fmt.Errorf("responder not configured") | |||
| } | |||
| if len(m.Ranges) == 0 { | |||
There was a problem hiding this comment.
Also, because that isn't entirely obvious to me, do the aws ranges include the aws_region ranges? If they overlap, maybe a list of clean default ranges would be better.
There was a problem hiding this comment.
Also, because that isn't entirely obvious to me, do the
awsranges include theaws_regionranges? If they overlap, maybe a list of clean default ranges would be better.
Hmm. This does seem like a better solution.
What do you suggest? All services that don't have overlap - private?
There was a problem hiding this comment.
Yes, that might be a good thing. A list referencing all the default IP ranges that are useful as an out-of-the-box setup.
E.g.: aws azurepubliccloud deepseek gcloud githubcopilot openai
There was a problem hiding this comment.
Okay okay 👌
If you want to add that into your documentation PR, I'll merge that in as well
There was a problem hiding this comment.
The PR is ready but includes private right now (to be correct).
There was a problem hiding this comment.
What do you mean by to be correct?
Didn't you say it wouldn't be beneficial to have private be in the defaults?
There was a problem hiding this comment.
I meant it includes private, to reflect the current state.
After your PR that would have to be changed, if you decided to remove private/local from the default rwnge.
There was a problem hiding this comment.
I meant it includes private, to reflect the current state.
After your PR that would have to be changed, if you decided to remove private/local from the default rwnge.
Yes, I removed it from the default range and updated the docs accordingly. You can see plugin.go#L23 for the list
Default the ranges to a smaller subset of ranges if none are specified. This avoids accidentally blocking large swaths of the internet if no ranges are specified. The new default ranges are: - aws - gcloud - azurepubliccloud - openai - deepseek - githubcopilot #35
Adds a whitelist of IPs which bypasses the CIDR checks.
closes #31
see commits/issue for context