FreshRSS / Extensions

A repository containing all the official FreshRSS extensions
GNU Affero General Public License v3.0
322 stars 52 forks source link

Allow Apache-friendly fine-tuning #77

Closed cedric-dufour closed 4 years ago

cedric-dufour commented 4 years ago

Hello,

This PR adds additional parameters to the ImageProxy extension such as to allow easy proxying via Apache mod_rewrite (which: 1. needs a hard-coded absolute path ; 2. can not URL-decode passed URL)

See the updated README.md for further information (and corresponding Apache configuration example).

I've taken care to remain backward-compatible with existing setups; in particular, the (former) force parameter will automatically be migrated to the (new) scheme_https counterpart.

Best,

Cédric

Alkarex commented 4 years ago

I trust you are the best to review this PR, @Frenzie :-) In principle, that sounds good. One thing though - but I believe this is something to configure in Apache and not in this FreshRSS extension - is that it might allow an attacker to trigger some requests that would otherwise be disallowed. For instance querying a service running on localhost. So we should keep an eye on those aspects.

Frenzie commented 4 years ago

You mean whether the sample Apache configuration is safe enough?

Alkarex commented 4 years ago

Probably safe enough for thie PR @Frenzie , but could potentially be improved: I have only quickly looked at it, but it seems to filter on incoming IP (which is good) but not on destination (it could be good to exclude e.g. localhost in IPv4 and IPv6).

cedric-dufour commented 4 years ago

on destination (it could be good to exclude e.g. localhost in IPv4 and IPv6).

Indeed! I believe https://github.com/FreshRSS/Extensions/pull/77/commits/5881c20661120d7ac375e7a7e64f19b0861d570f addresses this issue (or even stricter: https://github.com/FreshRSS/Extensions/pull/77/commits/5a6dc4940ce8213645085863be3c212b5a25a7ae).

Hopefully the CRITICAL: ...!!! stanza will raise awaress that care must be taken.

Frenzie commented 4 years ago

Thanks!