Accept Datatype Sensitive for Secrets

[lbetz]

Add Variant Sensitive[String] to String types:

icinga2::feature::api::ticket_salt
icinga2::feature::api::ticket_id
icinga2::feature::elasticsearch::password
icinga2::feature::icingadb::password
icinga2::feature::idomysql::password
icinga2::feature::idopgsql::password
icinga2::feature::influxdb::password
icinga2::feature::influxdb::basic_auth['password']
icinga2::feature::influxdb2::auth_token
icinga2::object::apiuser::password

[to-kn]

This sadly is broken atm, since for example the ido-config contains now:

object IdoPgsqlConnection "ido-pgsql" {
  host = "localhost"
  port = 5432
  user = "icinga"
  password = "Sensitive [value redacted]"
  database = "icingaido"
}

This is only on puppet 6, on puppet 7 everything works as expected...

[lbetz]

Strange. I tested it on different platform and different puppet versions. What kind of puppet version in detail and on what distribution it fails?

[zilchms]

I have the same issue.
Puppetmaster and Agent (the icinga node with the broken config) are SLES 12.
Puppetversion: 6.25.0 for both.
Icinga-icinga2 module version 2.3.0 from puppetforge (duh)

Addition: I am pulling the plaintext password from my hiera. The problem seems to be between wrapping the plaintext password and unwrapping it again.

[zilchms]

After a bit of investigating, i would wager that the problem lies somewhere in the idomysql.pp
More specifically:

icinga2::object { 'icinga2::object::IdoMysqlConnection::ido-mysql':
object_name => 'ido-mysql',
object_type => 'IdoMysqlConnection',
attrs => delete_undef_values(merge($attrs, $attrs_ssl)),
attrs_list => concat(keys($attrs), keys($attrs_ssl)),
target => "${conf_dir}/features-available/ido-mysql.conf",
order => 10,
notify => $_notify,
}

Since we dont unwrap content of the attrs, more specifically the $_password on line 278, the content is written as Sensitive data instead of the password into the file. (Just my current best guess)

[lbetz]

Hi Markus,

that's correct, the unwrap is done in the parser in

https://github.com/Icinga/puppet-icinga2/blob/master/lib/puppet_x/icinga2/utils.rb#L243
https://github.com/Icinga/puppet-icinga2/blob/master/lib/puppet_x/icinga2/utils.rb#L263
https://github.com/Icinga/puppet-icinga2/blob/master/lib/puppet_x/icinga2/utils.rb#L322

the returned string will be written to file.

[zilchms]

True. I got to there in the afternoon. Remains to be found, why the password does not get unwrapped correctly. I will go and do some more debugging now.

Edit: I got down to the unwrap parts in the utils.rb, but I dont know enough about Rubyprogramming to do serious debugging there (at least what i feel comfortable with)
My intuition was that the Sensitive type as described here (I know it is outdated, but this shouldnt have changed since then) is not working with the Puppet::Pops::Types::PSensitiveType::Sensitive comparison, since it is a Resource reference and should be Puppet::Pops::Types::PSensitiveType instead (the Data Type). I tried this in my own installation, but had no success. Maybe someone else has some more leads? Or maybe some guidance as to how i can actually write something akin to a notify in the ruby code, so i can actually confirm some things while debugging (had no luck with the documentation on that front until now).

[cocker-cc]

Edit: deleted, as I made a Mistake in my Comment

[lbetz]

I still cannot reproduce the problem. It would be helpful if you could tell me what I can do.

[sircubbi]

Same problem here on puppet 7.

[lbetz]

Please, what puppet version in detail. Here I haven't any problems with the last versions of Puppet 5, 6, and 7.

[sircubbi]

Puppet 7.9.0 (PE 2021.3). (Running on Rocky 8)

[rwaffen]

same here

2 root@localhost /etc/icinga2 # facter os
{
  architecture => "x86_64",
  family => "RedHat",
  hardware => "x86_64",
  name => "CentOS",
  release => {
    full => "7.9.2009",
    major => "7",
    minor => "9"
  },
  selinux => {
    config_mode => "permissive",
    config_policy => "targeted",
    current_mode => "permissive",
    enabled => true,
    enforced => false,
    policy_version => "31"
  }
}

root@localhost /etc/icinga2 # facter aio_agent_version
6.24.0

root@localhost /etc/icinga2 # facter -p pp_icinga_version
2.12.4

mod 'icinga-icinga2',            '3.2.0'

[zilchms]

Ok lets start by getting a baseline:

OS Versions: SLES 12
puppet versions: 6.25.0
Module versions: icinga-icinga2 (v3.2.0), puppetlabs-stdlib (v7.1.0), puppetlabs-concat (v7.1.1)

Hiera Entry:

icinga2::feature::idomysql::user: 'icingaservice'
icinga2::feature::idomysql::password: 'testpassword'

Site Entry:

class{ '::icinga2::feature::idomysql':}

This results in a broken idomysql file:

object IdoMysqlConnection "ido-mysql" {
  host = "localhost"
  user = "icingaservice"
  password = "Sensitive [value redacted]"
  database = "icinga"
  enable_ssl = false
}

@lbetz can you reproduce this? Or is this setup working on your end? Maybe it has something to do with the ruby version or some other things that seem unrelated. If you have an idea how to debug this on my end, I am all ears. I am just not sure how to help debug this right now.

[lbetz]

@zilchms

I tested your code on puppet 6.25.0 and stdlib/concat 7.1.0 on a centos8, because I haven't a SLES12 test platform, yet. And my result look like:

This file is managed by Puppet. DO NOT EDIT.

library "db_ido_mysql"

object IdoMysqlConnection "ido-mysql" {
host = "localhost"
user = "icingaservice"
password = "testpassword"
database = "icinga"
enable_ssl = false
}

I am very confused SLES or RedHat should be no difference here.

[zilchms]

Hmmm, now thats some funky stuff. Aaaand I found the problem (at least in my setup) ....
I am ensuring environment isolation with the generate types mechanic from puppet and my scripts are not generating those consistently .... After regenerating the types it works without issue.
@to-kn @sircubbi @rwaffen Do you have environment isolation enabled too and did not regenerate after updating like I did?

[lbetz]

Arrrg, that explains it.

@rwaffen I tested your setup and it sill works...

[lbetz]

environment caching could be a problem after updating to the new module version...!?

[zilchms]

Aye, this can happen everytime a variable changes type. To top that off, that isnt even guaranteed to happen for everyone and everytime. Some with the exact same setup might experience that, others dont. That bug is pretty old and is a result to a bug referred to as environment bleed. Generating types was introduced to solve that (but brings its own problems).
Though I am not sure if environment bleed is still a problem in current puppet versions and if generating types is still sometimes necessary or just introduces new problems without benefits now.

[zilchms]

Nevermind.... On my test VM unwrapping now works as intended, on my production environment though... it doesnt...
Even after regenerating types. Back to debugging.....

Edit: Now its non-deterministically writing the password or the Redacted Value message....

[sircubbi]

I have yet to try regenerating types. We are using PE with r10k here (which isn't the recommended setup I guess), so maybe there is some issue with our different environments.

[zilchms]

@lbetz is your testenvironment consistent in its ido-mysql.conf content? I successfully reproduced my non-deterministic switching of contents from 'testpassword' to 'Sensitive [value redacted]' and vice versa on a second pretty clean server now.

[lbetz]

@zilchms :Ach shit. My "Testumgebung" is just a standalone puppet agent. And, yes, my result is consistent. A test server environment first I have to deploy.

[zilchms]

Ok. My test environment consists of a puppet master and some agents. Every agent I tried to give the idomysql feature shows the same result -> "Sensitive [content redacted]"
I did confirm that the hiera hash containing the password got handed through to the class correctly every time, so fancy facter problems seem to be out the window. Also they always arrive wrapped correctly when arriving at the class, no problems there either.
My problem right now is, that I cant debug the utils.rb since I dont know how i can get it to display contents of variables in the puppetrun (akin to a notify). So I got a pretty big black box still, in which something seems to go wrong. Any ideas how to debug that in any usefull manner?

[rwaffen]

will test this on monday again. and yes, we also have environment isolation.

[rwaffen]

i updated all our environments to get around some bugs in environment isolations. normaly we update only environments which have changes.

normally we do this per environment, but i also did it in general

/opt/puppetlabs/puppet/bin/puppet generate types

but still i get this:

 object ApiUser "patch" {
-  password = "my_real_password"
+  password = "Sensitive [value redacted]"
   permissions = [ "actions/schedule-downtime", "actions/remove-downtime", "objects/query/Hosts", "objects/query/Services", ]
 }

[zilchms]

Huh. That is interesting. I managed to fix my problems just now (you can believe the feeling of happiness I am experiencing right now). Let me try to write down my journey to a solution:

---- Please read in full first and dont do it step by step, unless you want to suffer like i did ----

1. Upgrade all your environments to the latest puppet-icinga2 version.
2. Check if you have any environments without icinga2 and if they are never the less exporting icinga2 resources via filling in modules from the default path. If that is the case -> directly include puppet-icinga2 in the current version into that environment. (That one was a bitch to debug; sometimes I want to do horrible things to my coworkers....)
3. Regenerate all types individually per environment and after that once in general.
4. Stop your icinga master puppet for a weekend.
5. In that time delete the cache under /opt/puppetlabs/server/data/puppetserver
6. Realize you deleted all vendor rubys by mistake too
7. Uninstall puppetserver and reinstall it again
8. Fix RubyVM settings after finding out those are gone too and all your puppetruns are now timing out due to too much load on your puppetmaster
9. Restart the puppetserver again for good measure <- this seems to be important due to caching
10. Start all puppetruns on all agents again
11. Start the puppetrun on your icingamaster again
12. Hope and pray

After doing all that (not necessarily in that exact order), my Icingamaster finally got the memo that there is a new puppet-icinga2 version with sensitive types and only this is to be used.

On a more serious note:

I would suspect that an older puppet-icinga2 version bled over from other environments and the generating types way with puppet didnt correct that behaviour in full. If I had to guess, i would assume that this is only a problem since the unwrap part was in a ruby function and not in a puppet template or site (generate types solves this pretty reliably). There is nothing the module authors can do to prevent this as far as my understanding goes. And these bugs are bound to happen from time to time.

[rwaffen]

okay, so when i update icinga2-module in all environments, this schould be okay? 🤔
maybe can test this next week at my project,

[zilchms]

I would believe so yes. If that does not fix it on your deployment, I am running out of ideas. I dont even have a testenvironment left now, since I fixed everything on my end.

[sircubbi]

okay, so when i update icinga2-module in all environments, this schould be okay? thinking maybe can test this next week at my project,

Actually, that alone is not enough. In our case (using r10k with pe-puppet) I had to update all environments, then regenerate the types in all environments and after all that the puppetserver needs a restart (otherwise it doesn't seem to catch up with the new types). At least for now everything looks working again.

[lbetz]

Interessting. I used the same type as puppet does in the postgresql and mysql modules, I thought. I've to check it again.

https://github.com/puppetlabs/puppetlabs-postgresql/blob/dd190cd4105020f0b04d469b685e2c80b8e613ac/lib/puppet/functions/postgresql/postgresql_password.rb#L25

Maybe one of you would like to write a few lines for the documentation?

[lbetz]

I'm not sure, but maby it's better to move the unwrap from ruby to puppet?

[cocker-cc]

I'm not sure, but maby it's better to move the unwrap from ruby to puppet?

My Opinion (not referencing these particular Cases): The unwrap should always be at least possible Level in the Call-Stack, so that as many as possible previous Levels do not have to bother with it.

[rweigle]

We have the same issue in our api-users.conf:

object ApiUser "icingaweb2" {
  password = "Sensitive [value redacted]"
  permissions = [ "status/query", "actions/*", "objects/modify/*", "objects/query/*", ]
}

Additonally we can see the problem in ido-mysql.conf as well.

We read our password from an eyaml file so our hiera entry looks like this:
icinga::web::api_pass: "%{lookup('secrets::icinga::server::web_api_pass')}"

We are using puppet 7.12.1 on Debian bullseye.
Environments are used via r10k and the module was only updated in a test environment.

[zilchms]

@rweigle Yea, that was kind of to be expected. This bug should be affecting all files where the utils.rb is used to build the contents (all of those should be listed in the first comment of this issue).
@cocker-cc & @lbetz I would agree, moving it from the Ruby Level to the Puppet Level would be bad style in my opinion. We dont have any proof suggesting that moving it, would fix the bug. Additionally it could cause more problems down the line.
We might want to use an approach similar to this for the logic fork in the utils.rb maybe? (specifically the .responds_to)
https://github.com/puppetlabs/puppetlabs-postgresql/blob/dd190cd4105020f0b04d469b685e2c80b8e613ac/lib/puppet/functions/postgresql/postgresql_password.rb#L22
Albeit I am not sure if it would even make a difference or what would be the best-practice solution according to the puppet style guide. As stated, I dont see a way to reliably fix the environment bleed from a module perspective (or even a puppet setup perspective; the problem seems to be as old as puppet itself)

[cocker-cc]

The Approach…

if value.is_a?(Puppet::Pops::Types::PSensitiveType::Sensitive)

… and the Approach…

if value.respond_to?(:unwrap)

… should not make any Difference.

[lbetz]

Maybe one of you would like to write a few lines for the documentation?

[zilchms]

Can do, would that fall under the "How Configuration is parsed" section? Or where should information about that be placed? Maybe a new section "Known Issues"?

[lbetz]

@zilchms The last one. A description of when and why the problem occurs and how to fix it. I think, the README.md is a good place, Known Issues would be ok for me.

[rweigle]

I would suggest a "Know Issues" section, as it seem to be an environment bleed and apparently does not happen to everyone. I was able to fix it for me by cleaning up the environments (deleting the .resource_types directories) than regenerate them for all affected environments with puppet generate types --environment xxx and restarting the puppet server afterwards.

[lbetz]

I'm open to any suggestions since I can't reproduce it myself.

[grant-veepshosting]

Previously we could use plaintext passwords in Hiera instead of the better security that this change provides.
I think we could leave the Hiera data "as is", and add some Sensitive directives to the lookup_options instead ?
Would you consider doing some documentation on exactly how to do this with the new version of the code and module please?
Reference https://puppet.com/blog/my-journey-securing-sensitive-data-puppet-code/

Would something like this work inside the file /data/common.yaml ?
lookup_options:
"^icinga2::.*password$":
convert_to: Sensitive

Ref https://stackoverflow.com/questions/54445767/puppet-lookup-fails-with-expects-a-sensitive-value-got-string

[lbetz]

@grant-veepshosting The use of Sensitive Datatype should be optional, so that a plaintext transfer should also be possible. Have you read #693? If you are still affected by this, an info about it would be very important for us.

[grant-veepshosting]

Thanks for that - I have read through that link, the documentation and the underlying code changes.
All I needed to do to fix the problem was to add single quotes around each of the password '' in Hiera.
This is probably a best practice anyway, but hopefully this helps someone else in the future!

[lbetz]

Thx so much. That is an important info. If you think about it, not even far-fetched.

[lbetz]

I spent several hours yesterday and today recreating the issue, unsuccessfully. I installed a puppetserver (puppet 7.13.0), started with v3.1.0 of puppet-icinga2 to get an Icinga with features ido-mysql (password via Hiera), api (Constant TicketSalt via Hiera) and three ApiUser objects with passwords from Hiera and set in the manifest within declaration.
Then I updated the module to v3.2.0 and rerun puppet and the result was as expected, no changes. And yes, I didn't use quotes for strings in Hiera.

[bastelfreak]

Hi,
I had the same problem after upgrading the module.
puppetserver: Puppet Enterprise 2019.8.7
puppet agent: 6.23.0

I was able to fix this by running puppet generate types --environment $my_env on the puppetserver. This is usually done by PE, but that got disabled in my environment by accident. @lbetz I guess this is only reproducible when you've multiple environments and some have the older module version. https://puppet.com/docs/puppet/6/environment_isolation.html has some more infos about environment isolation issues.

[rwaffen]

can confirm this. tested on a fresh puppet7 setup (new puppetserver and icinga server) with 3 branches:

• one without the module
• one with 3.1.3
• one with 3.2.0

working in my 3.2.0 branch/environment:
got strange behavior. when i did the generated i got the "sensitive" message in my config file. than i restarted my puppetserver and all got really strange. now i dont get the sensitive at all. not in the output not in the file. is see clear text in the output now.

have to retry another time…. all strange behavior. but agree with @bastelfreak that this is all an environment isolation thingy

[lbetz]

Without environment isolation it is as @rwaffen report it. A simple restart solves the problem.

But switching between the versions break the correct behaivor again.