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

Blocking in WebSessionServerCsrfTokenRepository #8128

Closed
cbornet opened this issue Mar 17, 2020 · 6 comments · Fixed by #8534
Closed

Blocking in WebSessionServerCsrfTokenRepository #8128

cbornet opened this issue Mar 17, 2020 · 6 comments · Fixed by #8534
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@cbornet
Copy link
Contributor

cbornet commented Mar 17, 2020

Summary

Detected by blockhound: WebSessionServerCsrfTokenRepository and CookieServerCsrfTokenRepository make blocking calls to UUID.randomUUID when generating the token.

It would be nice to have a non-blocking SecureRandom to solve this.
It can of course be offloaded to the boundedElastic scheduler but that looks sub optimal.

Version

5.2.2.RELEASE

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 17, 2020
@rwinch
Copy link
Member

rwinch commented Mar 17, 2020

Thanks for the report @cbornet!

I'm not aware of a non-blocking secure random source. Are you? If we don't have a non-blocking secure random source then I agree our best bet is to use the boundedElastic.

Would you be interested in submitting a PR?

@rwinch rwinch added in: web An issue in web modules (web, webmvc) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 17, 2020
@rwinch rwinch added this to the 5.0.15 milestone Mar 17, 2020
@cbornet
Copy link
Contributor Author

cbornet commented Mar 17, 2020

I'm not aware of a non-blocking secure random source. Are you?

No, I'm not either. I guess even reading /dev/random with NIO is still offloading to a thread-pool. Would be nice to have it in Java one day though.

I'll do the PR for boundedElastic, no problem

@cbornet
Copy link
Contributor Author

cbornet commented Mar 18, 2020

I want to put a publishOn in generateToken but I don't find a good place
I coud do

	@Override
	public Mono<CsrfToken> generateToken(ServerWebExchange exchange) {
		return Mono.just(exchange)
			.publishOn(Schedulers.boundedElastic())
			.fromCallable(() -> createCsrfToken());
	}

but maybe there's a better way without wrapping exchange (which is not used) ?

@rwinch
Copy link
Member

rwinch commented Mar 18, 2020

That looks correct to me.

@eleftherias eleftherias modified the milestones: 5.0.15, 5.0.16 Apr 1, 2020
@eleftherias eleftherias modified the milestones: 5.0.16, 5.0.17 May 6, 2020
@rwinch
Copy link
Member

rwinch commented May 12, 2020

@cbornet Are you still interested in submitting this?

@cbornet
Copy link
Contributor Author

cbornet commented May 12, 2020

Yes. Sorry I've been quite busy. I'll do it this week.

cbornet added a commit to cbornet/spring-security that referenced this issue May 15, 2020
The CSRF token is created with a call to UUID.randomUUID which is blocking.
This change ensures this blocking call is done on the bounded elastic scheduler which supports blocking calls.

Fixes spring-projectsgh-8128
rwinch pushed a commit that referenced this issue May 18, 2020
The CSRF token is created with a call to UUID.randomUUID which is blocking.
This change ensures this blocking call is done on the bounded elastic scheduler which supports blocking calls.

Fixes gh-8128
@rwinch rwinch changed the title Blocking call to UUID.randomUUID in WebSessionServerCsrfTokenRepository (blockhound) Blocking in WebSessionServerCsrfTokenRepository May 18, 2020
@rwinch rwinch modified the milestones: 5.0.17, 5.4.0-M2 May 18, 2020
rwinch pushed a commit that referenced this issue May 18, 2020
The CSRF token is created with a call to UUID.randomUUID which is blocking.
This change ensures this blocking call is done on the bounded elastic scheduler which supports blocking calls.

Fixes gh-8128
@spring-projects-issues spring-projects-issues added the status: backported An issue that has been backported to maintenance branches label May 18, 2020
rwinch pushed a commit that referenced this issue May 18, 2020
The CSRF token is created with a call to UUID.randomUUID which is blocking.
This change ensures this blocking call is done on the bounded elastic scheduler which supports blocking calls.

Fixes gh-8128
rwinch pushed a commit that referenced this issue May 18, 2020
The CSRF token is created with a call to UUID.randomUUID which is blocking.
This change ensures this blocking call is done on the bounded elastic scheduler which supports blocking calls.

Fixes gh-8128
rwinch pushed a commit that referenced this issue May 18, 2020
The CSRF token is created with a call to UUID.randomUUID which is blocking.
This change ensures this blocking call is done on the bounded elastic scheduler which supports blocking calls.

Fixes gh-8128
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants