pageTokenSynchronizationTolerance check looks wrong

[bpapez]

I wanted to upgrade csrfguard from 3.1.0 to 4.1.1 in order to activate TokenPerPage in our application. We have a page, where on initialization three AJAX POST requests are sent to the same URL (with different Payload). Our automated test was failing after activating TokenPerPage. On debugging I found that the following method does not match the description:

www-project-csrfguard/csrfguard/src/main/java/org/owasp/csrfguard/token/service/TokenService.java

Lines 282 to 288 in fca27e9

	* @return true if the page token creation is within the configured time tolerance and the application should also accept the master token for validation
	*/
	private boolean initIsWithinTimeTolerance(final CsrfGuard csrfGuard, final boolean isAjaxRequest, final PageTokenValue tokenTimedPageToken) {
	return isAjaxRequest
	&& !csrfGuard.isTokenPerPagePrecreate()
	&& tokenTimedPageToken.getCreationTime().plus(this.csrfGuard.getPageTokenSynchronizationTolerance()).isBefore(LocalDateTime.now());
	}

I think that isBefore should be replaced with isAfter. To return true the current time should be before the sum of token creation time and tolerance, but currently its exactly the opposite.

[forgedhallpass]

Hello @bpapez,

Indeed, thank you for pointing it out. You can provide a PR if you want to, otherwise will make the change myself.

[bpapez]

Thanks for the quick response. I created the PR.
Are there already plans when a new release will be done? (just to know whether we will be able to package an official release instead of a patched one)

[forgedhallpass]

We don't have any plans yet. When would you like to do your release? I'd say give it some testing, make sure that the solution works fine with your set of configurations/requirements (there can be a lot of combinations and potential corner-cases) and then if all goes well, we can create a new patch version for you.

[bpapez]

Thank you, sounds like a good plan. We still need to do some enhancement work, because our app can work on a Tomcat cluster and in that case we don't use the standard Tomcat sessions, but Spring sessions and we need to be able to continue working when a server goes down, so the load balancer then directs those sessions to other running servers. So we probable need to replace the SessionTokenKeyExtractor and InMemoryTokenHolder.

Anyway, I still have a problem with the three AJAX requests sent on page initialization. Now with the fix in place it works fine on the first access to the page. However when clicking the reload button on the browser, we run into issues, because now pageTokens would exist, but the request header still sends the masterToken.

I see in the debugger that when handleDynamicallyCreatedNodes handles the url of that AJAX request, the /CsrfServlet has returned the current pageTokens in the response, but it has not yet been parsed, so pageTokenWrapper.pageTokens is still empty and thus these three AJAX urls triggered in the init all are sent with the master token and fail. How is this supposed to work correctly? Is there anything I need to adapt? Should we move this comment to a new discussion or issue?

[forgedhallpass]

You could try making sure that the csrfguard.js logic is executed first.

[bpapez]

Currently the request/response flow when loading the page looks like this:

URL	Start Time	End Time	Descriptions
GET settings.html	0,09 s	0,16 s	Main HTML page
GET CsrfServlet	0,23 s	0,23 s	returns the csrfguard.js
POST CsrfServlet	0,45 s	0,46 s	triggered by requestPageTokens - returns pageTokens (empty on first access)
POST settingsAction.do	0,45 s	0,46 s	AJAX call triggered by AngularJS - sends masterToken and gets pageToken for settingsAction.do in return

On page reload (or navigating to other page and coming back)

URL	Start Time	End Time	Descriptions
settings.html	15,70 s	15,70 s	Main HTML page
GET CsrfServlet	15,70 s	15,70 s	returns the csrfguard.js
POST CsrfServlet	15,71 s	15,71 s	triggered by requestPageTokens - returns pageTokens (same as previously returned from settingsAction.do)
POST settingsAction.do	15,71 s	15,71 s	AJAX call triggered by AngularJS - still sends masterToken and returns a 302 error

So the problem is that the POST request to /CsrfServlet receiving the pageTokens does not yet have xhr.readyState == 4 (DONE) to get the result parsed, when already the POST request to settingsAction.do is sent.
Do we have to change all the pages, where this is happening to explicitly wait for the csrfguard POST request to be finished, before sending the other AJAX requests? Is there no other seamless way to force that?

[forgedhallpass]

Hello @bpapez,

It is a bit hard to suggest a solution for a scenario I don't fully see/understand. There are multiple ways in which this could potentially be solved, but you'd might need to analyze a couple of things like:

• is it necessary for the front-end to auto-submit a request against the settings endpoint?
• does that request need to be a POST request? If not, and you are not making state changing operations with e.g. GET requests, you could just simply mark the GET HTTP verb/method as "unprotected". See:

  www-project-csrfguard/csrfguard/src/main/resources/csrfguard.properties

  Line 72 in e95aedf

  ## Protected Methods ##

• there are options to mark protected/unprotected pages depending on your needs. See:

  www-project-csrfguard/csrfguard/src/main/resources/csrfguard.properties

  Line 176 in e95aedf

  ## Protecting Pages ##

• if you use JSPs, you can use the provided tag library to inject CSRF tokens on the server side. If you don't have a huge amount of dynamic endpoints, you can also pre-create them. See:

  www-project-csrfguard/csrfguard/src/main/resources/csrfguard.properties

  Line 129 in e95aedf

  org.owasp.csrfguard.TokenPerPagePrecreate = false

• you can extend the main filter and write some custom logic that skips the CSRF check in certain cases
• if your front-end logic dynamically introduces new nodes, I've also introduced support for that. See:

  www-project-csrfguard/csrfguard/src/main/resources/csrfguard.properties

  Line 404 in e95aedf

  org.owasp.csrfguard.JavascriptServlet.injectIntoDynamicNodes = false

• there is an option to force synchronous AJAX requests (but this is not recommended). See

  www-project-csrfguard/csrfguard/src/main/resources/csrfguard.properties

  Line 492 in e95aedf

  org.owasp.csrfguard.forceSynchronousAjax = false

• you can also disable page tokens and rely on the master token only

These are the high level ideas I can think of right now, but ofc there might be other, even more suitable solutions depending on your use case. Last but not least, this is an open-source project, hence you can tailor it to your needs. We are accepting suggestions and PRs.

[bpapez]

Hello,

thank you very much for listing these options. It will come handy as I need to write a documentation how our customers will be able to solve problems on their pages.

To fix the failures we had I was renaming the endpoints getting the information on page load, so that they could get unprotected, while the endpoint on the submit action remained CSRF protected.

Due to that, we may not have a pressure to get a new csrfguard patch version. We are currently working on a custom implementation of the LogicalSessionExtractor and TokenHolder, as we use Spring Session with failover support in cluster, where a server can take over the sessions of another failing or shutdown server.

[forgedhallpass]

@bpapez you are welcome. I'm glad you've managed to sort it out!

[forgedhallpass]

@bpapez version 4.1.2 has been released and it should soon appear on the maven central as well.