From 4f664f0a3d338673f4b554230345b89c580bccbb Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Wed, 1 Feb 2023 14:02:53 +0800 Subject: [PATCH] Add serializable check for pojo (#11430) --- .../beanutil/JavaBeanSerializeUtil.java | 16 ++++---- .../common/constants/CommonConstants.java | 6 ++- .../apache/dubbo/common/utils/PojoUtils.java | 9 +++-- .../common/utils/SerializeClassChecker.java | 22 ++++++++--- .../apache/dubbo/common/beanutil/Bean.java | 11 +++--- .../org/apache/dubbo/common/model/Person.java | 3 +- .../org/apache/dubbo/common/model/User.java | 3 +- .../dubbo/common/utils/PojoUtilsTest.java | 39 ++++++++++--------- .../utils/SerializeClassCheckerTest.java | 34 ++++++++++++---- 9 files changed, 91 insertions(+), 52 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/beanutil/JavaBeanSerializeUtil.java b/dubbo-common/src/main/java/org/apache/dubbo/common/beanutil/JavaBeanSerializeUtil.java index c4463a20ac..6b621db2d9 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/beanutil/JavaBeanSerializeUtil.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/beanutil/JavaBeanSerializeUtil.java @@ -16,12 +16,6 @@ */ package org.apache.dubbo.common.beanutil; -import org.apache.dubbo.common.logger.Logger; -import org.apache.dubbo.common.logger.LoggerFactory; -import org.apache.dubbo.common.utils.LogHelper; -import org.apache.dubbo.common.utils.ReflectUtils; -import org.apache.dubbo.common.utils.SerializeClassChecker; - import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.lang.reflect.Field; @@ -32,6 +26,12 @@ import java.util.HashMap; import java.util.IdentityHashMap; import java.util.Map; +import org.apache.dubbo.common.logger.Logger; +import org.apache.dubbo.common.logger.LoggerFactory; +import org.apache.dubbo.common.utils.LogHelper; +import org.apache.dubbo.common.utils.ReflectUtils; +import org.apache.dubbo.common.utils.SerializeClassChecker; + public final class JavaBeanSerializeUtil { private static final Logger logger = LoggerFactory.getLogger(JavaBeanSerializeUtil.class); @@ -466,7 +466,9 @@ public final class JavaBeanSerializeUtil { name = name.substring(1, name.length() - 1); } SerializeClassChecker.getInstance().validateClass(name); - return Class.forName(name, false, loader); + Class aClass = Class.forName(name, false, loader); + SerializeClassChecker.getInstance().validateClass(aClass); + return aClass; } private static boolean isArray(String type) { diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/constants/CommonConstants.java b/dubbo-common/src/main/java/org/apache/dubbo/common/constants/CommonConstants.java index dd4ede8066..9acd76434b 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/constants/CommonConstants.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/constants/CommonConstants.java @@ -17,13 +17,13 @@ package org.apache.dubbo.common.constants; -import org.apache.dubbo.common.URL; - import java.net.NetworkInterface; import java.util.Properties; import java.util.concurrent.ExecutorService; import java.util.regex.Pattern; +import org.apache.dubbo.common.URL; + public interface CommonConstants { String DUBBO = "dubbo"; @@ -397,6 +397,8 @@ public interface CommonConstants { String CLASS_DESERIALIZE_BLOCKED_LIST = "dubbo.security.serialize.blockedClassList"; + String CLASS_DESERIALIZE_CHECK_SERIALIZABLE = "dubbo.application.check-serializable"; + String ENABLE_NATIVE_JAVA_GENERIC_SERIALIZE = "dubbo.security.serialize.generic.native-java-enable"; String SERIALIZE_BLOCKED_LIST_FILE_PATH = "security/serialize.blockedlist"; diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/PojoUtils.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/PojoUtils.java index d23a4b5ea7..3ae928d490 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/PojoUtils.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/PojoUtils.java @@ -16,10 +16,6 @@ */ package org.apache.dubbo.common.utils; -import org.apache.dubbo.common.constants.CommonConstants; -import org.apache.dubbo.common.logger.Logger; -import org.apache.dubbo.common.logger.LoggerFactory; - import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.lang.reflect.Field; @@ -57,6 +53,10 @@ import java.util.concurrent.ConcurrentSkipListMap; import java.util.function.Consumer; import java.util.function.Supplier; +import org.apache.dubbo.common.constants.CommonConstants; +import org.apache.dubbo.common.logger.Logger; +import org.apache.dubbo.common.logger.LoggerFactory; + import static org.apache.dubbo.common.utils.ClassUtils.isAssignableFrom; /** @@ -413,6 +413,7 @@ public class PojoUtils { } catch (ClassNotFoundException e) { // ignore } + SerializeClassChecker.getInstance().validateClass(type); } // special logic for enum diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/SerializeClassChecker.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/SerializeClassChecker.java index 477eeecc4f..4ef826e11e 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/SerializeClassChecker.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/SerializeClassChecker.java @@ -16,18 +16,19 @@ */ package org.apache.dubbo.common.utils; +import java.io.IOException; +import java.io.Serializable; +import java.util.Arrays; +import java.util.Locale; +import java.util.Set; +import java.util.concurrent.atomic.AtomicLong; + import org.apache.dubbo.common.beanutil.JavaBeanSerializeUtil; import org.apache.dubbo.common.config.ConfigurationUtils; import org.apache.dubbo.common.constants.CommonConstants; import org.apache.dubbo.common.logger.Logger; import org.apache.dubbo.common.logger.LoggerFactory; -import java.io.IOException; -import java.util.Arrays; -import java.util.Locale; -import java.util.Set; -import java.util.concurrent.atomic.AtomicLong; - public class SerializeClassChecker { private static final Logger logger = LoggerFactory.getLogger(SerializeClassChecker.class); @@ -42,6 +43,8 @@ public class SerializeClassChecker { private final LFUCache CLASS_ALLOW_LFU_CACHE = new LFUCache<>(); private final LFUCache CLASS_BLOCK_LFU_CACHE = new LFUCache<>(); + private final boolean checkSerializable; + private final AtomicLong counter = new AtomicLong(0); private SerializeClassChecker() { @@ -85,6 +88,7 @@ public class SerializeClassChecker { CLASS_DESERIALIZE_BLOCKED_SET.addAll(Arrays.asList(classStrings)); } + checkSerializable = Boolean.parseBoolean(ConfigurationUtils.getProperty(CommonConstants.CLASS_DESERIALIZE_CHECK_SERIALIZABLE, "true")); } public static SerializeClassChecker getInstance() { @@ -143,6 +147,12 @@ public class SerializeClassChecker { CLASS_ALLOW_LFU_CACHE.put(name, CACHE); } + public void validateClass(Class aClass) { + if (checkSerializable && !Serializable.class.isAssignableFrom(aClass)) { + error(aClass.getName()); + } + } + private void error(String name) { String notice = "Trigger the safety barrier! " + "Catch not allowed serialize class. " + diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/beanutil/Bean.java b/dubbo-common/src/test/java/org/apache/dubbo/common/beanutil/Bean.java index e17a538ae6..382d0df3a5 100644 --- a/dubbo-common/src/test/java/org/apache/dubbo/common/beanutil/Bean.java +++ b/dubbo-common/src/test/java/org/apache/dubbo/common/beanutil/Bean.java @@ -1,12 +1,13 @@ package org.apache.dubbo.common.beanutil; +import java.io.Serializable; +import java.util.Collection; +import java.util.Date; +import java.util.Map; + import org.apache.dubbo.rpc.model.person.FullAddress; import org.apache.dubbo.rpc.model.person.PersonStatus; import org.apache.dubbo.rpc.model.person.Phone; - -import java.util.Collection; -import java.util.Date; -import java.util.Map; /* * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with @@ -23,7 +24,7 @@ import java.util.Map; * See the License for the specific language governing permissions and * limitations under the License. */ -public class Bean { +public class Bean implements Serializable { private Class type; diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/model/Person.java b/dubbo-common/src/test/java/org/apache/dubbo/common/model/Person.java index 5a551a7f36..05e4fd01d6 100644 --- a/dubbo-common/src/test/java/org/apache/dubbo/common/model/Person.java +++ b/dubbo-common/src/test/java/org/apache/dubbo/common/model/Person.java @@ -16,9 +16,10 @@ */ package org.apache.dubbo.common.model; +import java.io.Serializable; import java.util.Arrays; -public class Person { +public class Person implements Serializable { byte oneByte = 123; private String name = "name1"; private int age = 11; diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/model/User.java b/dubbo-common/src/test/java/org/apache/dubbo/common/model/User.java index 33d20bbc6c..484218cf50 100644 --- a/dubbo-common/src/test/java/org/apache/dubbo/common/model/User.java +++ b/dubbo-common/src/test/java/org/apache/dubbo/common/model/User.java @@ -17,6 +17,7 @@ package org.apache.dubbo.common.model; +import java.io.Serializable; import java.util.Objects; /** @@ -24,7 +25,7 @@ import java.util.Objects; * @date: 2019-05-13 18:41 * @description: this class has no nullary constructor and some field is primitive */ -public class User { +public class User implements Serializable { private int age; private String name; diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/PojoUtilsTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/PojoUtilsTest.java index 8aaf2b96a0..9c1dc14844 100644 --- a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/PojoUtilsTest.java +++ b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/PojoUtilsTest.java @@ -16,19 +16,7 @@ */ package org.apache.dubbo.common.utils; -import org.apache.dubbo.common.model.Person; -import org.apache.dubbo.common.model.SerializablePerson; -import org.apache.dubbo.common.model.User; -import org.apache.dubbo.common.model.person.BigPerson; -import org.apache.dubbo.common.model.person.FullAddress; -import org.apache.dubbo.common.model.person.PersonInfo; -import org.apache.dubbo.common.model.person.PersonStatus; -import org.apache.dubbo.common.model.person.Phone; - -import com.alibaba.fastjson.JSONObject; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - +import java.io.Serializable; import java.lang.reflect.Method; import java.lang.reflect.Type; import java.text.SimpleDateFormat; @@ -46,6 +34,19 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import org.apache.dubbo.common.model.Person; +import org.apache.dubbo.common.model.SerializablePerson; +import org.apache.dubbo.common.model.User; +import org.apache.dubbo.common.model.person.BigPerson; +import org.apache.dubbo.common.model.person.FullAddress; +import org.apache.dubbo.common.model.person.PersonInfo; +import org.apache.dubbo.common.model.person.PersonStatus; +import org.apache.dubbo.common.model.person.Phone; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import com.alibaba.fastjson.JSONObject; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ -815,7 +816,7 @@ public class PojoUtilsTest { SUNDAY, MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, SATURDAY } - public static class BasicTestData { + public static class BasicTestData implements Serializable { public boolean a; public char b; @@ -887,7 +888,7 @@ public class PojoUtilsTest { } - public static class Parent { + public static class Parent implements Serializable { public String gender; public String email; String name; @@ -950,7 +951,7 @@ public class PojoUtilsTest { } } - public static class Child { + public static class Child implements Serializable { public String gender; public int age; String toy; @@ -990,7 +991,7 @@ public class PojoUtilsTest { } } - public static class TestData { + public static class TestData implements Serializable { private Map children = new HashMap(); private List list = new ArrayList(); @@ -1019,7 +1020,7 @@ public class PojoUtilsTest { } } - public static class InnerPojo { + public static class InnerPojo implements Serializable { private List list; public List getList() { @@ -1031,7 +1032,7 @@ public class PojoUtilsTest { } } - public static class ListResult { + public static class ListResult implements Serializable { List result; public List getResult() { diff --git a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/SerializeClassCheckerTest.java b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/SerializeClassCheckerTest.java index b58f399808..b02ecbd871 100644 --- a/dubbo-common/src/test/java/org/apache/dubbo/common/utils/SerializeClassCheckerTest.java +++ b/dubbo-common/src/test/java/org/apache/dubbo/common/utils/SerializeClassCheckerTest.java @@ -16,18 +16,18 @@ */ package org.apache.dubbo.common.utils; -import org.apache.dubbo.common.constants.CommonConstants; - -import javassist.compiler.Javac; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - import java.net.Socket; import java.util.LinkedList; import java.util.List; import java.util.Locale; +import org.apache.dubbo.common.constants.CommonConstants; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javassist.compiler.Javac; + public class SerializeClassCheckerTest { @BeforeEach @@ -56,6 +56,26 @@ public class SerializeClassCheckerTest { }); } + @Test + public void testSerializable1() { + SerializeClassChecker serializeClassChecker = SerializeClassChecker.getInstance(); + Assertions.assertThrows(IllegalArgumentException.class, ()-> serializeClassChecker.validateClass(List.class)); + } + + @Test + public void testSerializable2() { + System.setProperty(CommonConstants.CLASS_DESERIALIZE_CHECK_SERIALIZABLE, "false"); + SerializeClassChecker serializeClassChecker = SerializeClassChecker.getInstance(); + + try { + serializeClassChecker.validateClass(List.class); + } catch (Throwable t) { + Assertions.fail(t); + } + + System.clearProperty(CommonConstants.CLASS_DESERIALIZE_CHECK_SERIALIZABLE); + } + @Test public void testAddAllow() { System.setProperty(CommonConstants.CLASS_DESERIALIZE_ALLOWED_LIST, Socket.class.getName() + "," + Javac.class.getName());