Feature Proposal: Change Access control to avoid built-in security issues

Motivation

I experienced security issues with a client, which I had to address in due course.

Please be aware that there are a lot of people, who might not grasp this issue although it is described correctly in the documentation!

Description and Documentation

Change the access control system, so that it is more flexible by addressing the needs of a user (company). The current access control allows a user to set access rights to a topic for somebody (even TWikiGuest) outside the work area. This should not be allowed or at least optional.

Examples

Imagine you create an area (web), where you store confidential information. This might be a special internal document web, a project web or whatever. Then you deny access (view,change) to this web, but you do not want to set ALLOWTOPICHANGE or ALLOWTOPICVIEW in the Final Preferences, since you want to restrict access to some pages to members (users) of this web.

I had several occasions, where this was the case! Thus I do not think it is exceptional.

Now you have one member (user) of this web, who incidentally or on purpose sets access rights for TWikiGuest to a topic, telling a friend the url, let him download attachments and copy or just read the content, revoke the settings and does this within the timeframe, before a new revision is saved. Nobody ever will know, unless somebody suspects something like that the very day and check the log files.

Personally and from my clients perspective I do not think this is the best solution. Thus I changed Access.pm (see below). But this is not the final solution as there should be something like that implemented in TWiki.

Proposal

1. I suggest to enhance the access control management by implementing a new variable for WebPreferences (like ACCESSTOPIC = on/off) to switch the feature for each web, where appropriate. This would be compatible to earlier TWiki releases.
2. Another option would be to integrate this feature in configure. To switch it on/off for the whole TWiki.
3. Implement a new access control system an get rid of the access variables. This could be one per TWiki or one per web access control files in the working or data space (like .htpasswd), where all access information are stored. This could enhance speed, since just one or two files have to be searched. It would be like an access control registry.

I am happy to volunteer to program that as far as I am capable.

Please find the changed Access.pm below:

sub checkAccessPermission {
    my( $this, $mode, $user, $text, $meta, $topic, $web ) = @_;

    undef $this->{failure};

    print STDERR "Check $mode access $user to ". ($web||'undef'). '.'. ($topic||'undef')."\n" if MONITOR;

    # super admin is always allowed
    if( $this->{session}->{users}->isAdmin( $user ) ) {
        print STDERR "$user - ADMIN\n" if MONITOR;
        return 1;
    }

    $mode = uc( $mode );  # upper case

    my $prefs = $this->{session}->{prefs};

    my $allowTopic;
    my $denyTopic;
    my $allowWeb;
    my $denyWeb;


    # extract the * Set (ALLOWTOPIC|DENYTOPIC)$mode
    if( defined $text ) {
        # override topic permissions.
        $allowTopic = $prefs->getTextPreferencesValue(
            'ALLOWTOPIC'.$mode, $text, $meta, $web, $topic );
        $denyTopic = $prefs->getTextPreferencesValue(
            'DENYTOPIC'.$mode, $text, $meta, $web, $topic );
    } elsif( $topic ) {
        $allowTopic = $prefs->getTopicPreferencesValue( 'ALLOWTOPIC'.$mode,
                                                       $web, $topic );
        $denyTopic = $prefs->getTopicPreferencesValue( 'DENYTOPIC'.$mode,
                                                      $web, $topic );
    }

    # Check DENYTOPIC
    if( defined( $denyTopic )) {
        if( $denyTopic =~ /\S$/ ) {
            if( $this->{session}->{users}->isInList( $user, $denyTopic )) {
                $this->{failure} = $this->{session}->i18n->maketext('access denied on topic');
                print STDERR $this->{failure}." ($denyTopic)\n" if MONITOR;
                return 0;
            }
        } else {
            # If DENYTOPIC is empty, don't deny _anyone_
            print STDERR "DENYTOPIC is empty\n" if MONITOR;
            return 1;
        }
    }


    # Check DENYWEB, but only if DENYTOPIC is not set (even if it
    # is empty - empty means "don't deny anybody")
    unless( defined( $denyTopic )) {
        $denyWeb =
          $prefs->getWebPreferencesValue( 'DENYWEB'.$mode, $web );
        if( defined( $denyTopic ) &&
              $this->{session}->{users}->isInList( $user, $denyTopic )) {
            $this->{failure} = $this->{session}->i18n->maketext('access denied on web');
            print STDERR $this->{failure}."\n" if MONITOR;
            return 0;
        }
    }

    # Check ALLOWWEB. If this is defined and not overridden by
    # ALLOWTOPIC, the user _must_ be in it.
    $allowWeb = $prefs->getWebPreferencesValue( 'ALLOWWEB'.$mode, $web );

    if( defined( $allowWeb ) && $allowWeb =~ /\S/ ) {
        unless( $this->{session}->{users}->isInList( $user, $allowWeb )) {
            $this->{failure} = $this->{session}->i18n->maketext('access not allowed on web');
            print STDERR $this->{failure}."\n" if MONITOR;
            return 0;
        }
    }

    # Check ALLOWTOPIC. If this is defined the user _must_ be in it
    if( defined( $allowTopic ) && $allowTopic =~ /\S/ ) {
        if( $this->{session}->{users}->isInList( $user, $allowTopic )) {
            print STDERR "in ALLOWTOPIC\n" if MONITOR;
            return 1;
        }
        $this->{failure} = $this->{session}->i18n->maketext('access not allowed on topic');
        print STDERR $this->{failure}." ($allowTopic)\n" if MONITOR;
        return 0;
    }

    # Check DENYROOT and ALLOWROOT, but only if web is not defined
    unless( $web ) {
        $denyWeb =
          $prefs->getPreferencesValue( 'DENYROOT'.$mode, $web );
        if( defined( $denyWeb ) &&
              $this->{session}->{users}->isInList( $user, $denyWeb )) {
            $this->{failure} = $this->{session}->i18n->maketext('access denied on root');
            print STDERR $this->{failure}."\n" if MONITOR;
            return 0;
        }

        $allowWeb = $prefs->getPreferencesValue( 'ALLOWROOT'.$mode, $web );

        if( defined( $allowWeb ) && $allowWeb =~ /\S/ ) {
            unless( $this->{session}->{users}->isInList( $user, $allowWeb )) {
                $this->{failure} = $this->{session}->i18n->maketext('access not allowed on root');
                print STDERR $this->{failure}."\n" if MONITOR;
                return 0;
            }
        }
    }

    if( MONITOR ) {
        print STDERR "OK, permitted\n";
        print STDERR "ALLOW: $allowWeb\n" if defined $allowWeb;
        print STDERR "DENY: $denyWeb\n" if defined $denyWeb;
    }
    return 1;
}

1;

Impact

WhatDoesItAffect:

Security %WHATDOESITAFFECT%

edit

Implementation

-- Contributors: WolfMarbach - 10 Oct 2008

Discussion

It seems that in your case, a more appropriate solution would be have TWiki make a revision for each change, and make it the default behavior for new installs. I personally configured all my wikis this way, as the default twiki behavior of timeframe is troublesome in many ways.

This is done in the skin template, by for instance by defining in the edit template of your skin if it is pattern-based:

%TMPL:DEF{"forcerevisioncheckbox"}%<input type="hidden" name="forcenewrevision" checked="checked" />%TMPL:END%

-- ColasNahaboo - 10 Oct 2008

Colas, even with that fix it is possible to make TWiki to save a text into a topic without using the edit script. To be really secure, this is something that must be done in the core.

-- RafaelAlvarez - 10 Oct 2008

Thanks Rafael. The point is that I do not need the change in the revision control. I could bypass this by using the CommentPlugin anyway. As soon as somebody allows e.g. TWikiGuest to access the confidential content the harm is done and you just don't know, if you don't look for it.

-- WolfMarbach - 10 Oct 2008

One solution would be to make access control aware of the FINALPREFERENES setting. That way you could selectively prevent access control on a topic.

-- PeterThoeny - 12 Oct 2008

My understanding of FINALPREFERENCES is that I cannot set the ALLOWTOPICCHANGE (or VIEW) in a Topic any more? If so it would be better to introduce a preference variable like ALLOWOVERRIDE and let access control check if it is set on or off. If on or not defined access control works as usual (compatibility to earlier releases), if it is set 'on' you can set ALLOWTOPIV(VIEW|CHANGE), in a web, but are not allowed to override it by let somebody outside the web seeing it.

Example: Hans, Peter and Tom are allowed to view a web. I want to allow ALLOWTOPICVIEW = Peter, Tom to prevent Hans from seeing it, but do not want to have Tom to set ALLOWTOPICVIEW for TWikiGuest ! As you can see this matter is important to me and I will have to change access.pm for my client as it is a government department and very sensible about access rights.

-- WolfMarbach - 12 Oct 2008

This proposal reminds me an earlier related discussion: TheMissingAccessControlLevel

My concern is that access control itself is useless if someone (allowed to access) wants to make private content public.

-- GilmarSantosJr - 12 Oct 2008

Your concern is mine and that of my client. I agree that content can be copied and pasted, but TWiki claims that it is not just another wiki, but more. If I do not want the features of access control I use perhaps another open wiki. But if I want them, they should be well designed. Changing the TWiki core myself, I do not see as the appropriate solution.

-- WolfMarbach - 13 Oct 2008

Wait one more week until PluggableAccessControlImplementation is in place, so new strategies, mechanisms and backends can be plugged into the core without modifying it. Of course, this will be only in trunk.

-- RafaelAlvarez - 13 Oct 2008

I really do not understand the problem. In your example, if Tom wants to give the info to Hans, he just have to copy the page and paste it in an email. I you want to prevent this kind of scenario (Tom can read thus can send info to Hans), I guess you will need some really secure system, and maybe a secure browser and a secure military grade Operating System (with credentials enforcement on copy paste). TWiki is definitely not at this level, nor any web software I know. And what about if Tom just take a photograph of the screen and send it to Hans?

-- ColasNahaboo - 13 Oct 2008

I had the exact thought as Colas.

If someone wants to disable the feature of saving same revision within one hour simply reduce the setting in configure to 1 second (you cannot edit and save this fast in practical so it is equivalent to disabling the feature).

But this is a pain in the butt! You need to allow people to make typos and correct them without storing a new version. You need to allow people working on a large document to save along the way without making new versions.

Opening up access control without leaving an audit trail for a few minutes is a pseudo problem because as Colas says - anyone can copy the content in an email and send it and you will never know.

If we start enhancing the access control to meet this requirement we fail to solve an actual problem and we complicate the access control feature which is already pretty hard to understand for many users. The more complicated access control is - the more likely people will set them wrong. An ALLOWOVERRIDE feature is in my view a real messy feature which will be hard to both document and understand.

One thing we should consider is a setting which for the entire TWiki installation denies access to TWikiGuest.

Today you have to define such a setting in each web in WebPreferences and you can override it in each topic.

I think we are many that have a TWiki where we have defined all reading as not available unless you are authenticated simply to set a base security level.

I think this simple to understand (and probably also simple to implement) feature with a setting defined in configure will be a much better solution to both my need and Wolf's need.

-- KennethLavrsen - 13 Oct 2008

Coalas - Sorry to say so, but that sounds pretty polemic to me. I wish a more rational discussion.

Kenneth - As my background is not IT but general industry, I understand that there is quite a big gap in the way of thinking between user and IT. I cannot follow your arguments, since any OS if it is Linux or Windows provides this feature. You have folders areas and group you can assign space to and restrict access. If access is restricted in Win or Linux a group member cannot just assign access rights to any other user which is not allowed, right? Why should TWiki be the big exception? I do not get it.

I thought that TWiki is very much similar to the Linux access control with Groups a root admin group and so on.

The only thing I ask for is to adapt this common feature to TWiki as some people I spoke to and I find it very important. Perhaps we can let the TWiki Community vote?

BTW: The ALLOWOVERRIDE, was just an idea to keep compatabilty to earlier releases. I do not need it. And don't bother about the documentation, I would volunteer to revise that part.

Rafael - Your proposal could work for me as well and I think it was the same intention, which brought this idea up. Still I think it would be good to have such a feature in the core to avoid to much confusion. But I am interested in further information, when your trunk is completed.

-- WolfMarbach - 13 Oct 2008

The current TWiki access rights have little in common with the access control rules in Unix and in Windows. What does this mean? Wolf

Surely the Twiki access rules can be improved but I do not see this proposal as an enhancement. It just adds further complications.What kind of complications? Wolf

An additional thing to consider. Wiki's are supposed to do away with the one webmaster syndrome. The key to this is soft security. I live today with a Livelink system where I work and the constant battle with access rights and the fact that noone understands them tell me that this is the wrong way to go.Why does TWiki then has such a fine grained access control system? Wolf

-- KennethLavrsen - 13 Oct 2008

Wolf,

The current TWiki ACL supposes a collaborative environment, where there is some trust among members. Only people with write access can change the access rules, and this group is (or can be) well-defined.

Recall that Unix permissions have the concept of owner. TWiki is about collaborative work. There is no owner of a web or topic. (unless you define ALLOWTOPIC(VIEW|CHANGE) to some user. This possibility is used a lot and I see it as one of the reasons to have such fine grained access control system).

It seems to me that you are concerned about bad behaviored users and what Coalas, Kenneth and I say is that this change doesn't solve the problem and make ACL go higher on NerdoMeter. It seems to me that you are concerned about bad behaviored users and what Colas, Kenneth and I say is that this change doesn't solve the problem and make ACL go higher on NerdoMeter. All proposals, including this one, follow the TWikiReleaseManagementProcess, that we agreed as being a good and democratic process.

Currently we have no consensus, so we can reach it or vote in a release meeting, I mean, the decision is made by the twiki community

-- GilmarSantosJr - 13 Oct 2008

Thanks Gilmar: Although I still have different view, which does not relate to just bad behavoured users, I am glad that I got at least a more factual statement and look forward to a decision made by the community.

PS: I would be really happy to get more detailed argument, why there is a DENYWEBVIEW, when it can be overridden by users. It would help me arguing with my client. Perhaps I am just too stupid to grasp it .

-- WolfMarbach - 13 Oct 2008

Wolf, on the technical side, a problem is that in TWiki access control is in the file contents.That is no problem at all as you can see in the solution above.Wolf In unix, it is outside, in metadata. This makes trying to implement this kind of control in the current TWiki quite complex (you cannot separate rights to edit contents to rights to edit metadata easily)Again that is not complex. Just redirect the proper oops and the user cannot access the manage script for meta preferences.Wolf. Moreover, as Gilmar said, TWiki was not really designed to be protected against people you give write access to.Again: Nobody answered my question: Why do I need access control then?.Wolf For instance, on my personal site, I do not use the comment plugin to enable visitor comments, as it is too risky, I make them leave comments via external comment sites (I removed the comments feature in the blog addon)Aha. So we need access control, don't we?.Wolf. And for my company extranet and intranet I just advise people to have 2 webs, one enabled for write by only trusted people and one for others. Do not mix both levels in the same web.I do not understand what that means.Wolf

-- ColasNahaboo - 13 Oct 2008

Colas I feel very sad as you do not understand me. I added some remarks though and will not comment any more.

-- WolfMarbach - 13 Oct 2008

DENYWEBVIEW is there to help those "admins" by providing a default: Instead of specifying the permissions on each topic, it is defined at each web and then refined by the "admins" on each topic (much like the Windows permission hierarchy)

Wolf's problem came from the fact that, with the current ACL mechanism, all users with write access to a topic can be considered "admins" or "owners" of a topic. It is a "hole"/"feature" that is there basically by design.

I agree with Colas and Kenneth that, by default, TWiki should not be more "restrictive" than it should, and that's why I think than having different PluggableAccessControlImplementation that can be chosen by the TWiki Admin is the right way to go, as different corporation will have different rules.

-- RafaelAlvarez - 14 Oct 2008

There is a wide range of common use cases and scenarios for both schemes. I completely understand you, Wolf, when you are irritated by the fact that a web-level setting can be overridden by a topic-level setting. This is paralleled by the general mechanism preference variables work out. As others pointed out already, ACLs should not be tightened to preference variables, i.e. in the sense they are stored, but also in the way they work. Semantics of both diverge already too much thus irritating users all too often.

The scenario as outlined in the proposal is, that a web-admin wants to have a secure means to block further tinkering with the access rights by providing an upper boundary. However, registered users can "shoot holes" in any such setting made in Legacy.WebPreferences by overriding access rights in the topic.

That's the point at hand.

The desire to provide a hard upper boundary of all access rights to all topics in a web, is absolutely justified. TWiki is used in so many different environments where collaboration in isolated webs also means securing and hiding that area from "unwanted collaboration". Yes, sometimes information silos are wanted and needed, contrary to the pure wiki doctrine (which we don't need; instead we need to listen to users carefully). The most justified use case of webs is creating information silos anyway, but that's a different discussion.

Now, let's have a look at those scenarios where "shooting holes" into the standard web ACLs is actually used as a feature. Imagine a research group using a dedicated web to foster collaboration among that group: collaborative paper authoring, 3rd party paper collection & reviews, conference reports, issue tracking, time tracking, speculative development processes and all sorts of stuff that should only be accessible to this one specific research group. However, what this research group also wants is to open up Legacy and a dedicated Publications topic to be accessible to TWikiGuest, like googleagent, read only. This is done done by using the mechanism discussed here: by shooting specific holes into the default access protection. By nature this is a very controlled and intentional setup that TWiki supports quite nicely.

The difference between those two scenarios is if information is kind of leaking unintentionally (people might shoot holes and create information leakage just by accident), or by creating dedicated pubic topics in an otherwise closed web.

Please, understand that both of these scenarios are perfectly reasonable and justified by the different user needs.

However, as Wolf tried to explain, TWiki does not properly support the need to create a hard upper boundary for access rights, because it is only a two level scheme: topic-web. The proposed solution is to make it a three level scheme: topic-web-root.

And this makes sense. Although the name "root" might not be easily understandable what it refers to. "Web" and "topic" both refer to existing entities, while "root" does not. This might have irritated most people that have commented here so far.

For now, I am still undecided if the proposed augmentation of ACLs is the right way to implement the wanted behavior. TWiki is so so complicated and manyfold that there might be some other way to do the trick without a core change. I simply can't say it 100% that there is no other way to stop users from shooting holes into the ACL harness.

Let me say something not related to this proposal on PluggableAccessControlImplementation. That's all fine and good, but IMHO, TWiki is already "overpluggable". It is more like a camelion that you can tweak up to a completely different product. The more plugins and handlers and layers we create, the more blurry the vanilla product gets. It is already too blurry. It is too hard to configure. It is too easy to miss-configure. It uses bad default settings for the majority of users (IMHO), most of which need detailed reviewing (visiting all Legacy.WebPreferences, even those in the _default and _empty web). Which all makes supporting TWiki quite expensive, given a world where everybody talks of mashups, ease of situational applications and "just click on edit". Most of the time, when the community as a whole does not reach consensus and the proposal got steamroller all over beyond recognition - they go the plugin way. So think twice if you draw the "pluggable" card.

And don't draw the "nerdometer" card either. Instead, try to understand the real need of Wolf and surely that of many others.

Does anybody here have a better idea how to address this specific need, something that really works out? Wolf's proposal does work out. So come up with something else that is at least on par. If you can't, then we have to face the fact that Wolf has made a good point and offerend the code that fixes the issue. I don't want to say that the proposed solution is ready for use, just from the fact that it patches the very core of the most important function in TWiki, which we must not get wrong by all means. I have deep respect for Wolf, having learned perl just recently, pinpoints it down like this. Wolf, now you only have to withstand a community that tends towards endless discussions

-- MichaelDaum - 14 Oct 2008

I agree completely with Wolf's user requirement.

The arguments above, trying to tell Wolf that he does not require the requirement are pretty naive, and un-useful.

One implementation idea, would be to treat this in a similar way as spam-prevention. Given a list of topics in which the addition of ALLOW/DENY statements is to be disallowed, simply abort the save. In fact, this is Wolf's suggestion too - I've just re-phrased it in the hope that the dissenters can understand that this functionality has utility.

As Micha points out, its not actually necessary to implement this in the ACL system - just convenient.

given that we have DENYWEBVIEW - why wouldn't adding DENYTOPICVIEW to the FINALPREFERENCES also make it impossible to override that setting in topics?

consistency and simplicity really is useful.....

-- SvenDowideit - 14 Oct 2008

I think I understood this better after Michael's comments. But I still don't understand how this solves the scenario illustrated by Colas, that is an extended form of the one that motivated this proposal.

Maybe it would prevent accidental leakage (how does a user write " * Set ALOWTOPICVIEW = ..." accidentally?) or "web admins" could state that "this web is accessible only by the Foo group and no one, except us, can change the access rules", but does it solve the problem? Probably many user would be satisfied with this statement, but the leakage problem still exists, so other mechanisms are needed (company's information policy...). And my point is: since other mechanism is needed, there is no difference between the current and the proposed system. IMHO.

I'm in favor of Rafael's way and my concern will be over as soon as there is a specification that doesn't make TWiki harder to use and satisfies this requirement. Unfortunately I don't have suggestions on this, yet.

-- GilmarSantosJr - 14 Oct 2008

wow! here on twiki.org, we are in need of something like this too. Someone just created the TWiki.NotExistingYet topic, which is used in the docco as an example of a non-existant link. I'm going to toss this on the list of things to add to the TWikiDotOrgTeamPlugin

-- SvenDowideit - 14 Oct 2008

Yes, people can make screenshots and send that via email, or simply learn the topic text by heart and write it down using a pencil when they go home. That's however in validation with typical NDAs employees have to sign anyway. So, I am unsure if Colas scenario is helpful on this specific proposal. What we want is really locked down webs in terms of ACLs. We can't sell a "this is impossible" (or even a " we don't want this") to anybody.

-- MichaelDaum - 14 Oct 2008

Thanks Michael and Sven for your helpful and factual comments.

Here we go again:

Proposal:	Enhance the ACL by adding a variable like ALLOWACCESSOVERRIDE = off to enable TWiki Users and Admins to really choose, if they want allow Users to override e.g. the ALLOWWEBVIEW setting or not.

This solution is 100% compatible to all earlier releases and adds a new feature without any hazzle.

I volunteer to revise the doc page.

Please be aware that the actual documentation in TWiki-Web is misleading:

Controlling access to a Web

You can define restrictions on who is allowed to view a Compliance Wiki web. You can restrict access to certain webs to selected Users and Groups, by:

    * authenticating all webs and restricting selected webs: Topic access in all webs is authenticated, and selected webs have restricted access.
    * authenticating and restricting selected webs only: Provide unrestricted viewing access to open webs, with authentication and restriction only on selected webs.

Hence I suggest to think about a new ACL system for TWiki 5.

-- WolfMarbach - 14 Oct 2008

Mmm, I think I see where is the misunderstanding. Wolf, your use case was just an example to explain your feature, not a user requirement. You do not want (or so I guess) to pursue the ultimate goal of having a 100% secure (miltary grade) TWiki. This is fine with me. I was just afraid that you were engaging in an endless battle to make TWiki a CMS.

-- ColasNahaboo - 14 Oct 2008

I understand and like this last proposal better than the original

-- RafaelAlvarez - 14 Oct 2008

Thanks for your comments. My apologies, if the initial proposal was not expressed clearly enough, as it actually has never changed from my point of view . Now this might be a better base for a decision.

-- WolfMarbach - 14 Oct 2008

Commenting is disabled while not logged in