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

Fix encryption in pipeline for Glowstone #748

Merged
merged 5 commits into from Nov 26, 2017

Conversation

Projects
None yet
2 participants
@momothereal
Contributor

momothereal commented Nov 26, 2017

It looks like encryption was only working for the Spigot pipeline, so I added a check for Glowstone and replicated the correct order for GS's channel pipeline.

After this fix, a client receiving a chat packet (0x0F), like the join message, will crash. I'm not sure if this is caused by this particular fix or by another issue in ProtocolSupport. This is fixed, I swapped the encryption/decryption modes by accident.

@Shevchik

This comment has been minimized.

Member

Shevchik commented Nov 26, 2017

I think enabling encryption should be moved to PlatformMiscUtils instead

@Override
public void enableEncryption(ChannelPipeline pipeline, SecretKey key, boolean fullEncryption) {
pipeline.addBefore(GlowStoneChannelHandlers.FRAMING, ChannelHandlers.ENCRYPT, new PacketEncrypter(MinecraftEncryption.getCipher(Cipher.ENCRYPT_MODE, key)));
pipeline.addBefore(ChannelHandlers.ENCRYPT, ChannelHandlers.DECRYPT, new PacketDecrypter(MinecraftEncryption.getCipher(Cipher.DECRYPT_MODE, key)));

This comment has been minimized.

@Shevchik

Shevchik Nov 26, 2017

Member

You forgot to check if encryption is full

This comment has been minimized.

@momothereal

momothereal Nov 26, 2017

Contributor

Is full encryption a cross-platform thing? If so, what's the pipeline handler ordering for it?

This comment has been minimized.

@Shevchik

Shevchik Nov 26, 2017

Member

Full encryption means that both encryption handlers are present. Not full means that only decryption handler should be present.

@Shevchik Shevchik merged commit ff72187 into ProtocolSupport:master Nov 26, 2017

@momothereal momothereal deleted the momothereal:glowstone-fix branch Nov 26, 2017

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