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

Verify passwords in Type3 messages #15

Merged
merged 0 commits into from
Dec 9, 2014

Conversation

jlee-r7
Copy link

@jlee-r7 jlee-r7 commented Dec 8, 2014

Type3#parse was failing to grab the session key. Since all inactive fields are at the end, we can simply enable them and try again until the full data string is parsed or we run out of fields to enable. This simplifies parsing for all the Message types.

Once parsing works, we can verify that a given Type3 is for a blank password (or an arbitrary one, if you're into that). This is useful to determine whether an authentication attempt is worth recording (see https://github.com/rapid7/metasploit-framework/blob/master/modules/auxiliary/server/capture/smb.rb)

@jlee-r7
Copy link
Author

jlee-r7 commented Dec 8, 2014

Not sure what's up with JRuby failures. Looks like the same problem as this build: https://travis-ci.org/WinRb/rubyntlm/builds/43373552 from this branch: https://github.com/WinRb/rubyntlm/tree/dan/ntlm-client

@zenchild
Copy link
Member

zenchild commented Dec 8, 2014

I just fixed that failure in my branch. The cipher wasn't calling #final like it should have. This just happened to work fine in C Ruby but JRuby's OpenSSL implementation is different so it's not too surprising.

@zenchild
Copy link
Member

zenchild commented Dec 8, 2014

BTW, I'll try and have a look at your pull request in the next day or so while all the code is fresh in my mind. I have a pile of changes coming as well. See the dan/ntlm-client branch for more details.

@jlee-r7
Copy link
Author

jlee-r7 commented Dec 9, 2014

Yeah, I noticed that branch when I was trying to figure out the travis failure. Looks like you're working on what I was planning on doing in the next few weeks, so yay! 😄

There's a partial implementation of signing and sealing over at https://github.com/rapid7/metasploit-framework/tree/master/lib/rex/proto/ntlm . I don't know if it would be much use as a reference for your work as there are no tests and the code is pretty gnarly, but you might want to take a look. The SMB portion is at https://github.com/rapid7/metasploit-framework/tree/master/lib/rex/proto/smb

@zenchild
Copy link
Member

zenchild commented Dec 9, 2014

Thanks for the links. I've been working through these docs:
http://davenport.sourceforge.net/ntlm.html and http://msdn.microsoft.com/en-us/library/cc236621.aspx

So far I have a working signing + sealing that works for the WinRM gem but I don't have anywhere else to try it. If you have other areas to try it in I'd love extra hands on the code. I hope to have pull request for it very soon.

@zenchild
Copy link
Member

zenchild commented Dec 9, 2014

Everything looks pretty good. The only questions I have are about the Type3#password? and Type3#blank_password? methods and why they should belong there. The Type# messages kind of act as a model of the data and those methods seem better positioned elsewhere. Maybe in the forthcoming Net::NTLM::Client class or something. Maybe you can shed some light on how they're being used.

@jlee-r7
Copy link
Author

jlee-r7 commented Dec 9, 2014

Client doesn't really make sense because if we're building the Type3 from a Type2, we already know the plaintext password; the idea here is for a server to decide whether a captured authentication attempt is worth recording.

The main reason for putting them in Type3 was because it already has all the data (minus the server challenge) to determine whether it's blank. Adding #password? was really just an expedient way to write tests without having to re-generate packet data for the various NTLM versions.

@zenchild
Copy link
Member

zenchild commented Dec 9, 2014

I understand now. I was looking at it from a client's perspective and it didn't make sense to me. I think it's fine for now then. At some point it might be worth having a complementary Net::NTLM::Server to house such utility methods.

@zenchild zenchild merged commit 9687f68 into WinRb:master Dec 9, 2014
@jlee-r7 jlee-r7 deleted the feature/verify-password branch April 16, 2015 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants