Feature Request: Remove rest from the default list of AuthScripts

Motivation

Equal ajax browsing rights for guests!

Description and Documentation

By default rest is part of the list of scripts that require a valid user. {AuthScripts} is an EXPERT setting in configure and if ApacheLogin is being used, then it needs to be changed there as well. Not very intuative especially as the registerRESTHandler now has the authenticate option for a finer control.

Examples

Impact

A friendlier experience for browsing guests by default.

íITTABLE{format="|label,1|text,70|" changerows="off"}%

WhatDoesItAffect:

%WHATDOESITAFFECT%

Implementation

-- Contributors: DavidPatterson - 12 Aug 2010

Discussion

I believe this is already fixed in 1.1. See registerRESTHandler at http://trunk.foswiki.org/System/PerlDoc?module=Foswiki::Func:

Additional options are set in the %options hash. These options are important to ensuring that requests to your handler can't be used in cross-scripting attacks, or used for phishing.

• authenticate - use this boolean option to require authentication for the handler. If this is set, then an authenticated session must be in place or the REST call will be rejected with a 401 (Unauthorized) status code. By default, rest handlers do not require authentication.

-- AndrewJones - 12 Aug 2010

It's also in 1.0.9. See http://foswiki.org/System/PerlDoc?module=Foswiki::Func#registerRESTHandler_alias_fn_opt

-- CrawfordCurrie - 12 Aug 2010

Ok, but I'm not seeing this behaviour in practice.

-- DavidPatterson - 12 Aug 2010

Doh! rest is in the list of {AuthScripts} as default. This trips the rest request before the handler can even be checked for its preference.

I'm updating this feature request accordingly.

-- DavidPatterson - 12 Aug 2010

And don't forget foswiki_httpd_conf.txt, for those using Apache for autentication.

Current entry:

#<FilesMatch "(attach|edit|manage|rename|save|upload|mail|logon|rest|.*auth).*">
#    require valid-user
#</FilesMatch>

-- AndreLichtsteiner - 12 Aug 2010

the root trouble is that rest needs to be secure by default - as in, any plugin that adds a resthandler should require auth unless it specifies that its safe for guest use in the registerHandler.

mostly because its trivial to write a quick and dirty resthandler that not only disobeys permissions, but also avoids the CSRF and other protections that users have the right to expect.

iirc someone already did the work to make this true without the brutish apache.conf and {AuthScripts}, I bring it up to make sure that the implementor knows they need to test for it.

-- SvenDowideit - 12 Aug 2010

I never did commit the work that would make validation, authentication and only HTTP_POST allowed methods the default in registerRESTHandler(). I wasn't ready at feature freeze.

Probably, instead of {AuthScripts}, it should be {AllowUnauthScripts} or similar.

Anyway, this feature proposal can't go ahead unless we also change registerRESTHandler() defaultsto be secure by default, and move the onus of un-security onto the person calling it explicitly specifying authenticate => 0

-- PaulHarvey - 13 Aug 2010

an added help would be an addendum to InstalledPlugins (ok, so it probably desreves to live in configure)) that lists all installed handlers and restHandlers, and their permissions etc - that way an admin (and attacker) has the possibility to see what insecurities have been installed.

-- SvenDowideit - 13 Aug 2010

{AllowUnauthScripts} presses all the right buttons for me.

-- CrawfordCurrie - 13 Aug 2010

With the support we have in for requiring auth for individual rest requests, plsu the new 401 feature, I reckon it's time to do this. So marking for 1.2, and let's see what the cat drags in.

-- CrawfordCurrie - 17 Feb 2012

What's required to do this? Just remove rest from AuthScripts ?

-- MichaelDaum - 24 Apr 2013

We distinguish between rest and introduced restauth. We had to modify a couple of distributed scripts though.

-- AndreLichtsteiner - 24 Apr 2013

We now have the situation where:

• rest has been removed from AuthScripts
• The default for validate / authenticate / POST has NOT been changed.

This leaves us insecure by default. This needs to be corrected before we can release 1.2, or the work that removed rest from AuthScripts needs to be reversed.

Opening a task.

-- GeorgeClark - 02 Apr 2014

As noted above by SvenDowideit, there is no way for the admin to know which rest handlers are active. the current plugin diagnostics page doesn't process rest handlers because they are not registered in the normal path. This should probably block 1.2.0 as well.

-- GeorgeClark - 02 Apr 2014

Plans to resolve this:

• Extend the %FALIEDPLUGINS% macro to list the registered REST handlers, along with their security settings, only visible to AdminGroup
• Change the defaults in Foswiki::Func::registerRESTHandler and in Foswiki::UI::Rest::registerRESTHandlerto be secure:
  • authenticate default to TRUE
  • validate default to TRUE
  • http_allow default to POST
• Document this change in the release notes and release highlights.

In order to improve backwards compatibility, add an EXPERT configuration option under the Security & Authentication / Environment tab:

• {DefaultInsecureREST} = 'FALSE'; Enable this setting to restore the pre-1.2 defaults. This will be flagged as an error if the rest script is not in AuthScripts list.

-- GeorgeClark - 02 Apr 2014

One more idea, Add a {description} option to the handler, so that the diags can show what the rest verb is used for. Just a bit of fluff, but in order to do the audit we'll need to know that information.

-- GeorgeClark - 03 Apr 2014

I've made all the documentation changes consistent with the above changes. Foswiki::Func and Foswiki::Plugins::EmptyPlugin have been updated. Also GuidelinesForSecureExtensions

I think we still need to review a few things:

• Are changes to all defaults appropriate
• Should CLI mode bypass all the checks automatically? I think that the answer to that is yes. CLI already runs as admin by default, so authenticate should not be necessary. http_allow wouldn't make sense for CLI, and validate for CSRF also doesn't make sense for the CLI engine.
• Timing. When should changes to the defaults be made?

I'm leaning toward the more draconian defaults to hopefully force an explicit decision by the developer to not require all security checks rather than be less secure by default. It also makes the explicit determination to not enforce security requirements somewhat more self documenting, maybe with a comment or two rather than an automatic authenticate=>0, validate=>0, http_allow=>'GET,POST'.

-- GeorgeClark - 05 Apr 2014

Investigating the CLI http_allow checking for Rest, For some reason, Engine::CLI sets the Request method to the script name.

../lib/Foswiki/Engine/CLI.pm:    $req->method( $ENV{FOSWIKI_ACTION} );

So rather than being a valid HTTP protocol method, it's actually our script name. We could either leave that, and set http_allow to 'GET,POST,REST' to permit cli operation, or we should bypass the check for the CLI engine.

-- GeorgeClark - 05 Apr 2014

I couldn't figure out why the UI::Rest had to do password checking and username / password url param processing. That's handled elsewhere. What I've found is that:

• When the rest script runs, a Foswiki session is established.
• Foswiki::LoginManager::loadSession() which validates the username/password parameters, and does a bunch of other work and security checks.
• It then calls userLoggedIn(). This routine tests if context is command_line, and if true, returns without setting the enterContext('authenticated')
• Then =UI::Rest runs and re-checks everything.

The end result is that when run from cli, the identity is established by LoginManager, but the 'authenticated' context is missed. UI::Rest still works though because it repeats it's own login process, all be it with different checks from LoginManager. I think that this is all unnecessary. If LoginManager set context authenticated for cli, (I think it should) then UI::Rest could be significantly trimmed, to just check for the authenticated context, and other rest specific checks. This also lets the CLI version run as admin by default, which is what is documented in CommandAndCGIScripts.

The code I'm questioning is at the top of userLoggedIn. This cause context authenticated to be missed for cli. If I understand what remoteUser is, it also might be failing to set the context for some remote authenticated configurations?

    return
      if $session->inContext('command_line')
       || $session->{remoteUser}
       && $authUser
      && $authUser eq $session->{remoteUser};    # same user

Crawford, I need to chat with you about all this. I've got the refactoring mostly done, but due to the area of code, I'd like to review it before I check anything in.

-- GeorgeClark - 05 Apr 2014

OK, you got me extremely confused until I realised that "the code" you mention above is Release01x01, and not trunk.

When I added the username/password command-line parameter handling, it was a specific requirement for the REST scripts and no other script. Hence the checking in Rest.pm, and the bypass code in LoginManager.pm. By supporting CLI login for all scripts, that can be refactored into the login manager, as you appear to have done (i.e. I don't see anything obvious wrong with your refactoring).

I noted in one of the comments that this requires a review. The reason for that is that I've never been happy with the REST handlers requiring a fully initialised session. It has a major impact on performance that the preferences and plugins have to be loaded for a REST session. For preferences, this is because of the legacy of far too much initialisation information being tied up in the preferences code (such as authentication). For the plugins, the problem is the conflation of REST handler and tag handler initiation into a single initPlugin. In an ideal world, a REST handler would be able to say whether it needed these features up front. However that's not going to happen any time soon

BTW I changed the "CommittedDeveloper" to you, as you have done all this work, not me.

-- CrawfordCurrie - 28 Apr 2014

The current configure messages and warnings about REST security are irritating and potentially misleading. They are hard to understand and users probably decide on the wrong thing to do trying to understand Foswiki security. Let me explain.

(1) There's a boolean flag called InsecureREST defaulting to true. This generates a warning classified as an Error .... Users may think "Is Foswiki insecure by default? What the hell is this flag doing anyway? I better disable the flag because I don't want to enable insecurity."

(2) Yet another double-negotiation in a newly named parameter: enable/disable in-security. And again negated in the perl code unless ($Foswiki::cfg{InsecureREST}) {...}

Gawd. Can we please remove this flag?

(3) Explanatory text in configure

... Instead of providing blanket security for rest, each handler is now responsible to set its individual requirements for 3 options: authentication, validation and http_allowed methods (POST vs. GET). 

Users may think: "I want security. Always. Everywhere. Where can I set these three options? I've been looking everywhere but can't find them. And what will InsecureREST then have for an effect on them?"

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.

Users: "Why do I have to know this? This does not help me in deciding what to do about InsecureREST and all this?"

Enable this setting to restore the original insecure defaults. A warning will be displayed if this parameter is enabled and rest is not listed in {AuthScripts}

Users: "Well this is how it is configured out of the box anyway. Do I have to action upon this now, yes or no? I really don't like warnings. They make me feel uncertain."

I can't see what InsecureREST is good for other than creating an irritation about REST that users can't resolve on this level anyway.

There is no way for users of configure to find out whether their setting are secure or not. Far better is the analysis on InstalledPlugins. However even there no information is available whether plugins properly check ACLs on the course of their rest evaluation ... or not ... and thus create an insecure situation being installed on Foswiki 1.2.

-- MichaelDaum - 28 May 2014

That setting is supposed to change. It's only there to allow plugin authors a chance to update extensions. The default should be unchecked which elimnates the warning, and the three rest security checks are given more secure defaults.    I intended to flip it once you got all your rest handlers updated.   It exists to restore foswiki to the 1.1 defaults if you have rest handlers that are not working.    So out of the box, the warnings should all go away.

-- GeorgeClark - 28 May 2014

How about calling it {LegacyRESTSecurity}?

-- MichaelDaum - 28 May 2014

Thanks for the suggestion. That works great. It's been implemented and Item12839 is now waiting for release.

-- GeorgeClark - 29 May 2014

Commenting is disabled while not logged in