Skip to content
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 a decorator that normalizes the request duration to prevent enumeration #3190

Open
the-c0d3br34k3r opened this issue Sep 29, 2022 · 2 comments
Labels

Comments

@the-c0d3br34k3r
Copy link

For example, if we have an API /server/login and it takes on average ~3 ms for failure scenarios and ~7 ms for success scenarios, this would help an attacker enumerate different inputs and gauge the validity of fields (Maybe usernames) based on the response duration.

If we can have a decorator that will ensure that the API call takes at least the specified duration, this will render such enumeration techniques ineffective.
For eg:

@api_duration_in_ms(10)
def get(self):
    ...

The above code will ensure that the API takes at least 10ms before returning a response irrespective of success/failure.
This addition will be useful for administrative operations that aren't as frequent and where a small delay will not affect functionality.

Please help review the request and share your thoughts. If it sounds good, I can work on it :)

@the-c0d3br34k3r
Copy link
Author

@bdarnell Your feedback would be appreciated whenever possible :)

@bdarnell
Copy link
Member

bdarnell commented Oct 2, 2022

My initial reaction is that this doesn't seem like something I'd like to include in Tornado, because it's such an uncommon desire (I've never seen anyone try to mask timing side channels like this in practice). You can get a large part of it fairly easily (untested):

@contextlib.AsyncContextManager
async def min_duration(d):
    event = asyncio.Event()
    asyncio.get_event_loop.call_later(d, event.set)
    result = yield
    await event.wait()
    return result

# usage:
with min_duration(0.010):
    user  = lookup_user(...)
    check_permissions(user)
self.render(...)

But the problem with this is that it's easy to make it leaky: if you call methods like self.flush(), self.finish() or self.render() during the with min_duration block, they'll start writing out the HTTP response right away and leak the timing information you're trying to hide.

This seems messy to solve within RequestHandler. I think it's probably better to handle this at the level of the HTTPMessageDelegate since a wrapper of that object can intercept the HTTP output directly (without complexity related to separate error paths, etc). I think it's been possible to do this since the introduction of tornado.routing but I haven't tried to do it in practice (and it's been a long time since I've looked at the routing module). So that's where I'd point you to start:

  1. Implement a subclass of HTTPMessageDelegate that wraps an existing delegate (the real Application) and intercepts the output interfaces to add a delay.
  2. Implement tornado.routing.Router to wrap an existing router and wrap the returned HTTPMessageDelegate in your subclass.
  3. Add a top-level RuleRouter above your Application that contains rules for all the routes that should have a delay added.
  4. In the regular routes, the target is simply the Application. In the delayed routes, it's the Application wrapped by your Router from step 2.

@bdarnell bdarnell added the web label Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants