Merge 157c773fcd into be3207ab65

dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java

@@ -18,18 +18,25 @@ package org.thingsboard.server.dao.service.validator;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.stereotype.Component;
+import org.thingsboard.common.util.JacksonUtil;
 import org.thingsboard.server.common.data.Device;
 import org.thingsboard.server.common.data.StringUtils;
+import org.thingsboard.server.common.data.device.credentials.BasicMqttCredentials;
 import org.thingsboard.server.common.data.id.TenantId;
 import org.thingsboard.server.common.data.security.DeviceCredentials;
+import org.thingsboard.server.common.data.security.DeviceCredentialsType;
 import org.thingsboard.server.dao.device.DeviceCredentialsDao;
 import org.thingsboard.server.dao.device.DeviceService;
 import org.thingsboard.server.dao.exception.DeviceCredentialsValidationException;
 import org.thingsboard.server.dao.service.DataValidator;
 
+import java.util.regex.Pattern;
+
 @Component
 public class DeviceCredentialsDataValidator extends DataValidator<DeviceCredentials> {
 
+    private static final Pattern CONTROL_CHARS = Pattern.compile("[\\x00-\\x1F\\x7F]");
+
     @Autowired
     private DeviceCredentialsDao deviceCredentialsDao;
 
@@ -69,9 +76,24 @@ public class DeviceCredentialsDataValidator extends DataValidator<DeviceCredenti
         if (StringUtils.isEmpty(deviceCredentials.getCredentialsId())) {
             throw new DeviceCredentialsValidationException("Device credentials id should be specified!");
         }
+        rejectControlChars(deviceCredentials.getCredentialsId(), "credentialsId");
+        if (deviceCredentials.getCredentialsType() == DeviceCredentialsType.MQTT_BASIC) {
+            BasicMqttCredentials mqtt = JacksonUtil.fromString(deviceCredentials.getCredentialsValue(), BasicMqttCredentials.class);
+            if (mqtt != null) {
+                rejectControlChars(mqtt.getClientId(), "clientId");
+                rejectControlChars(mqtt.getUserName(), "userName");
+                rejectControlChars(mqtt.getPassword(), "password");
+            }
+        }
         Device device = deviceService.findDeviceById(tenantId, deviceCredentials.getDeviceId());
         if (device == null) {
             throw new DeviceCredentialsValidationException("Can't assign device credentials to non-existent device!");
         }
     }
+
+    private static void rejectControlChars(String value, String fieldName) {
+        if (value != null && CONTROL_CHARS.matcher(value).find()) {
+            throw new DeviceCredentialsValidationException(fieldName + " must not contain control characters!");
+        }
+    }
 }

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

