diff --git a/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java b/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java index 7035fcfd9b..96ffa613c7 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java +++ b/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 { + private static final Pattern CONTROL_CHARS = Pattern.compile("[\\x00-\\x1F\\x7F]"); + @Autowired private DeviceCredentialsDao deviceCredentialsDao; @@ -69,9 +76,24 @@ public class DeviceCredentialsDataValidator extends DataValidator 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; + } + +} diff --git a/dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java b/dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java index d4c93a1d5f..b69fefeea0 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java +++ b/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:"); + } + } + }