Skip to content
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

expand hashes identify library and add spec #11722

Merged
merged 5 commits into from Apr 24, 2019
Merged

expand hashes identify library and add spec #11722

merged 5 commits into from Apr 24, 2019

Conversation

@h00die
Copy link
Contributor

h00die commented Apr 12, 2019

This PR adds additional checks to the identify_hash function such as length and character type when appropriate. It adds checks for des, windows, osx, phpass (wordpress/drupal/phpbb3), mysql, mssql, oracle, and postgres.

Also add a spec to make sure things are working as expected.

This is my first test written in metasploit, ruby, or ever, so I'm very open to input on how to do it right/better.

# rspec spec/lib/metasploit/framework/hashes/identify_spec.rb 
WARN: Unresolved specs during Gem::Specification.reset:
      rake (>= 0.8.7)
      thor (< 2.0, >= 0.18.1)
      pg (>= 0)
      crass (~> 1.0.2)
WARN: Clearing out unresolved specs.
Please report a bug if this causes problems.
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 61794
hashes/identify ...................................

Top 10 slowest examples (0.08621 seconds, 78.1% of total time):
  hashes/identify identify_blofish_a returns bf
    0.06527 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:38
  hashes/identify identify_oracle11_S returns raw-sha1,oracle
    0.01476 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:200
  hashes/identify identify_blofish_y returns bf
    0.00091 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:60
  hashes/identify identify_qnx_md5 returns qnx,md5
    0.0008 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:102
  hashes/identify identify_empty_string returns empty string
    0.00078 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:235
  hashes/identify identify_mysql returns mysql
    0.00078 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:158
  hashes/identify identify_blofish_x returns bf
    0.00076 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:53
  hashes/identify identify_sha512_rounds returns sha512,crypt
    0.00073 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:74
  hashes/identify identify_md5 returns md5
    0.00072 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:24
  hashes/identify identify_nil returns empty string
    0.00071 seconds ./spec/lib/metasploit/framework/hashes/identify_spec.rb:242

Finished in 0.11045 seconds (files took 16 seconds to load)
35 examples, 0 failures

Randomized with seed 61794
Coverage report generated for RSpec to /root/metasploit-framework/coverage. 755 / 5624 LOC (13.42%) covered.
@h00die

This comment has been minimized.

Copy link
Contributor Author

h00die commented Apr 13, 2019

Thinking more about this, I think forcing/hoping each person to add a call to this library before they pass to the create_credential function may be a hassle. What about adding the call inside of that function? This way no one needs to call it, it sets the hash type automatically, and then when everyone forgets to set jtr anyways, we'll have no issues!
Thoughts?

@h00die h00die removed the delayed label Apr 13, 2019
@h00die

This comment has been minimized.

Copy link
Contributor Author

h00die commented Apr 14, 2019

Now has OSX as well

@busterb

This comment has been minimized.

Copy link
Member

busterb commented Apr 17, 2019

So create_credential can automatically identify a hash type if it isn't overridden by the caller? Seems like a reasonable default behavior to me!

@busterb

This comment has been minimized.

Copy link
Member

busterb commented Apr 17, 2019

Regarding the spec, I think you did capture the correct idea of how to write a test: given a set of test inputs, do you get the expected outputs.

No complaints here about the approach you took, though you might throw in some non-empty strings that are also not valid or understood password hashes, and verify those are no-match as well.

@h00die

This comment has been minimized.

Copy link
Contributor Author

h00die commented Apr 18, 2019

@busterb bingo! if given, let be. If not, throw it at the library for an educated guess.
This will MASSIVELY help out. I won't have time to implement, a few jobs about to kick off, but can put in an issue for reference if you want.
While whoever is looking at the DB creds stuff, #11686 and #11668 may be easy to solve as well.

@h00die

This comment has been minimized.

Copy link
Contributor Author

h00die commented Apr 18, 2019

Added a few more checks, but I think this is ready

@busterb

This comment has been minimized.

Copy link
Member

busterb commented Apr 18, 2019

While you're in there, do you think you could poke at why that credential spec is still failing (maybe that's a different PR).

@jrobles-r7

This comment has been minimized.

Copy link
Contributor

jrobles-r7 commented Apr 18, 2019

It looks like the spec is checking that creds -u will be used as a regex but that was changed in #11742.

@jrobles-r7

This comment has been minimized.

Copy link
Contributor

jrobles-r7 commented Apr 18, 2019

Spec fix #11749

@bcoles

This comment has been minimized.

Copy link
Contributor

bcoles commented Apr 18, 2019

@msjenkins-r7 test this please

@h00die h00die added the msf5 label Apr 19, 2019
@busterb busterb self-assigned this Apr 24, 2019
@busterb busterb merged commit 20934f1 into rapid7:master Apr 24, 2019
3 checks passed
3 checks passed
Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
busterb added a commit that referenced this pull request Apr 24, 2019
@busterb

This comment has been minimized.

Copy link
Member

busterb commented Apr 24, 2019

Release Notes

This updates the password hash identification library code to handle many more hash types.

@h00die h00die deleted the h00die:hash_lib branch May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.