monkey icon indicating copy to clipboard operation
monkey copied to clipboard

Switch to token-based authentication

Open VakarisZ opened this issue 3 years ago • 0 comments

Follow up to #2130

By-reference vs by-value tokens

The main libraries for session-based authentication are Flask-Login and more robust version of it Flask-Security. Although, based on this article setting up flask-login is not trivial for a setup such as ours and the author just ended up with JWT + blocklist approach. All things considered, I lean towards session-based approach, because it seems like a better fit for the use case of the project. Managing JWT blocklist could become a pain and a bottleneck, because for each agent run we'll have to create a JWT blocklist entry. Those entries will then have to be removed after the original JWT is outdated? example implementations

Token in authorization header

Another option to strongly consider is moving the session token to authorization header: https://flask-login.readthedocs.io/en/latest/#custom-login-using-request-loader This would provide unified interface for the UI and the agents. Also, this would prevent CSRF. The downside is the manual token handling (which we already do with JWT).

There are a couple of possible ways to handle this:

  1. Add authorization header: + Requests remain stateless + Faster, no need to keep fetching CSRF token - More custom code is required on the client side. We need to store the token in local storage and include it in requests

  2. Include CSRF token in all requests: + Less custom code, only hook all requests to fetch and include CSRF - Requests become stateful. This has potential to make unit/bb tests and agent communication harder

  3. Add custom header and use only json content type: + Easy - Stateful - Good enough?

All things considered I think solution 3 is the easiest. If we enforce the content type and custom header on the island side, it should be enough protection.

Edit: after discussion with @mssalvatore we decided to go with 1, because stateless API is easier to document and use for third parties and it's more orthodox solution.

More on CSRF protection

CSRF protection (add custom request headers. The CSRF is also irrelevant as long as we use application/json content type) Here's what Flask-security has to say about it: https://flask-security-too.readthedocs.io/en/stable/patterns.html#csrf

Prototype location

Working example with key changes are here. Registration/login/logout are working.

Tasks for session based token

  • [ ] Move user to database
    • [ ] Define user model (inherits UserMixin example model)
    • [ ] Define role model (inherits role mixin)
    • [ ] Create database user repository. It should return a user object (also inherits user mixin?)
    • [ ] Remove file user repository (users are now being stored in the database)
  • [ ] Change authentication service on the island
    • [ ] Setup flask-security (add MongoEngine connection, example here)
    • [ ] Implement registration
    • [ ] Implement login. Try using verify_and_update_password. Make sure verify_and_update_password is using salt correctly (basically make sure it does bcrypt.checkpw behind the scenes). Make sure login creates a user with the "user" role.
  • [ ] Change monkey/monkey_island/cc/ui/src/services/AuthService.js component. It needs to include Authentication-token header. Login and registration requests will also need specific query param to retrieve an authentication token.
  • [ ] Implement request loader to check for content type and custom header, like in the example of oauth
  • [ ] Fix custom PBA upload to include custom header or even better: use authentication service
  • [ ] Secure all endpoints with auth_token_required
  • [ ] Secure all user endpoints with roles_accepted decorator. Roles will allow us to segregate endpoints that are agent-only, user-only or both
  • [ ] Fix datastore encryptor to be unlocked on login

Note: These tasks will take more than expected because we won't resist the temptations to refactor the services and front end, which could use improvements.

VakarisZ avatar Aug 03 '22 11:08 VakarisZ