Modify

Ticket #1029 (closed defect: fixed)

Opened 11 years ago

Last modified 11 years ago

Action entries cannot be whitelisted/blacklisted

Reported by: https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA Owned by: solj
Priority: major Milestone: Bcfg2 1.2.0 Release
Component: bcfg2-client Version: 1.0
Keywords: Action, Decisions, whitelist, blacklist Cc: [email protected]

Description

<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

Attachments

Change History

comment:1 Changed 11 years ago by 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.

comment:2 Changed 11 years ago by 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

comment:3 Changed 11 years ago by solj

  • Owner changed from desai to solj
  • Status changed from new to accepted

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.

comment:4 follow-up: ↓ 5 Changed 11 years ago by 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)

comment:5 in reply to: ↑ 4 Changed 11 years ago by https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA

The patch solves my issue.

Thanks! :)

comment:6 Changed 11 years ago by solj

Fixed in 8059a36.

comment:7 Changed 11 years ago by solj

  • Status changed from accepted to closed
  • Resolution set to fixed

comment:8 follow-up: ↓ 9 Changed 11 years ago by https://www.google.com/accounts/o8/id?id=AItOawkfHvWdYf7g8kSZA32s7dhK0Xig9JKo_CA

  • Status changed from closed to reopened
  • Resolution fixed deleted

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

comment:9 in reply to: ↑ 8 Changed 11 years ago by solj

  • Status changed from reopened to accepted

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?

comment:10 follow-up: ↓ 11 Changed 11 years ago by 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

comment:11 in reply to: ↑ 10 Changed 11 years ago by 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.

comment:12 Changed 11 years ago by solj

  • Status changed from accepted to closed
  • Resolution set to fixed

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

comment:13 Changed 11 years ago by 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.)

comment:14 Changed 11 years ago by 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.

WARNING! You need to establish a session before you can create or edit tickets. Otherwise the ticket will get treated as spam.

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.