Feature Proposal: Remove the -T flag from the foswiki scripts

Motivation

There are 4 major reasons:

• (1) locales are not really usable with -T enabled
• (2) performance: It's been reported a 10% boost in performance by disabling -T
• (3) bug in old perls as reported in Item12887
• (4) local perl friendly ( #!/usr/bin/env perl does not support flags.)

Description and Documentation

This was under informal discussion for a future I18N release. Perl taints the results of any regular expression that references locales. This breaks foswiki in many places, and in some cases there appears to be no way to get perl to actually untaint the results.

This proposal is to accelerate this change into release 1.2. Taint checking is reasonable in a development environment where issues exposed can be caught and resolved. But once foswiki is in production, it doesn't add much value.

It would probably still be valuable to run our test systems with -T enabled.

Examples

Impact

WhatDoesItAffect:

%WHATDOESITAFFECT%

edit

Implementation

-- Contributors: GeorgeClark - 04 Aug 2014

Discussion

I can confirm the 10 % speedboost. I have tried to remove the -T from both production and backup server and both dropped 0.1 second off a 1 second time. That is significant.

Best in my view is to develop with -T and ship without.

-- KennethLavrsen - 06 Aug 2014

I'd agree.

Of course some tests will have to run without the -T switch as they affect I18N. I'm not sure how that will impact the test infrastructure. I'm not particularly concerned right now because we do not have tests that cover I18N and locales - at least not until we make this change to make these tests possible.

-- JulianLevens - 06 Aug 2014

The obvious questions are "what does taint mode do for us?" and following on from that "can we craft tests that cover what taint mode does?"

For those that don't know, taint mode tries to ensure that no data sourced from outside the program can be used in a way that may compromise the server. In Foswiki, that applies mainly to data sourced from command-line arguments, and data coming from topics, as these are the main attack surfaces for a hacker. Other attack surfaces include cookie data and session data, though taint mode is of limited value when defending these. Command line parameters are the most open, and the most often attacked, facet of Foswiki.

So what does taint mode do for us? It marks any data coming from command-line arguments and topics as "tainted", so that if that data is then used in a potentially dangerous way - such as in an eval - that usage can be detected and rejected with an "Insecure dependency" exception. The risk of command-line parameters (or topic content) being used to craft an attack on the server is slightly moderated by the use of taint mode. I say "slightly" because the effectiveness of taint mode in the Foswiki codebase is severely limited by programmer behaviour. It is trivial to untaint data, and as I recall, every case highlighted to the Foswiki security list where taint mode should have detected the problem, the taint had been rendered ineffective by an unvalidated untaint. These untaints were added during development, presumably by the programmer seeking a "quick fix" to the taint exception, without fully realising the impact of what they are doing. More often than not the problems exist with the rarely-used parameters to an extension that in some cases have clearly never even been tested by the developer. It's hard to quantify what problems taint mode has detected in production systems; however we can be sure that it has blocked some (though far from all) problems reaching that stage.

So, the major advantage of taint mode is during development, especially of plugins and third party extensions, where it provides immediate feedback regarding a potential attack surface. The chances of that attack surface persisting into production depends entirely on the quality of the testing performed on the code. Which leads us on to the question about testing. Can we craft tests (using the existing infrastructure) that cover what taint mode does in a production system? In a word, no. The Foswiki testing regime is based heavily on unit testing. While some developers actively use unit testing during development, many don't, and the main way unit testing is used in Foswiki is to help avoid already-detected and-in-theory-fixed problems re-emerging during later phases of development. Unit testing is not really the best tool for the kind of exhaustive exercising of the command-line that would help avoid the problems that taint mode might detect.

Another consideration is that taint mode is deeply embedded in the Perl psyche, many sysadmins have sufficient knowledge to spot that taint mode is enabled, and will see that as a good thing. Turn off taint mode, and we lose that comfort blanket. Further, there is a real case where taint mode in a production system has value; where we want to install a not totally trusted extension. Taint mode gives us some level of testing and a bit more confidence in that code.

In summary, while a 10% performance advantage is desirable for stable systems, probably behind firewalls and with small attack surfaces, the trade-off is against a small - say 5% - increase in security confidence. I would not be happy shipping a system in which taint mode cannot be enabled. Some (all?) web servers support the taint flag being passed in on the perl command line - that may suffice, and negate the need for -T in the scripts, but it needs to be carefully documented so the sysadmins understand the tradeoff.

-- CrawfordCurrie - 07 Aug 2014

Foswiki is okay running in taint mode as long as you don't want to have locales at the same time. This is a known deficiency in perl caused by strings being considered tainted even though they are not provided by the user. For web apps requiring locales this is a killer. I don't see this being fixed any time soon in newer perls, nor organizations adopting newer perls anyway.

Therefore we have two options:

1. forget about locales
2. forget about taint mode

For a long time I have been undecided which of the two is the worse one, until after I learned about the performance implications of taint checks in perl as well as the problem of switching on taint mode in local perl environments (such as plenv or perlbrew).

In addition to that I am unsure whether taint checks are well suited to stop any attack vector at all on a production system, i.e. those that we really have to care about. From what I see taint mode delivers more of a cozy feeling that once activated everything is fine and secure ... which is not the case all too often.

What we really have to take care of is proper validation of user strings independent of taint mode or not. Granted getting a taint mode alarm is a good indicator that validation is missing. Therefore taint mode should be enabled during development as long as practical given the drawbacks when used in combination with locales.

Foswiki should always be able to be deployed with taint mode enabled ... as long as you don't enable locales at the same time. This is something we can test for in configure and add a bold warning on a configuration error along these lines.

If you need (a) more speed (b) locales and (c) deploy using a local perl then you might strongly consider disabling taint mode. Doing so should be sufficiently easy for users.

-- MichaelDaum - 07 Aug 2014

Opened task Item13028. I'll plan to leave -T in place in the git source, and we can use rewriteshebang to remove it when building a release.

-- GeorgeClark - 15 Sep 2014

I'd rather remove it in git as well to pave the way for developing locales support.

-- MichaelDaum - 15 Sep 2014

Developers who are doing I18N / locales work will be able to easily use rewriteshebang to remove the -T. Add the bin scripts to the .gitignore and it should be pretty unobtrusive, while still defaulting to taint checking for routine development.

If we remove it from git (ie during development) and we remove it from shipped code, what's left. Just give up on any checking of developed code for insufficient validations?

I thought that removing it from shipped code, and making it easy for devs to turn it off when doing locale work seemed like a fair compromise. Another option might be to use -t, so that the code issues warnings instead of fail. But since devs hopefully run with ASSERTS enabled, those warnings become failures, so I don't think that this buys much.

-- GeorgeClark - 15 Sep 2014

Hm, adding bin scripts (and some in the tools directory as well being used in cronjobs etc) to .gitignore is actually not a good idea as they definitely need versioning.

How about enabling taint mode for the unit tests only?

-- MichaelDaum - 16 Sep 2014

You are right, .gitignore is not the right answer. That's supposed to only affect untracked files anyway. Check out git update-index --skip-worktree or the --assume-unchanged bits, Setting either of those bit on files in the git index causes it to ignore any local changes.

With --assume-unchanged, you can still get the changes checked in by using an explicit "git commit <filename> " command, and if it changes upstream, git fetch will let you know. With the --skip-worktree bit, you need to remove the bit before you can update, and upstream changes are made to the index, but the working copy is untouched.

Either of these solutions would probably work for the bin scripts. They are very seldom changed.

These can also be really helpful if you want to change AdminGroup.txt or other shipped files on your local install and don't want to check them in for everyone.

See http://stackoverflow.com/questions/13630849/git-difference-between-assume-unchanged-and-skip-worktree

git ls-files -v will show the flags for all files. "h" is assume-unchanged, "S" is skip-worktree, and "s" is the combination   So git ls-files -v | grep ^[hsS] shows you everything being ignored and/or skipped.

-- GeorgeClark - 16 Sep 2014

My proposal is as follows:

1. Remove -T from all perl scripts
2. Check for the presence of Taint::Runtime and only enable taint if:
   1. DEBUG is on,
   2. Taint::Runtime is present, and
   3. {UseLocale} is off

Thus a developer should install Taint::Runtime to get taint checking. Normal users will not.

I have implemented this proposal (including removing -T from all scripts)

-- CrawfordCurrie - 09 Feb 2015

If the road to 2.0 covers PSGI(Plackup) too, the standard plackup doesn't handles taint mode. Because me personally agree with the usefulness of the taint-mode development (really is *not needed* for production), will need to use some "special" PSGI-development process. Check this: http://stackoverflow.com/a/6169852/632407 .

-- JozefMojzis - 09 Feb 2015

Commenting is disabled while not logged in