Uh oh!
There was an error while loading. Please reload this page.
Managed implementation of NTLM for Android and tvOS - #66879
Conversation
ghost
commented
Mar 19, 2022
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
ghost
commented
Mar 19, 2022
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsContributes to #62264 This is based on @wfurt's managed NTLM code and extended to cover couple more scenarios (SPNs, channel bindings, message integrity code). It integrates it into HTTP authentication for SocketsHttpHandler for Android and tvOS platforms. Notably, it does not integrate into SMTP client and NegotiateStream implementations but it would be rather trivial to add after the basic code is merged. TODO:
|
There was a problem hiding this comment.
It looks to me like Transform is only ever called once on any given instance. Rather than an instantiable class, this should just be
internalstaticclassRC4{internalstaticvoidTransform(ReadOnlySpan<byte>key,ReadOnlySpan<byte>input,Span<byte>output){Span<byte>state=stackallocbyte[256];
...}}There was a problem hiding this comment.
That's the case for the MIC calculation but it would be reused for Encrypt/Decrypt if we implement it later (for SMTP authentication and NegotiateStream).
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Exception messages should be in resx (and thus use format placeholders instead of interpolation)
There was a problem hiding this comment.
Yep, I have this one in the TODO in the initial comment.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
wfurt
left a comment
There was a problem hiding this comment.
generally LGTM.
Thanks @filipnavara.
This looks like great start.
wfurt
commented
Mar 25, 2022
enterprise tests failures are tracked by #66970 |
filipnavara
commented
Mar 25, 2022
Some of the failures are/were from the temporary test I added. I'll remove it on next pass. |
steveisok
commented
Mar 31, 2022
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
On tvOS devices, it's crashing pretty early. Quite likely not related to this change directly, but we do want to enable System.Net.Security as a result. |
wfurt
commented
Apr 12, 2022
Should we disable tvOS for now and focus on Android? It seems like it is failing as well but from the logs I cannot see what is wrong. |
…ommit. Read NegTokenResp explicitly. Add mechListMIC reading and verification.
The System.Net.Security.Tests crashes are completely unrelated to these changes. In fact, the only relevant test in that batch is the MD4 one for the managed implementation. I did NOT enable the implementation of |
wfurt
commented
Apr 19, 2022
Tests may be #68206 |
* Move MD4 implementation into Common/src/System/Net/Security * Add minimal RC4 implementation * WIP: Integrate managed Ntlm implementation into SocketsHttpHandler Co-authored-by: Tomas Weinfurt <tweinfurt@yahoo.com> * WIP: Makeshift tests for NTLM/Negotiate authentication * Fix compilation, clean up some of the hashing * Avoid using a temporary buffer * Add computation of signing keys, sealing keys and mechListMIC * Various cleanups * Send SPN in target information * Add some validation, mark spots with missing validation * Clean up some of the memory offset manipulation * Move NTLM version into static variable * Add support for channel bindings, clean up * Fix hash calculation in makeNtlm2Hash accidentally broken with last commit. Read NegTokenResp explicitly. Add mechListMIC reading and verification. * Verify last authentication token in HTTP Negotiate authentication * Address feedback * Fix tvOS builds by making few methods static * Enable System.Net.Security tests on Android and iOS Co-authored-by: Tomas Weinfurt <tweinfurt@yahoo.com> Co-authored-by: Steve Pfister <steve.pfister@microsoft.com>
EatonZ
commented
May 4, 2022
@filipnavara Thank you for adding a native RC4 implementation to .NET. Would you consider making the class public so it can be used? I have some old code that uses RC4 (not related to NTLM for Android and tvOS) and it would be nice to be able to call into .NET directly. |
filipnavara
commented
May 4, 2022
@EatonZ Every new API would have to go through API review process. RC4 is unlikely to pass through that process because the algorithm is insecure and deprecated. Feel free to copy the implementation under the current MIT license though, it is really small. |
EatonZ
commented
May 4, 2022
@filipnavara Thanks for the fast response. That is what I was going to do anyway, but thought I'd ask. |
Contributes to #62264
This is based on @wfurt's managed NTLM code and extended to cover couple more scenarios (SPNs, channel bindings, message integrity code). It integrates it into HTTP authentication for SocketsHttpHandler for Android and tvOS platforms. Notably, it does not integrate into SMTP client and NegotiateStream implementations but it would be rather trivial to add after the basic code is merged.
TODO: