[Bug]: Sensitive values are stripped from resources passing through OpenvoxDB

[smortex]

Is this a critical security issue?

• This is not a security issue.

Describe the Bug

While working on voxpupuli/puppet-bacula#241, I realized that OpenvoxDB strips Sensitive parameters from resources. This is not compatible with exporting a resource with a Sensitive password on one host, and collecting it on another host (so that the same password can be used to setup the client and the server without having to repeat passwords).

It looks like this was done on purpose in fdd4cef 😲.

Expected Behavior

PuppetDB store the resource as exported, without stripping Sensitive parameters.

When collecting exported resources, Sensitive parameters are as they where when the resource was exported.

Maybe puppet query shows [REDACTED] by default if this is a concern?

Steps to Reproduce

1. Have a class with a required Sensitive parameter
2. Export this class
3. Collect the exported class

Environment

n/a

Additional Context

No response

Relevant log output

[hlindberg]

That would be a security issue since the sensitive value could be read by anything having access to puppet db.

To fix this PuppetDB would have to be made aware of what Sensitive is, and this would mean handling a new datatype throughout the code base.

It would then need to handle access to sensitive values based on more than the identity of the user.

The export/import mechanism is not suited for what you are doing. Instead, all instances that should have the password should get it from the same source in a safe way.

[smortex]

That would be a security issue since the sensitive value could be read by anything having access to puppet db.

Well, it quite depends on your threat model: beyond the PuppetServer, who (should have) access (to) PuppetDB? The PuppetServer already manipulates these secrets so is out of the equation. Direct access to the database / puppet manifests is not something that fits in our threat model (that basically mean you are root on our systems, we would have already lost 🤷). Some tools rely on PuppetDB (e.g. PuppetBoard) and these tools should probably only have access to some and not all content in PuppetDB (this is also out of our threat model due to the way we give access to this tool, but I guess PuppetDB has support for filtering what is accessible on a per-user basis, so it should not be an issue).

On the other hand, configuring both sides of a client-server tool with a single resource is really handy, as it avoids duplication of information. If you track the history of this bacula module that rely on PuppetDB to implement this, you will see that its root came from the PuppetLabs Operations Team (smart people who had this neat idea 😁).

The export/import mechanism is not suited for what you are doing. Instead, all instances that should have the password should get it from the same source in a safe way.

I am not sure to follow. I basically have a profile::bacula_server (director + storage daemon) on backup nodes, profile::bacula_client (file daemon) on all nodes, and a bunch of bacula::job in many profiles.

Each file daemon has a password that is also in the director configuration (in clear text in the config on both side, as it is some sort of pre-shared-key). Not having to touch the configuration of the director when we add a node is the key point of this setup: adding a bacula::job without including profile::bacula_client will cause a catalog compilation failure, including profile::bacula_client will add the corresponding configuration on the node classified as a bacula director. So it is impossible to forget one part: either a catalog apply and the director will know how to reach the file daemon, schedule backups, and run them; or a catalog will not compile. Misconfiguration is still possible (e.g. wrong hostname) but in this case this will break quite hard (backup will be scheduled, run and fail). It is not possible to have backup configured but silently not running because some piece of configuration is missing. That is a HUGE safety net.

How could we securely share passwords common to two nodes? Exporting the director configuration fragment from the file daemon catalog seems really straightforward, and I can't see how to achieve this without having 2 resources, one on each side, what we want to avoid?

To fix this PuppetDB would have to be made aware of what Sensitive is, and this would mean handling a new datatype throughout the code base.

It would then need to handle access to sensitive values based on more than the identity of the user.

As far as I see, PuppetDB receive a catalog that include sensitive information and does the filtering (so currently, if PuppetDB is compromised, Sensitive data is already accessible to the attacker, end of the game). The first step would be to revert the commit that does this removal, and make sure we also store which parameters where sensitive in the database. Then when building resources from PuppetDB, we should use this information to do proper wrapping of values to make them sensitive again. That seems pretty doable to me?

[hlindberg]

I was thinking of the more secure option to use Vault, and a deferred lookup of the secret in Vault directly from the client rather than making it available on the server side (and in PDB). This makes use of the client's certificate to identify the node. Without this, the less safe is that any compilation for any node can look up any value in PDB. If you think that is ok, then you could just change the value to not be Sensitive. Using Vault, you grant the nodes that should have access to a particular key (the shared password), and you set it up in Vault. The only thing in clear text is the key name, and it is visible in manifests or in Hiera data.

If you are working on a module where you don't want it to require the use of Vault, you could allow users to choose between Vault (deferred lookup) or look up using server-side Hiera with EYAML (or even clear text in YAML/JSON if they don't care).

PDB has no notion of sensitive values - the filtration is there because it would otherwise just be redacted, or PDB would have to unwrap the sensitive to a clear text value. Doing that was deemed less secure. The way PDB is constructed, there is no type information beyond the fundamental; it does not have anywhere to flag these fundamental data values as being different. To add this would be a major change. Hence, the options are to a) unwrap the value and then it is no longer sensitive, b) store "redacted" (which it cannot do for things like integer, since it would also change the data type, so it would have to be an actual integer value - which will screw up a lot of other things), or c) drop the values it cannot deal with (current solution), d) redo the entire data type handling and DB layer in PDB (huge effort).

The attack on PDB would be for someone to obtain information from a node they gained access to. Any catalog compilation for any node can access any piece of data stored in PDB. By using Vault, the possible attack vector is much smaller since information is only available to those who should have it (it is not in PDB).

Personally, I wanted PDB to handle all of the rich-data data types (and Sensitive in particular), but the PDB team was already decimated at that point, and the only practical option was to filter things out (or leave it as it was with unwanted side effects).