Repository navigation
fix: follow GNOME Remote Login into the user's session - #44
Open
ZacharyZcR wants to merge 1 commit into
Open
ZacharyZcR wants to merge 1 commit into
ZacharyZcR wants to merge 1 commit into
Conversation
The built-in engine followed GNOME's Server Redirection but never reached the session: - The routing token lost its "Cookie: msts=" prefix, which IronRDP's opaque token does not add back, so GNOME read the request as a new connection and failed its negotiation (UnexpectedMessageType got 240). The token now goes back as the server sent it, as FreeRDP does. - The redirect flags its one-time password as encrypted, which the engine dropped, then tried NLA with the original password. GNOME's one-time account names also contain random characters, and one with a backslash is split into a domain by NLA. The hop now logs on with RDSTLS, sending the redirection GUID, the account name as is and the password cookie back, which is what GNOME expects and FreeRDP does. - The redirect parser read the target addresses before the GUID and certificate, and skipped them when an address was set, so later fields were misread. It now follows the PDU order. - GNOME sends data on a dynamic channel the client refused, which ended the session. That PDU is dropped, as FreeRDP does. Rebuilt the bundled engine. Fixes Termix-SSH#28
|
AI review | Looks safe to merge Adds RDSTLS authentication support, fixes routing token prefix handling, corrects Server Redirection PDU field parsing order, and gracefully drops data for refused dynamic channels to enable GNOME Remote Login redirections. Partial review: 1 file(s) could not be read, a maintainer should look at them. Reviewed 7551db8. AI can be wrong, a maintainer still decides. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #28. Reproduced on Ubuntu 26.04.1 with GNOME Remote Login (gnome-remote-desktop system mode + GDM). The built-in engine reached the login screen, followed the redirect and failed with
UnexpectedMessageType { got: 240 }, as reported. Four problems were stacked:redirectTargetstrippedCookie: msts=on the assumption that the engine adds it back.with_load_balance_infosends anOpaqueRoutingToken, which is written as is, so the X.224 request carried only the digits. GNOME's system daemon looks forCookie: msts=, treated it as a new connection and failed negotiation on the token's first digit (Incorrect negotiation request type 49in its log). The token now goes back as the server sent it, minus the trailing CRLF the engine adds. FreeRDP's server writes the prefix into the redirect itself, and its client sendsLoadBalanceInfoverbatim. This also matters for Windows load balancing tokens, which carry the same prefix, and leavestsv://broker tokens untouched.LB_PASSWORD_IS_PK_ENCRYPTED, which the engine dropped. It then tried NLA with the redirect's account name and the user's original password (STATUS_LOGON_FAILURE). The account names are 16 random printable characters, so about one in six contains a backslash, which NLA splits off as a domain (Could not find user in SAM database).sspicannot express such a name. A redirected hop that carries a password cookie now logs on with RDSTLS (MS-RDPBCGR 2.2.17), as FreeRDP/Remmina does. The engine sends its own X.224 request asking forPROTOCOL_RDSTLS, because the connector only enables SSL/HYBRID. After TLS it reads the server's capabilities, sends the Authentication Request with the redirection GUID, the account name as is, the domain and the cookie, and checks the response. Then it hands back to the connector, which skips CredSSP for RDSTLS.TargetNetAddressesstraight after the TSV URL, and only consumed it when no address was set. The PDU order is …, TSV URL, redirection GUID, target certificate, target net addresses. The parser now follows that order, reads the GUID, skips the certificate (whose cap is raised from 4 KB to the PDU's 64 KB) and always consumes the address list.DrdynvcClientreturnsaccess to non existing DVC channel, which ended the session. That one PDU is dropped and logged, as FreeRDP does.Validation:
Not covered: RDSTLS with an auto-reconnect cookie or FedAuth token. Only the password form is implemented, which is what a Server Redirection PDU provides.