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

Work around a race condition between server join and auth validation. #15756

Merged
merged 1 commit into from Nov 3, 2018

Conversation

Projects
None yet
2 participants
@pchote
Member

pchote commented Oct 29, 2018

Fixes #15642 - see the issue discussion for a testcase.

@pchote pchote added this to the Hotfix release milestone Oct 29, 2018

@pchote

This comment has been minimized.

Member

pchote commented Oct 30, 2018

Elaborating on this a bit, from IRC:

[18:51:35] | <abcdefg30> pchote so a null AuthSignature means we assume the fingerprint is valid?
[18:51:55] | <abcdefg30> (why do we sign and send an auth signature when the connection state is LinkState.ConnectionFailed, though)
[19:22:38] | <pchote> abcdefg30: no
[19:55:14] | <pchote> abcdefg30: we store the private key locally only
[19:55:31] | <pchote> the online check is only to fetch the local user data (forum name, badges) and check whether the key has been revoked
[19:55:42] | <pchote> but neither of those things are needed for authing with a server
[19:56:20] | <pchote> so there is no need to wait for that check to complete before signing the auth data when connecting to the server
[19:57:00] | <pchote> either the keys are valid and everything proceeds as normal, or the keys aren't valid (either revoked or not yet added to the account), and the server will discover that itself when querying the forum, log an error, and not auth them
[19:57:39] | <pchote> the CheckingLink and ConnectionFailed statuses relate to the forum data check
[19:57:49] | <pchote> the other ones relate to the keys themselves

@pchote

This comment has been minimized.

Member

pchote commented Oct 30, 2018

Updated comment and check as discussed in IRC.

@abcdefg30 abcdefg30 merged commit 4ea3e83 into OpenRA:bleed Nov 3, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Member

abcdefg30 commented Nov 3, 2018

@pchote pchote deleted the pchote:fix-auth-join-race branch Nov 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment