Skip to content

Correct bcrypt salt regex #1279

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

Merged
merged 3 commits into from
Oct 20, 2022
Merged

Conversation

sabo
Copy link
Contributor

@sabo sabo commented Oct 14, 2022

This corrects the regex used by the pw_hash function for bcrypt-type hashes. The first two characters are interpreted as a strength parameter by bcrypt, and must take a value between 4 and 31 inclusive.

@sabo sabo requested a review from a team as a code owner October 14, 2022 19:30
@CLAassistant
Copy link

CLAassistant commented Oct 14, 2022

CLA assistant check
All committers have signed the CLA.

@puppet-community-rangefinder
Copy link

pw_hash is a function

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

This module is declared in 318 of 579 indexed public Puppetfiles.


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.

@jordanbreen28
Copy link
Contributor

Hi @sabo - thanks for submitting this PR. Whilst the changes look good, could I ask that you look to resolve the present test failures and sign our CLA so we can proceed with this. :-)
Thanks!

@jordanbreen28
Copy link
Contributor

@sabo - please disregard my previous comment regarding the test failures, the failures present at first appeared to be transient. (With exception of Windows 2012 R2, these tests are regularly failing in our nightlies and not thought to be due to your PR)

Could you please rebase with the current main branch, and sign our CLA, so that we can proceed with this.
Thanks!

@sabo
Copy link
Contributor Author

sabo commented Oct 17, 2022

Hi @jordanbreen28, working on getting approval for that CLA.

@jordanbreen28
Copy link
Contributor

Great @sabo - in the meantime could you rebase this PR with the current main :-)

sabo added 3 commits October 18, 2022 14:21
The two-digit header of a bcrypt salt is a strength parameter. Valid
values for the strength parameter range from 4 to 31 inclusive. Update
the regex used to check the salt to only match against values from 04 to
31.
mention the meaning of the first two characters as a strength parameter
@sabo sabo force-pushed the fix_bcrypt_salt_regex branch from 4ffce2d to 90a8760 Compare October 18, 2022 18:21
@sabo
Copy link
Contributor Author

sabo commented Oct 18, 2022

@jordanbreen28, branch rebased and CLA signed, should be good to go

@jordanbreen28
Copy link
Contributor

@sabo Great work :-) Just one more small thing before merging, could you please squash your commits into a single commit for this PR.

@kenyon
Copy link
Contributor

kenyon commented Oct 19, 2022

You can squash on merge using GitHub's web interface.

@jordanbreen28 jordanbreen28 merged commit e1977c5 into puppetlabs:main Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants