Don't update password encrypted with SCRAM-SHA-256 if not changed

[Bouke]

Currently we're seeing a lot of spurious updates to a user, when the given password_hash is encrypted with SCRAM-SHA-256. For more information about the format, see pg_authid.

[puppet-community-rangefinder]

postgresql::server::role is a type

The enclosing module is declared in 50 of 578 indexed public Puppetfiles.

Breaking changes to this file WILL impact these modules (exact match):

• groupbuddies-gb
• nicopaez-dockerserver
• maestrodev-maestro
• stackstorm-st2
• jjulien-trac
• deric-barman
• Aethylred-gitlab
• it2ndq-barman
• lwo-dataverse
• nnutter-testdb
• nicopaez-alfred
• hetzner-roundcube
• npwalker-pe_external_postgresql
• braiins-sentry
• landcareresearch-ckan
• braiins-zabbix
• landcareresearch-pidservice
• blackknight36-bacula
• nicopaez-triage

Breaking changes to this file MAY impact these modules (near match):

• echoes-wrappers
• tedivm-hieratic
• Aethylred-puppetdashboard
• Firebladee-moodle
• soli-wrappers

---

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

[carabasdaniel]

Hi @Bouke

Thanks for submitting this PR. Could you please provide a test for this functionality ?

[Bouke]

@carabasdaniel I'm not familiar with writing tests, but if you can provide me with some pointers for existing test that I can extend, I'd be happy to. Is there a similar unit test for the MD5 case?

[daianamezdrea]

Hi @Bouke, you can find acceptance tests here: puppetlabs-postgresql/spec/acceptance/server
and unit tests: puppetlabs-postgresql/spec/unit/defines/server. Let me know if you need help!

[michaeltlombardi]

Hi, @Bouke!

It looks like this pull request hasn't seen any activity in a while. Is this something that you're still able to work on? We're happy to keep this pull request open if there's forward movement, and we'll leave it open for another two weeks from now. If there isn't any activity by then we may have to close this pull request due to lack of activity.

Closing the pull request wouldn't mean we don't consider this change valuable. However we try to keep all open pull requests moving towards completion, and closing stale pull requests help us focus on more active pull requests.

If you have any questions or concerns, please don't hesitate to ping us in the Slack channel on slack.puppet.com.

[Bouke]

Hi @daianamezdrea @michaeltlombardi, thank you for your feedback. It's indeed the case that there hasn't been any additional progress on this PR. I've tried working on this thing for a few times now, but I am getting nowhere with the tests.

It's a trivial change that adds scram-sha-256 in the same way that md5 is whitelisted. I've looked at the existing tests, and they don't cover any of the flow that I'm making changes. So what you're asking of me is to create a unit test not only for my changes, but something of a much bigger scope.

Looking at the role spec, it looks like an unsupported version of postgres (9.1) is being used for the tests, which doesn't even support the scram-sha-256 password hashing. So to be able to run the tests, I would also have to upgrade the test definitions to target some newer version of postgresql.

I'm sorry but if that's what you're asking of me, feel free to close this PR.

[daianamezdrea]

Hi @Bouke, yes, I understand your concerns. After a short discussion with the team, we decided that we are going to update our tests and I'll open a ticket for the new changes. Also, when the things are ready, could you please rebase your PR and add your tests? Thank you, cheers !

[david22swan]

@Bouke
Making a quick change to point your PR at the newly default main branch.
Apologies if this is inconvenient in any way.

[sanfrancrisko]

Hi @Bouke, yes, I understand your concerns. After a short discussion with the team, we decided that we are going to update our tests and I'll open a ticket for the new changes. Also, when the things are ready, could you please rebase your PR and add your tests? Thank you, cheers !

Raised IAC-1063

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[jplindquist]

Is there any update or progress on this? Postgres 14 dropped recently which now defaults to scram-sha-256

Change the default of the password_encryption server parameter to scram-sha-256 (Peter Eisentraut)

Previously it was md5. All new passwords will be stored as SHA256 unless this server setting is changed or the password is specified in MD5 format. Also, the legacy (and undocumented) Boolean-like values which were previously synonyms for md5 are no longer accepted.

[vaol]

Hello, I believe config.pp should be adjusted as well to create the pg_hba default rule with scram-sha-256 accordingly.
$password_encryption is already given as parameter in the class.

[kenyon]

This can be closed, done in #1313.

[Neustradamus]

Linked to:

• State of Play scram-sasl/info#1
• add scram-sha-256 support #1313