Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Online auth system testing #14

Closed
IotaBread opened this issue Jul 7, 2021 · 9 comments · Fixed by #15
Closed

Online auth system testing #14

IotaBread opened this issue Jul 7, 2021 · 9 comments · Fixed by #15

Comments

@IotaBread
Copy link
Contributor

IotaBread commented Jul 7, 2021

Describe the bug
The online mode bypass doesn't work.

To Reproduce
Steps to reproduce the behavior:

  1. Create a server in offline mode with the mod installed, and sessions disabled.
  2. Join the server with a Minecraft account, register, and leave the server
  3. Join the server again with the same account.
  4. Mod still asks to authenticate by the normal way.

Expected behavior
The mod should authenticate the player when it has already registered and has a Minecraft account.

Version
Mod version: 1.2.0 (I'm using the mod built from #13)
Minecraft version: 1.16.5
Fabric version: 0.36.0+1.16

The reason

Click to expand

(I have studied this part of Minecraft before when I tried to create an auth mod with online mode authentication.)
The reason this happens is simple: when the server is in offline mode, all players are assigned an UUID based on their username (see PlayerEntity#getOfflinePlayerUuid). (The UUID for all players is generated by the server, not by the client). Your code doesn't work because it compares the uuid generated by the offline server with the official one, which are always different.

The solution

Click to expand

I can only think of three solutions.

  • Reimplementing Mojang's/Minecraft's authentication to check if the player has a proper account. This would be overly complicated and it may not even be possible.
  • When the player logs on to the server (ServerLoginNetworkHandler#onHello), checking if a Minecraft account with the same username exists. If so, making the Minecraft server handle the authentication and if not, just letting the player pass and authenticate the normal way (/login). This would cause players with the username of a valid Minecraft account to not be able to join the server.
  • Letting each player opt-in to the online mode bypass feature. Like the previous solution, when the player logs on to the server, checking if the player has opted in to the feature. If the player has opted into the feature, let Minecraft handle the authentication part and, if not, let the player authenticate the normal way.
    Do note that the second and third would make players using an official account to use their online-mode-uuid, so they would be able to change their account username and save all their data, but losing the data from when they didn't have the option toggled on.

The letting pass an offline player or authenticate an online one part can be done with just a single mixin

Code
@Mixin(ServerLoginNetworkHandler.class)
public class ServerLoginNetworkHandlerMixin {
    @Redirect(method = "onHello", at = @At(value = "INVOKE", target = "Lnet/minecraft/server/MinecraftServer;isOnlineMode()Z"))
    private boolean authenticatePlayerWithMojang(MinecraftServer server) {
        if (!server.isOnlineMode() || !featureEnabled) return false;

        boolean playerHasFeatureEnabled = ...;
        return playerHasFeatureEnabled && playerHasMinecraftAccount(this.profile.getName());
    }

    private static boolean playerHasMinecraftAccount(String username) {
        // Code from OnPlayerConnect#testPlayerOnline, just checking if the server does return an uuid
    }
}
@IotaBread IotaBread added the bug Something isn't working label Jul 7, 2021
@Dqu1J
Copy link
Member

Dqu1J commented Jul 7, 2021

Huh, I tested in offline mode and for me everything worked. Can you send your configuration file and server log?

@IotaBread
Copy link
Contributor Author

IotaBread commented Jul 7, 2021

Configuration:

{
  "version": 1,
  "sessions-enabled": false,
  "sessions-valid-hours": "6",
  "skip-online-auth": true,
  "password-type": "local",
  "global-password": "123456"
}

Logs:
https://pastebin.com/jYTJ1MMQ
(ByMartrixX is the online player, MartrixX22 is the offline one)
Do note that I ran the server from a dev env

@IotaBread
Copy link
Contributor Author

Added a breakpoint to see the real and actual uuid
https://pastebin.com/6MLxXdDN

[17:06:45] [Server thread/INFO] (Minecraft) ByMartrixX joined the game
UUID: f3845173905533efa9b85e349fedf0e2; Real UUID: cdc80916cd0243b8938cf3105dc49c2f

@IotaBread
Copy link
Contributor Author

Forgot to mention in the reproduction steps that sessions have to be disabled

@Dqu1J
Copy link
Member

Dqu1J commented Jul 7, 2021

I don't see any good solutions for this. I will think about removing this feature at all.

@Dqu1J
Copy link
Member

Dqu1J commented Jul 12, 2021

This now needs testing. Please, download the build from:
https://github.com/Dqu1J/simplerauth/releases/tag/1.2.1-pre1
And test in all possible scenarios, including offline mode!
Report any bugs you found here.
Thank you.

@Dqu1J Dqu1J reopened this Jul 12, 2021
@Dqu1J Dqu1J added high priority and removed bug Something isn't working labels Jul 12, 2021
@Dqu1J Dqu1J changed the title Online mode bypass doesn't actually work Online auth system testing Jul 12, 2021
@Dqu1J Dqu1J pinned this issue Jul 12, 2021
@ghost
Copy link

ghost commented Jul 13, 2021

I have tested all combinations of online and offline players, everything works fine, the only problem is now susceptibility to bot attacks on [online mod] servers .. using many generated nicks like: Xd67dhsXkD
In the future you should think about limiting the maximum number of offline nicks per server per second/10seconds etc.

@Dqu1J
Copy link
Member

Dqu1J commented Jul 15, 2021

No issues was found, closing this now. Feel free to reopen this if you found an error before 1.2.1 release.

@Dqu1J Dqu1J closed this as completed Jul 15, 2021
@Dqu1J Dqu1J unpinned this issue Jul 15, 2021
@ghost
Copy link

ghost commented Jul 15, 2021

is it possible to modify the code so that when it tries to connect a non-original client that has used (stolen) the name of the original client... it writes a specific error phrase? For example : You can't use the nickname of the original player

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

Successfully merging a pull request may close this issue.

2 participants