Merge pull request #9050 from dashevchenko/valueNoxssValidation

Added optional noxss validation for attribute/telemetry value

application/src/main/java/org/thingsboard/server/service/telemetry/DefaultTelemetrySubscriptionService.java

@@ -21,6 +21,7 @@ import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.SettableFuture;
 import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.stereotype.Service;
 import org.thingsboard.common.util.ThingsBoardThreadFactory;
@@ -78,6 +79,9 @@ public class DefaultTelemetrySubscriptionService extends AbstractSubscriptionSer
 
     private ExecutorService tsCallBackExecutor;
 
+    @Value("${sql.ts.value_no_xss_validation:false}")
+    private boolean valueNoXssValidation;
+
     public DefaultTelemetrySubscriptionService(AttributesService attrService,
                                                TimeseriesService tsService,
                                                @Lazy TbEntityViewService tbEntityViewService,
@@ -135,7 +139,7 @@ public class DefaultTelemetrySubscriptionService extends AbstractSubscriptionSer
         checkInternalEntity(entityId);
         boolean sysTenant = TenantId.SYS_TENANT_ID.equals(tenantId) || tenantId == null;
         if (sysTenant || apiUsageStateService.getApiUsageState(tenantId).isDbStorageEnabled()) {
-            KvUtils.validate(ts);
+            KvUtils.validate(ts, valueNoXssValidation);
             if (saveLatest) {
                 saveAndNotifyInternal(tenantId, entityId, ts, ttl, getCallback(tenantId, customerId, sysTenant, callback));
             } else {

application/src/main/resources/thingsboard.yml

@@ -269,11 +269,13 @@ sql:
     batch_max_delay: "${SQL_ATTRIBUTES_BATCH_MAX_DELAY_MS:100}"
     stats_print_interval_ms: "${SQL_ATTRIBUTES_BATCH_STATS_PRINT_MS:10000}"
     batch_threads: "${SQL_ATTRIBUTES_BATCH_THREADS:3}" # batch thread count have to be a prime number like 3 or 5 to gain perfect hash distribution
+    value_no_xss_validation: "${SQL_ATTRIBUTES_VALUE_NO_XSS_VALIDATION:false}"
   ts:
     batch_size: "${SQL_TS_BATCH_SIZE:10000}"
     batch_max_delay: "${SQL_TS_BATCH_MAX_DELAY_MS:100}"
     stats_print_interval_ms: "${SQL_TS_BATCH_STATS_PRINT_MS:10000}"
     batch_threads: "${SQL_TS_BATCH_THREADS:3}" # batch thread count have to be a prime number like 3 or 5 to gain perfect hash distribution
+    value_no_xss_validation: "${SQL_TS_VALUE_NO_XSS_VALIDATION:false}"
   ts_latest:
     batch_size: "${SQL_TS_LATEST_BATCH_SIZE:10000}"
     batch_max_delay: "${SQL_TS_LATEST_BATCH_MAX_DELAY_MS:100}"

application/src/test/java/org/thingsboard/server/controller/TelemetryControllerTest.java

@@ -16,6 +16,7 @@
 package org.thingsboard.server.controller;
 
 import org.junit.Test;
+import org.springframework.test.context.TestPropertySource;
 import org.thingsboard.server.common.data.Device;
 import org.thingsboard.server.common.data.SaveDeviceWithCredentialsRequest;
 import org.thingsboard.server.common.data.security.DeviceCredentials;
@@ -25,6 +26,10 @@ import org.thingsboard.server.dao.service.DaoSqlTest;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 @DaoSqlTest
+@TestPropertySource(properties = {
+        "sql.attributes.value_no_xss_validation=true",
+        "sql.ts.value_no_xss_validation=true"
+})
 public class TelemetryControllerTest extends AbstractControllerTest {
 
     @Test
@@ -39,6 +44,18 @@ public class TelemetryControllerTest extends AbstractControllerTest {
         doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", invalidRequestBody, String.class, status().isBadRequest());
     }
 
+    @Test
+    public void testValueConstraintValidator() throws Exception {
+        loginTenantAdmin();
+        Device device = createDevice();
+        String correctRequestBody = "{\"data\": \"value\"}";
+        doPostAsync("/api/plugins/telemetry/" + device.getId() + "/SHARED_SCOPE", correctRequestBody, String.class, status().isOk());
+        doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", correctRequestBody, String.class, status().isOk());
+        String invalidRequestBody = "{\"data\": \"<object data=\\\"data:text/html,<script>alert(document)</script>\\\"></object>\"}";
+        doPostAsync("/api/plugins/telemetry/" + device.getId() + "/SHARED_SCOPE", invalidRequestBody, String.class, status().isBadRequest());
+        doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", invalidRequestBody, String.class, status().isBadRequest());
+    }
+
     private Device createDevice() throws Exception {
         String testToken = "TEST_TOKEN";
 

dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java

@@ -30,12 +30,12 @@ public class AttributeUtils {
         Validator.validateString(scope, "Incorrect scope " + scope);
     }
 
-    public static void validate(List<AttributeKvEntry> kvEntries) {
+    public static void validate(List<AttributeKvEntry> kvEntries,  boolean valueNoXssValidation) {
-        kvEntries.forEach(AttributeUtils::validate);
+        kvEntries.forEach(tsKvEntry -> validate(tsKvEntry, valueNoXssValidation));
     }
 
-    public static void validate(AttributeKvEntry kvEntry) {
+    public static void validate(AttributeKvEntry kvEntry, boolean valueNoXssValidation) {
-        KvUtils.validate(kvEntry);
+        KvUtils.validate(kvEntry, valueNoXssValidation);
         if (kvEntry.getDataType() == null) {
             throw new IncorrectParameterException("Incorrect kvEntry. Data type can't be null");
         } else {

dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java

@@ -18,6 +18,7 @@ package org.thingsboard.server.dao.attributes;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Primary;
 import org.springframework.stereotype.Service;
@@ -45,6 +46,9 @@ import static org.thingsboard.server.dao.attributes.AttributeUtils.validate;
 public class BaseAttributesService implements AttributesService {
     private final AttributesDao attributesDao;
 
+    @Value("${sql.attributes.value_no_xss_validation:false}")
+    private boolean valueNoXssValidation;
+
     public BaseAttributesService(AttributesDao attributesDao) {
         this.attributesDao = attributesDao;
     }
@@ -82,14 +86,14 @@ public class BaseAttributesService implements AttributesService {
     @Override
     public ListenableFuture<String> save(TenantId tenantId, EntityId entityId, String scope, AttributeKvEntry attribute) {
         validate(entityId, scope);
-        AttributeUtils.validate(attribute);
+        AttributeUtils.validate(attribute, valueNoXssValidation);
         return attributesDao.save(tenantId, entityId, scope, attribute);
     }
 
     @Override
     public ListenableFuture<List<String>> save(TenantId tenantId, EntityId entityId, String scope, List<AttributeKvEntry> attributes) {
         validate(entityId, scope);
-        AttributeUtils.validate(attributes);
+        AttributeUtils.validate(attributes, valueNoXssValidation);
         List<ListenableFuture<String>> saveFutures = attributes.stream().map(attribute -> attributesDao.save(tenantId, entityId, scope, attribute)).collect(Collectors.toList());
         return Futures.allAsList(saveFutures);
     }

dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java

@@ -69,6 +69,8 @@ public class CachedAttributesService implements AttributesService {
 
     @Value("${cache.type:caffeine}")
     private String cacheType;
+    @Value("${sql.attributes.value_no_xss_validation:false}")
+    private boolean valueNoXssValidation;
 
     public CachedAttributesService(AttributesDao attributesDao,
                                    StatsFactory statsFactory,
@@ -212,7 +214,7 @@ public class CachedAttributesService implements AttributesService {
     @Override
     public ListenableFuture<String> save(TenantId tenantId, EntityId entityId, String scope, AttributeKvEntry attribute) {
         validate(entityId, scope);
-        AttributeUtils.validate(attribute);
+        AttributeUtils.validate(attribute, valueNoXssValidation);
         ListenableFuture<String> future = attributesDao.save(tenantId, entityId, scope, attribute);
         return Futures.transform(future, key -> evict(entityId, scope, attribute, key), cacheExecutor);
     }
@@ -220,7 +222,7 @@ public class CachedAttributesService implements AttributesService {
     @Override
     public ListenableFuture<List<String>> save(TenantId tenantId, EntityId entityId, String scope, List<AttributeKvEntry> attributes) {
         validate(entityId, scope);
-        AttributeUtils.validate(attributes);
+        AttributeUtils.validate(attributes, valueNoXssValidation);
 
         List<ListenableFuture<String>> futures = new ArrayList<>(attributes.size());
         for (var attribute : attributes) {

dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java

@@ -15,6 +15,7 @@
  */
 package org.thingsboard.server.dao.util;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
 import org.thingsboard.server.common.data.kv.KvEntry;
@@ -36,11 +37,11 @@ public class KvUtils {
                 .maximumSize(100000).build();
     }
 
-    public static void validate(List<? extends KvEntry> tsKvEntries) {
+    public static void validate(List<? extends KvEntry> tsKvEntries, boolean valueNoXssValidation) {
-        tsKvEntries.forEach(KvUtils::validate);
+        tsKvEntries.forEach(tsKvEntry -> validate(tsKvEntry, valueNoXssValidation));
     }
 
-    public static void validate(KvEntry tsKvEntry) {
+    public static void validate(KvEntry tsKvEntry, boolean valueNoXssValidation) {
         if (tsKvEntry == null) {
             throw new IncorrectParameterException("Key value entry can't be null");
         }
@@ -55,14 +56,20 @@ public class KvUtils {
             throw new DataValidationException("Validation error: key length must be equal or less than 255");
         }
 
-        if (validatedKeys.getIfPresent(key) != null) {
+        if (validatedKeys.getIfPresent(key) == null) {
-            return;
+            if (!NoXssValidator.isValid(key)) {
+                throw new DataValidationException("Validation error: key is malformed");
+            }
+            validatedKeys.put(key, Boolean.TRUE);
         }
 
-        if (!NoXssValidator.isValid(key)) {
+        if (valueNoXssValidation) {
-            throw new DataValidationException("Validation error: key is malformed");
+            Object value = tsKvEntry.getValue();
+            if (value instanceof CharSequence || value instanceof JsonNode) {
+                if (!NoXssValidator.isValid(value.toString())) {
+                    throw new DataValidationException("Validation error: value is malformed");
+                }
+            }
         }
-
-        validatedKeys.put(key, Boolean.TRUE);
     }
 }