-
Notifications
You must be signed in to change notification settings - Fork 150
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
NTLM Support with Full Domain and Server Authentication #87
Conversation
pmorton
commented
Mar 20, 2013
- You can now specify a domain as the last parameter of ntlm authentication ['user','password','domain']. If the domain is omitted we assume that you want to authenticate to the local server. (Note that this is a behavior change from the current implementation) Currently, if you did not specify a domain it would select the default target which is the machine for a non-domain joined machine, or the domain for a domain joined machine.
- Some newer microsoft services only allow Negotiate authentication. Negotiate will degrade to NTLM authentication and is 100% compatible with the NTLM scheme. The client will now query the server for valid auth types and use Negotiate or NTLM in that order. If the auth type is not supported we try NTLM anyhow.
- Type 1 Packets where malformed because they did not include the domain and workstation. In the general case, these should always be null for a type 1 packet. This is purely aesthetic, but the inclusion of this information quiets wireshark warnings about the packet being malformed. Note: Chrome, Firfox and Curl also fill these values in a null by default.
- Type 2 Packets were missing the Workstation name. This is now included by default.
- Type 2 Packets were missing the target domain. Without this the server would try to authenticate using the default target (my first comment on the behavior change).
* You can now specify a domain as the last parameter of ntlm authentication ['user','password','domain']. If the domain is omitted we assume that you want to authenticate to the local server. (Note that this is a behavior change from the current implementation) Currently, if you did not specify a domain it would select the default target which is the machine for a non-domain joined machine, or the domain for a domain joined machine. * Some newer microsoft services only allow Negotiate authentication. Negotiate will degrade to NTLM authentication and is 100% compatible with the NTLM scheme. The client will now query the server for valid auth types and use Negotiate or NTLM in that order. If the auth type is not supported we try NTLM anyhow. * Type 1 Packets where malformed because they did not include the domain and workstation. In the general case, these should always be null for a type 1 packet. This is purely aesthetic, but the inclusion of this information quiets wireshark warnings about the packet being malformed. Note: Chrome, Firfox and Curl also fill these values in a null by default. * Type 2 Packets were missing the Workstation name. This is now included by default. * Type 2 Packets were missing the target domain. Without this the server would try to authenticate using the default target (my first comment on the behavior change).
wow. thank you very much @pmorton! @rogerleite can you check on this? we need to run the manual integration test i think. |
Below are the manual testing steps that I have taken. Test Script HTTPI.adapter = :net_http
r = HTTPI::Request.new('http://localhost:8080')
r.auth.ntlm 'domain user', 'domain password', 'domain'
t = HTTPI.get(r)
puts t.inspect
HTTPI.adapter = :net_http
r = HTTPI::Request.new('http://localhost:8080')
r.auth.ntlm 'vagrant', 'vagrant'
t = HTTPI.get(r)
puts t.inspect
|
@pmorton 🤘 we need to update https://github.com/savonrb/httpi/wiki/NTLM-Integration-Test-Plan guide? |
@rogerleite I think that it would make sense to update https://github.com/savonrb/httpi/wiki/NTLM-Integration-Test-Plan . The nice part of what you have setup already is that it is straight forward and covers most use cases. By adding domain authentication to the manual test we add a huge amount of complexity and or raise the bar for who can develop in this area (I.e. you need to have a domain to develop easily). How do you think we best cover the added complexity in the Wiki? The matrix that needs to be covered is
|
Hi guys, just looked at this, I like this a lot. I had a basic knowledge of NTLM auth, and a use case at my work that guided me through the first stab, but it seems like @pmorton has more experience with all the different variations. @pmorton, I threw together the NTLM-Integration-Test-Plan just enough to get the basics working, it could stand expansion. I think you're right, configuring a domain server is probably a bit more work as well as the variations of user. (I'm not a windows net admin, so my knowledge gets a bit fuzzy here). If the test setup is too complex, we could possibly look at packaging it into a VM image (like the rails team). |
@coldnebo thank you very much for your feedback. regarding the test image idea: the problem with this is the windows evaluation image, right? doesn't that expire after n days? |
I am thinking that some vagrant magic with windows trials will do the Would it be possible to merge this change and open a new issue to make the I am working on a major update to the WinRM and vagrant windows gem. Once I P On Mar 21, 2013, at 1:31 AM, Daniel Harrington notifications@github.com @coldnebo https://github.com/coldnebo thank you very much for your — |
@pmorton sounds like a plan 👍 |
@pmorton I forgot about that wrinkle. Another solution might be to come up with a Chef recipe... while I'm fairly sure a recipe could be made for setting up our NTLM test environment, there seems to be quite a few steps to get chef installed on a fresh Windows trial image... 😦 So... what other rabbits in the hat? We could use a VM based on a private Win server license -- but I really have no idea how to make such a VM private for an open source project. A hosted Win server is an option, but since Microsoft itself advises solution developers to not use NTLM it's going to be hard if not impossible to find a hosting provider who would open their server or network up to NTLM auth unless it could be sufficiently sandboxed. Which leaves us with a few scattered devs running manual tests? 👍 @pmorton one thing we could do is to automate your manual tests similar to the way in which I did it by setting a console var and including the appropriate connection info in the test:
Caveats are (of course) if your manual test is done in your own intranet and you can't very well put your private company information in a public test spec... 👎 📓 Note: we may wonder why bother since Microsoft advises against NTLM and these other factors complicate support for it. The reason NTLM is still so prevalent (in spite of these problems) is that it enables Windows workstations on the intranet to seamlessly autologin to IIS when using IE. This means Windows users using IE get a drastically better experience logging into things like Sharepoint and other IIS authenticated intranet sites. Other browsers/OSes still have to log in manually. I agree this is a pretty weak reason to keep an antiquated security protocol, but there it is. |
agreed. |
@coldnebo i think you could use chef solo to solve this. we should definitely use some sort of automated provisioning. |
Hold off on the merge. I have another commit that will fix a bug that I found... Specifically if the server does not offer authentication methods the code will fail. |
In addition, I need to push a new version of the rubynltm gem to rubygems and update the gemspec on this gem. The reason that this was not caught in testing was because I had a custom version of the gem installed. |
I have pushed a new version of the gem to rubygem and will update the dependencies. @rubiii I am working at addressing anther issue. If you issue a request to a server that does not support the HEAD verb, the remote server will return a 405 not supported. I looked at how other libraries handle this and what they seem to do it piggyback on the verb that was originally requested. Do you have any pointers on how I might be able to find the verb of the request that I am trying to authenticate. |
@pmorton can you open another issue for that? i think i need a more detailed explanation of the problem. |
@rubiii I would be happy to open a new issue, but let me see if I can summarize it here to see if it makes sense. If you call HEAD seems like a logical choice except some servers have elected to disable the So when the user has called |
Yeah, I chose head initially because it seemed more efficient than a full get or post, but I didn't support this legacy case. There's another optimization that I didn't pursue, which is caching the auth for subsequent requests. Sent from my iPhone On Mar 22, 2013, at 9:55 AM, Paul Morton notifications@github.com wrote:
|
@pmorton i see. i don't think that information is available, but i could be wrong. /cc @rogerleite |
@pmorton this PR is kind of blocking @rubiii to release new version of savon (savonrb/savon#405). Do you need some help? What can i do to help you? |
@pmorton sorry for spamming you with notifications, but i feel like this is almost done and it would be very sad if we could not get it released. so if you could just give us a quick update, i would really appreciate it. |
I think @pmorton said the hold up was trying to figure out how to find the verb of the request being authenticated. This is so he can issue the existing verb instead of (my solution) issuing a HEAD request for the first part. I can look at this tonight for you. @rubiii, @rogerleite is there anything else that needs to be addressed? I too would like to see this in the release! 😄 |
Hey All - Sorry for going silent. There are still two things to do here.
|
@pmorton this constraint "rubyntlm requires Ruby version >= 1.9.2" is really necessary? Travis is "broken" because of this and HTTPI support many ruby versions https://travis-ci.org/savonrb/httpi/builds/6929272 @rubiii what are the directives about Ruby versions? @coldnebo To launch this new release, it should be good to close this two issues too. #88 and #90. I don't know if is mandatory. |
Another thing. Using rubyntlm last version, when running
@pmorton and @rubiii some idea what should be? Using version 0.1.1 doesn't occur. Weird. 😕 |
@rogerleite @pmorton i think it would be good to have it support 1.8 if it's an easy fix. |
Rubyntlm works only with 1.9 version because replaced Kconv for "native 1.9 encoding". WinRb/rubyntlm@3f54039. It seems a no way back. |
@rogerleite you could still conditionally use kconv on 1.8 in a separate method and remove it as soon as support for 1.8 is dropped. of course this is not my decision. but i haven't thought about dropping support for 1.8 for httpi and therefore savon just yet. |
@rogerleite @rubiii I would happily accept a pull request that supports 1.8 and (with passing tests Note rbx is broken already so no need to update). There was a non-trivial amount of work to make encoding work properly in both versions of ruby. I dropped 1.8 support because of EOL. 1.8 is very close to EOL and IMHO there are few reasons to use it. Note: I can unlock the dependency, but it will no longer support some NTLM scenarios. In specific older versions of the rubyntlm gem did not respect the domain and would always use the domain received in the type 2 request. I am not a fan of unlocking it because the behavior is determined by the dependency that is taken. |
@pmorton, it's not pretty, but checkout these hacks, especially the one to figure out the current verb in the request. The only problem is it delays the request by 20 seconds... not sure why. Maybe this will help you get to the next part? That's all for tonight. P.S. I fiddled with rubyntlm looking at 1.8, but realized it was non-trivial to adapt Iconv and the new encoding at the same time. It's possible, but I realized you're probably looking at having a conditional adapter -- don't know if you want that much refactoring, so decided to punt on helping with that part for now. |
Hey Guys - I just pulled down a copy of @pmorton branch and tested it against my production servers. Everything worked great. Performance looked fine. I can't speak to all of the variations of Windows Servers mentioned in this discussion, but it certainly worked for me on ruby 2.0 and jruby 1.7.3 talking to my production servers (Microsoft Dynamics CRM 2012). @pmorton I would be happy to run the integration tests as a second set of verifications, but I'm not sure if you are talking about the integrations tests in rubyntlm or httpi. |
Hi everybody! @pmorton and @coldnebo, i did a pull request to support Ruby 1.8.7. This way, httpi can still support 1.8.7 too. If the PR needs change, just notify me. @hqmq thanks for the feedback. |
finally merged and pushed this to master. there are three failing tests which appear to be related to ntlm. |
I'll take a look tonight. Sent from my iPhone On Jul 20, 2013, at 11:24 AM, Daniel Harrington notifications@github.com wrote:
|
thanks @coldnebo. i really appreciate it. |
continue at #97 |
this has finally been released with version 2.1.0. |