Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow PAKID_CORE_CLIENTID_CONFIRM in RDPDR_CHANNEL_STATE_NAME_REQUEST #10200

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tomsci
Copy link

@tomsci tomsci commented May 17, 2024

Occasionally servers can send this sequence which breaks the channel state machine leading to all smartcard operations failing. This patch modifies the state transitions to tolerate the observed sequence of operations.

In my testing, this fixes the issue I mentioned in #9506 (comment) - but it's maybe not the best way to fix the problem. For example it's possible that adding RDPDR_CHANNEL_STATE_NAME_REQUEST to the allowable states in PAKID_CORE_CLIENTID_CONFIRM breaks something else I didn't see in my testing - I'm not familiar enough with this codebase or protocol to say for sure. So consider this PR a suggestion and a chance for discussion rather than a perfect solution :)

Occasionally servers can send this sequence which breaks the channel
state machine leading to all smartcard operations failing. This patch
modifies the state transitions to tolerate the observed sequence of
operations.
@freerdp-bot
Copy link

Can one of the admins verify this patch?

Copy link

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Member

@akallabeth akallabeth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like a works for me patch.

  1. will break server side (proxy) rdpdr
  2. I´d like to know a few more details about your setup (you attached some to a already closed issue), specifically if the systems are stock or if there is some additional software installed.
  3. what would be awesome a session recording of such a situation (that is if you can do this for a test user, the recording will contain the whole session data and desktop) with xfreerdp /dump:record,file:somefile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants