Broken Encryption

[Fourmisain]

Problem

I had a gander at this project's encryption code and the symmetric key encryption using AES looked fine to me, so the next question was how client and server are sharing their secret key used for said encryption. I was pretty shocked (and a bit amused) to see that secret key is simply sent as "plaintext", unencrypted over the wire.

It completely defeats the point of using symmetric key encryption if anyone tapping the wire also gets the "secret" key.

Solution

So how can a secret get shared between two parties over an insecure channel? One of the earliest answers is the Diffie–Hellman key exchange (DH for short) and while it is still very useful to learn about it, it is not exactly considered secure anymore.

Luckily, there is a variant of Diffie-Hellman which is considered secure: Elliptic-curve Diffie-Hellman (ECDH for short).

Even luckier, Java 11 has implementations for ECDH (for Curve25519 and Curve448), so they can be used for Minecraft 1.17 without any issue.

I don't feel comfortable contributing to an All Rights Reserved project, so I won't create a Pull Request, but here's some example code taken verbatim from https://bugs.openjdk.java.net/browse/JDK-8189806:

KeyPairGenerator kpg = KeyPairGenerator.getInstance("XDH");
NamedParameterSpec paramSpec = new NamedParameterSpec("X25519");
kpg.initialize(paramSpec); // equivalent to kpg.initialize(255)
//alternatively: kpg = KeyPairGenerator.getInstance("X25519")
KeyPair kp = kpg.generateKeyPair();

KeyFactory kf = KeyFactory.getInstance("XDH");
BigInteger u = ...
XECPublicKeySpec pubSpec = new XECPublicKeySpec(paramSpec, u);
PublicKey pubKey = kf.generatePublic(pubSpec);

KeyAgreement ka = KeyAgreement.getInstance("XDH");
ka.init(kp.getPrivate());
ka.doPhase(pubKey, true);
byte[] secret = ka.generateSecret();

The example code is a bit confusing since the middle part generates a public key for demonstration, but in practice this key is received from the second party.

The key exchange is actually really simple to use:

1. generate a key pair (part 1 of the example code)
2. send the public key to the second party
3. on receiving the public key from the second party, use it together with the private key generated in step 1 to generate the secret (part 3 of the example code)

If both parties do this, they will have the same secret (called the shared secret) which can then be used for symmetric key encryption.

(When implementing this, make sure everything is executed on the server thread, not on the networking threads.)

Remaining Concerns

This is still not perfectly secure because Diffie-Hellman is known to be vulnerable to Man-in-the-middle attacks, in other words: If someone not only listens to the wire but can also interfere with it, they can initiate a key exchange with the client and another one with the server - both the client and the server will think they are talking to each other, while in reality, the middle-man can decrypt and even tamper with all messages being sent.

This is an issue which basically can only be solved by authentication - a much harder topic. Maybe the switch to Microsoft accounts can be used to enable authentication? I have no idea.

Even with authentication - in the current implementation the server still decrypts all messages, meaning you must not only trust all clients (because they can obviously hear you ingame) but also the server - but this is honestly fine.

[henkelmax]

Minecraft packets are encrypted.

[Fourmisain]

I don't think you understood what I am saying.

Here's the piece of code that is sending the "secret" from the server to the client - this is completely unencrypted, so anyone listening the the connection can read it: https://github.com/henkelmax/simple-voice-chat/blob/1a7c30c3c421b3151668ddf9fcf31f52b0e80f1b/src/main/java/de/maxhenkel/voicechat/voice/server/ServerVoiceEvents.java#L50-L58

[henkelmax]

You don't understand what I mean. This is a piece of code sending a packet via Minecrafts networking. Not mine. And Minecrafts networking is encrypted.

[Fourmisain]

Minecrafts networking is encrypted.

I was under the impression that only the login packets are encrypted while the actual gameplay packets which Fabric API's ServerPlayNetworking sends aren't.

But I also have trouble finding a good source for either claim.

One random forum post here says: https://www.spigotmc.org/threads/enable-protocol-encryption-for-offline.211631/?__cf_chl_jschl_tk__=pmd_b83b4daaeab8215eff06cb7e2e0886792ee556c9-1627405157-0-gqNtZGzNAg2jcnBszQYO

Master_chan: Minecraft protocol only encrypts authentication data, all game packets are transmitted without encryption.

This links to this page as well: https://wiki.vg/Protocol_Encryption

While this does say

As of 12w17a, Minecraft uses encrypted connections for online-mode servers.

It only talks about login packets, so I'm not sure if this proves anything.

But, if Minecraft actually encrypts traffic, what is the point of another layer of (broken) AES?

[henkelmax]

I was under the impression that only the login packets are encrypted while the actual gameplay packets which Fabric API's ServerPlayNetworking sends aren't.

This would make absolutely no sense. Only servers running in offline mode are unencrypted.

But, if Minecraft actually encrypts traffic, what is the point of another layer of (broken) AES?

What should be broken there?

It is also not another "layer". The voice chat itself doesn't use Minecraft networking.

[Fourmisain]

The voice chat itself doesn't use Minecraft networking.

Oh, right, it's using a separate UDP connection for that - I was missing that entirely! So it all comes down to whether all Minecraft packets are encrypted or not.

This would make absolutely no sense.

Only login information is crucial to protect, it would make sense to not encrypt gameplay packets for increased performance.

I'll probably be looking into this to check whether this is true or not, because this probably applies to e.g. text chat as well and I want a definite answer to this.

[henkelmax]

https://wiki.vg/Protocol_Encryption

  C->S : Handshake State=2
  C->S : Login Start
  S->C : Encryption Key Request
  (Client Auth)
  C->S : Encryption Key Response
  (Server Auth, Both enable encryption)
  S->C : Login Success

The encryption keys are exchanged in the login networking. The only packet encrypted in the login phase is the login success packet. This implies that all other traffic from then on is encrypted. I doubt Minecraft only encrypts one packet xD

[Fourmisain]

You were right!

Inside ClientLoginNetworkHandler.onHello(), there's

         this.connection.send(loginKeyC2SPacket2, (future) -> {
            this.connection.setupEncryption(cipher3, cipher4);
         });

which, on response, executes inside ClientConnection:

      this.encrypted = true;
      this.channel.pipeline().addBefore("splitter", "decrypt", new PacketDecryptor(decryptionCipher));
      this.channel.pipeline().addBefore("prepender", "encrypt", new PacketEncryptor(encryptionCipher));

This is never undone, so it really is all packets being encrypted.

That's actually really good to know, I'm glad we had this talk!

[henkelmax]

That's actually really good to know, I'm glad we had this talk!

Let me know if you find any other potential flaws in my encryption. I always appreciate people looking into the code to find potential bugs.

[Fourmisain]

Let me know if you find any other potential flaws in my encryption.

I will, though I'll keep it short next time to not totally embarass myself again 😄

[henkelmax]

I will, though I'll keep it short next time to not totally embarass myself again 😄

That happened too many times to me as well. It is always a chance to learn something, so no worries :)