Unsigned responses are accepted while signing is active (signature stripping)

[rwbh]

Summary

smb2 accepts a server response that arrives without SMB2_FLAGS_SIGNED, even when signing is active for the session. Verification is gated on a flag read from the untrusted message, so an on-path attacker can clear that bit and forge the entire response -- the client never verifies, because it was told not to.

MS-SMB2 section 3.2.5.1.3 requires the client to reject such a response.

src/client/connection.rs:

let is_signed = (flags & HeaderFlags::SIGNED) != 0;
...
if is_signed && !is_pending {
    // verify
}
// no else -- an unsigned response falls through and is accepted

Impact

An attacker positioned on the network (SMB's threat model is typically the enterprise LAN) can rewrite file contents, directory listings and metadata undetected. Any consumer that makes a trust decision from a server response -- for example resolving a file's canonical name to decide whether policy permits reading it -- can be fed a forged answer.

Encryption is not an available mitigation on its own: should_encrypt is read from the server's session-setup response (session.rs), so the same attacker can clear that flag too. The AEAD tag is verified when encryption is active; the gap is that the client never insists on it.

Reproduction

A ~100-line MITM proxy clears the flag on every server frame and rewrites the payload. Against a Samba 4.23.8 fixture:

	direct	via proxy
content returned	corpus-spike-payload	TAMPERED-BY-ATTACKER

No error was raised. PoC proxy: https://github.com/wasmagents/xmp-monorepo (docs/sprints/active/831-smb-provider-sprint/research/strip_sign_proxy.py) -- happy to paste it inline if the repo is not reachable.

Suggested fix

Reject an unsigned, non-PENDING response while signing is active. PENDING interim responses stay exempt: servers send those unsigned by design and the final response carries the signature.

Branch (based on v0.17.0): https://github.com/wasmagents/smb2/tree/xmp/reject-unsigned-responses

It also makes the signature comparison constant-time (received_sig != expected_sig on [u8; 16] short-circuits on the first differing byte), with a test that forges signatures sharing 0..15 correct leading bytes.

Test fallout, and why it is expected

Two mock-based tests relied on the old behaviour and are corrected in the branch:

• make_mock_client used SessionFlags(0), so the real Session::setup concluded the session was authenticated and activated signing -- while every canned response arrived unsigned. That combination cannot occur against a real server. The mock now declares the unsigned (guest) session it actually is; those tests exercise reconnect and message-id semantics, not signing.
• a_revival_leaves_no_state_belonging_to_the_dead_session staged a fake signing key before its warm-up write. Staging now happens after; the test still proves staged state does not survive a revival.

975 tests pass, unchanged from before the fix.

Happy to open this as a PR if you would prefer that.