fix(credssp): complete the SPNEGO mechListMIC exchange in CredSspServer

[Ki Hyun Park (kihyun1998)]

Closes #687.

Problem

CredSspServer cannot authenticate sspi's own CredSspClient when both sides run NTLM inside SPNEGO (ClientMode::Negotiate / ServerMode::Negotiate — the pairing RDP requires, since Windows RDP servers reject bare NTLM inside CredSSP). The handshake fails on the AUTHENTICATE leg with:

Error { error_type: InvalidToken, description: "CredSSP server expected an encrypted public key", nstatus: None }

Because the client always includes an NTLM MIC, the SPNEGO mechListMIC exchange is mandatory (MS-SPNG 3.3.5.5 / RFC 4178 §5), so the client defers pubKeyAuth until it has received and verified the server's final SPNEGO token. The server side, however, dropped that final token (ts_request.nego_tokens = None) at the moment its acceptor completed, and demanded pubKeyAuth in the same TSRequest — one leg earlier than its own client sends it.

Fix

In CredSspServer's NegoToken state, when the acceptor completes (Ok/CompleteNeeded) but the incoming TSRequest carries no pubKeyAuth and the acceptor produced a final output token:

• reply ServerState::ReplyNeeded carrying that final SPNEGO token (accept-completed + mechListMIC) instead of dropping it, and
• accept pubKeyAuth on the following leg (awaiting_pub_key_auth flag; calling the acceptor again on that leg would hit NegotiateState::Ok → OutOfSequence).

This matches the client's behavior and what Windows RDP servers do on the wire. Pairings where pubKeyAuth arrives together with the last nego token (bare NTLM, Kerberos) take the unchanged single-leg path — the new branch only activates when pubKeyAuth is absent and a final token exists. The pubKeyAuth decrypt/encrypt sequence is extracted into exchange_pub_key_auth to avoid duplicating it across the two legs.

Resulting wire flow (Negotiate-NTLM)

before: NEGOTIATE → CHALLENGE → AUTHENTICATE+mechListMIC → ❌ InvalidToken
after:  NEGOTIATE → CHALLENGE → AUTHENTICATE+mechListMIC → accept-completed+mechListMIC
        → pubKeyAuth → pubKeyAuth (server) → authInfo → Finished ✅

Tests

• New credssp_negotiate_ntlm client↔server test pins the four-client-leg handshake — it fails with the pre-fix error on master and passes with this change. (The suite previously paired only Ntlm↔Ntlm and Negotiate↔Negotiate-Kerberos, so this path was never exercised.)
• Existing credssp_ntlm (3 legs, unchanged) and credssp_kerberos (single-leg pubKeyAuth, unchanged) still pass — cargo test --test sspi --features "network_client,__test-data": 19 passed; cargo test -p sspi --lib: 230 passed.

Found while building an RDP client that drives CredSspClient in Negotiate mode against real Windows hosts (which complete this exact client flow) and attempting to stand up an sspi-based loopback CredSSP server for integration tests.

CI

f8959ca adds __test-data to the three root-manifest test-matrix rows, so the client_server suite (gated on all(network_client, __test-data)) actually runs in CI from now on — it previously never did, which is how #687 shipped unnoticed. The suite is hermetic (KdcMock / NetworkClientMock); verified locally with the exact win-row feature combo and the shared minimal combo, all green.

[Ki Hyun Park (kihyun1998)]

One observation from reading the CI config while preparing this PR: the client_server test module is gated on all(feature = "network_client", feature = "__test-data"), but no row of the CI test matrix enables __test-data — so the client↔server pairing tests (including credssp_ntlm, credssp_kerberos, and the regression test added here) currently only run locally, never in CI. That is likely how #687 went unnoticed.

Happy to add __test-data to the matrix rows in this PR if you'd like — or leave it to a separate change if you prefer to keep this one scoped to the fix.

[Ki Hyun Park (kihyun1998)]

Went ahead and added it in f8959ca (kept as a separate commit so it's easy to drop if you'd rather scope this PR to the fix). Verified locally with the win-row combo network_client,dns_resolver,scard,tsssp,__test-data — the suite is hermetic and green.

[Pavlo Myroniuk (TheBestTvarynka)]

Hi,
Thank you for the fix! 💟 And sorry for the late review. I was on a vacation last week 😅

I understand how we allowed this to happen. We have tests for CredSSP + bare NTLM and CredSSP + SPNEGO + Kerberos. But we did not have CredSSP + SPNEGO + NTLM.

since Windows RDP servers reject bare NTLM inside CredSSP

I don't think it's true. As far as I know, the mstsc (default Windows RDP client) always sends bare NTLM over CredSSP, and it has always worked well.

I left a comment about your approach.

[Pavlo Myroniuk (TheBestTvarynka)]

praise: good test 💟

[Pavlo Myroniuk (TheBestTvarynka)]

This change should be in a separate PR. We try not to mix CI and library code changes

[Pavlo Myroniuk (TheBestTvarynka)]

Honestly, I do not like this field 😅 It seems like a workaround due to the bad state machine.

I explored the code more deeply and thought about it for some time.

The problem is in the server-side CredSSP state machine. It expects the encrypted public key immediately after the internal protocol (NTLM) completes authorization. And that is wrong. It worked in the past, before the big SPNEGO refactoring.

Instead of adding a new bool flag, I recommend improving the state machine. I already developed a fix and will create a PR today.

Thank you for showing up to this problem!

[Pavlo Myroniuk (TheBestTvarynka)]

closed in favor of #689

[Ki Hyun Park (kihyun1998)]

Yeah, this is a much better fix. The flag I added was really just a workaround, your state machine change actually fixes it. Thanks for taking the time to dig into it.

I'll send the __test-data CI matrix bit as its own PR so these client_server tests actually run in CI. They never have, which is probably why #687 went unnoticed for so long.