-
-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for netblocks to CookiesMiddlewareProcessor
.
#340
base: main
Are you sure you want to change the base?
Conversation
…s test pass: - Allow requests with invalid cookies to proceed if they are authenticated or not required to authenticate. Improvements to the Stelline server test support needed by the Stelline cookies test: - Advance mock system time in the Stelline server tests. - Move Stelline server tests under src/ to permit #cfg(test) based swap out of real system time for mock system time. - Use the thread_local version of mock_instant to ensure parallel mock time dependent tests don't interfere with each other (such tests also use tokio::time which only works if they run in a single thread). - Set mock system time to start at zero for each Stelline server test (as expected by the cookies .rpl test script). - Pass the IP address of the test client to the server so that the cookies middleware can match it against its deny list. Other: - Add support for net blocks in the IP deny list of the cookie middleware processor, ala Unbound, otherwise the deny list is difficult to use beyond a few simple speciifc IP addresses. - Enable tracing log output in the server Stelline tests. - Move unit server tests and Stelline server tests together under src/net/server/tests/. - Additional logging.
…s on never type fallback being `()`
…s on never type fallback being `()`
…use a task based waker instead of a thread based waker for scenarios where CPU resource is low such as in GH Actions (or when simualted with taskset -c 0).
This reverts commit dd226dc.
This reverts commit 5f522f6.
This reverts commit 35ca1c9.
…ate PR." This reverts commit ca0f08a.
You might want to consider using inetnum rather than |
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 I have updated this PR to use |
…es without a prefix length.
I've updated the PR again to use its own |
/// If the given IP address is on our deny list it is required to | ||
/// authenticate itself. | ||
fn must_authenticate(&self, ip: IpAddr) -> bool { | ||
self.deny_list.iter().any(|netblock| netblock.contains(ip)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might not be very efficient for larger numbers of blocks. Maybe use (optionally, i.e. behind a feature gate) https://docs.rs/rotonda-store/latest/rotonda_store/struct.MultiThreadedStore.html here?
Also, looking up configuration based on incoming IP address is also needed for handling incoming XFR requests, so maybe this functionality should be moved to a common module under src/net/
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this a bit more, the lookup result when using a simple vec of netblocks is dependent on the order they are added, so it's not just about efficiency, a proper prefix store will also return the best match rather than just the first match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There’s probably also going to be rate limiting code that needs to work on prefixes, so having something more general may be useful. Would it make sense to do something generic here? Or are we happy to settle on rotonda-store at least under the hood?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this needs turning around like was done for XFR. For XFR the code used to apply access control but there was quite a lot of configuration and exactly how you want to restrict access is perhaps not something we should decide here. So for XFR I moved the checking out to an XfrDataProvider
trait that the middleware uses a user supplied impl of. The only sample impls of XfrDataProvider
actually don't do any access control at all, that's left to the end user. @partim: Should we maybe do the same here?
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.