Skip to content

Commit

Permalink
A few small code cleanups for cryptography
Browse files Browse the repository at this point in the history
* Remove some unused cryptographic code
* Add some notes about how Minecraft's cryptography choices have not quite survived the test of time
  • Loading branch information
astei committed Dec 21, 2024
1 parent 3919195 commit af97fff
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 54 deletions.
9 changes: 9 additions & 0 deletions native/src/main/c/jni_cipher_macos.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions native/src/main/c/jni_cipher_openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ public VelocityCipher forDecryption(SecretKey key) throws GeneralSecurityExcepti

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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,15 @@ void start() {

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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
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;
Expand Down Expand Up @@ -111,42 +109,6 @@ public static boolean verifySignature(String algorithm, PublicKey base, byte[] s
}
}

/**
* 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);
}
Expand All @@ -155,22 +117,6 @@ public static byte[] decodeUrlEncoded(String toParse) {
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<String, String> 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.
*
Expand Down

0 comments on commit af97fff

Please sign in to comment.