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

[Android] Provide the option to use NTLM Authentication #62264

Closed
3 tasks done
steveisok opened this issue Dec 2, 2021 · 23 comments
Closed
3 tasks done

[Android] Provide the option to use NTLM Authentication #62264

steveisok opened this issue Dec 2, 2021 · 23 comments
Assignees
Labels
area-System.Net.Http os-android User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@steveisok
Copy link
Member

steveisok commented Dec 2, 2021

Unlike our other configurations, there is no library we rely on and can safely ship that provides NTLM authentication. As a result, we will need to incorporate a managed implementation largely based off of @wfurt's work in https://github.com/wfurt/Ntlm.

Work Items

@steveisok steveisok added os-android User Story A single user-facing feature. Can be grouped under an epic. labels Dec 2, 2021
@steveisok steveisok added this to the 7.0.0 milestone Dec 2, 2021
@steveisok steveisok added this to To do in Mobile Backlog via automation Dec 2, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net.Http untriaged New issue has not been triaged by the area owner labels Dec 2, 2021
@ghost
Copy link

ghost commented Dec 2, 2021

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Unlike our other configurations, there is no library we rely on and can safely ship that provides NTLM authentication. As a result, we will need to incorporate a managed implementation largely based off of @wfurt's work in https://github.com/wfurt/Ntlm.

Work Items

  • Port mono's MD4 implementation
  • Investigate what needs changed in AndroidMessageHandler and add support for NTLM
  • Add support into SocketsHttpHandler
Author: steveisok
Assignees: steveisok, MaximLipnin, wfurt
Labels:

os-android, User Story

Milestone: 7.0.0

@steveisok steveisok removed the untriaged New issue has not been triaged by the area owner label Dec 2, 2021
@filipnavara
Copy link
Member

JFYI I have an open PR on @wfurt's Ntlm repository that implements couple of the missing features and security mechanisms. It should be reviewed/updated/merged before you proceed with integrating the code.

@MaximLipnin
Copy link
Contributor

Hello @wfurt ! Managed MD4 implementation has been added in #62074, so we can move further and try to use it in the AndroidMessageHandler. Since you know a lot in this area, it would be great to get any advice from you or highlight any obstacles on this way.

@wfurt
Copy link
Member

wfurt commented Jan 25, 2022

AFAIK @dotMorten has working implementation on Android. Is that something you can share/contribute @dotMorten?
The next step would be cleanup and merge with PR @filipnavara did while back. And then bringing this to runtime repo.
I can either hide it behind existing helper methods or it can be used directly. We will need to choose.

@dotMorten
Copy link

I can definitely share it. Need to dig it out. Not sure I’d call that contribute as I just put together a few things from other contributors and got help to get it working

@dotMorten
Copy link

Here you go: https://github.com/dotMorten/NtlmAndroid

Most of how this differs is it has 0-dependencies. I lifted the MD4 and Asn1 implementations from this repo, and just modified @wfurt's code to work with these classes instead. So most of the code in the repo can be deleted since it is already available (as internal) in the .net runtime sdk.

@MaximLipnin
Copy link
Contributor

Thanks for your feedback @wfurt and @dotMorten ! To make progress, I can pull the solution, put it into a separate branch against runtime repo and start cleaning it up.

@filipnavara
Copy link
Member

@dotMorten Uff, that is based on the code before I sent out PR wfurt/Ntlm#2. It's missing couple of important security features and most importantly, some of these are required with newer Windows setups. It also looks quite close to https://github.com/JeroenBer/Ntlm where I found couple of bugs too and sent a PR (also not merged).

I'd be happy to review any PR but at the moment I cannot commit to coding this myself. I'd strongly urge to fix the security and compatibility issues/features first before proceeding with the integration. Some of the changes fundamentally require to keep the state throughout the authentication on the client side (to calculate checksum) and it may influence how the code is integrated.

@MaximLipnin
Copy link
Contributor

@filipnavara Glad to have you as a reviewer, thanks! If we need to follow any formal procedure to preserve authoring, then perhaps, it'd make sense that @dotMorten opened a draft PR with the "as-is" solution against dotnet/runtime and I could join this work and address any issues on that.

@dotMorten
Copy link

dotMorten commented Jan 27, 2022

No need to have me be author, considering I really wasn't for 99% of the code, but it is mainly wfurt's code, and he's already assigned here.

@wfurt
Copy link
Member

wfurt commented Jan 27, 2022

I'll try to stabilize the core NTLM with @filipnavara (probably next week) and then I'll bring it to runtime. That should allow to unblock work on the android specific part e.g. integration with platform handler.

@filipnavara
Copy link
Member

filipnavara commented Feb 12, 2022

I have started experimenting with an integration here: https://github.com/filipnavara/runtime/tree/managed_ntlm. It's very much work in progress and I expect to do a lot of cleanup and testing tomorrow. It basically just wires the code into System.Net.Http and gets the basic scenario working.

Aside from correctness of the code itself (eg. validation, buffer usage, etc.) there's an issue with Negotiate authentication against Windows Server 2016. After the last AUTHENTICATE_MESSAGE from the client the server replies with accept-incomplete SPNEGO message with mechlistMIC field. This is not handled correctly yet.

Update: I added basic implementation of the mechlistMIC computation. Now it works with both NTLM and Negotiate against Windows Server 2016 and 2019. Other versions likely work too but I lack the test infrastructure.

@filipnavara
Copy link
Member

The branch linked in the comment above should compile and it works against my test servers. I've marked couple of the spots that need to be fixed in the source code comments. There's definitely more things than that though (big endian compatibility, unnecessary usage of Marshal.OffsetOf, etc.) that would need more thorough review.

How do we move forward from here? I can open a PR to allow you to commit to the branch and to make it easier to discuss issues with the code.

(Note that there's a temporary test case that connects to my hosted server, including plain text credentials. The server is a burner so it's not a problem for the moment. It also crashes about once per day. [Thanks, Azure!] We just need to remember to remove it before merging.)

@MaximLipnin
Copy link
Contributor

How do we move forward from here? I can open a PR to allow you to commit to the branch and to make it easier to discuss issues with the code.

@filipnavara Thanks! AFAIU we need to integrate it with XA's AndroidMessageHandler. So perhaps it would be a couple of option:

  • make some additional managed logic (in an android application?) which does all the NTLM authentication and then somehow passes any given tokens to AndroidMessageHandler class which just uses it in any further requests;
  • extend AndroidMessageHandler class to allow to do more then one authentication request - either in managed part or at java api level.

Perhaps, you might want to add a couple more thoughts on that?

@filipnavara
Copy link
Member

It took me a bit of a time to give it a thought and see what is actually the problem scope and if there's an easy way to approach it.

Let's start with several observations:

  • Android is not the only platform that lacks GSSAPI support, and thus in turn the NTLM and SPNEGO authentication. tvOS suffers from the exact same issue.
  • macOS does have GSSAPI support and has NTLM implementation that is working but due to issue in how it handles credentials (splitting domain and user name if specified in the user@DOMAIN form) it behaves in different way than the Windows implementation. While it may be strictly correct on the technical level it poses a compatibility problem¹. Keeping that in mind there may be a future need to use the NTLM/SPNEGO implementation on macOS with a fallback to real GSSAPI for other algorithms like Kerberos.
  • AndroidMessageHandler currently doesn't handle challenge-response authentication at all. Moreover, NTLM specifically has additional requirements on the HTTP connection level. It needs to use HTTP/1.x and it is session-based authentication where it strictly needs to run within the same connection. None of these requirements are currently implemented in the Android code.
  • AndroidMessageHandler is in different assembly and thus doesn't have access to the NT Authentication / GSSAPI interop that exists in the dotnet/runtime repository since it's not a public API.

That leads to a question what is the minimal viable implementation. Implementing the NTLM/SPNEGO on SocketsHttpHandler (as done my branch) for Android/tvOS is somewhat straightforward. SocketsHttpHandler already knows the quirks of NTLM (HTTP version and connection limitations). It is just a matter of replacing the internal NTAuthentication class (or a different layer - there are multiple options; but this one was the one easiest to reuse and also enables scenarios like SMTP in System.Net.Mail) with a managed implementation. NTAuthentication is already a no-op for these platforms so this is trivial switch. Obviously the downside is that users would need to use SocketsHttpHandler and not a platform handler if they need NTLM. Depending on circumstances it may be a reasonable tradeoff.

If we wanted AndroidMessageHandler to handle NTLM a more work would need to be done. I'll blindly assume that it's possible to implement the challenge-response session based authentication on top of the Java API for the moment. Since there's no public API for the NT authentication at the moment that would lead to three options:

  1. Maintain a copy of the NTLM implementation in the xamarin-android code base.
  2. Use reflection in xamarin-android to access some private API on the dotnet/runtime side
  3. Introduce a public API

Each of these options have significant downsides and/or costs. Maintaining a copy of the code seems like a non-starter to me. We'd want SocketsHttpHandler to support NTLM too so the implementation has to exist on the dotnet/runtime side. The code would need to stay in sync which is maintenance burden, and finally on mobile platform we don't want to duplicate code due to size constraints. Using reflection is error-prone and not quite linker friendly. Public API needs a lot of design work and approval. It is already tracked in #29270 though. Is anyone willing to push it through API review? If yes then it may open up possibilities.

¹) I'll open separate issue to track this. The gist is that the same credentials work on Windows but don't work on macOS. A different variation of the credentials works on both. The scenario involves a typical Exchange setup with server E and AD, E being the Exchange server and AD being the Active Directory server. User tries to authenticate with user name "u@b.c" to E. "b.c" is email domain that exist on the Exchange server but it's neither an AD domain nor a DNS domain name setup on the AD. On Windows the credentials are sent as user u@b.c + domain (empty). On macOS they are sent as user u + domain b.c. Since the server doesn't know any AD way to resolve domain b.c it fails the authentication. Using just user name u with no domain doesn't work either because then the server E tries to resolve it as local user. However, it seems that in the Windows case it recognizes that it's not a local user based on the @ in user name and forwards it to its default domain controller AD. The domain controller AD tries to resolve the authentication then. Ordinarily that would fail because there's no domain b.c on the AD and it has no knowledge of it. However, section 3.2.5.1.2. of NTLM specification has a fallback where the domain name is stripped and then the lookup is retried. Essentially it turns the user name into equivalent of AD\a. That passes the checks and authentication succeeds.

@wfurt
Copy link
Member

wfurt commented Feb 18, 2022

Thanks @filipnavara for great detailed summary. I agree that session-based auth and challenge-response are important concepts to consider.
Also as I was exploring https://github.com/dotnet/Kerberos.NET recently, there is some hope for Negotiate/Kerberos even for platforms without GSSAPI.

@filipnavara
Copy link
Member

The Kerberos.NET implementation is quite amazing but at the moment I am looking into breaking this up into more managable chunks.

Btw, the reflection over NTAuthentication already exists on the ASP.NET side: https://github.com/dotnet/aspnetcore/tree/main/src/Security/Authentication/Negotiate/src/Internal

I don't like the reflection based approach and it reinforces my belief that public API is in order. However, worst case, it is possible to reuse that on the xamarin-android side.

@wfurt
Copy link
Member

wfurt commented Feb 18, 2022

I agree. That was more note about what we could do in the future. And while #29270 was focused on server IMHO, we can push that forward if that makes it easier for us.

@steveisok
Copy link
Member Author

Since there's no public API for the NT authentication at the moment that would lead to three options:

  1. Maintain a copy of the NTLM implementation in the xamarin-android code base.
  2. Use reflection in xamarin-android to access some private API on the dotnet/runtime side
  3. Introduce a public API

I believe there is a 4th option. Move AndroidMessageHandler into runtime and give it some JNI treatment. The difficult part would be if we need to subclass any of the java API's to pull NTLM off. In runtime, we've resisted having to do such a thing in the past, but maybe this is enough to reopen that discussion.

@wfurt
Copy link
Member

wfurt commented Jun 22, 2022

#29270 is merged and there is now API sufficient for the handler.

@simonrozsival
Copy link
Member

Thanks to #66879 the SocketsHttpHandler in .NET 7 now supports NTLM and Negotiate on Android (tested on my phone). The integration of the new NegotiateAuthentication API (#29270) into AndroidMessageHandler is now waiting for a .NET 7 build bump in Xamarin.Android and it should be finished soon (dotnet/android#6999).

jonpryor pushed a commit to dotnet/android that referenced this issue Jul 26, 2022
Context: dotnet/runtime#62264
Context? https://github.com/wfurt/Ntlm

Update `Xamarin.Android.Net.AndroidMessageHandler` to *optionally*
support NTLMv2 authentication in .NET 7+.  This authentication method
is recommended only for legacy services that do not provide any more
secure options.

If an endpoint requires NTLMv2 authentication and NTMLv2 is not
enabled, then the endpoint will return HTTP-401 errors.

NTLMv2 authentication can be enabled by setting the
`$(AndroidUseNegotiateAuthentication)` MSBuild property to True.
If this property is False or isn't set, then NTLMv2 support is linked
away during the package build.

Example `.csproj` changes to enable NTLMv2 support:

	<PropertyGroup>
	  <AndroidUseNegotiateAuthentication>true</AndroidUseNegotiateAuthentication>
	</PropertyGroup>

Example C# `HttpClient` usage using NTLMv2 authentication:

	var cache = new CredentialCache ();
	cache.Add (serverUri, "Negotiate", new NetworkCredential(username, password, domain));

	var handler = new AndroidMessageHandler {
	    Credentials = cache,
	};
	var client = new HttpClient (handler);
	var response = await client.GetAsync (requestUri);      // 200 OK; 401 is NTLMv2 isn't enabled
@filipnavara
Copy link
Member

Fixed by dotnet/android#6999

Mobile Backlog automation moved this from To do to Done Jul 26, 2022
@dotMorten
Copy link

Thank you everyone! I have quite a few customers who's going to be happy about this.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http os-android User Story A single user-facing feature. Can be grouped under an epic.
Projects
Development

No branches or pull requests

6 participants