Skip to content

Rework passwords management#241

Draft
smortex wants to merge 8 commits intomasterfrom
rework-passwords
Draft

Rework passwords management#241
smortex wants to merge 8 commits intomasterfrom
rework-passwords

Conversation

@smortex
Copy link
Member

@smortex smortex commented Jan 5, 2026

In this PR, we drop the default value for all passwords, and mandate the use of a Sensitive data type.

Rationale

The module used to provide default values for password, which is prone to errors because forgetting to change them do not cause catalog compilation failures. This issue is mitigated by the use of TLS, but we should rather stop providing default value for these passwords completely.

The module also support usage of Sensitive[String] to provide passwords since version 8.1.0, in addition to String which was available before. It make sense to only support the former.

What needs to be done

If your already provide custom passwords, and provide them as Sensitive values, no change is required.

If you provide custom passwords as String, you need to transform them to Sensitive:

class { 'bacula::director':
  password => Sensitive('...'),
  # ...
}

If you did not provided a custom password, add one :-)

@smortex smortex force-pushed the rework-passwords branch 2 times, most recently from 0ae14f8 to e58649e Compare January 5, 2026 20:52
@smortex smortex added this to the 10.0.0 milestone Jan 5, 2026
@smortex smortex added the backwards-incompatible This change will lead to a major version bump for the next release label Jan 5, 2026
This fix a bunch of deprecation warnings.
When data types where added in version 5.4.0 a few years ago (#117),
backward compatibility with old catalog that used String for all
parameters was preserved.

We already did a few major releases so keeping this backward
compatibility does not make sense and we can make the types more
consistent by removing them.
Default values for passwords is a terrible idea.  Removing them is a
backwards-incompatible change, so we jump on this occasion to mandate
the use of Sensitive data type to pass passwords.  In order to be able
to test each change, we first switch these default values to Sensitive.
This will allow us to remove support for String passwords next, and
then the default values.
@smortex smortex force-pushed the rework-passwords branch 2 times, most recently from a82194f to 18c19b4 Compare January 20, 2026 20:01
This allows us to get rid of the `show_diff` trickery as an epp template
that render a Sensitive value return a Sensitive.

While here, fix some data-type mismatches.
Passwords shall be site-specific.  Not setting them should cause a
compilation failure, not fallback on a trivial password.
Copy link
Member Author

Choose a reason for hiding this comment

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

What? Exported resources do not include sensitive values? $password above is mandatory, provided as a Sensitive[String], yet the exported resource does not include it:

romain@zappy ~ % puppet query  'resources[certname,type,title,file,line,parameters] { type = "Bacula::Director::Client" and title = "ns3006942.ip-151-80-35.eu" and exported = true }'
[
  {
    "certname": "ns3006942.ip-151-80-35.eu",
    "type": "Bacula::Director::Client",
    "title": "ns3006942.ip-151-80-35.eu",
    "file": "/etc/puppetlabs/code/environments/romain/modules/bacula/manifests/client.pp",
    "line": 111,
    "parameters": {
      "tag": "bacula-vps-bee4e253.vps.ovh.net",
      "port": 9102,
      "address": "ns3006942.ip-151-80-35.eu",
      "autoprune": true,
      "job_retention": "6 months",
      "file_retention": "45 days"
    }
  }
]

and the catalog that collect this exported resource is not valid:

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Bacula::Director::Client[ns3006942.ip-151-80-35.eu]: expects a value for parameter 'password' on node vps-bee4e253.vps.ovh.net

One level up, the class that pass the Sensitive password around also does not show passwords in puppetdb:

romain@zappy ~ % puppet query  'resources[certname,type,title,file,line,parameters] { title = "Bacula::Client" and certname = "ns3006942.ip-151-80-35.eu"  }'
[
  {
    "certname": "ns3006942.ip-151-80-35.eu",
    "type": "Class",
    "title": "Bacula::Client",
    "file": "/etc/puppetlabs/code/environments/romain/site-modules/profile/manifests/bacula_client.pp",
    "line": 127,
    "parameters": {
      "port": 9102,
      "client": "ns3006942.ip-151-80-35.eu",
      "ensure": "present",
      "address": "ns3006942.ip-151-80-35.eu",
      "messages": {
        "Standard-fd": {
          "mname": "Standard",
          "append": "\"/var/log/bacula/bacula-fd.log\" = all, !skipped",
          "daemon": "fd",
          "director": "ns3006942.ip-151-80-35.eu-dir = all, !skipped, !restored"
        }
      },
      "packages": [
        "bacula-client"
      ],
      "services": "bacula-fd",
      "autoprune": true,
      "pki_keypair": "/etc/bacula/ssl/ns3006942.ip-151-80-35.eu_pki_crt+key.pem",
      "default_pool": "OpusLabs",
      "director_name": "vps-bee4e253.vps.ovh.net",
      "job_retention": "6 months",
      "file_retention": "45 days",
      "listen_address": [
        "::"
      ],
      "pki_encryption": true,
      "pki_signatures": true,
      "max_concurrent_jobs": 2
    }
  }
]

Am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened OpenVoxProject/openvoxdb#177 to discuss the issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

So follow-up to the above discussion: passing them as String is fine for our use case given the current limitations of PuppetDB.

Another issue is raised as concatenated fragments of sensitive data are not sensitive and result in passwords being logged. I opened puppetlabs/puppetlabs-concat#828 to fix this issue. Maybe we should wait for a release of the concat module that include this, and bump the required version of concat in this module?

Sensitive parameters are stripped from resources before they are stored
in PuppetDB.  As a result, these required parameters are undef when
collecting exported resources, and catalog compilation fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards-incompatible This change will lead to a major version bump for the next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants