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

REST Resource path matching behaves differently than HTTP policy path matching #13756

Closed
alex-lmco opened this issue Dec 8, 2020 · 8 comments
Assignees
Labels
area/security kind/bug Something isn't working

Comments

@alex-lmco
Copy link

alex-lmco commented Dec 8, 2020

Describe the bug

REST Resources not prefixed with "/" match against "/" or "//" (note: not "///", see below) but HTTP policies not prefixed with "/" only match against "/".

For clarity (or look at example): Assume a REST Resource annotated with @Path("test/path"). Do not change from default quarkus.resteasy.path or set @ApplicationPath (defaults to /). It will match against /test/path or //test/path (but not ///test/path. However, defining an HTTP policy of test/path only matches against /test/path. If this policy has required roles associated with it then going to /test/path gives you a 401 as appropriate but going to //test/path gives you a 200 even though this is clearly not the expected behavior.

Expected behavior

HTTP policy paths behave identically to REST resource paths for basic cases, ignoring extra stuff specified in JAX-RS like template variables, regular expressions, etc.

REST resource paths and HTTP policies behave according to RFC 3986 Section 3.3. Per RFC 3986 Section 3.3 the following paths produce the following distinct path segments:

http://localhost:8080/test/path = path segments {"test", "path"}
http://localhost:8080//test/path = path segments {"", "test", "path"}
http://loclahost:8080///test/path = path segments {"", "", "test", "path"}

Reading the JAX-RS Spec, it says that:

Prior to matching, request URIs are normalized by following the rules for case, path segment, and percent encoding normalization described in section 6.2.2 of RFC 3986. The normalized request URI MUST be reflected in the URIs obtained from an injected UriInfo.

For Path Normalization in RFC 3986 (Section 6.2.2.3) it just talks about relative paths being normalized, not empty path segments.

Glancing over the algorithm specified in JAX-RS for matching against URIs (mapping a URI to a function), it doesn't explicitly address empty path segments. The closest I could find matching this scenario was Section 3.4 of the spec:

3.4 URI Templates
...
1 @path("widgets/{path:.+}")
2 public class Widget {
3 ...
4 }
In the above example the Widget resource class will be matched for any request whose path starts with widgets and contains at least one more path segment

I couldn't find what the base application path should be if left unspecified in the spec, it seemed to imply it had to be specified (based on @ApplicationPath or equivalent). Based on the quarkus all-config documentation, I found that it is set to the value in quarkus.resteasy.path which is defaulted to / see here which, all together, would imply, I think, that @Path("test/path") should only match against /test/path and not also against //test/path.

Actual behavior

HTTP policies do not behave identically to REST resource paths. This leads to issues like the following:

$ curl --noproxy 'localhost' -X GET -i http://localhost:8080/test/path
HTTP/1.1 401 Unauthorized
www-authenticate: Bearer {token}
content-length: 0

$ curl --noproxy 'localhost' -X GET -i http://localhost:8080//test/path
HTTP/1.1 200 OK
Content-Length: 7
Content-Type: application/octet-stream

To Reproduce

minimal-quarkus-replicator.zip

Steps to reproduce the behavior:

  1. ./gradlew quarkusDev
  2. curl --noproxy 'localhost' -X GET -i http://localhost:8080/test/path = 401
  3. curl --noproxy 'localhost' -X GET -i http://localhost:8080//test/path = 200

Configuration

# quarkus configuration
quarkus:
  smallrye-jwt:
    enabled: true
  # logger configuration
  log:
    level: INFO
    console:
      format: "%d{yyyy-MM-dd HH:mm:ss,SSS} %-5p [%c{3.}] [%X] (%t) %s%e%n"
      json: false
    category:
      # root log level (default if nothing more specific is specified)
      io.quarkus:
        level: INFO
  # http configuration
  http:
    port: 8080
    auth:
      policy:
        policy:
          roles-allowed: admin
      permission:
        pathPermission:
          paths: test/path
          methods: GET
          policy: policy

Screenshots

N/A

Environment (please complete the following information):

  • Output of uname -a or ver: Microsoft Windows [Version 10.0.17763.1577]
  • Output of java -version: Java 11
  • GraalVM version (if different from Java): N/A
  • Quarkus version or git rev: 1.10.2.Final
  • Build tool (ie. output of mvnw --version or gradlew --version): Gradle 6.7.1

Additional context

N/A

@alex-lmco alex-lmco added the kind/bug Something isn't working label Dec 8, 2020
@ghost ghost added the env/windows Impacts Windows machines label Dec 8, 2020
@sberyozkin
Copy link
Member

My understanding is that HTTP policy path matching algorithm is not meant to be equivalent to the JAX-RS matching algorithm - I think one should use the annotations if all the variations need to be covered.
Otherwise indeed we need to support the regular expressions, template vars matching some sub-segments, etc.
CC @stuartwdouglas

@alex-lmco
Copy link
Author

Is the HTTP policy path matching algorithm documented somewhere as being 'different' in regards like this to the JAX-RS matching algorithm?

It seems, to me at least, that regardless of that extra stuff specified in JAX-RS like template variables, regular expressions, etc. that the basic case of URI matching should be functionally identical (e.g. the /test/path case vs //test/path case). Is it expected that /test/path and //test/path would match in JAX-RS but not match in HTTP policy paths specified identically (again, basic case)?

(I had forgotten about that extra stuff when I specified the 'identical' part, I really just meant in terms of this basic case. A bit too over-broad sorry.)

@sberyozkin
Copy link
Member

sberyozkin commented Dec 8, 2020

Np; taking care of the duplicate segment slashes makes sense.

@SpazzMarticus
Copy link

Found this issue after a client reported that any user could access routes that were configured to be accessible only by a certain role. This is still a security problem and not mentioned in the docs.

@michalvavrik
Copy link
Member

Found this issue after a client reported that any user could access routes that were configured to be accessible only by a certain role. This is still a security problem and not mentioned in the docs.

Yeah, I agree this is a security issue, if endpoint can be matched with //test/path while you secured /test/path, that's a problem. I'm making changes in that very code for #14047, so I'll try to address this as well.

@michalvavrik
Copy link
Member

note: I didn't test above mentioned example, just judging by comments you report.

@michalvavrik michalvavrik self-assigned this Aug 18, 2023
@michalvavrik michalvavrik removed the env/windows Impacts Windows machines label Aug 18, 2023
@SpazzMarticus
Copy link

SpazzMarticus commented Aug 21, 2023

note: I didn't test above mentioned example, just judging by comments you report.

Correct, I reproduced the bug with the latest quarkus version (3.2.4).

Thanks for taking a look at this.

@gsmet
Copy link
Member

gsmet commented Sep 14, 2023

This has been fixed in 2.16.11.Final, 3.2.6.Final and 3.3.3.

@gsmet gsmet closed this as completed Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants