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

security-proxy: Allow semicolons in URLs #2636

Merged
merged 2 commits into from
Jun 16, 2019

Conversation

groldan
Copy link
Member

@groldan groldan commented Jun 14, 2019

Configure org.springframework.security.web.firewall.StrictHttpFirewall"
with allowSemicolon = true to avoid errors
like 'the request was rejected because the URL contained a potentially malicious String ";"'

";" in URL's come in the form http:///;jsessionid=xxxx
In general, proxied applications are discouraged to disclose jsessionid this way,
and shall use a cookie instead.
Some applications like CKAN though can't be modified to use cookies, hence this configuration.

In general, proxied applications are discouraged to disclose jsessionid this way, and shall use a cookie instead.
Some applications like CKAN though can't be modified to use cookies, hence this configuration.
-->
<property name="allowSemicolon" value="true"/>
Copy link
Member

Choose a reason for hiding this comment

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

I would find it useful to have this setting configurable from the datadir, with a default value set to false.
Platforms using CKAN would have to switch it to true.

Copy link
Member Author

Choose a reason for hiding this comment

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

sounds doable, let me see

Copy link
Member Author

Choose a reason for hiding this comment

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

@groldan groldan force-pushed the bug/GSHDF-95 branch 2 times, most recently from 4a7a079 to 8c96818 Compare June 14, 2019 14:38
Configure org.springframework.security.web.firewall.StrictHttpFirewall"
with allowSemicolon = true to avoid errors
like 'the request was rejected because the URL contained a potentially malicious String ";"'

";" in URL's come in the form http://<domain>/<path>;jsessionid=xxxx OR
in static resource URLs (e.g to reference bundles of JS files (minified on the fly)).

In general, proxied applications are discouraged to disclose jsessionid this way,
and shall use a cookie instead.
Some applications like though can't be modified to use cookies, hence this configuration.
@fvanderbiest
Copy link
Member

Excellent, thanks !

@fvanderbiest fvanderbiest merged commit e1847c5 into georchestra:master Jun 16, 2019
@fvanderbiest fvanderbiest added this to the 19.06 milestone Jun 16, 2019
@fvanderbiest fvanderbiest added this to In progress in Geo2France via automation Jun 16, 2019
@groldan groldan deleted the bug/GSHDF-95 branch June 24, 2019 14:04
@pmauduit
Copy link
Member

This should also be ported back to 19.04

@pmauduit pmauduit mentioned this pull request Jul 12, 2019
@landryb
Copy link
Member

landryb commented Jul 15, 2019

Would it be possible to also allow // in urls too while here ? this is a regression from 18.06..

@pmauduit
Copy link
Member

Would it be possible to also allow // in urls too while here ? this is a regression from 18.06..

where have you encountered such urls ?

@landryb
Copy link
Member

landryb commented Jul 15, 2019

georchestra/cadastrapp@db5b008 & georchestra/cadastrapp@42a01a1 from georchestra/cadastrapp#445 among others, but this can cause issues for generated links in geoserver or geonetwork (ie interlinks between md & layer, when the url is generated by some code -> this sometimes has //)

I know i had to fix/remove some slashes here and there in the geor datadir to account for this change

@pmauduit
Copy link
Member

Ok, it's in the path, not in the arguments, so it would be somewhere else

@fvanderbiest fvanderbiest modified the milestones: 19.09, 19.12 Oct 22, 2019
@landryb
Copy link
Member

landryb commented Apr 7, 2020

Would it be possible to also allow // in urls too while here ? this is a regression from 18.06..

where have you encountered such urls ?

i was having broken extent thumbnails on a metadata view, because the url generated by geonetwork looked like https://ids.craig.fr/geocat/srv//fre/region.getmap.png?... -> double-slash triggered a 500 code with the sec-proxy firewall.

Looking at upstream repo and after digging a good while to find out where the html was generated, if we want to fix it on 19.04 geonetwork/core-geonetwork@05836b4 should be cherry-picked on top of georchestra-gn3.4-19.04 branch. Yes, i'm still fighting with this horror...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Geo2France
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants