Skip to content

(#19514) trusted facts#1991

Merged
zaphod42 merged 3 commits intopuppetlabs:masterfrom
hlindberg:feature/issue-19514-trusted-facts
Oct 11, 2013
Merged

(#19514) trusted facts#1991
zaphod42 merged 3 commits intopuppetlabs:masterfrom
hlindberg:feature/issue-19514-trusted-facts

Conversation

@hlindberg
Copy link
Contributor

This adds trusted facts - see commit messages.

This adds a new feature where it is possible to opt in to getting a top
scope variable $trusted to contain a hash of trusted data.

The setting that controls this is called hashed_node_data (it is
intended to later also be used for hashed facts as well as the hashed
trusted data; hence its more generic name).

The variable $trusted is only available if :hashed_node_data is set to
true (the default in 3.x is false).

The hash contains the key 'authenticated' which is always set to one of
'remote', 'local' or boolean false. The key 'clientcert' is set if
'authenticated' is set to 'remote' or 'local'.
Other tests were passing nil as node and there were no protection
against nil node in the new indirector code.
@puppetcla
Copy link

CLA signed by all contributors.

Some tests did not expect a call to Node.set_trusted_data. This commit
now only sets this if Puppet[hashed_node_data] is true.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will need some more documentation of what keys can be found in $trusted, and in what way they are "trusted" - your trust may not be the same as mine! From what I understand, the keys there are trusted on the assumption that the client certificate is valid. Would we ever add additional forms of trust?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it needs more docs - currently there is a note in the predocs repo about how it works. The first implementation only has clientsert as a key. There is also a key called "authenticated" which is set to "remote", "local" or false indicating if this was a remote (certificate validated) request, a local request (i.e. puppet apply, or internal runs like rspec testing), or false (remote and not authenticated). In a way, this key is like "authority" (the method used to assert that the values in trusted are just that. Maybe it is too simplistic going forward...

Copy link
Contributor

Choose a reason for hiding this comment

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

The idea was also to use this to store other information from the certificate going forward. The basic idea of what is trusted is that it it is either information directly from the certificate or entirely derived from trusted, non-spoofable data (so something from the certificate combined with information on the master).

@hlindberg, thanks for remember the pre-docs. I'll take a look at those too.

@zaphod42
Copy link
Contributor

@hlindberg I'm going to pull this down and make the few fixups around my comments and then merge it in.

@hlindberg
Copy link
Contributor Author

ok, great, ping if there something else on this I should do.

zaphod42 added a commit that referenced this pull request Oct 11, 2013
* hlindberg-feature/issue-19514-trusted-facts:
  (#19514) Fix up tests that were not providing a valid Node
  (19514) Fix failing tests for expectations of not setting trusted
  (#19514) Fix issues in trusted data support when running tests
  (#19514) Add $trusted as hash with trusted node data

Closes: GH-1991
@zaphod42 zaphod42 merged commit ceeb13b into puppetlabs:master Oct 11, 2013
@hlindberg hlindberg deleted the feature/issue-19514-trusted-facts branch October 11, 2013 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants