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

[release/8.0-staging] Fix calculation of channel bindings hash in managed NTLM implementation #102565

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

filipnavara
Copy link
Member

@filipnavara filipnavara commented May 22, 2024

Backport of #95898 to release/8.0-staging

Fixes #95725

Customer Impact

Reported by customers in issue #95898 and by eM Client in #95725 (comment) - they cannot connect to Microsoft Exchange and quite a few other host providers on Android client (which uses ManagedNtlm by default).

Authenticating to HTTPS servers using NTLM fails with "Unauthorized" error code when Extended Authentication option is enabled on the server. This is an increasingly common setting on Microsoft Exchange deployments. The bug affects primarily the Android platform where the Managed NTLM implementation is used. We also allow the Managed NTLM as opt-in on macOS and Linux which would be likewise affected if the developer chooses to enable the System.Net.Security.UseManagedNtlm app context switch (which is rare).

Regression

No

Testing

Validated by eM Client on private Exchange instance in Azure VM + against one of the largest host providers in Germany.
The fix has been in .NET 9 since December 2023 and there were no observed issues.

Note: There are no unit tests, the set up is too complicated to test it meaningfully.

Risk

Low; the modified code path is in platform specific code for a single authentication scheme.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 22, 2024
@filipnavara filipnavara added the Servicing-consider Issue for next servicing release review label May 22, 2024
@filipnavara
Copy link
Member Author

/cc @rzikm @wfurt

We have another customer of our product affected by this issue. They provided a test account on their server and we verified that the fix resolves the issue.

@rzikm rzikm removed the Servicing-consider Issue for next servicing release review label May 23, 2024
@rzikm rzikm added this to the 8.0.x milestone May 23, 2024
@rzikm
Copy link
Member

rzikm commented May 23, 2024

LGTM.

Removed Servicing-Consider for now, in case @karelz wants to edit the template first.

@vitek-karas
Copy link
Member

/fyi @simonrozsival

@JeroenBer
Copy link

Yes we urgently need this for our customers. Not only for Android but also iOS and macOS since the unmanaged implementations cause crashes and are not usable (#97966)

Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

@JeroenBer
Copy link

@rzikm , not sure how this works but I see the PR is missing the "Servicing-approved" label before it can be merged. Who can do this ?

@rzikm
Copy link
Member

rzikm commented May 31, 2024

@JeroenBer I am keeping eye on this issue. The earliest this fix can get released is in July servicing release, and deadline for that release is mid June, we are still on track for that release.

@JeroenBer
Copy link

@rzikm Thanks for keeping an eye on this. A lot of our customers are waiting for this. July is still a long time but please let's make that. If there's anything we can do to speed this up, like creating a service case at Microsoft, then please let me know.

@rzikm
Copy link
Member

rzikm commented May 31, 2024

Creating a service case is unlikely to speed things up.

.NET has monthly cadence of servicing releases (with release dates coinciding with patch Tuesdays). Around the time one release is released, next one is branched off and starts to be prepared.

Restarting the process mid-way to include additional fixes would be costly (I don't know the figures, it's just my assumption based on my observations).

At any rate, there is potentially a way to unblock you and your customers earlier by using a private build of System.Net.Security.dll containing the fix. We use the process for validating that the fix is working. For desktop platforms the steps are simple (self-contained publish of the app+ overwriting the changed dll), I can provide you with the right dll for macOS if this option is available for you.

Unfortunately, I don't know if a similar process is possible for mobile platforms. Maybe @simonrozsival knows.

@simonrozsival
Copy link
Member

simonrozsival commented Jun 3, 2024

Unfortunately, I don't know if a similar process is possible for mobile platforms.

It is possible to use custom runtime build in a mobile app, but I wouldn't recommend it. An example how to use custom dll on Android (or other mobile platforms) is shown here in the "Known workarounds" section: #95295

@JeroenBer
Copy link

Well in that case please try to get this fix in the July servicing release.

@rzikm rzikm added the Servicing-consider Issue for next servicing release review label Jun 3, 2024
@rzikm
Copy link
Member

rzikm commented Jun 3, 2024

Approved via email on 6/3 and on 6/6 (with corrected customer info).

@rzikm rzikm added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jun 3, 2024
@rzikm rzikm merged commit ca28411 into dotnet:release/8.0-staging Jun 4, 2024
110 of 121 checks passed
@filipnavara filipnavara deleted the backport-95898 branch June 4, 2024 07:57
@JeroenBer
Copy link

@rzikm this is merged into dotnet:release/8.0-staging I think, does this mean it will come into the next servicing release in July ?

For Android / iOS / macOS does this come available with a workload update for android / ios / macos or does this come available with a SDK update ?
I mean Android / iOS / macOS have their own runtimes or is this something that is shared between all runtimes ? Or is this not in the runtime ?

@wfurt
Copy link
Member

wfurt commented Jun 7, 2024

The libraries are shared across all runtimes. For Android, the managed implementation is the only one option because there is no GSSAPI. For iOS & macOS you need to subscribe to it.

@JeroenBer
Copy link

Thanks for the info, so just updating the SDK and rebuilding would be enough then ? No need to update workloads for android etc. For iOS / macOS we opted in for managed NTLM.

@wfurt
Copy link
Member

wfurt commented Jun 7, 2024

Thanks for the info, so just updating the SDK and rebuilding would be enough then ? No need to update workloads for android etc. For iOS / macOS we opted in for managed NTLM.

yes. cc @filipnavara just in case I missed something.

@filipnavara
Copy link
Member Author

It’s correct. SDK update is all that’s needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Security community-contribution Indicates that the PR has been added by a community member os-android Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants