Improve encryption metadata validation (#925)
diff --git a/java/common/src/main/resources/org/apache/tsfile/i18n/messages.properties b/java/common/src/main/resources/org/apache/tsfile/i18n/messages.properties
index ffea7c0..036b8cb 100644
--- a/java/common/src/main/resources/org/apache/tsfile/i18n/messages.properties
+++ b/java/common/src/main/resources/org/apache/tsfile/i18n/messages.properties
@@ -1069,6 +1069,15 @@
# === encrypt ===
+# EncryptUtils — unsupported encryption type
+error.encrypt.type_not_supported = Unsupported encryption type: %1$s
+
+# EncryptUtils.getSecondKeyFromStr — key exceeds the supported length (length arg)
+error.encrypt.key_too_long = Encryption key exceeds the maximum length of %1$s bytes
+
+# EncryptUtils.getSecondKeyFromStr — null key
+error.encrypt.key_null = Encryption key must not be null
+
# IEncryptor.getEncryptor — class not found (class name arg)
error.encrypt.encryptor_class_not_found = Get encryptor class failed, class not found: %1$s
diff --git a/java/common/src/main/resources/org/apache/tsfile/i18n/messages_zh.properties b/java/common/src/main/resources/org/apache/tsfile/i18n/messages_zh.properties
index 453977c..5b2eb47 100644
--- a/java/common/src/main/resources/org/apache/tsfile/i18n/messages_zh.properties
+++ b/java/common/src/main/resources/org/apache/tsfile/i18n/messages_zh.properties
@@ -1069,6 +1069,15 @@
# === encrypt ===
+# EncryptUtils — unsupported encryption type
+error.encrypt.type_not_supported = 不支持的加密类型: %1$s
+
+# EncryptUtils.getSecondKeyFromStr — key exceeds the supported length (length arg)
+error.encrypt.key_too_long = 加密密钥长度超过支持的最大值 %1$s 字节
+
+# EncryptUtils.getSecondKeyFromStr — null key
+error.encrypt.key_null = 加密密钥不能为 null
+
# IEncryptor.getEncryptor — class not found (class name arg)
error.encrypt.encryptor_class_not_found = 获取 encryptor class 失败，class not found: %1$s
diff --git a/java/tsfile/src/main/java/org/apache/tsfile/encrypt/EncryptUtils.java b/java/tsfile/src/main/java/org/apache/tsfile/encrypt/EncryptUtils.java
index 42ccec8..4018406 100644
--- a/java/tsfile/src/main/java/org/apache/tsfile/encrypt/EncryptUtils.java
+++ b/java/tsfile/src/main/java/org/apache/tsfile/encrypt/EncryptUtils.java
@@ -44,6 +44,10 @@
private static final String encryptClassPrefix = "org.apache.tsfile.encrypt.";
+ private static final int MAX_SECOND_KEY_LENGTH = 1024;
+
+ private static final int MAX_SECOND_KEY_STRING_LENGTH = MAX_SECOND_KEY_LENGTH * 5;
+
private static volatile String normalKeyStr;
private static volatile EncryptParameter encryptParam;
@@ -69,16 +73,36 @@
}
public static String getEncryptClass(String encryptType) {
- String classNameRegex = "^(\\p{Alpha}\\w*)(\\.\\p{Alpha}\\w+)+$";
- if (IEncrypt.encryptTypeToClassMap.containsKey(encryptType)) {
- return IEncrypt.encryptTypeToClassMap.get(encryptType);
- } else if (encryptType.matches(classNameRegex)) {
- IEncrypt.encryptTypeToClassMap.put(encryptType, encryptType);
- return encryptType;
- } else {
- IEncrypt.encryptTypeToClassMap.put(encryptType, encryptClassPrefix + encryptType);
- return encryptClassPrefix + encryptType;
+ if (encryptType == null || encryptType.isEmpty()) {
+ throw new EncryptException(
+ Messages.format("error.encrypt.type_not_supported", String.valueOf(encryptType)));
}
+ String mappedClassName = IEncrypt.encryptTypeToClassMap.get(encryptType);
+ if (mappedClassName != null) {
+ return validateEncryptClassName(mappedClassName, encryptType);
+ }
+ String className =
+ encryptType.startsWith(encryptClassPrefix) ? encryptType : encryptClassPrefix + encryptType;
+ String previousClassName = IEncrypt.encryptTypeToClassMap.putIfAbsent(encryptType, className);
+ return validateEncryptClassName(
+ previousClassName == null ? className : previousClassName, encryptType);
+ }
+
+ private static String validateEncryptClassName(String className, String encryptType) {
+ if (!className.startsWith(encryptClassPrefix)) {
+ throw new EncryptException(Messages.format("error.encrypt.type_not_supported", encryptType));
+ }
+ return className;
+ }
+
+ static Class<? extends IEncrypt> loadEncryptClass(String encryptType)
+ throws ClassNotFoundException {
+ Class<?> encryptClass =
+ Class.forName(getEncryptClass(encryptType), false, EncryptUtils.class.getClassLoader());
+ if (!IEncrypt.class.isAssignableFrom(encryptClass)) {
+ throw new EncryptException(Messages.format("error.encrypt.type_not_supported", encryptType));
+ }
+ return encryptClass.asSubclass(IEncrypt.class);
}
public static byte[] getEncryptKeyFromToken(String token, byte[] salt) {
@@ -291,11 +315,11 @@
if (IEncrypt.encryptMap.containsKey(className)) {
return ((IEncrypt) IEncrypt.encryptMap.get(className).newInstance(dataEncryptKey));
}
- Class<?> encryptTypeClass = Class.forName(className);
- java.lang.reflect.Constructor<?> constructor =
+ Class<? extends IEncrypt> encryptTypeClass = loadEncryptClass(encryptType);
+ java.lang.reflect.Constructor<? extends IEncrypt> constructor =
encryptTypeClass.getDeclaredConstructor(byte[].class);
IEncrypt.encryptMap.put(className, constructor);
- return ((IEncrypt) constructor.newInstance(dataEncryptKey));
+ return constructor.newInstance(dataEncryptKey);
} catch (ClassNotFoundException e) {
throw new EncryptException(
Messages.format("error.encrypt.encrypt_class_not_found", encryptType), e);
@@ -331,11 +355,26 @@
}
public static byte[] getSecondKeyFromStr(String str) {
- String[] strArray = str.split(",");
+ if (str == null) {
+ throw new EncryptException(Messages.get("error.encrypt.key_null"));
+ }
+ validateSecondKeyStringLength(str.length());
+ String[] strArray = str.split(",", MAX_SECOND_KEY_LENGTH + 1);
+ if (strArray.length > MAX_SECOND_KEY_LENGTH) {
+ throw new EncryptException(
+ Messages.format("error.encrypt.key_too_long", MAX_SECOND_KEY_LENGTH));
+ }
byte[] key = new byte[strArray.length];
for (int i = 0; i < strArray.length; i++) {
key[i] = Byte.parseByte(strArray[i]);
}
return key;
}
+
+ public static void validateSecondKeyStringLength(int length) {
+ if (length < 0 || length > MAX_SECOND_KEY_STRING_LENGTH) {
+ throw new EncryptException(
+ Messages.format("error.encrypt.key_too_long", MAX_SECOND_KEY_LENGTH));
+ }
+ }
}
diff --git a/java/tsfile/src/main/java/org/apache/tsfile/encrypt/IDecryptor.java b/java/tsfile/src/main/java/org/apache/tsfile/encrypt/IDecryptor.java
index b906e5b..de7735d 100644
--- a/java/tsfile/src/main/java/org/apache/tsfile/encrypt/IDecryptor.java
+++ b/java/tsfile/src/main/java/org/apache/tsfile/encrypt/IDecryptor.java
@@ -39,11 +39,11 @@
if (IEncrypt.encryptMap.containsKey(className)) {
return ((IEncrypt) IEncrypt.encryptMap.get(className).newInstance(key)).getDecryptor();
}
- Class<?> encryptClass = Class.forName(className);
- java.lang.reflect.Constructor<?> constructor =
+ Class<? extends IEncrypt> encryptClass = EncryptUtils.loadEncryptClass(type);
+ java.lang.reflect.Constructor<? extends IEncrypt> constructor =
encryptClass.getDeclaredConstructor(byte[].class);
IEncrypt.encryptMap.put(className, constructor);
- return ((IEncrypt) constructor.newInstance(key)).getDecryptor();
+ return constructor.newInstance(key).getDecryptor();
} catch (ClassNotFoundException e) {
throw new EncryptException(
Messages.format("error.encrypt.decryptor_class_not_found", type), e);
diff --git a/java/tsfile/src/main/java/org/apache/tsfile/encrypt/IEncryptor.java b/java/tsfile/src/main/java/org/apache/tsfile/encrypt/IEncryptor.java
index 2c0b6cf..7d60ecf 100644
--- a/java/tsfile/src/main/java/org/apache/tsfile/encrypt/IEncryptor.java
+++ b/java/tsfile/src/main/java/org/apache/tsfile/encrypt/IEncryptor.java
@@ -39,11 +39,11 @@
if (IEncrypt.encryptMap.containsKey(className)) {
return ((IEncrypt) IEncrypt.encryptMap.get(className).newInstance(key)).getEncryptor();
}
- Class<?> encryptClass = Class.forName(className);
- java.lang.reflect.Constructor<?> constructor =
+ Class<? extends IEncrypt> encryptClass = EncryptUtils.loadEncryptClass(type);
+ java.lang.reflect.Constructor<? extends IEncrypt> constructor =
encryptClass.getDeclaredConstructor(byte[].class);
IEncrypt.encryptMap.put(className, constructor);
- return ((IEncrypt) constructor.newInstance(key)).getEncryptor();
+ return constructor.newInstance(key).getEncryptor();
} catch (ClassNotFoundException e) {
throw new EncryptException(
Messages.format("error.encrypt.encryptor_class_not_found", type), e);
diff --git a/java/tsfile/src/main/java/org/apache/tsfile/file/metadata/TsFileMetadata.java b/java/tsfile/src/main/java/org/apache/tsfile/file/metadata/TsFileMetadata.java
index 7caae06..95759ae 100644
--- a/java/tsfile/src/main/java/org/apache/tsfile/file/metadata/TsFileMetadata.java
+++ b/java/tsfile/src/main/java/org/apache/tsfile/file/metadata/TsFileMetadata.java
@@ -122,6 +122,9 @@
for (int i = 0; i < propertiesSize; i++) {
String key = ReadWriteIOUtils.readVarIntString(buffer);
int valueSize = ReadWriteForEncodingUtils.readVarInt(buffer);
+ if ("encryptKey".equals(key) && valueSize >= 0) {
+ EncryptUtils.validateSecondKeyStringLength(valueSize);
+ }
byte[] value = null;
if (valueSize >= 0) {
value = new byte[valueSize];
diff --git a/java/tsfile/src/test/java/org/apache/tsfile/encrypt/EncryptTest.java b/java/tsfile/src/test/java/org/apache/tsfile/encrypt/EncryptTest.java
index 51b8f8a..9d60ff4 100644
--- a/java/tsfile/src/test/java/org/apache/tsfile/encrypt/EncryptTest.java
+++ b/java/tsfile/src/test/java/org/apache/tsfile/encrypt/EncryptTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.tsfile.encrypt;
+import org.apache.tsfile.exception.encrypt.EncryptException;
import org.apache.tsfile.file.metadata.enums.EncryptionType;
import org.junit.After;
@@ -26,9 +27,12 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.util.Collections;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
public class EncryptTest {
private final String inputString = "AES, a fast encryptor/decryptor.";
@@ -75,6 +79,73 @@
}
@Test
+ public void GetEncryptorDoesNotLoadExternalClassName() {
+ assertThrows(
+ EncryptException.class,
+ () ->
+ IEncryptor.getEncryptor(
+ "java.io.ByteArrayInputStream", key.getBytes(StandardCharsets.UTF_8)));
+ }
+
+ @Test
+ public void GetEncryptorUsesRegisteredClassMapping() {
+ String type = "CUSTOM";
+ IEncrypt.encryptTypeToClassMap.put(type, UNENCRYPTED.class.getName());
+ try {
+ IEncryptor encryptor = IEncryptor.getEncryptor(type, key.getBytes(StandardCharsets.UTF_8));
+ assertEquals(EncryptionType.UNENCRYPTED, encryptor.getEncryptionType());
+ } finally {
+ IEncrypt.encryptTypeToClassMap.remove(type);
+ }
+ }
+
+ @Test
+ public void GetEncryptorRejectsExternalRegisteredClassMapping() {
+ String type = "CUSTOM_EXTERNAL";
+ IEncrypt.encryptTypeToClassMap.put(type, "java.io.ByteArrayInputStream");
+ try {
+ assertThrows(
+ EncryptException.class,
+ () -> IEncryptor.getEncryptor(type, key.getBytes(StandardCharsets.UTF_8)));
+ } finally {
+ IEncrypt.encryptTypeToClassMap.remove(type);
+ }
+ }
+
+ @Test
+ public void GetEncryptorValidatesEncryptionClassBeforeInstantiation() {
+ NonEncryptClass.constructorCalled = false;
+
+ assertThrows(
+ EncryptException.class,
+ () ->
+ IEncryptor.getEncryptor(
+ NonEncryptClass.class.getName(), key.getBytes(StandardCharsets.UTF_8)));
+
+ assertFalse(NonEncryptClass.constructorCalled);
+ }
+
+ @Test
+ public void GetSecondKeyFromStrLimitsKeyLength() {
+ String oversizedKey = String.join(",", Collections.nCopies(1025, "0"));
+
+ assertThrows(EncryptException.class, () -> EncryptUtils.getSecondKeyFromStr(oversizedKey));
+ }
+
+ @Test
+ public void GetSecondKeyFromStrRejectsNullKey() {
+ assertThrows(EncryptException.class, () -> EncryptUtils.getSecondKeyFromStr(null));
+ }
+
+ public static class NonEncryptClass {
+ private static boolean constructorCalled;
+
+ public NonEncryptClass(byte[] key) {
+ constructorCalled = true;
+ }
+ }
+
+ @Test
public void HexStringTransverse() {
byte[] SboxTable = {
(byte) 0xd6,
diff --git a/java/tsfile/src/test/java/org/apache/tsfile/file/metadata/TsFileMetadataTest.java b/java/tsfile/src/test/java/org/apache/tsfile/file/metadata/TsFileMetadataTest.java
index 6488acd..2f42942 100644
--- a/java/tsfile/src/test/java/org/apache/tsfile/file/metadata/TsFileMetadataTest.java
+++ b/java/tsfile/src/test/java/org/apache/tsfile/file/metadata/TsFileMetadataTest.java
@@ -21,6 +21,7 @@
import org.apache.tsfile.common.conf.TSFileConfig;
import org.apache.tsfile.compatibility.DeserializeConfig;
import org.apache.tsfile.constant.TestConstant;
+import org.apache.tsfile.exception.encrypt.EncryptException;
import org.apache.tsfile.file.metadata.utils.TestHelper;
import org.apache.tsfile.file.metadata.utils.Utils;
@@ -65,6 +66,19 @@
Assert.assertTrue(Utils.isFileMetaDataEqual(tsfMetaData, readMetaData));
}
+ @Test
+ public void testRejectOversizedEncryptKeyDuringDeserialization() {
+ TsFileMetadata tsfMetaData = TestHelper.createSimpleFileMetaData();
+ tsfMetaData.addProperty("encryptLevel", "1".getBytes(TSFileConfig.STRING_CHARSET));
+ tsfMetaData.addProperty(
+ "encryptType",
+ "org.apache.tsfile.encrypt.UNENCRYPTED".getBytes(TSFileConfig.STRING_CHARSET));
+ tsfMetaData.addProperty("encryptKey", new byte[5121]);
+ serialized(tsfMetaData);
+
+ Assert.assertThrows(EncryptException.class, this::deSerialized);
+ }
+
private TsFileMetadata deSerialized() {
FileInputStream fileInputStream = null;
TsFileMetadata metaData = null;
