Skip to content

Adding str2saltedpbkdf2 function #1040

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 1 commit into from
Feb 25, 2020
Merged

Adding str2saltedpbkdf2 function #1040

merged 1 commit into from
Feb 25, 2020

Conversation

genebean
Copy link
Contributor

@genebean genebean commented Aug 1, 2019

This is designed to aid in setting passwords for OS x / macOS 10.8+

Pinging @binford2k and @DavidS since they helped me figure out what to do here.

@binford2k
Copy link
Contributor

will this fail gracefully on older or new yet-to-be-released OS versions?

@genebean
Copy link
Contributor Author

genebean commented Aug 1, 2019

Not exactly sure... what kind of scenario were you thinking of @binford2k?

Sent with GitHawk

@binford2k
Copy link
Contributor

Apple changes the algorithm unexpectedly sometimes and we don't know until the generated password suddenly doesn't work. I'm not sure if that should just be a documented/known thing, or whether you should explicitly emit warnings when used for an OS that's not in the test matrix.

tbh, it's not something easily tested for, so I don't really know that it should be our responsibility.

@genebean
Copy link
Contributor Author

genebean commented Aug 2, 2019

Ah, gotcha. I don’t think that needs to be handled here but I do need to add some related documentation. This was discovered to be an issue in the first place by way of the surprisingly helpful error message emitted by macOS when not passing it the type of password it wanted... in such a case the user resource emits that error and fails. I think that’s all that’s needed guardrail wise. I’ll fix the docs tonight or this weekend and look into what’s failing in CI also.

Sent with GitHawk

@genebean
Copy link
Contributor Author

genebean commented Aug 5, 2019

@binford2k Updated docs slightly and fixed rubocop errors

@tphoney
Copy link
Contributor

tphoney commented Aug 19, 2019

Hey @genebean the code looks good, and great work on the documentation. I would ask that you add some unit tests for this. To make sure that it continues to work over time.

thanks for your patience, i was on PTO for the last few weeks.

Copy link
Contributor

@tphoney tphoney left a comment

Choose a reason for hiding this comment

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

All good, just needs a few unit tests. to make sure we dont lose functionality going forward.

@genebean
Copy link
Contributor Author

I just returned from PTO myself. I will look at this this week. (pinging @SowmyaJujjuri for visibility)

@michaeltlombardi
Copy link
Contributor

@genebean are you still planning to work on this or is it no longer needed?

@genebean
Copy link
Contributor Author

I just forgot... sorry! It is still wanted.

Sent with GitHawk

@sheenaajay
Copy link
Contributor

Thanks, @genebean. Could you please add some unit tests to verify the changes. Thank you.

@genebean genebean requested a review from a team as a code owner February 24, 2020 16:34
@genebean
Copy link
Contributor Author

Initial pass at testing added. Will revisit after I get back from lunch. Also, I rebased on current master.

This is designed to aid in setting passwords for OS x / macOS 10.8+
@genebean
Copy link
Contributor Author

genebean commented Feb 24, 2020

Spec tests for this PR pass:

$ pdk bundle exec rspec spec/functions/str2saltedpbkdf2_spec.rb
pdk (INFO): Using Ruby 2.5.7
pdk (INFO): Using Puppet 6.12.0
Run options: exclude {:bolt=>true}

str2saltedpbkdf2
  is expected not to eq nil
  is expected to run str2saltedpbkdf2() and raise an ArgumentError with the message matching /wrong number of arguments/i
  is expected to run str2saltedpbkdf2("Pa55w0rd", 2) and raise an ArgumentError with the message matching /wrong number of arguments/i
  is expected to run str2saltedpbkdf2(1, "Using s0m3 s@lt", 50000) and raise an ArgumentError with the message matching /first argument must be a string/
  is expected to run str2saltedpbkdf2("Pa55w0rd", 1, 50000) and raise an ArgumentError with the message matching /second argument must be a string/
  is expected to run str2saltedpbkdf2("Pa55w0rd", "U", 50000) and raise an ArgumentError with the message matching /second argument must be at least 8 bytes long/
  is expected to run str2saltedpbkdf2("Pa55w0rd", "Using s0m3 s@lt", "50000") and raise an ArgumentError with the message matching /third argument must be an integer/
  is expected to run str2saltedpbkdf2("Pa55w0rd", "Using s0m3 s@lt", 1) and raise an ArgumentError with the message matching /third argument must be between 40,000 and 70,000/
  when running with "Pa55w0rd", "Using s0m3 s@lt",and "50000" as params
    is expected to run str2saltedpbkdf2("Pa55w0rd", "Using s0m3 s@lt", 50000) and return {"password_hex"=>"3577f79f7d2e73df1cf1eecc36da16fffcd3650126d79e797a8b227492d13de4cdd0656933b43118b7361692f755e5b3c1e0536f826d12442400f3467bcc8fb4ac2235d5648b0f1b0906d0712aecd265834319b5a42e98af2ced81597fd78d1ac916f6eff6122c3577bb120a9f534e2a5c9a58c7d1209e3914c967c6a467b594", "salt_hex"=>"5573696e672073306d332073406c74", "iterations"=>50000}

There are also no lint errors related to this PR:

$ pdk validate
pdk (INFO): Running all available validators...
pdk (INFO): Using Ruby 2.5.7
pdk (INFO): Using Puppet 6.12.0
[✔] Checking metadata syntax (metadata.json tasks/*.json).
[✔] Checking module metadata style (metadata.json).
[✔] Checking YAML syntax (["**/*.yaml", "*.yaml", "**/*.yml", "*.yml"]).
[✔] Checking Puppet manifest syntax (**/*.pp).
[✔] Checking Puppet manifest style (**/*.pp).
[✔] Checking Ruby code style (**/**.rb).
warning: puppet-lint: types/filemode.pp:2:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/cidr.pp:2:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/alternative.pp:2:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/alternative.pp:3:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/alternative.pp:4:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/alternative.pp:5:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/alternative.pp:6:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/alternative.pp:7:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/alternative.pp:8:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/nosubnet/alternative.pp:4:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/nosubnet/alternative.pp:5:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/nosubnet/alternative.pp:6:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v6/nosubnet/alternative.pp:7:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v4/nosubnet.pp:1:140: line has more than 140 characters
warning: puppet-lint: types/ip/address/v4/cidr.pp:1:140: line has more than 140 characters
info: puppet-epp: ./: Target does not contain any files to validate (**/*.epp).
info: task-name: ./: Target does not contain any files to validate (tasks/**/*).
info: task-metadata-lint: ./: Target does not contain any files to validate (tasks/*.json).%

@codecov-io
Copy link

Codecov Report

Merging #1040 into master will increase coverage by 0.29%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1040      +/-   ##
=========================================
+ Coverage    4.35%   4.64%   +0.29%     
=========================================
  Files         185     186       +1     
  Lines        5401    5338      -63     
=========================================
+ Hits          235     248      +13     
+ Misses       5166    5090      -76
Impacted Files Coverage Δ
lib/puppet/parser/functions/str2saltedpbkdf2.rb 0% <0%> (ø)
lib/puppet/parser/functions/assert_private.rb 0% <0%> (-92.31%) ⬇️
lib/puppet/type/file_line.rb 96.36% <0%> (+1.81%) ⬆️
lib/facter/facter_dot_d.rb 19% <0%> (+5.8%) ⬆️
lib/facter/puppet_settings.rb 50% <0%> (+20.37%) ⬆️
lib/facter/root_home.rb 69.23% <0%> (+20.65%) ⬆️
lib/facter/pe_version.rb 68.75% <0%> (+23.85%) ⬆️
lib/facter/util/puppet_settings.rb 100% <0%> (+44.44%) ⬆️
lib/puppet/provider/file_line/ruby.rb 62.62% <0%> (+47.47%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1bb15b...a521aa2. Read the comment docs.

@sheenaajay
Copy link
Contributor

@genebean Thanks for your contribution.

@sheenaajay
Copy link
Contributor

Screen Shot 2020-02-25 at 12 58 32

@sheenaajay sheenaajay merged commit 275e9b5 into puppetlabs:master Feb 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants