Skip to content

Commit

Permalink
Item12839: Implement default REST security changes
Browse files Browse the repository at this point in the history
Defines new InsecureREST setting in Foswiki.spec.   TEMPORARILY ENABLED.
This will be disabled prior to Foswiki 1.2

Add warning log records, with referer, to the REST security rejections,
so that admins can figure out which pages are making incorrect rest
requests.

Disable core checks for WysiwygPlugin - These should be enforcing their
own requirements.

Update release notes

Fix unit tests to disable rest default security checking.  Each test
enables settings as needed.

git-svn-id: http://svn.foswiki.org/trunk@17592 0b4bb1d4-4e5a-0410-9cc4-b2b747904278
  • Loading branch information
GeorgeClark authored and GeorgeClark committed Apr 27, 2014
1 parent 26341ce commit a0ac38f
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 6 deletions.
1 change: 1 addition & 0 deletions UnitTestContrib/test/unit/RESTTests.pm
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ our $UI_FN;

sub set_up {
my $this = shift;
$Foswiki::cfg{InsecureREST} = 1;
$this->SUPER::set_up();
$UI_FN ||= $this->getUIFn('rest');

Expand Down
16 changes: 12 additions & 4 deletions WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin.pm
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,22 @@ sub initPlugin {
Foswiki::Func::registerTagHandler( 'WYSIWYG_SECRET_ID',
sub { _execute( '_SECRET_ID', @_ ) } );

# The WYSIWYG REST handlers all check for appropriate access.
# Core does not need to enforce access or validation.
my %opts = (
authenticate => 0,
validate => 0,
http_allow => 'GET,POST',
);

Foswiki::Func::registerRESTHandler( 'tml2html',
sub { _execute( '_restTML2HTML', @_ ) } );
sub { _execute( '_restTML2HTML', @_ ) }, %opts );
Foswiki::Func::registerRESTHandler( 'html2tml',
sub { _execute( '_restHTML2TML', @_ ) } );
sub { _execute( '_restHTML2TML', @_ ) }, %opts );
Foswiki::Func::registerRESTHandler( 'upload',
sub { _execute( '_restUpload', @_ ) } );
sub { _execute( '_restUpload', @_ ) }, %opts );
Foswiki::Func::registerRESTHandler( 'attachments',
sub { _execute( '_restAttachments', @_ ) } );
sub { _execute( '_restAttachments', @_ ) }, %opts );

# Plugin correctly initialized
return 1;
Expand Down
12 changes: 11 additions & 1 deletion core/data/System/ReleaseNotes01x02.txt
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,17 @@ A conversion tool is availabe in the tools directory. =tools/convertTopicSettin
#
</verbatim>

---++ New Pluggable Access Control implementations.
---+++ REST Script default security has changed:

Foswiki 1.2 has removed the =rest= script from the list of ={AuthScripts}=. Instead of providing blanket
security for =rest=, each handler is now responsible to set its individual requirements for 3 options:
<i>authentication</i>, <i>validation</i> and <i>http_allow</i>ed methods (POST vs. GET). The defaults for these
3 options have been changed to default to be secure, and handlers can exempt these checks based upon their specific requirements.

A new configuration option has been added to the _Security and Authentication_ section, _Login_ tab: ={InsecureREST}=. Enable this setting to restore the old insecure
defaults for REST handlers. If enabled, and =rest= is not listed in ={AuthScripts}=, a warning will be displayed.

---+++ New Pluggable Access Control implementations.

Foswiki has made the Access Control implementation "Pluggable". New ACL
methods may be more easily implemented in the future. The default method is
Expand Down
9 changes: 9 additions & 0 deletions core/lib/Foswiki.spec
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,15 @@ $Foswiki::cfg{Trace}{LoginManager} = 0;
$Foswiki::cfg{AuthScripts} =
'attach,compareauth,edit,manage,previewauth,rdiffauth,rename,restauth,save,statistics,upload,viewauth,viewfileauth';

# **BOOLEAN**
# Foswiki 1.2 has removed the <tt>rest</tt> script from the list of <tt>{AuthScripts}</tt>. Instead of providing blanket
# security for <tt>rest</tt>, each handler is now responsible to set its individual requirements for 3 options:
# <i>authentication</i>, <i>validation</i> and <i>http_allow</i>ed methods (POST vs. GET). The defaults for these
# 3 options have been changed to default to be secure, and handlers can exempt these checks based upon their specific requirements.
# <p>Enable this setting to restore the original insecure defaults. A warning will be displayed if this parameter is enabled
# and <tt>rest</tt> is not listed in <tt>{AuthScripts}</tt></p>
$Foswiki::cfg{InsecureREST} = $TRUE;

# **REGEX EXPERT**
# Regular expression matching the scripts that should be allowed to accept the
# <tt>username</tt> and <tt>password</tt> parameters other than the login script. Older versions of
Expand Down
34 changes: 33 additions & 1 deletion core/lib/Foswiki/UI/Rest.pm
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ sub rest {
my $res = $session->{response};
my $err;

# Referer is useful for logging REST request errors
my $referer = ( defined $ENV{HTTP_REFERER} ) ? $ENV{HTTP_REFERER} : '';

# Must define topic param in the query to avoid plugins being
# passed the path_info when the are initialised. We can't affect
# the path_info, but we *can* persuade Foswiki to ignore it.
Expand All @@ -100,6 +103,8 @@ sub rest {
$err = 'ERROR: (400) Invalid REST invocation'
. " - Invalid topic parameter $topic\n";
$res->print($err);
$session->logger->log( 'warning', "REST rejected: " . $err,
" - $referer", );
throw Foswiki::EngineException( 400, $err, $res );
}
}
Expand Down Expand Up @@ -169,6 +174,8 @@ sub rest {
$err =
"ERROR: (400) Invalid REST invocation - $pathInfo is malformed\n";
$res->print($err);
$session->logger->log( 'warning', "REST rejected: " . $err,
" - $referer", );

$res->print(
"\nusage: ./rest /PluginName/restHandler param=value\n\n" . join(
Expand Down Expand Up @@ -196,10 +203,20 @@ sub rest {
'ERROR: (404) Invalid REST invocation - '
. $pathInfo
. ' does not refer to a known handler';
$session->logger->log( 'warning', "REST rejected: " . $err,
" - $referer", );
$res->print($err);
throw Foswiki::EngineException( 404, $err, $res );
}

# This allows us to remove rest from the list of {AuthScripts} list
# Individual rest handlers should explicitly set their own requirements.
unless ( $Foswiki::cfg{InsecureREST} ) {
$record->{http_allow} = 'POST' unless defined $record->{http_allow};
$record->{authenticate} = 1 unless defined $record->{authenticate};
$record->{validate} = 1 unless defined $record->{validate};
}

# Check the method is allowed
if ( $record->{http_allow} && defined $req->method() ) {
unless ( $session->inContext('command_line') ) {
Expand All @@ -211,6 +228,11 @@ sub rest {
'ERROR: (405) Bad Request: '
. uc( $req->method() )
. ' denied';
$session->logger->log(
'warning',
"REST rejected: " . $err,
" $subject/$verb - $referer",
);
$res->print($err);
throw Foswiki::EngineException( 405, $err, $res );
}
Expand All @@ -226,6 +248,11 @@ sub rest {
{
$res->header( -type => 'text/html', -status => '401' );
$err = "ERROR: (401) $pathInfo requires you to be logged in";
$session->logger->log(
'warning',
"REST rejected: " . $err,
" $subject/$verb - $referer"
);
$res->print($err);
throw Foswiki::EngineException( 401, $err, $res );
}
Expand Down Expand Up @@ -253,6 +280,11 @@ sub rest {
{
$res->header( -type => 'text/html', -status => '403' );
$err = "ERROR: (403) Invalid validation code";
$session->logger->log(
'warning',
"REST rejected: " . $err,
" $subject/$verb - $referer"
);
$res->print($err);
throw Foswiki::EngineException( 403, $err, $res );
}
Expand Down Expand Up @@ -312,7 +344,7 @@ sub rest {
-charset => 'UTF-8'
);
$session->{response}
->print( 'ERROR: (404) Invalid REST invocation - '
->print( 'ERROR: (403) Invalid REST invocation - '
. ' redirectto does not refer to a valid redirect target'
);
return;
Expand Down

0 comments on commit a0ac38f

Please sign in to comment.