[Rt-commit] rt branch, 4.0-trunk, updated. rt-4.0.0rc4-62-g0ee6ad4

Alex Vandiver alexmv at bestpractical.com
Tue Mar 1 16:10:37 EST 2011


The branch, 4.0-trunk has been updated
       via  0ee6ad45c8fc42217d30b515e87bab8c56842416 (commit)
       via  9dfd68c3ae86938f892d7f59d5dcc211e261090b (commit)
       via  7bb4a58bef34a258d1fa6041ed62ee5a1b134578 (commit)
       via  e533b561918c97d3964e789179a14dedde6e1d3f (commit)
       via  6b962e49657c1dd3b68c2d007c1544bd78b80dd9 (commit)
       via  75bdb3e0f7bd0fb26aed1197cd685c9d9a8e8c9d (commit)
      from  7c6f4d68264296ef9b2029fca1c1308a89db3cd1 (commit)

Summary of changes:
 etc/RT_Config.pm.in              |   15 +++++++++++++++
 lib/RT/Config.pm                 |   14 ++++++++++++++
 lib/RT/Interface/Web/Handler.pm  |    7 +++++++
 share/html/Elements/Framekiller  |   20 ++++++++++++++++++++
 share/html/Elements/Header       |    2 ++
 share/html/m/_elements/header    |    1 +
 t/web/clickjacking-preventions.t |   33 +++++++++++++++++++++++++++++++++
 7 files changed, 92 insertions(+), 0 deletions(-)
 create mode 100644 share/html/Elements/Framekiller
 create mode 100644 t/web/clickjacking-preventions.t

- Log -----------------------------------------------------------------
commit 75bdb3e0f7bd0fb26aed1197cd685c9d9a8e8c9d
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Dec 23 19:51:17 2010 -0500

    Add a javascript framekiller to all pages
    
    This makes sure to clear all the page content even if the redirect is
    caught by an anti-framekiller script on the framing page.
    
    Swiped from Twitter after comparing it to versions on Wikipedia and
    elsewhere.

diff --git a/share/html/Elements/Framekiller b/share/html/Elements/Framekiller
new file mode 100644
index 0000000..cc985b3
--- /dev/null
+++ b/share/html/Elements/Framekiller
@@ -0,0 +1,15 @@
+<script>
+if (window.top !== window.self) {
+    document.write = "";
+
+    window.top.location = window.self.location;
+
+    setTimeout(function(){
+        document.body.innerHTML = "";
+    }, 1);
+
+    window.self.onload = function(){
+        document.body.innerHTML = "";
+    };
+}
+</script>
diff --git a/share/html/Elements/Header b/share/html/Elements/Header
index 58f7696..47786a1 100755
--- a/share/html/Elements/Header
+++ b/share/html/Elements/Header
@@ -54,6 +54,8 @@
   <head>
     <title><%$Title%></title>
 
+    <& /Elements/Framekiller &>
+
 % if ($Refresh && $Refresh =~ /^(\d+)/ && $1 > 0) {
     <meta http-equiv="refresh" content="<% $Refresh %>" />
 % }
diff --git a/share/html/m/_elements/header b/share/html/m/_elements/header
index faf5765..007c3ac 100644
--- a/share/html/m/_elements/header
+++ b/share/html/m/_elements/header
@@ -55,6 +55,7 @@ $r->headers_out->{'Cache-control'} = 'no-cache';
 </%init>
 <html>
 <head>
+<& /Elements/Framekiller &>
 <link rel="stylesheet" type="text/css" href="<%RT->Config->Get('WebPath')|n%>/m/style.css"/>
 <title><%$title%></title>
 <meta name="viewport" content="width=device-width height=device-height user-scalable=yes"/>

commit 6b962e49657c1dd3b68c2d007c1544bd78b80dd9
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Dec 30 12:12:54 2010 -0500

    Add an X-Frame-Options: DENY header to all responses

diff --git a/lib/RT/Interface/Web/Handler.pm b/lib/RT/Interface/Web/Handler.pm
index 67fcb13..9115e66 100644
--- a/lib/RT/Interface/Web/Handler.pm
+++ b/lib/RT/Interface/Web/Handler.pm
@@ -262,6 +262,8 @@ sub _psgi_response_cb {
     Plack::Util::response_cb
             ($ret,
              sub {
+                 my $res = shift;
+                 Plack::Util::header_set($res->[1], 'X-Frame-Options' => 'DENY');
                  return sub {
                      if (!defined $_[0]) {
                          $cleanup->();

commit e533b561918c97d3964e789179a14dedde6e1d3f
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Dec 30 13:19:35 2010 -0500

    Add an @ExtraSecurity config option to control clickjacking countermeasures
    
    This adds an RT->Config->ExtraSecurity() method which lets you easily
    check if a certain countermeasure is enabled.

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index cc69e23..6efe087 100755
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -1730,6 +1730,21 @@ Default: C<< Set( $DisplayTicketAfterQuickCreate, 0 ) >>
 
 Set( $DisplayTicketAfterQuickCreate, 0 );
 
+=item C<@ExtraSecurity>
+
+This is a list of extra security measures to enable that help keep your RT
+safe.  If you don't know what these mean, you should almost certainly leave the
+defaults alone.
+
+    Clickjacking - Enables framekiller javascript and adds an X-Frame-Options:
+                   DENY header to all requests
+
+Default: C<< Set( @ExtraSecurity, qw(Clickjacking) ) >>
+
+=cut
+
+Set( @ExtraSecurity, qw(Clickjacking) );
+
 =back
 
 =head1 UTF-8 Configuration
diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 4a87382..89688e5 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -1141,6 +1141,20 @@ sub UpdateOption {
     return 1;
 }
 
+=head2 ExtraSecurity NAME
+
+Returns true if NAME is included in the C<@ExtraSecurity> list, false if not.
+
+This is currently a convenience method for C<< grep { lc $_ eq lc $name } RT->Config->Get('ExtraSecurity') >>.
+
+=cut
+
+sub ExtraSecurity {
+    my $self = shift;
+    my $name = lc shift;
+    return grep { lc $_ eq $name } $self->Get('ExtraSecurity');
+}
+
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/Interface/Web/Handler.pm b/lib/RT/Interface/Web/Handler.pm
index 9115e66..f9a1481 100644
--- a/lib/RT/Interface/Web/Handler.pm
+++ b/lib/RT/Interface/Web/Handler.pm
@@ -263,7 +263,12 @@ sub _psgi_response_cb {
             ($ret,
              sub {
                  my $res = shift;
-                 Plack::Util::header_set($res->[1], 'X-Frame-Options' => 'DENY');
+
+                 if ( RT->Config->ExtraSecurity('Clickjacking') ) {
+                     # XXX TODO: Do we want to make the value of this header configurable?
+                     Plack::Util::header_set($res->[1], 'X-Frame-Options' => 'DENY');
+                 }
+
                  return sub {
                      if (!defined $_[0]) {
                          $cleanup->();
diff --git a/share/html/Elements/Framekiller b/share/html/Elements/Framekiller
index cc985b3..7af5f62 100644
--- a/share/html/Elements/Framekiller
+++ b/share/html/Elements/Framekiller
@@ -1,3 +1,7 @@
+% if ( RT->Config->ExtraSecurity('Clickjacking') ) {
+%# This is defeatable.  The current best known implemention uses CSS to hide
+%# the content and JS to re-show it, but that fails poorly for clients that
+%# don't run JS.
 <script>
 if (window.top !== window.self) {
     document.write = "";
@@ -13,3 +17,4 @@ if (window.top !== window.self) {
     };
 }
 </script>
+% }

commit 7bb4a58bef34a258d1fa6041ed62ee5a1b134578
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Dec 30 14:06:22 2010 -0500

    Add basic tests for the presence of clickjacking countermeasures

diff --git a/t/web/clickjacking-preventions.t b/t/web/clickjacking-preventions.t
new file mode 100644
index 0000000..fda0877
--- /dev/null
+++ b/t/web/clickjacking-preventions.t
@@ -0,0 +1,33 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+
+use RT::Test tests => 14;
+
+my ($url, $m);
+
+# Enabled by default
+{
+    ok(RT->Config->ExtraSecurity('Clickjacking'), "RT->Config->ExtraSecurity reports Clickjacking enabled");
+    ok(RT->Config->ExtraSecurity('clickjacking'), "RT->Config->ExtraSecurity reports clickjacking enabled");
+
+    ($url, $m) = RT::Test->started_ok;
+    $m->get_ok($url);
+    $m->content_contains('if (window.top !== window.self) {', "Found the framekiller javascript");
+    is $m->response->header('X-Frame-Options'), 'DENY', "X-Frame-Options is set to DENY";
+
+    RT::Test->stop_server;
+}
+
+# Disabled
+{
+    RT->Config->Set('ExtraSecurity' => grep { !/clickjacking/i } RT->Config->Get('ExtraSecurity'));
+    ok(!RT->Config->ExtraSecurity('Clickjacking'), "RT->Config->ExtraSecurity reports Clickjacking disabled");
+    ok(!RT->Config->ExtraSecurity('clickjacking'), "RT->Config->ExtraSecurity reports clickjacking disabled");
+
+    ($url, $m) = RT::Test->started_ok;
+    $m->get_ok($url);
+    $m->content_lacks('if (window.top !== window.self) {', "Didn't find the framekiller javascript");
+    is $m->response->header('X-Frame-Options'), undef, "X-Frame-Options is not present";
+}
+

commit 9dfd68c3ae86938f892d7f59d5dcc211e261090b
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Tue Mar 1 14:41:04 2011 -0500

    Ensure that RT->Config->ExtraSecurity always returns a scalar

diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index 89688e5..432a4bb 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -1152,7 +1152,7 @@ This is currently a convenience method for C<< grep { lc $_ eq lc $name } RT->Co
 sub ExtraSecurity {
     my $self = shift;
     my $name = lc shift;
-    return grep { lc $_ eq $name } $self->Get('ExtraSecurity');
+    return scalar grep { lc $_ eq $name } $self->Get('ExtraSecurity');
 }
 
 RT::Base->_ImportOverlays();

commit 0ee6ad45c8fc42217d30b515e87bab8c56842416
Merge: 7c6f4d6 9dfd68c
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Mar 1 15:51:21 2011 -0500

    Merge branch '4.0/disable-iframe-embedding' into 4.0-trunk


-----------------------------------------------------------------------


More information about the Rt-commit mailing list