@@ -50,6 +50,11 @@ public class DeviceConnectivityUtil {
     public static final String MQTT_IMAGE = "thingsboard/mosquitto-clients ";
     public static final String COAP_IMAGE = "thingsboard/coap-clients ";
     private final static Pattern VALID_URL_PATTERN = Pattern.compile("^(https?)://[-a-zA-Z0-9+&@#/%?=~_|!:,.;]*[-a-zA-Z0-9+&@#/%=~_|]");
+    private final static Pattern CONTROL_CHARS = Pattern.compile("[\\x00-\\x1F\\x7F]");
+
+    private static String sanitize(String value) {
+        return value == null ? null : CONTROL_CHARS.matcher(value).replaceAll("_");
+    }
 
     public static String getHttpPublishCommand(String protocol, String host, String port, DeviceCredentials deviceCredentials) {
         return String.format("curl -v -X POST %s://%s%s/api/v1/%s/telemetry --header Content-Type:application/json --data " + JSON_EXAMPLE_PAYLOAD,
@@ -122,7 +127,7 @@ public class DeviceConnectivityUtil {
         switch (deviceCredentials.getCredentialsType()) {
             case ACCESS_TOKEN:
                 dockerComposeBuilder.append("      - TB_GW_SECURITY_TYPE=accessToken\n");
-                dockerComposeBuilder.append("      - TB_GW_ACCESS_TOKEN=").append(deviceCredentials.getCredentialsId()).append("\n");
+                dockerComposeBuilder.append("      - TB_GW_ACCESS_TOKEN=").append(sanitize(deviceCredentials.getCredentialsId())).append("\n");
                 break;
             case MQTT_BASIC:
                 dockerComposeBuilder.append("      - TB_GW_SECURITY_TYPE=usernamePassword\n");
@@ -130,13 +135,13 @@ public class DeviceConnectivityUtil {
                         BasicMqttCredentials.class);
                 if (credentials != null) {
                     if (StringUtils.isNotEmpty(credentials.getClientId())) {
-                        dockerComposeBuilder.append("      - TB_GW_CLIENT_ID=").append(credentials.getClientId()).append("\n");
+                        dockerComposeBuilder.append("      - TB_GW_CLIENT_ID=").append(sanitize(credentials.getClientId())).append("\n");
                     }
                     if (StringUtils.isNotEmpty(credentials.getUserName())) {
-                        dockerComposeBuilder.append("      - TB_GW_USERNAME=").append(credentials.getUserName()).append("\n");
+                        dockerComposeBuilder.append("      - TB_GW_USERNAME=").append(sanitize(credentials.getUserName())).append("\n");
                     }
                     if (StringUtils.isNotEmpty(credentials.getPassword())) {
-                        dockerComposeBuilder.append("      - TB_GW_PASSWORD=").append(credentials.getPassword()).append("\n");
+                        dockerComposeBuilder.append("      - TB_GW_PASSWORD=").append(sanitize(credentials.getPassword())).append("\n");
                     }
                 }
                 break;

dao/src/test/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidatorTest.java

@@ -0,0 +1,129 @@
+/**
+ * Copyright © 2016-2026 The Thingsboard Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.thingsboard.server.dao.service.validator;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.thingsboard.common.util.JacksonUtil;
+import org.thingsboard.server.common.data.Device;
+import org.thingsboard.server.common.data.device.credentials.BasicMqttCredentials;
+import org.thingsboard.server.common.data.id.DeviceId;
+import org.thingsboard.server.common.data.id.TenantId;
+import org.thingsboard.server.common.data.security.DeviceCredentials;
+import org.thingsboard.server.common.data.security.DeviceCredentialsType;
+import org.thingsboard.server.dao.device.DeviceCredentialsDao;
+import org.thingsboard.server.dao.device.DeviceService;
+import org.thingsboard.server.dao.exception.DeviceCredentialsValidationException;
+
+import java.util.UUID;
+
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.BDDMockito.willReturn;
+
+@ExtendWith(MockitoExtension.class)
+class DeviceCredentialsDataValidatorTest {
+
+    @Mock
+    DeviceCredentialsDao deviceCredentialsDao;
+    @Mock
+    DeviceService deviceService;
+    @InjectMocks
+    DeviceCredentialsDataValidator validator;
+
+    final TenantId tenantId = TenantId.fromUUID(UUID.fromString("9ef79cdf-37a8-4119-b682-2e7ed4e018da"));
+    final DeviceId deviceId = new DeviceId(UUID.fromString("11111111-1111-1111-1111-111111111111"));
+
+    @Test
+    void rejectsNewlineInAccessToken() {
+        DeviceCredentials creds = accessToken("safe_token\nentrypoint: [\"/bin/sh\"]");
+
+        assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds))
+                .isInstanceOf(DeviceCredentialsValidationException.class)
+                .hasMessageContaining("credentialsId")
+                .hasMessageContaining("control characters");
+    }
+
+    @Test
+    void rejectsCarriageReturnInAccessToken() {
+        DeviceCredentials creds = accessToken("token\rprivileged: true");
+
+        assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds))
+                .isInstanceOf(DeviceCredentialsValidationException.class)
+                .hasMessageContaining("control characters");
+    }
+
+    @Test
+    void rejectsNewlineInMqttClientId() {
+        DeviceCredentials creds = mqttBasic("cid\nentrypoint: x", "user", "pwd");
+
+        assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds))
+                .isInstanceOf(DeviceCredentialsValidationException.class)
+                .hasMessageContaining("clientId");
+    }
+
+    @Test
+    void rejectsNewlineInMqttUserName() {
+        DeviceCredentials creds = mqttBasic("cid", "user\nprivileged: true", "pwd");
+
+        assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds))
+                .isInstanceOf(DeviceCredentialsValidationException.class)
+                .hasMessageContaining("userName");
+    }
+
+    @Test
+    void rejectsNewlineInMqttPassword() {
+        DeviceCredentials creds = mqttBasic("cid", "user", "pwd\nentrypoint: x");
+
+        assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds))
+                .isInstanceOf(DeviceCredentialsValidationException.class)
+                .hasMessageContaining("password");
+    }
+
+    @Test
+    void acceptsValidCredentials() {
+        willReturn(new Device()).given(deviceService).findDeviceById(tenantId, deviceId);
+        DeviceCredentials creds = accessToken("safe_token_123");
+
+        assertThatCode(() -> validator.validateDataImpl(tenantId, creds))
+                .doesNotThrowAnyException();
+    }
+
+    private DeviceCredentials accessToken(String token) {
+        DeviceCredentials c = new DeviceCredentials();
+        c.setDeviceId(deviceId);
+        c.setCredentialsType(DeviceCredentialsType.ACCESS_TOKEN);
+        c.setCredentialsId(token);
+        return c;
+    }
+
+    private DeviceCredentials mqttBasic(String clientId, String userName, String password) {
+        BasicMqttCredentials inner = new BasicMqttCredentials();
+        inner.setClientId(clientId);
+        inner.setUserName(userName);
+        inner.setPassword(password);
+        DeviceCredentials c = new DeviceCredentials();
+        c.setDeviceId(deviceId);
+        c.setCredentialsType(DeviceCredentialsType.MQTT_BASIC);
+        c.setCredentialsId("mqtt-credentials-id");
+        c.setCredentialsValue(JacksonUtil.toString(inner));
+        return c;
+    }
+
+}

dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java

@@ -16,6 +16,13 @@
 package org.thingsboard.server.dao.util;
 
 import org.junit.jupiter.api.Test;
+import org.thingsboard.common.util.JacksonUtil;
+import org.thingsboard.server.common.data.device.credentials.BasicMqttCredentials;
+import org.thingsboard.server.common.data.security.DeviceCredentials;
+import org.thingsboard.server.common.data.security.DeviceCredentialsType;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -29,4 +36,87 @@ class DeviceConnectivityUtilTest {
         assertThat(DeviceConnectivityUtil.CA_ROOT_CERT_PEM).doesNotContainAnyWhitespaces();
     }
 
+    @Test
+    void validAccessTokenIsRenderedAsIs() throws Exception {
+        String yaml = renderCompose(accessToken("safe_token_123"));
+
+        assertThat(yaml).contains("- TB_GW_ACCESS_TOKEN=safe_token_123\n");
+        assertNoInjectedSiblingKeys(yaml);
+    }
+
+    @Test
+    void newlineInAccessTokenIsSanitized() throws Exception {
+        String malicious = "safe_token\n    entrypoint: [\"/bin/bash\",\"-c\",\"id\"]";
+
+        String yaml = renderCompose(accessToken(malicious));
+
+        assertNoInjectedSiblingKeys(yaml);
+    }
+
+    @Test
+    void carriageReturnInAccessTokenIsSanitized() throws Exception {
+        String yaml = renderCompose(accessToken("token\rprivileged: true"));
+
+        assertNoInjectedSiblingKeys(yaml);
+    }
+
+    @Test
+    void newlineInMqttClientIdIsSanitized() throws Exception {
+        String yaml = renderCompose(mqttBasic("cid\n    entrypoint: [\"/bin/sh\"]", "user", "pwd"));
+
+        assertNoInjectedSiblingKeys(yaml);
+    }
+
+    @Test
+    void newlineInMqttUserNameIsSanitized() throws Exception {
+        String yaml = renderCompose(mqttBasic("cid", "user\n    privileged: true", "pwd"));
+
+        assertNoInjectedSiblingKeys(yaml);
+    }
+
+    @Test
+    void newlineInMqttPasswordIsSanitized() throws Exception {
+        String yaml = renderCompose(mqttBasic("cid", "user", "pwd\n    entrypoint: [\"/bin/sh\"]"));
+
+        assertNoInjectedSiblingKeys(yaml);
+    }
+
+    private static String renderCompose(DeviceCredentials credentials) throws Exception {
+        var resource = DeviceConnectivityUtil.getGatewayDockerComposeFile(
+                "host.docker.internal", "3.8-stable", credentials);
+        try (var in = resource.getInputStream()) {
+            return new String(in.readAllBytes(), StandardCharsets.UTF_8);
+        }
+    }
+
+    private static DeviceCredentials accessToken(String token) {
+        DeviceCredentials c = new DeviceCredentials();
+        c.setCredentialsType(DeviceCredentialsType.ACCESS_TOKEN);
+        c.setCredentialsId(token);
+        return c;
+    }
+
+    private static DeviceCredentials mqttBasic(String clientId, String userName, String password) {
+        BasicMqttCredentials inner = new BasicMqttCredentials();
+        inner.setClientId(clientId);
+        inner.setUserName(userName);
+        inner.setPassword(password);
+        DeviceCredentials c = new DeviceCredentials();
+        c.setCredentialsType(DeviceCredentialsType.MQTT_BASIC);
+        c.setCredentialsId("mqtt-credentials-id");
+        c.setCredentialsValue(JacksonUtil.toString(inner));
+        return c;
+    }
+
+    private static void assertNoInjectedSiblingKeys(String yaml) throws IOException {
+        for (String line : yaml.split("\n")) {
+            String trimmed = line.replaceFirst("^\\s+", "");
+            assertThat(trimmed)
+                    .as("unexpected sibling key — possible YAML injection: %s", line)
+                    .doesNotStartWith("entrypoint:")
+                    .doesNotStartWith("privileged:")
+                    .doesNotStartWith("command:");
+        }
+    }
+
 }