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

There is a draft plan for implementing support for deferred mode #104

Closed
1 task
izgeri opened this issue Jul 8, 2020 · 7 comments
Closed
1 task

There is a draft plan for implementing support for deferred mode #104

izgeri opened this issue Jul 8, 2020 · 7 comments

Comments

@izgeri
Copy link
Contributor

izgeri commented Jul 8, 2020

In Milestone 3 of #20, we'll be adding support for the Puppet integration to leverage deferred functions to delegate secret retrieval to Puppet agents.

From the epic:

Deferred Secret Retrieval – Leveraging the new deferred function feature in Puppet 6, the Conjur module can now delegate secret retrieval to the node itself. Previously, secrets were always retrieved by the Puppet master and provided to each node via the requested catalog. This follows Puppet’s recommend best practices for secret store integrations, as it removes the Puppet master as an unnecessary intermediary.

In this spike, we'll research what will be required to add this support. Note: we must add this support so that this module continues to support Puppet 5, which means that it must be configurable whether to operate in deferred mode or not.

AC:

  • There is a comment on this issue with a draft plan for adding optional support for running the Conjur Puppet module with deferred functions when using Puppet 6. The plan should include:
    • Suggestion for how to configure the module to use deferred functions (does the configuration need to be on the server side or the agent side? can it be a new variable in the conjur class definition of the agent? etc)
    • Proposal for how deferred function support should be implemented / invoked when the module is configured to use it
    • What the requirements that must be met to use deferred functions? (eg are Puppet 6 server AND agents required?)
    • What error checking should be added to the module to "fail fast" if the env doesn't meet the requirements?
@izgeri
Copy link
Contributor Author

izgeri commented Jul 13, 2020

Noting here: this might be as simple as wrapping the Conjur lookup in Deferred. See the docs here. It would be valuable in this spike to validate whether this works, and if so we can document that "when using puppet 6, wrap the lookup function call in Deferred to run the lookup on the node instead of on the puppet server".

@doodlesbykumbi
Copy link
Contributor

Tried to run a manifest with Deferred using the following, using the docs for reference https://puppet.com/docs/puppet/6.17/deferring_functions.html.

  notify { "Grabbing 'inventory/db-password' secret...": }
  $variables = {
    'secret' => Deferred('conjur::secret', ['inventory/db-password']),
  }

  # WARNING: You should not print secrets like this to console in
  #          non-development environments!
  file { $pem_file:
    ensure  => file,
    content => Deferred('inline_epp',['<%= $secret %>', $variables]),
  }

The result was an error on the agent side:

Error: Failed to apply catalog: undefined method `variable_value' for nil:NilClass

It seems that the conjur::secret method doesn't just assume deferred functionality for free.

@sgnn7 sgnn7 self-assigned this Jul 29, 2020
@sgnn7
Copy link
Contributor

sgnn7 commented Jul 29, 2020

A bit deeper info on the error:

Test manifest:

File { backup => false }

node default {
  if ($facts['windows_puppet_agent']) {
    $pem_file  = 'c:/tmp/test.pem'
  } else {
    $pem_file  = '/tmp/test.pem'
  }

  notify { "Including conjur module...": }
  include conjur

  notify { "Grabbing 'inventory/db-password' secret...": }
  $secret =  Sensitive(Deferred(conjur::secret, ['inventory/db-password']))

  notify { "Writing secret '${secret.unwrap}' to $pem_file...": }
  file { $pem_file:
    ensure  => file,
    content => $secret,
  }

  notify { "Done!": }
}

Trace output:

Error: Failed to apply catalog: undefined method `variable_value' for nil:NilClass                                                                                                                                                                                           
/opt/puppetlabs/puppet/cache/lib/puppet/functions/conjur/secret.rb:18:in `secret'                                                                                                                                                                                            
/opt/puppetlabs/puppet/cache/lib/puppet/functions/conjur/secret.rb:23:in `with_defaults'                                                                                                                                                                                     
/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatch.rb:60:in `invoke'                         

Relevant code section where scope['conjur::client'] is nil (which needs to not be nil for Deferred to work): https://github.com/cyberark/conjur-puppet/blob/master/lib/puppet/functions/conjur/secret.rb#L16-L24:

  def secret client, account, id, token
    token = token.unwrap if token.respond_to? :unwrap
    sensitive.new client.variable_value account, id, token #Exception occurs here since client is 'nil'
  end

  def with_defaults id
    scope = closure_scope
    secret scope['conjur::client'], scope['conjur::authn_account'], id, scope['conjur::token']
  end

@sgnn7
Copy link
Contributor

sgnn7 commented Jul 29, 2020

Preliminary research:

  • We need to switch from scope to Facter-based variable retrieval.
    • There may be some subtle differences between scope variables that master might modify vs directly provided facts from the Agent. I'm guessing this comes into play heavily if we use server-provided credentials.
  • We cannot encrypt the token the same way as the agent will not have the master's private key to decrypt it.

Not a valid diff but it shows the most relevant changes to get Deferred working with agent-provided host/apikey combo:
https://gist.github.com/sgnn7/01b88e192fb91ca2b89c06874662a398

@sgnn7
Copy link
Contributor

sgnn7 commented Aug 3, 2020

PoC with agent-provided credentials Deferred working: https://gist.github.com/sgnn7/3aebb6705f9953a90ce81af4462f3407. In the PoC, we still send the encrypted api key up as facts but since it's encrypted and master is the only one that can decrypt it, it's effectively just throwaway data that gets re-pulled when agent encounters the deferred in the manifest.

@sgnn7 sgnn7 changed the title There is a draft plan for implementing backwards-compatible support for deferred mode There is a draft plan for implementing support for deferred mode Aug 10, 2020
@sgnn7
Copy link
Contributor

sgnn7 commented Aug 11, 2020

This is now complete with #179 merged with the following items left to be done:

@sgnn7
Copy link
Contributor

sgnn7 commented Aug 11, 2020

Suggestion for how to configure the module to use deferred functions (does the configuration need to be on the server side or the agent side? can it be a new variable in the conjur class definition of the agent? etc)

We support both

Proposal for how deferred function support should be implemented / invoked when the module is configured to use it

Included in the documentation on main branch

What the requirements that must be met to use deferred functions? (eg are Puppet 6 server AND agents required?)

Puppet 6 server and agent

What error checking should be added to the module to "fail fast" if the env doesn't meet the requirements?

Puppet will prevent running this code on puppet 5 as it does not support deferred functions.

@sgnn7 sgnn7 closed this as completed Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants