diff --git a/native/src/main/c/jni_cipher_macos.c b/native/src/main/c/jni_cipher_macos.c index aa7d1aba..d658eb5d 100644 --- a/native/src/main/c/jni_cipher_macos.c +++ b/native/src/main/c/jni_cipher_macos.c @@ -26,6 +26,15 @@ Java_com_velocitypowered_natives_encryption_OpenSslCipherImpl_init(JNIEnv *env, return 0; } + // But, you're saying, *why* are we using the key as the IV? After all, reusing the key as + // the IV defeats the entire point - we might as well just initialize it to all zeroes. + // + // You can blame Mojang. For the record, we also don't consider the Minecraft protocol + // encryption scheme to be secure, and it has reached the point where any serious cryptographic + // protocol needs a refresh. There are multiple obvious weaknesses, and this is far from the + // most serious. + // + // If you are using Minecraft in a security-sensitive application, *I don't know what to say.* CCCryptorRef cryptor = NULL; CCCryptorStatus result = CCCryptorCreateWithMode(encrypt ? kCCEncrypt : kCCDecrypt, kCCModeCFB8, diff --git a/native/src/main/c/jni_cipher_openssl.c b/native/src/main/c/jni_cipher_openssl.c index 83515be5..06f09e11 100644 --- a/native/src/main/c/jni_cipher_openssl.c +++ b/native/src/main/c/jni_cipher_openssl.c @@ -32,6 +32,15 @@ Java_com_velocitypowered_natives_encryption_OpenSslCipherImpl_init(JNIEnv *env, return 0; } + // But, you're saying, *why* are we using the key as the IV? After all, reusing the key as + // the IV defeats the entire point - we might as well just initialize it to all zeroes. + // + // You can blame Mojang. For the record, we also don't consider the Minecraft protocol + // encryption scheme to be secure, and it has reached the point where any serious cryptographic + // protocol needs a refresh. There are multiple obvious weaknesses, and this is far from the + // most serious. + // + // If you are using Minecraft in a security-sensitive application, *I don't know what to say.* int result = EVP_CipherInit(ctx, EVP_aes_128_cfb8(), (byte*) keyBytes, (byte*) keyBytes, encrypt); if (result != 1) { diff --git a/native/src/main/java/com/velocitypowered/natives/encryption/JavaVelocityCipher.java b/native/src/main/java/com/velocitypowered/natives/encryption/JavaVelocityCipher.java index 58a7da58..00f5a1bf 100644 --- a/native/src/main/java/com/velocitypowered/natives/encryption/JavaVelocityCipher.java +++ b/native/src/main/java/com/velocitypowered/natives/encryption/JavaVelocityCipher.java @@ -48,6 +48,15 @@ public class JavaVelocityCipher implements VelocityCipher { private JavaVelocityCipher(boolean encrypt, SecretKey key) throws GeneralSecurityException { this.cipher = Cipher.getInstance("AES/CFB8/NoPadding"); + // But, you're saying, *why* are we using the key as the IV? After all, reusing the key as + // the IV defeats the entire point - we might as well just initialize it to all zeroes. + // + // You can blame Mojang. For the record, we also don't consider the Minecraft protocol + // encryption scheme to be secure, and it has reached the point where any serious cryptographic + // protocol needs a refresh. There are multiple obvious weaknesses, and this is far from the + // most serious. + // + // If you are using Minecraft in a security-sensitive application, *I don't know what to say.* this.cipher.init(encrypt ? Cipher.ENCRYPT_MODE : Cipher.DECRYPT_MODE, key, new IvParameterSpec(key.getEncoded())); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java index 8b5d4d3d..45c228b3 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java @@ -236,6 +236,15 @@ public class VelocityServer implements ProxyServer, ForwardingAudience { registerTranslations(); + // Yes, you're reading that correctly. We're generating a 1024-bit RSA keypair. Sounds + // dangerous, right? We're well within the realm of factoring such a key... + // + // You can blame Mojang. For the record, we also don't consider the Minecraft protocol + // encryption scheme to be secure, and it has reached the point where any serious cryptographic + // protocol needs a refresh. There are multiple obvious weaknesses, and this is far from the + // most serious. + // + // If you are using Minecraft in a security-sensitive application, *I don't know what to say.* serverKeyPair = EncryptionUtils.createRsaKeyPair(1024); cm.logChannelInformation(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/crypto/EncryptionUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/crypto/EncryptionUtils.java index 283c8d49..04107b39 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/crypto/EncryptionUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/crypto/EncryptionUtils.java @@ -23,8 +23,6 @@ import com.velocitypowered.proxy.util.except.QuietDecoderException; import it.unimi.dsi.fastutil.Pair; import java.io.IOException; import java.math.BigInteger; -import java.nio.ByteBuffer; -import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.security.Key; @@ -111,42 +109,6 @@ public enum EncryptionUtils { } } - /** - * Generates a signature for input data. - * - * @param algorithm the signature algorithm - * @param base the private key to sign with - * @param toSign the byte array(s) of data to sign - * @return the generated signature - */ - public static byte[] generateSignature(String algorithm, PrivateKey base, byte[]... toSign) { - Preconditions.checkArgument(toSign.length > 0); - try { - Signature construct = Signature.getInstance(algorithm); - construct.initSign(base); - for (byte[] bytes : toSign) { - construct.update(bytes); - } - return construct.sign(); - } catch (GeneralSecurityException e) { - throw new IllegalArgumentException("Invalid signature parameters"); - } - } - - /** - * Encodes a long array as Big-endian byte array. - * - * @param bits the long (array) of numbers to encode - * @return the encoded bytes - */ - public static byte[] longToBigEndianByteArray(long... bits) { - ByteBuffer ret = ByteBuffer.allocate(8 * bits.length).order(ByteOrder.BIG_ENDIAN); - for (long put : bits) { - ret.putLong(put); - } - return ret.array(); - } - public static String encodeUrlEncoded(byte[] data) { return MIME_SPECIAL_ENCODER.encodeToString(data); } @@ -155,22 +117,6 @@ public enum EncryptionUtils { return Base64.getMimeDecoder().decode(toParse); } - /** - * Parse a cer-encoded RSA key into its key bytes. - * - * @param toParse the cer-encoded key String - * @param descriptors the type of key - * @return the parsed key bytes - */ - public static byte[] parsePemEncoded(String toParse, Pair descriptors) { - int startIdx = toParse.indexOf(descriptors.first()); - Preconditions.checkArgument(startIdx >= 0); - int firstLen = descriptors.first().length(); - int endIdx = toParse.indexOf(descriptors.second(), firstLen + startIdx) + 1; - Preconditions.checkArgument(endIdx > 0); - return decodeUrlEncoded(toParse.substring(startIdx + firstLen, endIdx)); - } - /** * Encodes an RSA key as String cer format. *