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

guard zip export with a permission #35

Open
frisi opened this issue Feb 9, 2016 · 8 comments
Open

guard zip export with a permission #35

frisi opened this issue Feb 9, 2016 · 8 comments

Comments

@frisi
Copy link
Contributor

frisi commented Feb 9, 2016

currently anonymous users can simply download a folder's content as zip by visiting path/to/folder/zip_export

can't this be used for DOS attacks?

possible solution:
add a permission "ftw.zipexport.DownloadZip" "ftw.zipexport: Download ZIP"
that is given to manager and siteadmin by default.
and use this permission to guard the @@zip_export view.

integrators can grant this permission to other roles (even Anonymous) and overwrite the @@zipexport-enabled view.

@frisi
Copy link
Contributor Author

frisi commented Feb 9, 2016

we should also add a browserlayer to this package. currently zip_export can be invoked even if the package is not installed on the plonesite.

what do you think about this @jone?

@jone
Copy link
Member

jone commented Feb 9, 2016

Yes, we should definitively protect the view with a custom permission!

I'd not add a browserlayer because:

  • The custom permission would only be installed when installing the package (rolemap.xml) and therefore make it impossible to access the views when the package is not installed.
  • Since browserlayers are unordered, using browserlayers for views make it harder to customize the view because usually the overrides.zcml must be used for making sure that the customizations always apply, which is only possible once. Therefore I personally only define a browser-layer for customizing views and for viewlets.

@hvelarde
Copy link

hvelarde commented Feb 9, 2016

the permission should be also given to Anonymous role by default or you are going to create a problem OOTB; I personally prefer to handle that at the front end server: caching the whole file in the web server is quite easy.

some documentation should be provided for both use cases.

@frisi
Copy link
Contributor Author

frisi commented Feb 9, 2016

@jone and @hvelarde thanks for your feedback.

@jone: did not know about the browserlayer problem - the permission is sufficient then.

@hvelarde: i don't understand why we are creating a problem out of the box if we do not allow zip_export for anonymous users.
you are right from a BBB point of view - since it works ootb for anonymous atm.
however, caching on a front-end server does not stop bad guys crawling your site and append /zip_export to every url. which will lead to all zope threads being busy almost instantly.

personally i'd break with backward compatibility in favour of robustness. if @jone as a maintainer prefers to stay with the current behaviour i'm fine with that too and will change permission settings in my policy package.

and yes, the permission should be documented in the readme

@hvelarde
Copy link

hvelarde commented Feb 9, 2016

@frisi maybe I misunderstood the problem; I don't remember how this is implemented, but we're using it to allow anonymous users download full photo galleries and want to keep that without changes.

check an example: http://www.brasil2016.gov.br/pt-br/galeria-de-fotos/parque-olimpico-da-barra-dezembro-de-2015

@jone
Copy link
Member

jone commented Feb 10, 2016

Anonymous:
If we grant Anonymous the zip-permission in rolemap.xml, an anonymous user could probably download a private object even if the user has no View permission.
For correctly controlling that, the integrator must manage the zip-permission in the workflow, similar to the view permission.
Because of that problem I prefer a restrictive default and not granting the permission to Anonymous globally, but maybe to Reader and Manager.

DOS-Prevention with proxy config:
In our standard setup we usually use haproxy as load balancer, which proxies requests to one of multiple zope instances.
For preventing DOS haproxy could be configured so that zip-download requests always are proxied to the same zope instance in order to not impact the other instances.
In the event of a DOS attack the zip-requests would be queued for this zope instance, normal requests would be served normally be the other instances.

Configure downloadable content:
Another DOS scenario is to make a zip-download request to the root or a level 1 content, resulting in an export of the whole site. This should be prevented by properly configuring the content allowed to be export in the registry

View / Public:
When I yesterday looked at the issue I mixed up the permissions and thought that the views are currently protected with zope.Public, which would be very bad. Now I've realized that it is zope2.View, which is not so bad since the user cannot download content which is not visible, which was my worry.
That said, I no longer see a custom permission that urgent.

New ZIP permission:
I'd like to re-discuss the initial issue:
As far as I see it, it should not be possible to just download anything by appending /zip_export if the downloadable interfaces are properly configured (see above).

So as I see it now, the problem is not the permission, but the lack of documentation for the zip-export setting, combined with the default of IContentish, allowing to download anything.

I propose to leave the permissions as it is now and extend the documentation for the zipexport settings and maybe also with some of the thoughts about DOS attacks.

What do you think?

@hvelarde
Copy link

+1

@frisi
Copy link
Contributor Author

frisi commented Feb 17, 2017

just installed the addon in an intranet and went through this ticket again.
for intranets, all the defaults are perfectly fine.

for usage on big websites i think the additional permission is still a good idea.
not for @hvelarde's usecase: public available zip_export actions for fotoalbums.
but for cases where only a trusted group of users should be allowed to call zip_export
(because protecting the website from dos attacks that call /zip_export on all objects is an additional configuration-effort. and it's more work than going to the zmi securty tab and change roles for the zipdownload permission)

so i'm fine for keeping the default roles for the zip-download permission very open (same as view-permission - to not break usecases like the one of hector) but i'd still vote for a custom permission to not only limit the zip_export action to certain interfaces, but also the ability to show the action only to a subset of authenticated users.

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

No branches or pull requests

3 participants