Skip to content

Setup encryption before possible disconnect#1754

Open
WouterGritter wants to merge 1 commit intoPaperMC:dev/3.0.0from
WouterGritter:setup-encryption-before-disconnect
Open

Setup encryption before possible disconnect#1754
WouterGritter wants to merge 1 commit intoPaperMC:dev/3.0.0from
WouterGritter:setup-encryption-before-disconnect

Conversation

@WouterGritter
Copy link
Copy Markdown
Contributor

There's no reason to delay setting up encryption to after we have fetched the GameProfile from Mojang.

Before this PR, this could result in a disconnect() call before we had set up encryption if Mojang's servers are down:

if (throwable != null) {
  logger.error("Unable to authenticate player", throwable);
  inbound.disconnect(Component.translatable("multiplayer.disconnect.authservers_down"));
  return;
}

// Only here we were setting up encryption

Simulating this with throwable != null || true, we get disconnected on the client with a garbled exception message complaining about packets instead of the proper disconnect message. This is because as far as the client is concerned, encryption should already be in place.

This PR moves enabling encryption on the connection right when we receive the packet and after we're done with setting up the crypto stuff.

This whole code block is already in a giant try/catch block:

try {
  // ...
} catch (GeneralSecurityException e) {
  logger.error("Unable to enable encryption", e);
  mcConnection.close(true);
}

which still results in the same behavior when enableEncryption() fails (error log & disconnect), just without sending an unneeded fetch to Mojang.

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.

2 participants