No Chat Reports

No Chat Reports

43M Downloads

[Third-party server interoperability] When connecting to a Waterfall/BungeeCord server that is enforcing secure profiles, No Chat Reports will fail to detect that secure profiles are required due to a custom (non-standard/non-vanilla) disconnect message being sent by Waterfall/BungeeCord.

akemin-dayo opened this issue · 7 comments

commented

※ Updated issue

NCR determines whether or not a server is enforcing the use of secure profiles by seeing if the client was disconnected with one of several standardised disconnect reasons that vanilla Minecraft server implementations will send if an NCR'd client attempts to join an secure-profile-enforced server, as seen here:

private static final List<String> KEY_DISCONNECT_REASONS = ImmutableList.of(
"multiplayer.disconnect.missing_public_key",
"multiplayer.disconnect.invalid_public_key_signature",
"multiplayer.disconnect.invalid_public_key"
);

The core issue here is that BungeeCord (of which Waterfall is a fork of) is, for some strange reason, sending a fully custom disconnect reason instead of using one of the standardised Mojang-provided reasons.

The \u00a7cA secure profile is required to join this server. message seen in the screenshot above is defined here:

https://github.com/SpigotMC/BungeeCord/blob/696315615d67cc5387b502d1d5ac0ceaae4fadbf/proxy/src/main/resources/messages.properties#L25

And we can see that disconnect message being sent in this part of the code:

https://github.com/SpigotMC/BungeeCord/blob/78ca16dfe3bf9a21d5c054a1884d4f5f198a62bc/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java#L385-L408

The end result is that this causes NCR to erroneously believe that the client was disconnected for some other reason entirely, and not because the server requires secure profiles:

} else if (KEY_DISCONNECT_REASONS.contains(contents.getKey())) {
if (ServerSafetyState.getLastServerAddress() != null) {
if (!NCRConfig.getServerWhitelist().isWhitelisted(ServerSafetyState.getLastServerAddress()) && !NCRConfig.getClient().whitelistAllServers()) {
client.setScreen(new UnsafeServerScreen());
} else {
if (ServerSafetyState.getReconnectCount() <= 0) {
ServerSafetyState.setAllowsUnsafeServer(true);
reconnectLastServer();
} else {
ServerSafetyState.setReconnectCount(0);
}
}
}
}

I've confirmed that Velocity thankfully does not have this issue — they simply send the standard Vanilla multiplayer.disconnect.missing_public_key disconnect reason, as seen here:

https://github.com/PaperMC/Velocity/blob/1a3fba4250553702d9dcd05731d04347bfc24c9f/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialLoginSessionHandler.java#L115


Original issue

Environment

Minecraft version: 1.19.2
Modloader version: quilt-loader 0.17.4
No Chat Reports version: 1.11.2

Bug Description

My girlfriend occasionally joins an old server she used to play on ("Empire Minecraft") that apparently recently switched to enforcing secure profiles, but No Chat Reports somehow fails to detect this.

I'm not sure if this is some sort of configuration issue on the server's side (they are behind a Waterfall proxy), or if this is an issue/deficiency that should be addressed by NCR.

See also: #184.

※ NOTE: I don't play on this server at all, nor do I really know anything about it other than the fact that it exists.

Logs

Server address smp9.emc.gs (apparently directly connects to one of the backend servers, I think?):

[Render thread/INFO]: Connecting to: smp9.emc.gs:25565, will expose public key: false
[Render thread/INFO]: Server Data IP: smp9.emc.gs
[Render thread/INFO]: Connecting to smp9.emc.gs, 25565

Server address play.emc.gs:

[Render thread/INFO]: Connecting to: play.emc.gs:25565, will expose public key: false
[Render thread/INFO]: Server Data IP: play.emc.gs
[Render thread/INFO]: Connecting to play.emc.gs, 25565

Just for reference, here's a screenshot from her client:
image

commented

Can confirm. Perhaps it could be due to the speed of which NCR tries to reconnect?

commented

So, good news — I know why this is happening!

Basically, NCR determines whether or not a server is enforcing the use of secure profiles by seeing if the client was disconnected with one of several standardised disconnect reasons that vanilla Minecraft server implementations will send if an NCR'd client attempts to join an secure-profile-enforced server, as seen here:

private static final List<String> KEY_DISCONNECT_REASONS = ImmutableList.of(
"multiplayer.disconnect.missing_public_key",
"multiplayer.disconnect.invalid_public_key_signature",
"multiplayer.disconnect.invalid_public_key"
);

The core issue here is that BungeeCord (of which Waterfall is a fork of) is, for some strange reason, sending a fully custom disconnect reason instead of using one of the standardised Mojang-provided reasons.

The \u00a7cA secure profile is required to join this server. message seen in the screenshot above is defined here:

https://github.com/SpigotMC/BungeeCord/blob/696315615d67cc5387b502d1d5ac0ceaae4fadbf/proxy/src/main/resources/messages.properties#L25

And we can see that disconnect message being sent in this part of the code:

https://github.com/SpigotMC/BungeeCord/blob/78ca16dfe3bf9a21d5c054a1884d4f5f198a62bc/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java#L385-L408

The end result is that this causes NCR to erroneously believe that the client was disconnected for some other reason entirely, and not because the server requires secure profiles:

} else if (KEY_DISCONNECT_REASONS.contains(contents.getKey())) {
if (ServerSafetyState.getLastServerAddress() != null) {
if (!NCRConfig.getServerWhitelist().isWhitelisted(ServerSafetyState.getLastServerAddress()) && !NCRConfig.getClient().whitelistAllServers()) {
client.setScreen(new UnsafeServerScreen());
} else {
if (ServerSafetyState.getReconnectCount() <= 0) {
ServerSafetyState.setAllowsUnsafeServer(true);
reconnectLastServer();
} else {
ServerSafetyState.setReconnectCount(0);
}
}
}
}

commented

Well, this can be solved, I just need to add their special message to expected disconnect reasons for enforce-secure-profile. It is arcane to me, however, why would they want custom message for that in the first place.

commented

Yeah, I suppose that's a solution, too.

I've confirmed that Velocity thankfully does not have this issue — they simply send the standard Vanilla multiplayer.disconnect.missing_public_key disconnect reason, as seen here:

https://github.com/PaperMC/Velocity/blob/1a3fba4250553702d9dcd05731d04347bfc24c9f/proxy/src/main/java/com/velocitypowered/proxy/connection/client/InitialLoginSessionHandler.java#L115

I agree that it's… a really strange decision that the BungeeCord devs have made here. So I guess NCR will just have to have a special case to handle them l o l

commented

It is notable that server owners could adjust the messages manually as well by simply modifying the messages.properties file inside the bungeecord.jar. As the file is not provided outside the jar, it is unlikely that most server owners would, though.
See also

Edit: actually it is possible to include the file outside the jar, so some servers may use this for translations or branding.

commented

I think most servers would stick to the stock messages. For the rare cases of fully custom, server-specific disconnect reasons, #184 would probably suffice.

Could also consider adding an NCR-specific button to a corner of the disconnect reason screen or something that would function as a "Were you disconnected for requiring a secure profile?" type of thing, but I'm not sure how well the UX would work for that / if it's even worth the work of implementing.

commented

Could also consider adding an NCR-specific button to a corner of the disconnect reason screen or something that would function as a "Were you disconnected for requiring a secure profile?" type of thing, but I'm not sure how well the UX would work for that / if it's even worth the work of implementing.

Nah, I agree that #184 would suffice. For troubleshooting users would remove the mod either way, so there is no need to overengineer it.