chef-workstation icon indicating copy to clipboard operation
chef-workstation copied to clipboard

At creation of Policy.lock.json symbols are already replaced

Open TvG71 opened this issue 4 years ago • 2 comments

Description

Within the policy.lock.json (created during "install .\Policyfile.rb") chef symbols like Time.now.utc are already replaced. If the policy file contains something like this:

default['production']['base']['message'] = "This node was hardened by Chef. Policyfile created for 'production' at #{Time.now.utc}\n"

the lockfine will contain this:

"default_attributes": { "production": { "base": { "message": "This node was hardened by Chef. Policyfile created for 'production' at 2019-11-08 09:44:57 UTC\n" } } },

I expet it to be the same as within the policyfile without already executed replacement. Then the question comes up if this replacement is also done before the hash (SHA1/SHA256) calculation of any file. If this is the case it will have impact on the checksum. If for example a date is within a file the checksum changes with each new run of chef install...

Chef Workstation Version

Chef Workstation version: 0.7.4

Platform Version

Windows 10

TvG71 avatar Nov 08 '19 10:11 TvG71

Thanks for filing an issue @TvG71 but I'm a little confused - what is your ask here? I would expect chef install to generate a new Policyfile.lock with a new SHA so having that happen because of a timestamp does not seem unnatural to me. If nothing in the policyfile has changed chef install should not change the lock (and the timestamp update would change the lock) but I also assume users are running chef install when they expect to update their lock. What is your thinking on this flow?

This actually makes me think there is a security issue in Chef Workstation where we execute strings in Policyfile.rb. Someone could inject malicious code into a public cookbook and have it ran on users workstations.

My proposal would be for us to stop 'executing' the code within those strings. I'll have to look into whether that is possible, and it may fall outside the scope of what we want to do. Chef cookbooks are change oriented and it would be pretty easy to make them malicious if users do not check what their public cookbooks perform.

tyler-ball avatar Nov 08 '19 20:11 tyler-ball

Hi @tyler-ball, the example with the message inside the default attribute section of the Policy-file I have taken from the policy file training. The replacement of the variable #{...} I expect only on the managed node not on the chef workstation. Between creation of the lockfile on the chef workstation and the execution on the managed node can be a longer time so that the message created on the node contains a wrong time stamp. As you also stated if nothing is changed within the Policy.rb the lockfile is not changed. In this case the file was not modified (by hand / on purpose). What happens now with the lockfile if an other user does exactly the same on a different machine? He will get a different lock file ( and if any system variable replacement is done within any of the referenced cookbooks / recipes the checksum for this file and the revision_id of the lockfile is different. Is this really wanted? (maybe I am missing the full understanding of the policy concept here). If I only have a macro replacement inside the policy-file this changes also the revision_id of the lockfile. If I would like to have a message like “this lockfile was crated at ” on the managed nodes then it will be ok to do the replacement on the creation of the lockfile. Interesting thing that you discovered a problem based on my question here with malicious code. Some things like datetime could still be helpful in the policyfile. Maybe you should only allow a predefined set of macros where is also clear where the replacement should be done (workstation-side or managed-node-side) “replacement.workstation.getDateTime.utc or replacement.node.getDateTime.Utc

TvG71 avatar Nov 11 '19 08:11 TvG71