WUI: ambiently accessible pages should framebust in order to prevent UI redressing attacks #1455

New Issue

tahoe-lafs · 2011-07-30T23:33:18Z

davidsarah commented

2011-07-30 23:33:18 +00:00

If an ambiently accessible WUI page (one that does not require a capability to access, such as the Welcome page) is loaded in a frame or iframe, the loading frame might be able to perform some UI redressing or clickjacking attacks.

For example, the loading frame could entice the user to click the "Create a directory" button, when it should not have the authority to create a directory on that grid.

This is not a very strong attack. In any case, it can be prevented by including some framebusting code on ambiently accessible WUI pages (or all WUI pages).

If an ambiently accessible WUI page (one that does not require a capability to access, such as the Welcome page) is loaded in a frame or iframe, the loading frame might be able to perform some UI redressing or clickjacking attacks. For example, the loading frame could entice the user to click the "Create a directory" button, when it should not have the authority to create a directory on that grid. This is not a very strong attack. In any case, it can be prevented by including some framebusting code on ambiently accessible WUI pages (or all WUI pages).

tahoe-lafs added the

labels 2011-07-30 23:33:18 +00:00

tahoe-lafs added this to the undecided milestone 2011-07-30 23:33:18 +00:00

davidsarah commented

2011-07-31 04:36:13 +00:00

How not to framebust: http://seclab.stanford.edu/websec/framebusting/framebust.pdf

The way to do it securely seems to be to send an X-Frame-Options: DENY header.

How not to framebust: <http://seclab.stanford.edu/websec/framebusting/framebust.pdf> The way to do it securely seems to be to send an [X-Frame-Options: DENY](http://blogs.msdn.com/b/ieinternals/archive/2010/03/30/combating-clickjacking-with-x-frame-options.aspx) header.

davidsarah commented

2012-08-28 18:11:05 +00:00

According to https://developer.mozilla.org/en-US/docs/The_X-FRAME-OPTIONS_response_header , X-Frame-Options is supported by:

Internet Explorer 8.0
Firefox 3.6.9 (Gecko 1.9.2.9)
Opera 10.50
Safari 4.0
Chrome 4.1.249.1042

I think there might be some benefit in including the header for all WUI pages, not just ambiently accessible ones. In conjunction with #1797, that would simplify reasoning about some of the attacks we were worried about in the 2012-08-28 conference call.

According to <https://developer.mozilla.org/en-US/docs/The_X-FRAME-OPTIONS_response_header> , X-Frame-Options is supported by: * Internet Explorer 8.0 * Firefox 3.6.9 (Gecko 1.9.2.9) * Opera 10.50 * Safari 4.0 * Chrome 4.1.249.1042 I think there might be some benefit in including the header for all WUI pages, not just ambiently accessible ones. In conjunction with #1797, that would simplify reasoning about some of the attacks we were worried about in the 2012-08-28 conference call.

ChosenOne commented

2013-11-30 14:29:16 +00:00

I am trying to take this, but I have serious problems finding the appropriate code to patch.
It looks like a lot of HTTP handling is scattered through all files in src/web/, which makes it particularly hard to add a security header for all HTTP responses :(

It is unclear to me what the childFactory methods in web/root.py do and the nevow guides weren't very helpful.
If they are called for each incoming request, I would consider writing a function that takes a context (ctx) and sets the desired headers for the corresponding request (IRequest(ctx) it seems).

Otherwise I would have to patch the request handlers for every HTTP method (i.e. render_FOO). A pointer for this would also be helpful.

If it's not explicitly undesired, I am considering adding other security headers in a follow-up and might therefore do some changes that are not exactly required for the X-Frame-Options patch.

Oh, and on a side-note: I would suggest to set the X-Frame-Options header to SAMEORIGIN and not DENY. This way, HTML files hosted on tahoe can be framed by other files in the grid. I assume that some users might require this. If there is consensus to take DENY, I will happily obey ;-)

I am trying to take this, but I have serious problems finding the appropriate code to patch. It looks like a lot of HTTP handling is scattered through all files in src/web/, which makes it particularly hard to add a security header for all HTTP responses :( It is unclear to me what the childFactory methods in web/root.py do and the nevow guides weren't very helpful. If they are called for each incoming request, I would consider writing a function that takes a context (`ctx`) and sets the desired headers for the corresponding request (`IRequest(ctx)` it seems). Otherwise I would have to patch the request handlers for every HTTP method (i.e. `render_FOO`). A pointer for this would also be helpful. If it's not explicitly undesired, I am considering adding other security headers in a follow-up and might therefore do some changes that are not exactly required for the X-Frame-Options patch. Oh, and on a side-note: I would suggest to set the X-Frame-Options header to `SAMEORIGIN` and not `DENY`. This way, HTML files hosted on tahoe can be framed by other files in the grid. I assume that some users might require this. If there is consensus to take `DENY`, I will happily obey ;-)

ChosenOne commented

2013-11-30 15:25:19 +00:00

OK, it seems easy enough to do this for uri/ and file/ resources by patching FileHandler and URIHandler classes in root.py. See this commit on my feature branch

I have yet to figure out how to do this for things like /statistics, /status, etc. It seems undesirable to re-implement this for every subpage in its own file in src/web/. This would mean that every new subresource would have to remember doing this again to ensure that no URL is left behind ;)

Wouldn't it make sense to build a tiny abstraction layer on top of nevow's rend.Page that ensures proper response headers and is to be used in all files? This would still require changes in all files, but these could be less careful, as we just replace rend.Page references.

OK, it seems easy enough to do this for uri/ and file/ resources by patching FileHandler and URIHandler classes in root.py. See [this commit](https://github.com/freddyb/tahoe-lafs/commit/c73f46675e57c30c957fe87e0d87504051bac5e0) on my feature branch I have yet to figure out how to do this for things like /statistics, /status, etc. It seems undesirable to re-implement this for every subpage in its own file in src/web/. This would mean that every new subresource would have to remember doing this again to ensure that no URL is left behind ;) Wouldn't it make sense to build a tiny abstraction layer on top of nevow's `rend.Page` that ensures proper response headers and is to be used in all files? This would still require changes in all files, but these could be less careful, as we just replace `rend.Page` references.

ChosenOne commented

2013-11-30 22:37:10 +00:00

Ha, I think I solved it differently. Please see https://github.com/tahoe-lafs/tahoe-lafs/pull/72 or https://github.com/freddyb/tahoe-lafs/commits/ticket1455-x-frame-options

I think warner did the most WUI things, so review appreciated

Ha, I think I solved it differently. Please see <https://github.com/tahoe-lafs/tahoe-lafs/pull/72> or <https://github.com/freddyb/tahoe-lafs/commits/ticket1455-x-frame-options> I think warner did the most WUI things, so review appreciated

daira commented

2013-12-01 12:40:19 +00:00

Reviewing.

tahoe-lafs added

normal

and removed

minor

labels 2013-12-01 12:40:19 +00:00

tahoe-lafs modified the milestone from undecided to 1.11.0

2015-01-29 19:51:50 +00:00

daira commented

2015-02-09 01:57:42 +00:00

I seem to have lost track of this after saying I'd review it. I'll rebase it soon.

daira commented

2015-02-09 02:31:57 +00:00

Rebased to https://github.com/tahoe-lafs/tahoe-lafs/commits/1455.x-frame-options.1. (I changed SAMEORIGIN to DENY.)

Needs a test.

Rebased to <https://github.com/tahoe-lafs/tahoe-lafs/commits/1455.x-frame-options.1>. (I changed SAMEORIGIN to DENY.) Needs a test.

daira commented

2015-02-09 02:34:20 +00:00

BTW, it needs to be DENY rather than SAMEORIGIN because the attacking page might also be on the grid.

warner commented

2016-03-22 05:02:52 +00:00

Milestone renamed

warner modified the milestone from 1.11.0 to 1.12.0

2016-03-22 05:02:52 +00:00

warner commented

2016-06-28 18:20:37 +00:00

moving most tickets from 1.12 to 1.13 so we can release 1.12 with magic-folders

warner modified the milestone from 1.12.0 to 1.13.0

2016-06-28 18:20:37 +00:00

Jean-Paul Calderone <exarkun@twistedmatrix.com> commented

2018-05-28 12:12:41 +00:00

In 718fa44/trunk:

Add "X-Frame-Options: DENY" header to all pages. refs #1455

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

In [718fa44/trunk](/tahoe-lafs/trac-2024-07-25/commit/718fa4493c4b6dd9d897160ded2ea4831c17d3bf): ``` Add "X-Frame-Options: DENY" header to all pages. refs #1455 Signed-off-by: Daira Hopwood <daira@jacaranda.org> ```

exarkun added the

fixed

label 2018-05-28 12:16:14 +00:00

exarkun closed this issue

2018-05-28 12:16:14 +00:00

exarkun commented

2018-05-28 12:20:00 +00:00

Fixed in bfedd796338398b3d8ab21520d48cddca42c425d

Sign in to join this conversation.