Add alternate of sending clientcert as fact #49
Conversation
This just adds instructions for exposing the contents of the client's hostcert as a custom fact. The compiling master will use the contents of that fact if it doesn't already have the certificate on disk. Fixes #13
|
Any thoughts on this approach, @reidmv & @Sharpie? Is there any reason to not rewrite this as a native fact and move towards using this as the primary vector for encryption? I've been thinking on security weaknesses and I don't think I can find one.
The reason I've been holding off on this is that there's no guarantee that the certificate the agent sends as a fact is indeed the same one it's using to authenticate to Puppet, yet at the same time I can't actually come up with a scenario in which that could be true. Using this would drastically simplify things for the end user, because it would Just Work™️ and there'd be no need to shop around agent certificates with the node_encrypt::certificates class. |
There was a problem hiding this comment.
Certificates are intended to be "public" bits of info, so there shouldn't be any security concern there. This would allow a node to request that another certificate be used to encrypt data --- but that seems to me like it would end up being a feature rather than a bug.
As for the implementation its self,
Pros:
-
Simple approach. Unlikely to break due to changes in Puppet Server.
-
Would work in edge cases where the server may not have the agent's certificate at all --- for example when SSL is being terminated upstream on a load balancer and cert info is not forwarded.
-
Has the side effect of making certificates queryable via PuppetDB.
Cons:
- This is duplicating and re-shipping data that the Puppet Server should be making available via a well-documented API (the server doesn't do this today).
All in all, seems good to me 👍 
README.md
Outdated
| If you cannot sync agent public certificates to your compile masters, you can also | ||
| send them with the catalog request from the agent with a custom fact. Place this | ||
| file in `/etc/puppetlabs/facter/facts.d/clientcert_pem.rb` on each agent node and | ||
| make it executable with `chmod +x` or mode `0755`. |
There was a problem hiding this comment.
This will be a bit heavy as Facter has to fork a new process and load Puppet (this will take a few seconds) just to re-read some data that is already present in the current process. You may be able to do this as a plain Ruby custom fact that is confined by the presence of another fact or file on the file system --- this is what the PE package inventory fact does to allow folks to "opt-in" to its behavior (although, I find the implementation there a bit more convoluted than it needs to be).
There was a problem hiding this comment.
this was just a POC to validate that the idea worked and get some feedback before turning it into a real fact-and to keep -all- agents from sending the fact until I was sure that there wouldn't be any unfortunate side effects.
| @@ -0,0 +1,5 @@ | |||
| Facter.add(:clientcert_pem) do | |||
| setcode do | |||
There was a problem hiding this comment.
If this is a concern being always-on, we can set it to opt-in by adding an escape hatch here.
next unless File.exist? '/etc/puppetlabs/node_encrypt_fact'
This just adds instructions for exposing the contents of the client's
hostcert as a custom fact. The compiling master will use the contents
of that fact if it doesn't already have the certificate on disk.
Update: now switched to always-on native fact. It still defaults to looking for the clientcert on disk, just to be cautious.
Fixes #13