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

Add sensitive support for configs #275

Merged
merged 3 commits into from
Sep 27, 2022

Conversation

teluq-pbrideau
Copy link

Pull Request (PR) description

I would like the proxy_password config not be leaked in the puppet log. This PR address this issue by disabling the show_diff when the given config is of type Sensitive

class example {
  $secret = Sensitive('mysecretpassword')
  class { 'yum' :
    config_options => {
      'proxy'          => 'https://host.example.com:3128',
      'proxy_username' => 'user',
      'proxy_password' => $secret,
    },
  }
}

I could add support for Sensitive[Boolean] and Sensitive[Integer] also if you think it is necessary, feel free to ask for it. I was not sure it would be required.

@bastelfreak bastelfreak added the enhancement New feature or request label Sep 27, 2022
@bastelfreak bastelfreak merged commit e5f3ab4 into voxpupuli:master Sep 27, 2022
@teluq-pbrideau
Copy link
Author

@bastelfreak Thanks for this quick merge, but I’ve discovered a problem today about this feature:
The password is not censored when another config is changed:

Notice: Augeas[yum.conf_proxy_username](provider=augeas): 
--- /etc/yum.conf       2022-09-28 10:53:13.958280359 -0400
+++ /etc/yum.conf.augnew        2022-09-28 11:44:01.581689900 -0400
@@ -10,5 +10,5 @@
 metadata_expire=0
 mirrorlist_expire=0
 proxy=http://host.example.com:3128
-proxy_username=user
+proxy_username=anotheruser
 proxy_password=mysecretpassword

Notice: /Stage[main]/Yum/Yum::Config[proxy_username]/Augeas[yum.conf_proxy_username]/returns: executed successfully (corrective)

It might lure people in a false sense of security by using this config... The better way to deal with this would probably use a variable yum::show_diff and completely revert this commit. I’ll gladly provide the PR and tests. What do you think?

@bastelfreak
Copy link
Member

I still think that the sensitive type is a good idea, it should not be reverted. However, adding the show_diff param would be a good addition (and maybe supporting sensitive for all parameters in that file).

@teluq-pbrideau
Copy link
Author

All right, i’ll make another PR about the show_diff. 👍

I do not understand what you means when you say «supporting sensitive for all parameters». Do you mean supporting the sensitive type for $yum::repos for example? I don’t see a case where I would want to censor the repository I connect to, but I guess someone might want that somehow…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants