Action entries cannot be whitelisted/blacklisted

[https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA]

<Action ...> configuration entries bypass the Decisions' whitelist/blacklist filtering by the client.

A simple example:

<Bundle name='a'>
  <BoundAction name='a1' command='/usr/bin/touch /tmp/a1_executed' when='always' status='check' timing='post' />
</Bundle>

Then run bcfg2 -l whitelist. The Action's command will get executed, even though it's not whitelisted.

This behaviour doesn't seem to be documented. IMHO, the promise of white/blacklist safety is not fulfilled here. (Eg. I use Actions for setting up static routing using just the 'route' command, so an Action is the sole element of the Bundle, and, obviously, I need to control which hosts actually execute it).

I believe it happens in Client.Frame.Install(), where tool.BundleUpdated() and tool.BundleNotUpdated() are called unconditionally.

Also, in Client.Frame.Decide() some Actions get run regardless of their white/blacklist status. (Perhaps there are some other places where Actions can get run, but my understanding of the code is not good enough to find them.)

A partial workaround could be to add to the Bundle an artificial config file (crafted just to mirror the actual "volatile" configuration), and set the Action to when='modified'. This would still be fragile for some non-trivial Bundles and commands, though.

/mkd

[solj]

I can't reproduce this with the current code and it seems like this should be fixed in the most recent stable release. It looks like this was fixed in 92a7d555ff99e187d4583559f6abe82739221de9.

[https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA]

Sol,

I'm running >1.2.0pre3 (HEAD-9070f86, to be specific).

I think that on Linux you need to use /bin/touch instead of /usr/bin/touch. (We are primarily a Solaris/Aix? shop, so I'm a little biased :( ).

Regards, /mkd

[solj]

My fault, I was testing dryrun and not whitelist/blacklist. I can reproduce this reliably. I'll try and come up with a patch for you to try.

[solj]

Can you try this out and see if it fixes the problem for you?

diff --git a/src/lib/Client/Tools/Action.py b/src/lib/Client/Tools/Action.py
index bc57a0e..7eb81bc 100644
--- a/src/lib/Client/Tools/Action.py
+++ b/src/lib/Client/Tools/Action.py
@@ -83,6 +83,8 @@ class Action(Bcfg2.Client.Tools.Tool):
     def BundleNotUpdated(self, bundle, states):
         """Run Actions when bundles have not been updated."""
         for action in bundle.findall("Action"):
-            if action.get('timing') in ['post', 'both'] and \
-               action.get('when') != 'modified':
-                states[action] = self.RunAction(action)
+            if self.setup['decision'] == 'whitelist' and \
+               action in self.setup['decision_list']:
+                if action.get('timing') in ['post', 'both'] and \
+                   action.get('when') != 'modified':
+                    states[action] = self.RunAction(action)

[https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA]

The patch solves my issue.

Thanks! :)

[solj]

Fixed in 8059a36.

[solj]

8059a36335f4bc4baedfd7a6e0b434779e89fe84 for the trac link.

[https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA]

The fix in 8059a36 caused all Actions in BundleNotUpdated not be run at all when client was run in any other mode than 'whitelist'. Also, it did not handle '*' in blacklisted/whitelisted names.

Perhaps this code of Client/Tools/Action.py could take care of these issues too.

At the beginning of the file:

from Bcfg2.Client.Frame import matches_white_list, passes_black_list

At the end, replacing code of BundleUpdated and BundleNotUpdated:

    def _action_allowed(self, action):
        if self.setup['decision'] == 'whitelist' and \
           not matches_white_list(action, self.setup['decision_list']):
            self.logger.info("In whitelist mode: suppressing Action:" + \
                             action.get('name'))
            return False
        if self.setup['decision'] == 'blacklist' and \
           not passes_black_list(action, self.setup['decision_list']):
            self.logger.info("In blacklist mode: suppressing Action:" + \
                             action.get('name'))
            return False
        return True

    def BundleUpdated(self, bundle, states):
        """Run postinstalls when bundles have been updated."""
        for postinst in bundle.findall("PostInstall"):
            if not self._action_allowed(postinst):
                continue
            self.cmd.run(postinst.get('name'))
        for action in bundle.findall("Action"):
            if action.get('timing') in ['post', 'both']:
                if not self._action_allowed(action):
                    continue
                states[action] = self.RunAction(action)

    def BundleNotUpdated(self, bundle, states):
        """Run Actions when bundles have not been updated."""
        for action in bundle.findall("Action"):
            if action.get('timing') in ['post', 'both'] and \
               action.get('when') != 'modified':
                if not self._action_allowed(action):
                    continue
                states[action] = self.RunAction(action)

/mkd

[solj]

Replying to https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA:

The fix in 8059a36 caused all Actions in BundleNotUpdated not be run at all when client was run in any other mode than 'whitelist'. Also, it did not handle '*' in blacklisted/whitelisted names.

Can you please elaborate more (or maybe provide an example) of when things aren't working as expected? The Action is running properly for me when *not* in whitelist mode:

bcfg2 # grep '/root/test' /var/lib/bcfg2/Bundler/test.xml
        <BoundAction name='test' command='/usr/bin/touch /root/test' when='always' status='check' timing='post'/>

bcfg2 # ls /root/test
ls: cannot access /root/test: No such file or directory

bcfg2 # bcfg2 -vq -b test                                                                                                                              
     
bcfg2 # ls /root/test 
/root/test

Also, can you describe how you're trying to use '*' in Decisions so I can try and replicate the behavior on my end?

[https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA]

Ugh, now I see that the committed change is different than the prototype you've first proposed (and which I tested). I didn't checkout the code from git to test again. Well, this is embarrassing...

Regarding the use of '*': I have Actions named like staticroute-172.16.8.0, staticroute-172.16.3.0, staticroute-192.168.11.0, etc. (The names are dynamically generated, according to missing routes on the client.)

Then I permit appropriate 'safe' routes in whitelist:

  <Decision type='Action' name='staticroute-172.16.*' />

Other routes (and Actions) should be disallowed in whitelist mode.

I think that some explanation of reasoning behind my reopen and patch is due. I believe that the handling of Actions should be complete and consistent across the board. I.e. regardless whether it's a whitelist or blacklist, the bundle was updated or not updated, the action is run before bundle install or after - if a user wants to filter an Action out with Decisions, there should be no surprises for him.

I consider Actions a somewhat substandard configuration mechanism, and Bcfg2 seems to treat them as such, but sometimes they need to be used just like a first-class citizen. (Eg. in AIX one must use commands like chdev or odmadd to edit the binary ODM database - just to set up routing, beside other things). In such circumstances, being able to actually control their execution will be definitely helpful.

/mkd

[solj]

Replying to https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA:

Ugh, now I see that the committed change is different than the prototype you've first proposed (and which I tested). I didn't checkout the code from git to test again. Well, this is embarrassing...

Sorry about that. I should have mentioned that in the comment. The fix was definitely different.

Regarding the use of '*': I have Actions named like staticroute-172.16.8.0, staticroute-172.16.3.0, staticroute-192.168.11.0, etc. (The names are dynamically generated, according to missing routes on the client.)

Then I permit appropriate 'safe' routes in whitelist:

  <Decision type='Action' name='staticroute-172.16.*' />

Other routes (and Actions) should be disallowed in whitelist mode.

I haven't actually tested this. Let me do so and see if I can replicate things on my end.

I think that some explanation of reasoning behind my reopen and patch is due. I believe that the handling of Actions should be complete and consistent across the board. I.e. regardless whether it's a whitelist or blacklist, the bundle was updated or not updated, the action is run before bundle install or after - if a user wants to filter an Action out with Decisions, there should be no surprises for him.

I consider Actions a somewhat substandard configuration mechanism, and Bcfg2 seems to treat them as such, but sometimes they need to be used just like a first-class citizen. (Eg. in AIX one must use commands like chdev or odmadd to edit the binary ODM database - just to set up routing, beside other things). In such circumstances, being able to actually control their execution will be definitely helpful.

I agree with both of these.

[solj]

I tested your fix and it worked nicely for me. Committed in 3fc2476a0188d28cd0a3544d18a43f04950c78ce. Thanks!

[https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA]

Sol,

Well, *I* feel embarassed for carelessly not checking out the actual production code, knowing that the preliminary patch is just a proof of concept. You should *not* feel sorry :)

Back to the general issue:

I've been thinking about a driver which perhaps could alleviate the Action problem.

This driver should be able to run some command to get the actual value of a given parameter (this is the key part, I think), map the value to a common format, and be able to implement a new value. It should be generic enough to allow for most imaginable textual interactions.

This way any 'abstract' system/app parameters, providing existence of a command-based API to manipulate them, could be treated just like any other configuration elements.

(I've actually written a similar driver (in fact, a few of them, all based on a common 'engine'), to handle some AIX and Solaris oddities. But it's still not really general and not configurable enough to deserve pushing upstream.)

[solj]

I think I understand what you're getting at and I would be interested in helping you test out both of these. I have a few Solaris machines as well as AIX which have been fairly painful to manage. We are planning to do a release candidate this month, but perhaps we could plan this for 1.3.