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

FELIX-6698 Ability to configure URI Compliance mode #308

Merged

Conversation

paulrutter
Copy link
Contributor

@paulrutter paulrutter commented Apr 19, 2024

See https://issues.apache.org/jira/browse/FELIX-6698

  • Add org.eclipse.jetty.UriComplianceMode option
  • Possible values are: DEFAULT, LEGACY, RFC3986, UNAMBIGUOUS, UNSAFE
  • DEFAULT and RFC3986 are equivalent. Jetty uses DEFAULT as default, which is equivalent to RFC3986.
  • When UNSAFE, UNAMBIGUOUS or LEGACY is used, also apply workaround as suggested in UriCompliance.Violation ignored despite being set jetty/jetty.project#11448 (comment). This is not Servlet API 6 compliant though, so use on your own risk.

More info about the possible modes: https://github.com/jetty/jetty.project/blob/jetty-12.0.x/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java

@cziegeler sorry to bother you again, but i have another request ready.

- Add `org.eclipse.jetty.UriComplianceMode` option
- Possible values are: DEFAULT, LEGACY, RFC3986, UNAMBIGUOUS, UNSAFE
- When UNSAFE or LEGACY is used, also apply workaround as suggested in jetty/jetty.project#11448 (comment). This is not Servlet API 6 compliant though, so use on your own risk.
@paulrutter
Copy link
Contributor Author

@cziegeler any feedback on this one? Thanks!

@cziegeler
Copy link
Contributor

I think it would be good to also add metatype for that new property here: https://github.com/apache/felix-dev/blob/master/http/jetty12/src/main/java/org/apache/felix/http/jetty/internal/ConfigMetaTypeProvider.java#L57

@paulrutter
Copy link
Contributor Author

I processed the feedback.

@joakime
Copy link

joakime commented Oct 25, 2024

Please ensure that your documentation on this points out ...

  1. this is in direct violation of the Servlet 6 spec to enable this.
  2. enabling this and can disable (well, "allow bypass of" is probably a better way to describe it) many security safeguards you have on your servlet webapp.
  3. there is no guarantee that it will work or be available in future Jetty EE releases (we're good for support in Jetty 12.1.x and ee11 btw). It is very possible that the Servlet spec itself puts these kinds of enforcement into the Servlet spec itself, making Jetty's ability to bypass this impossible in a future release.

For point 2, if you enable this spec bypass, as a safeguard, do not put any content that you don't want people to access directly in the webapp in your WEB-INF directory. Also don't put raw JSP files in your WAR, make sure to precompile them into classes with no raw JSP files present. Don't get me wrong, Jetty will still make a best effort to prevent these kinds of access bypasses, but that's increasingly difficult, and the malicious actors are increasingly creative in this space, so don't let your guard down.

Btw, this is a servlet spec specific concern. Using non-servlet Jetty Core, you do not have the issue requiring this bypass.

Might be a good idea to document the types of URL that can trigger the need for this feature to be enabled, and provide alternative ways to define your URL space to not rely on these disallowed concepts in the servlet spec.

Similar to what was done at jetty/jetty.project#12346 (comment)

@paulrutter
Copy link
Contributor Author

paulrutter commented Oct 26, 2024

@joakime I'm aware of the security implications, but we should indeed have better documentation around using this.

The point is that it's sometimes required to remain backwards compatible, although there should be a plan in place to no longer use this configuration and thus be spec compliant.

See #340.

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

Successfully merging this pull request may close these issues.

3 participants