Integrate realmd authentication stream hardening#461
Conversation
Up to standards ✅🟢 Issues
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b7fae6a00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| catch [System.IO.IOException] { | ||
| return $false |
There was a problem hiding this comment.
When the invalid-stream case writes extra data after the server has already closed the socket, Windows/.NET can surface the close as an IOException with a connection-reset socket error instead of returning 0 from Read. In that correct prompt-close scenario this helper returns false, so the new smoke test can fail even though realmd rejected and closed the hostile connection as intended; distinguish timeouts from reset/closed errors here.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| function New-LogonChallenge { | ||
| $username = [System.Text.Encoding]::ASCII.GetBytes("NOUSER") |
There was a problem hiding this comment.
If the configured realm database happens to contain an account named NOUSER, these challenge responses are no longer deterministic because realmd returns an SRP challenge with per-connection random values, so the every-split SequenceEqual check later can fail even though TCP fragmentation handling is correct. Generate a unique unlikely username for each run or assert/remove it from the test database before comparing full responses.
Useful? React with 👍 / 👎.
2 participants
Summary
realmdsubmodule to the tested authentication-stream hardening commitDepends on mangos/realmd#30.
Why
The Classic client can fragment its login challenge at any TCP boundary, while the previous server path could dispatch handlers before a complete protocol frame was available. The integration also lacked an installed-binary regression for malformed streams and stalled sessions. Separately, realmd did not periodically drain the shared logger's buffered file sink, so debug authentication records could remain invisible until shutdown.
The linked submodule change centralizes framing and state validation, caps unauthenticated buffering, adds a pre-authentication deadline, preserves patch and authenticated realm-list behavior, and flushes logs without restoring per-line I/O.
Compatibility
realmdremains one multi-client application. The protocol guard is build-independent and its test covers every accepted build: 5875, 6005, 6141, 8606, 12340, 15595, 18273, 18414, 21742, 26972, 35662, and 40000.This parent intentionally pins the compatible realmd commit rather than current realmd
master: the latter includes the laterCommon/Locales.handCommon/TimeConstants.hsplit, while current Zeromasterdoes not yet provide those headers.Validation
RelWithDebInfobuild passed, including realmd, mangosd, modules, tools, and testsrealmd_auth_protocol_tests,auth_crypto_tests,lease_tests,network_regression_tests)realmd.exeSHA-256 hashes matchedAPPROVEwith no blocking or important findings after follow-upThis change is?