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

Issue with the token-per-page support for REST endpoint containing path parameters #146

Closed
forgedhallpass opened this issue Sep 14, 2020 · 13 comments

Comments

@forgedhallpass
Copy link
Collaborator

Example: /departments/{dept}/employees/{id}

The dept and id strings are dynamic path parameters. Creating page tokens for the whole path could rapidly lead to performance issues and might also defeat the reason of using page tokens, because an attacker could force the solution to revert to master token validation by providing unique identifier to path parameters. The severity of this problem is lower, because the attacker must have access to the master token.

Possible solution:

  1. make the (un)protected page options to support wildcards (e.g. /departments/.*/employees/.*)
  2. provide support for programmatic retrieval of (un)protected pages so that the definitions could be generated (e.g. using JEE/JAX-RS API or Swagger/OpenID support)
  3. this solution should be combined with Page tokens generated on first use are not sent back to the client #145 in a way that only page tokens for already accessed pages or page definitions (wildcard support) are returned to the UI to avoid disclosing all the available endpoints to the client
@jellisgwn
Copy link

jellisgwn commented Apr 14, 2021

hi, @forgedhallpass

We may have a peculiar use case, but the changes here have made it difficult / impossible for us to test resources which are protected by CSRFGuard.

Until 4.0.0 we were making a POST to JavaScriptServlet with the FETCH-CSRF-TOKEN header, and receiving back the master token, which could be used in subsequent requests. The test environment cannot execute Javascript, which is what forced us to directly retrieve the token.

Such testing (via frameworks with Javascript execution support) is presumably common, and i'm not sure how they can now be supported...

Edit: there is an interesting comment on this commit:

https://github.com/forgedhallpass/OWASP-CSRFGuard/commit/3e1991516d0d81238b77f74eb60bdf9bb067a880

if used token rotation is enabled, the new master token is returned in the response header

this does not seem to be the case, but if it was it might be enough for us to adapt tests to use.

@forgedhallpass
Copy link
Collaborator Author

hello @jellisgwn,

Until 4.0.0 we were making a POST to JavaScriptServlet with the FETCH-CSRF-TOKEN header, and receiving back the master token, which could be used in subsequent requests.

You can still mimic the same behavior in a slightly different manner:

  1. Do a GET request against the JavaScriptServlet
  2. Extract the master token value from the response
  3. Do a POST request using the extracted master token value to receive the current list of page tokens

The test environment cannot execute JavaScript, which is what forced us to directly retrieve the token.

Could you please elaborate on this? You mean you want to create automated (integration) tests that are not executed through a browser? Normally if you don't have a browser client, you don't need CSRF support, hence you could just disable CSRFGuard for the back-end (module) integration tests. For front-end/e2e test scenarios you might want to consider using Selenium or other frameworks that execute JavaScript through WebDriver or a similar technology.

this does not seem to be the case, but if it was it might be enough for us to adapt tests to use.

The customizable user-properties and their relations/valid combinations are mostly documented in the csrfguard.properties.

There is a sample test application intended for testing different combinations of such properties. You can build it yourself, or download the pre-built WAR from the Maven Central repository. If you find a combination of parameters that do not work together as expected, please create an issue, attach the used configurations and describe the steps so I can try to reproduce it.

The project has been moved to its official location, please use that one in the future.

@jellisgwn
Copy link

Hi, @forgedhallpass

Yep, i was trying to avoid having to do step 2, but most likely that is what it's going to take.

We have a bunch of tests using a fairly lightweight framework:

https://github.com/rest-assured/rest-assured

There are indeed lots of other ways to do the testing but these are the tests that we have :)

Ok, new issues in the new place. As these issues were all still open it seemed as good a place as any.

@forgedhallpass
Copy link
Collaborator Author

Yep, i was trying to avoid having to do step 2, but most likely that is what it's going to take.

You won't be able to do skip this step. The purpose of this approach is to make stealing page tokens harder.

Basically you are trying to mimic/mock the front-end logic of CSRFGuard. Extracting the master token from the JavaScript is pretty easy to implement using a regex, but depending on your requirements, certain parameter combinations might require additional (more complex) logic.

All in all, what you are doing is not a good practice, because most probably you will end up in similar situations every time a 3rd party library you are using is being modified.

Normally:

  • You should ONLY test your own code (for which you can disable the CSRF protection)
  • The 3rd party library (CSRFGuard in this case) should be tested in its own repository (you are more than welcome to help improve the test coverage either by writing new tests or by "running" and documenting manual test scenarios via the test application)
  • If you want to test the integration between your code and CSRFGuard, you'd need to use an approach/framework like mentioned in the previous comment.

@jellisgwn
Copy link

My only input here was that the tokens were returned from the POST prior to 4.0.0.

@forgedhallpass
Copy link
Collaborator Author

My only input here was that the tokens were returned from the POST prior to 4.0.0.

Yes, and I have tried to explain it to you:

  • the reason why it has been changed
  • what changes would you need to do in order to make your approach work with 4.0
  • how could you do it better

I hope I was able to make myself clear. Let me know if you run into problems :)

@jellisgwn
Copy link

All clear.

@forgedhallpass
Copy link
Collaborator Author

@Vidhi-123
Copy link

Vidhi-123 commented Dec 17, 2021

hello @jellisgwn,@forgedhallpass

Until 4.0.0 we were making a POST to JavaScriptServlet with the FETCH-CSRF-TOKEN header, and receiving back the master token, which could be used in subsequent requests.

You can still mimic the same behavior in a slightly different manner:

  1. Do a GET request against the JavaScriptServlet
  2. Extract the master token value from the response
  3. Do a POST request using the extracted master token value to receive the current list of page tokens

The test environment cannot execute JavaScript, which is what forced us to directly retrieve the token.

Could you please elaborate on this? You mean you want to create automated (integration) tests that are not executed through a browser? Normally if you don't have a browser client, you don't need CSRF support, hence you could just disable CSRFGuard for the back-end (module) integration tests. For front-end/e2e test scenarios you might want to consider using Selenium or other frameworks that execute JavaScript through WebDriver or a similar technology.

this does not seem to be the case, but if it was it might be enough for us to adapt tests to use.

The customizable user-properties and their relations/valid combinations are mostly documented in the csrfguard.properties.

There is a sample test application intended for testing different combinations of such properties. You can build it yourself, or download the pre-built WAR from the Maven Central repository. If you find a combination of parameters that do not work together as expected, please create an issue, attach the used configurations and describe the steps so I can try to reproduce it.

The project has been moved to its official location, please use that one in the future.

Hey, I'm trying to upgrade csrfGuard from 3.1.0 to 4.0.0. i tried to follow above 3 steps you given but i didn't get token back from last step which is post call. i just get empty object in pageToken. response preview {pageToken:{}};

@forgedhallpass
Copy link
Collaborator Author

Hello @Vidhi-123,

What exactly do you want to achieve?

Also note that the official repository is at: https://github.com/OWASP/www-project-csrfguard

@Vidhi-123
Copy link

hello @forgedhallpass,

I want to upgrade csrfguard jar from 3.1.0 to 4.0.0 , i have followed the same as given in https://github.com/OWASP/www-project-csrfguard. when i try to login my master token has generated , then i extract that token from javascriptservlet and try to make POST call for javascriptServlet to get page token but i got empty object in that.

@Vidhi-123
Copy link

Vidhi-123 commented Dec 17, 2021 via email

@forgedhallpass
Copy link
Collaborator Author

forgedhallpass commented Dec 17, 2021

I want to upgrade csrfguard jar from 3.1.0 to 4.0.0

You said upgrade, and the question from the previous user was about mimicking/replicating the JavaScript logic for integration testing purposes. These are two different topics, and the latter was answered above.

I'll also repeat, this is NOT the official OWASP CSRF Guard repository!

The new CSRF Guard dependencies are described here: https://github.com/OWASP/www-project-csrfguard#using-with-maven. There is also a test application that is configured to use the latest version of the solution. You can use that as an example and for testing. The details on how to build it can be found on the readme page's "Building the code section".

Relevant discussion: OWASP/www-project-csrfguard#43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants