flokli / puppet-wireguard

MIT License
4 stars 6 forks source link

security vulnerability with private key parameter #5

Open logicminds opened 6 years ago

logicminds commented 6 years ago

The private key is passed directly to the template without any kind of encryption. This exposes the key in hiera, catalogs, and log files which could be used in spoofing a tunnel.

You could pass the path to the private key if the key was made via puppet or existed previously. Additionally you could also read with the file function `$private_key_hash = file('/etc/wireguard/privateKey'). Then use the variable in the template as you are doing. However, this method only works when puppet apply is used.

Alternatively you can use https://forge.puppet.com/binford2k/node_encrypt to encrypt the content all the way to the node.

flokli commented 6 years ago

Thanks for reporting!

This depends a bit on your threat model - currently I'm assuming the puppet master is a trustful entity (as it practically has root on every machine anyhow) - and as long as wireguard::tunnel resources are not exported, they shouldn't be inspectable from other hostile agents, right? Please correct me if I'm wrong here!

Resorting to use file() and switching to use puppet apply local-mode only on each node is something I really don't want to do. This module should be something still usable in a standard puppet master - agent setup.

I could imagine adding support in copying over private encrypted keys to the agent via node_encrypt and decrypting locally (and would gladly accept pull requests for that too ;-) )

IIRC, passing a file path as PrivateKey= is not something supported by wg setconf (to which we pass the generated config file currently), so we might have to do some manual splicing together of templated config file and the private key transferred via node_encrypt.

flokli commented 6 years ago

@logicminds ping :-)

logicminds commented 6 years ago

To do this properly a few puppet types would be required. I don't have time right now to do this.