Unauthenticated request with a request path that contains a semicolon results in a server exception when using OIDC #31802
Comments
|
/cc @pedroigor (oidc), @sberyozkin (oidc) |
|
i would expect the cookie values to be properly escaped by the low level layers. @sberyozkin could you have a look at what's going on? |
|
@gsmet Sure, though I have to look after a few other issues first. It does appear that the lower level Vert.x code gets affected/confused by such a path, HttpAuthorizer code just tries to end the request where the exception is triggered, |
|
@sschellh Can you check please where the cookies are coming from unauthenticated requests when the path does not cause the exception ? Are these application specific cookies or set by Quarkus (please check what the browser console shows) ? |
|
@sberyozkin There are no cookies. I called the application using curl to make sure there is nothign the browser sends. Same result. |
|
@sschellh What 'curl -v' shows when you call a path without these special characters? Thanks |
|
|
@sschellh Thanks, so the request with special characters has no any server handler available, JAX-RS resource method or Vert.x route listening specifically to handle such requests. I think if you disable the proactive authentication then you might get 404 immediately since in that case the security layer runs as part of the JAX-RS chain so if the path handler does not exist 404 should be returned. |
|
I just saw I forgot to list one setting. The root path is protected and hence those (invalid) paths would require authentication. Hence 401 (or 302 redirect to idp) seems logical. |
|
There is nothing wrong with the URLs. According to https://www.rfc-editor.org/rfc/rfc3986#section-2 Are valid characters, so a Say you request: } else if (context.request().query() != null) {
CodeAuthenticationStateBean extraStateValue = new CodeAuthenticationStateBean();
extraStateValue.setRestorePath("?" + context.request().query());
cookieValue += (COOKIE_DELIM + encodeExtraStateValue(extraStateValue, configContext));
}Note that the there is no As
The solution is to escape the cookie data before trying to encode. Note that It seems (didn't test) it doesn't happen when |
|
Great stuff @pmlopes, thanks for doing this analysis, all right then, the culprit is But what about for ex See there, I'm a little bit wary about the encoding as then I guess we'd need to decode and there is always one short step to some unexpected results, if the path already contains an encoded |
|
@pmlopes I'm personally a bit skeptical that Vert.x doesn't handle the encoding of the cookie for you given you use a proper API to add one cookie with a specific value - I would at least expect that Now, I will create a PR with a possible fix but I have no idea how to test it so it's more to start a conversation. |
|
I created #42684 to initiate a discussion. |
Fix quarkusio#31802 (cherry picked from commit 3ffa6ec)
Fix quarkusio#31802 (cherry picked from commit 3ffa6ec)
Describe the bug
If an unauthenticated user submits a request with a request path that contains a semicolon, then that results in
This might be a problem because we see a lot of unauthenticated requests from the Internet for URIs like the ones below. Instead of just being rejected, they case a server exception.
Full
exceptiontrace:Expected behavior
The request is rejected with HTTP 401 unauthorized or HTTP 302 redirect to identity provider
Actual behavior
The request is running forever, the server log contains an exception.
How to Reproduce?
Make a call to a request URL like this one:
http://localhost:8080/svn///;@example.comIn the backend you find following exception:
Output of
uname -aorverNo response
Output of
java -versionopenjdk 11.0.7 2020-04-14 LTS
GraalVM version (if different from Java)
No response
Quarkus version or git rev
2.16.3.Final
Build tool (ie. output of
mvnw --versionorgradlew --version)Gradle 7.3.3
Additional information
No response
The text was updated successfully, but these errors were encountered: