Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(MODULES-8175) Munge Properties Hash for Sensitive #92

Merged
merged 1 commit into from
Dec 14, 2018
Merged

(MODULES-8175) Munge Properties Hash for Sensitive #92

merged 1 commit into from
Dec 14, 2018

Conversation

michaeltlombardi
Copy link
Contributor

Prior to this commit the PowerShell template code included a workaround
which munged a malformed hash (which should have been a sensitive value)
in the New-PSCredential method.

This commit adds a deserialization method and uses it in the munging
of the properties hash itself, obviating the need to handle this in
the PowerShell template code.

This also handles sensitive values for things other than the
MSFT_Credential DSC type, which the prior implementation did not.

@michaeltlombardi
Copy link
Contributor Author

@hlindberg, this is related to the mismatch in keys for Sensitive where the master and agent are of mismatched versions (Puppet 5 master / 6 agent and vice versa).

We were able to workaround the problem in the module, but not sure if this could/should be fixed in the agent code?

@michaeltlombardi michaeltlombardi changed the title (WIP) (MODULES-8175) Munge Properties Hash for Sensitive (MODULES-8175) Munge Properties Hash for Sensitive Dec 11, 2018
@michaeltlombardi
Copy link
Contributor Author

Note: The changes have passed ad-hoc.

@hlindberg
Copy link

I doubt this can be fixed as it it is a matter of having rich-data support or not, and if the catalog stringifies rich data or not. The work around handles one case, when the catalog contains rich data but the agent does not deserialize it (and ends up with the raw __pcore_xxx hashes). Note that later version of puppet uses __ptype and __pvalue instead of the longer tags - but then it should also be using the correct deserialization, so should not be a problem.

@michaeltlombardi
Copy link
Contributor Author

@hlindberg We found this to happen with newer master and older agent, and with older master / newer agent - how likely is this to pop up in other modules that use sensitive when there's a mismatch?

This code should handle both __pcore_* and __p* hashes, I think.

@hlindberg
Copy link

@michaeltlombardi I doubt it is common for modules to have nested structures with sensitive values inside them. We would have heard more complaints if this was common.

(I did indeed miss that __p* cases were in there...)

The "newer master, older agents" case worries me since there you you get stringification where the older master probably issued warnings and sent rich data. The other way around isn't really a supported case (new agents/old master) even if some combinations just happens to be compatible.

@Iristyle
Copy link
Contributor

Once we've run through the various combos / documented, and made sure this works in all cases, I think we can merge.

Since I think I'll be OoO tomorrow, was hoping you might have a look as well @joshcooper. Thanks!

@michaeltlombardi
Copy link
Contributor Author

michaeltlombardi commented Dec 12, 2018

So far I've tested against:

  • Master 6x
    • Agent 5x (rich_data false) - mismatched hash
    • Agent 5x (rich_data true) - mismatched hash
    • Agent 5x (apply)
    • Agent 6x (rich_data false)
    • Agent 6x (rich_data true)
    • Agent 6x (apply)
  • Master 5x
    • Agent 5x (rich_data false)
    • Agent 5x (rich_data true)
    • Agent 5x (apply)
    • Agent 6x (rich_data false) - mismatched hash
    • Agent 6x (rich_data true) - mismatched hash
    • Agent 6x (apply)

Anything else we should add to the matrix? I tried toggling rich_data off on the servers but it didn't seem to make a difference.

lib/puppet/type/dsc.rb Outdated Show resolved Hide resolved
Prior to this commit the module did not handle sensitive values in the
property hash when the master and agent were mismatched major versions -
for example, master 5.x and agent 6.x or vice versa. This problem stems
from the change in the core puppet language for handling rich data types
and caused the agent to be unable to properly deserialize the objects
marked sensitive.

This commit adds a munge_sensitive_hash method and uses it in the munging
of the properties hash itself, ensuring that sensitive datatypes will
be appropriately marked during an agent run.
@michaeltlombardi michaeltlombardi dismissed Iristyle’s stale review December 13, 2018 23:09

Updated to use code in dsc type helpers.

Copy link
Contributor

@Iristyle Iristyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Iristyle Iristyle merged commit 2bec339 into puppetlabs:master Dec 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants