Fix calculation of channel bindings hash in managed NTLM implementation

[filipnavara]

Fixes #95725

Tested on linux-x64 with Managed NTLM against Windows Server 2022 21H2 with Extended Protection set to Required.

[ghost]

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

Issue Details

---

Fixes #95725

Author:	filipnavara
Assignees:	-
Labels:	area-System.Net.Security
Milestone:	-

[rzikm]

LGTM

[rzikm]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rzikm]

/azp list

[azure-pipelines]

CI/CD Pipelines for this repository:

• runtime-coreclr outerloop
• runtime-coreclr jitstress
• runtime-coreclr jitstressregs
• runtime-coreclr jitstress2-jitstressregs
• runtime-coreclr gcstress0x3-gcstress0xc
• runtime-coreclr gcstress-extra
• runtime-coreclr r2r-extra
• runtime-coreclr jitstress-isas-x86
• runtime-coreclr jitstress-isas-arm
• runtime-coreclr jitstressregs-x86
• runtime-coreclr libraries-jitstressregs
• runtime-coreclr libraries-jitstress2-jitstressregs
• runtime-coreclr r2r
• runtime-coreclr runincontext
• runtime-coreclr crossgen2
• runtime-libraries-coreclr outerloop
• runtime-libraries-coreclr outerloop-windows
• runtime-libraries-coreclr outerloop-linux
• runtime-libraries-coreclr outerloop-osx
• runtime
• runtime-libraries enterprise-linux
• runtime-libraries stress-http
• runtime-libraries stress-ssl
• runtime-dev-innerloop
• runtime-coreclr crossgen2 outerloop
• coreclr-release-outerloop-nightly
• runtime-coreclr crossgen2-composite
• runtime-jit-experimental
• runtime-coreclr libraries-jitstress
• dotnet-linker-tests
• runtime-coreclr ilasm
• runtime-coreclr crossgen2-composite gcstress
• runtime-staging
• runtime-coreclr pgo
• runtime-coreclr libraries-pgo
• Antigen
• runtime-community
• Fuzzlyn
• runtime-coreclr superpmi-replay
• runtime-wasm
• runtime-coreclr superpmi-diffs
• runtime-coreclr superpmi-asmdiffs-checked-release
• runtime-extra-platforms
• jit-cfg
• runtime-wasm-perf
• runtime-llvm
• runtime-coreclr jitstress-random
• runtime-coreclr libraries-jitstress-random
• runtime-android-grpc-client-tests
• runtime-wasm-libtests
• runtime-android
• runtime-androidemulator
• runtime-ioslike
• runtime-ioslikesimulator
• runtime-linuxbionic
• runtime-maccatalyst
• runtime-coreclr pgostress
• runtime-coreclr jitstress-isas-avx512
• runtime-tools-tests
• runtime-libraries-mono outerloop
• runtime-sanitized
• runtime-wasm-dbgtests
• runtime-nativeaot-outerloop

[rzikm]

/azp run runtime-libraries enterprise-linux

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rzikm]

/azp run runtime-libraries-coreclr outerloop

[rzikm]

/azp run runtime-libraries enterprise-linux

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[wfurt]

LGTM.
Thanks @filipnavara for digging into this.
I assume we do not have good way how to write a test, right?

[filipnavara]

I assume we do not have good way how to write a test, right?

We can create a subclass of ChannelBinding for use in tests and then do a round-trip against the managed mock NTLM server implementation and check that the computed hash of the channel binding matches the expectation. The trouble is that we would not be really testing the end-to-end scenario in that case. It's probably better than nothing but not really adequate. I may look into that at some point but I would prefer not to delay the PR on this. It would be nice to have for any future refactoring but the particular scenario was already tested manually against a real Windows Server with different settings manually applied.

We cannot test against real NTLM server on local Windows machine either because the managed NTLM implementation is not compiled in that configuration, and we would still need to establish a TLS connection with some reasonable certificate. There may potentially be some global system settings affecting it as well.

[rzikm]

@dotnet/dnceng The outerloop pipeline seems is reporting lots of failures. For example https://dev.azure.com/dnceng-public/public/_build/results?buildId=498370&view=logs&j=631ed29b-f0dd-5b15-38a0-c6c555556dea&t=b9941189-b5a8-5df3-6b3a-0964fabb3acd

Individual work items don't indicate test failure (e.g. https://helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-95898-merge-dcd3063db7254525ba/System.IO.FileSystem.DriveInfo.Tests/1/console.61bb75be.log?helixlogtype=result), but it seems to always end with

ERROR: The process "corerun.exe" not found.
gen-debug-dump-docs.py: Did not find dumps, skipping dump docs generation.
['System.IO.FileSystem.DriveInfo.Tests' END OF WORK ITEM LOG: Command exited with 1]

Can you please have a look?

[dougbu]

@rzikm we haven't changed anything in a while that should impact this aspect of testing. is XUnitLogChecker something specific to this repo❔ has it changed lately❔

[rzikm]

I am not aware of any change on our side, maybe @carlossanlop would know. Looking at recent runs, it probably started at the beginning of December and I have seen it so far only on runtime-libraries-coreclr outerloop pipeline.

[carlossanlop]

I made changes in those failure reporting tools, as you can see from the logs you shared.

But the problem was not in gen-debug-dump-docs.py, it was in XUnitLogChecker:

----- start ===============  XUnitLogChecker Output =====================================================
XUnitLogChecker.dll does not exist in the expected location: C:\h\w\A22708CE\p\XUnitLogChecker.dll
----- end ===============  XUnitLogChecker Output - exit code 1 ===============

I think there might have been a fix around this area after I introduced the support for this tool, because I did validate against outerloop.

It seems in this queue, the tool is not getting added to the set of payloads that get extracted in the helix machines. I opened an issue to investigate it: #96035 cc @ivdiazsa @mangod9 @JulieLeeMSFT @ericstj

Thanks for reporting it, @rzikm .