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

Auth system and forwarding changes #8199

Closed
wants to merge 2 commits into from
Closed

Conversation

Xernium
Copy link
Member

@Xernium Xernium commented Jul 28, 2022

These changes modify the auth session handler to verify the player key early, if present.
This potentially saves session service requests on invalid connections.
The Velocity forwarding mode has also been updated to reflect the V2 key forwarding mode

@Xernium Xernium requested a review from a team as a code owner July 28, 2022 12:03
@Xernium
Copy link
Member Author

Xernium commented Jul 28, 2022

I just realized I need to patch the signature enforcement field with the value defined in the config for velocity-online-mode, doing that now

Comment on lines +1 to +10
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: "FivePB (Xer)" <admin@fivepb.me>
Date: Thu, 28 Jul 2022 11:36:36 +0000
Subject: [PATCH] Verify player key early

This patch refactors authentication with the player key in a way
that forces the key to be verified early using the (currently unused)
player UUID provided in the Server hello packet.
This helps verify connections early and may reduce requests to the session
service
Copy link
Member

@kennytv kennytv Jul 30, 2022

Choose a reason for hiding this comment

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

This patch seems like unnecessarily high-conflict diff for too little benefit in a normal server scenario, since it doesn't help in any other important way with specific targetting etc.

... or just needs fixing the first if on key != null

@Xernium
Copy link
Member Author

Xernium commented Aug 1, 2022

Closed in favor of #8219 and following

@Xernium Xernium closed this Aug 1, 2022
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.

3 participants