Bug 16210 - Bug 15111 breaks the OPAC if JavaScript is disabled
Summary: Bug 15111 breaks the OPAC if JavaScript is disabled
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 15111
Blocks: 16179
  Show dependency treegraph
 
Reported: 2016-04-05 18:42 UTC by Owen Leonard
Modified: 2019-06-27 09:24 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16210: Revert OPAC changes from Bug 15111 (3.61 KB, patch)
2016-04-08 09:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16210: Set X-Frame-Options to SAMEORIGIN in 2 other places (1.53 KB, patch)
2016-04-08 09:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16210: Revert OPAC changes from Bug 15111 (3.66 KB, patch)
2016-04-12 14:02 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 16210: Set X-Frame-Options to SAMEORIGIN in 2 other places (1.67 KB, patch)
2016-04-12 14:03 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 16210: Revert OPAC changes from Bug 15111 (3.73 KB, patch)
2016-04-14 05:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 16210: Set X-Frame-Options to SAMEORIGIN in 2 other places (1.73 KB, patch)
2016-04-14 05:40 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2016-04-05 18:42:26 UTC
Since Bug 15111, if you have JavaScript disabled the OPAC is broken. It only displays a blank page.
Comment 1 Jonathan Druart 2016-04-06 10:26:12 UTC
I use NoScript and Iceweasel 38.2.1 and the OPAC displays correctly.
Comment 2 Owen Leonard 2016-04-06 12:51:38 UTC
I tested in Iceweasel by disabling JS via the the Web Developer Toolbar (http://chrispederick.com/work/web-developer/), but it also works to use about:config and set the javascript.enabled preference to "false."

In Chromium I use an extension called "Quick Javascript Switcher."

I can reproduce the problem in both.
Comment 3 Jonathan Druart 2016-04-06 14:17:11 UTC
Same comment as bug 15111 comment 33:
Well, we have a problem.
After reading this https://www.owasp.org/images/0/0e/OWASP_AppSec_Research_2010_Busting_Frame_Busting_by_Rydstedt.pdf it seems that it is not possible not to be vulnerable to XFS and render something with JS disabled...
Comment 4 Jonathan Druart 2016-04-07 15:00:21 UTC
If I understand correctly, setting X-Frame-Options to SAMEORIGIN should be enough for mordern browsers: https://developer.mozilla.org/en-US/docs/Web/HTTP/X-Frame-Options

The antiClickjack trick could be removed if we decide not to support them anymore.
Comment 5 Mirko Tietgen 2016-04-07 15:26:16 UTC
(In reply to Jonathan Druart from comment #4)
> If I understand correctly, setting X-Frame-Options to SAMEORIGIN should be
> enough for mordern browsers:
> https://developer.mozilla.org/en-US/docs/Web/HTTP/X-Frame-Options
> 
> The antiClickjack trick could be removed if we decide not to support them
> anymore.

Supported are

Firefox 3.6.9	September 2010
IE 8		March 2008
Opera 10.5	March 2010
Safari 4	February 2009
Chrome 4.1.…	somewhen 2010

If that fixes the problem in general I vote for using it. One thing that needs to be checked if it works with recent mobile browsers, the website does not really say that.
Comment 6 Jonathan Druart 2016-04-08 09:19:53 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-04-08 09:19:56 UTC Comment hidden (obsolete)
Comment 8 Marc Véron 2016-04-12 14:02:27 UTC Comment hidden (obsolete)
Comment 9 Marc Véron 2016-04-12 14:03:53 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2016-04-12 14:15:11 UTC
This works for me to enable use of the OPAC without JavaScript, which I think is an important goal.

I think we can rationalize the vulnerability for older browsers by saying "If you're still using one of these browsers you are probably vulnerable to any number of other terrible security problems because of your old computer and/or browser and what's one more?"

I will leave it to someone who knows better than I to test whether this solves the security problem it's meant to fix.
Comment 11 Chris Cormack 2016-04-12 23:08:51 UTC
(In reply to Owen Leonard from comment #10)
> This works for me to enable use of the OPAC without JavaScript, which I
> think is an important goal.
> 
> I think we can rationalize the vulnerability for older browsers by saying
> "If you're still using one of these browsers you are probably vulnerable to
> any number of other terrible security problems because of your old computer
> and/or browser and what's one more?"
> 
> I will leave it to someone who knows better than I to test whether this
> solves the security problem it's meant to fix.

Yeah, it does what it should, and yep if you are running a 6 year old browser, chances are someone already is doing all your internet banking for you. Having someone put a hold on a book you don't want, is the least of your worries at that point.

I think it is better to allow those who run without JS turned on (often for very legitimate reasons) to be able to use the OPAC. Than to try to support browsers from last decade.
Comment 12 Katrin Fischer 2016-04-14 05:39:36 UTC
Created attachment 50220 [details] [review]
[PASSED QA] Bug 16210: Revert OPAC changes from Bug 15111

This patch reverts the changes made at the OPAC from the following
patches:

Do not include the antiClickjack legacy browser trick for greybox"

Revert "Bug 15111: Do not include the antiClickjack legacy browser trick for greybox"
This reverts commit fc640d2a86f395ad392f84314bce22e8b4dab1fe.

Revert "Bug 15111: Change X-Frame-Options with SAMEORIGIN"
This reverts commit fb167c0e4b897bf9a93b4fd6176b15e2d4dbd4df.

Revert "Bug 15111 - Koha is vulnerable to Cross-Frame Scripting (XFS) attacks"
This reverts commit dc03bca76cf5b7cb48d98d1ce245fc65b98be929.

Setting X-Frame-Options to SAMEORIGIN is enough for mordern browsers:
https://developer.mozilla.org/en-US/docs/Web/HTTP/X-Frame-Options

The antiClickjack trick should be removed at the OPAC as we want to keep
the OPAC usable even if the user has disabled JS.
That means the OPAC will be vulnerable to XFS if a user is navigating
with a prehistoric browser:
Firefox 3.6.9 September 2010
IE 8    March 2008
Opera 10.5  March 2010
Safari 4  February 2009
Chrome 4.1.…  somewhen 2010

Test plan:
Confirm that there are no regression of bug 15111 with modern browsers

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2016-04-14 05:40:31 UTC
Created attachment 50221 [details] [review]
[PASSED QA] Bug 16210: Set X-Frame-Options to SAMEORIGIN in 2 other places

The login page should not be displayed if the page is displayed in a
frame.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Brendan Gallagher 2016-04-20 16:07:33 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks!
Comment 15 Julian Maurice 2016-04-22 08:58:44 UTC
Patches pushed to 3.22.x, will be in 3.22.6
Comment 16 Frédéric Demians 2016-04-27 15:31:42 UTC
Pushed to 3.22.x, will be in 3.20.11.