Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add AllowEncodedSlashes to apache #5068

Merged
merged 2 commits into from
May 9, 2019
Merged

Conversation

colonelkrud
Copy link

Add AllowEncodedSlashes On to apache config to support encoding for v3 rooms. "The AllowEncodedSlashes setting is not inherited by virtual hosts, and virtual hosts are used in many default Apache configurations, such as the one in Ubuntu. The workaround is to add the AllowEncodedSlashes setting inside a container (/etc/apache2/sites-available/default in Ubuntu)." Source: https://stackoverflow.com/questions/4390436/need-to-allow-encoded-slashes-on-apache

Signed-off-by: TravisH colonelkrud@gmail.com

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file
  • Pull request includes a sign off

Add `AllowEncodedSlashes On` to apache config to support encoding for v3 rooms. "The AllowEncodedSlashes setting is not inherited by virtual hosts, and virtual hosts are used in many default Apache configurations, such as the one in Ubuntu. The workaround is to add the AllowEncodedSlashes setting inside a <VirtualHost> container (/etc/apache2/sites-available/default in Ubuntu)." Source: https://stackoverflow.com/questions/4390436/need-to-allow-encoded-slashes-on-apache
@Bubu
Copy link
Contributor

Bubu commented Apr 16, 2019

Docs here: https://httpd.apache.org/docs/2.4/mod/core.html#allowencodedslashes

Quoting:

The AllowEncodedSlashes directive allows URLs which contain encoded path separators (%2F for / and additionally %5C for \ on accordant systems) to be used in the path info.
With the default value, Off, such URLs are refused with a 404 (Not found) error.
With the value On, such URLs are accepted, and encoded slashes are decoded like all other encoded characters.
With the value NoDecode, such URLs are accepted, but encoded slashes are not decoded but left in their encoded state.
Turning AllowEncodedSlashes On is mostly useful when used in conjunction with PATH_INFO.

If encoded slashes are needed in path info, use of NoDecode is strongly recommended as a security measure. Allowing slashes to be decoded could potentially allow unsafe paths.

Shouldn't that then be AllowEncodedSlashes NoDecode?

@colonelkrud
Copy link
Author

Bubu is correct. It should be set to NoDecode.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks plausible!

@richvdh richvdh merged commit d9a02d1 into matrix-org:develop May 9, 2019
anoadragon453 added a commit that referenced this pull request May 10, 2019
* develop: (45 commits)
  URL preview blacklisting fixes (#5155)
  Revert 085ae34
  Add a DUMMY stage to captcha-only registration flow
  Make Prometheus snippet less confusing on the metrics collection doc (#4288)
  Set syslog identifiers in systemd units (#5023)
  Run Black on the tests again (#5170)
  Add AllowEncodedSlashes to apache (#5068)
  remove instructions for jessie installation (#5164)
  Run `black` on per_destination_queue
  Limit the number of EDUs in transactions to 100 as expected by receiver (#5138)
  Fix bogus imports in tests (#5154)
  add options to require an access_token to GET /profile and /publicRooms on CS API (#5083)
  Do checks on aliases for incoming m.room.aliases events (#5128)
  Remove the requirement to authenticate for /admin/server_version. (#5122)
  Fix spelling in server notices admin API docs (#5142)
  Fix sample config
  0.99.3.2
  include disco in deb build target list
  changelog
  Debian: we now need libpq-dev.
  ...
anoadragon453 added a commit that referenced this pull request May 10, 2019
* develop:
  Revert 085ae34
  Add a DUMMY stage to captcha-only registration flow
  Make Prometheus snippet less confusing on the metrics collection doc (#4288)
  Set syslog identifiers in systemd units (#5023)
  Run Black on the tests again (#5170)
  Add AllowEncodedSlashes to apache (#5068)
  remove instructions for jessie installation (#5164)
  Run `black` on per_destination_queue
  Limit the number of EDUs in transactions to 100 as expected by receiver (#5138)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants