diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/util/LogSanitizer.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/util/LogSanitizer.java new file mode 100644 index 0000000000..b5ee7a9641 --- /dev/null +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/util/LogSanitizer.java @@ -0,0 +1,20 @@ +package org.cloudfoundry.multiapps.controller.core.util; + +public final class LogSanitizer { + + private LogSanitizer() { + } + + public static String sanitize(String value) { + if (value == null) { + return null; + } + return value.replace("\r", "\\r") + .replace("\n", "\\n") + .replace("\t", "\\t"); + } + + public static String sanitize(Object value) { + return value == null ? null : sanitize(value.toString()); + } +} diff --git a/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/util/LogSanitizerTest.java b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/util/LogSanitizerTest.java new file mode 100644 index 0000000000..d305749244 --- /dev/null +++ b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/util/LogSanitizerTest.java @@ -0,0 +1,65 @@ +package org.cloudfoundry.multiapps.controller.core.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +class LogSanitizerTest { + + static Stream testSanitize() { + return Stream.of( + // null and empty pass through unchanged + Arguments.of(null, null), + Arguments.of("", ""), + // strings without control characters are returned as-is + Arguments.of("plain text", "plain text"), + Arguments.of("with spaces and punctuation!", "with spaces and punctuation!"), + // each control character is escaped to its two-character literal + Arguments.of("a\nb", "a\\nb"), + Arguments.of("a\rb", "a\\rb"), + Arguments.of("a\tb", "a\\tb"), + // combined and CRLF + Arguments.of("a\r\nb", "a\\r\\nb"), + Arguments.of("\tcol1\tcol2", "\\tcol1\\tcol2"), + Arguments.of("line1\nline2\r\nline3\tend", "line1\\nline2\\r\\nline3\\tend"), + // a realistic log-injection payload + Arguments.of("evil\nFAKE LOG ENTRY: forged", "evil\\nFAKE LOG ENTRY: forged"), + // input that already contains literal backslash-n is NOT touched; only real control characters are + Arguments.of("already escaped: \\n", "already escaped: \\n")); + } + + @ParameterizedTest + @MethodSource + void testSanitize(String input, String expected) { + assertEquals(expected, LogSanitizer.sanitize(input)); + } + + @Test + void testSanitizeIsIdempotent() { + String once = LogSanitizer.sanitize("a\nb\tc"); + String twice = LogSanitizer.sanitize(once); + assertEquals(once, twice); + } + + @Test + void testSanitizeObjectNull() { + assertNull(LogSanitizer.sanitize((Object) null)); + } + + @Test + void testSanitizeObjectDelegatesToToString() { + Object value = new Object() { + @Override + public String toString() { + return "value-with\nnewline"; + } + }; + assertEquals("value-with\\nnewline", LogSanitizer.sanitize(value)); + } +} diff --git a/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/stream/LazyArchiveInputStream.java b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/stream/LazyArchiveInputStream.java index 7e61ed0371..e2c3361386 100644 --- a/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/stream/LazyArchiveInputStream.java +++ b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/stream/LazyArchiveInputStream.java @@ -9,6 +9,7 @@ import org.apache.commons.io.IOUtils; import org.cloudfoundry.multiapps.common.SLException; +import org.cloudfoundry.multiapps.controller.core.util.LogSanitizer; import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; import org.cloudfoundry.multiapps.controller.persistence.services.FileService; import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException; @@ -102,10 +103,10 @@ public void close() throws IOException { private BufferedInputStream openBufferedInputStream(FileEntry archiveFileEntry) { try { - stepLogger.debug(Messages.OPENING_A_NEW_INPUT_STREAM_FOR_FILE_WITH_ID_0_AND_NAME_1, archiveFileEntry.getId(), - archiveFileEntry.getName()); + String sanitizedName = LogSanitizer.sanitize(archiveFileEntry.getName()); + stepLogger.debug(Messages.OPENING_A_NEW_INPUT_STREAM_FOR_FILE_WITH_ID_0_AND_NAME_1, archiveFileEntry.getId(), sanitizedName); LOGGER.info(MessageFormat.format(Messages.OPENING_A_NEW_INPUT_STREAM_FOR_FILE_WITH_ID_0_AND_NAME_1, archiveFileEntry.getId(), - archiveFileEntry.getName())); + sanitizedName)); InputStream inputStream = fileService.openInputStream(archiveFileEntry.getSpace(), archiveFileEntry.getId()); return new BufferedInputStream(inputStream, BUFFERED_SIZE); } catch (FileStorageException e) { diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java index 9dbcb55d33..16e4721aff 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java @@ -12,6 +12,7 @@ import org.cloudfoundry.multiapps.controller.api.model.ImmutableFileMetadata; import org.cloudfoundry.multiapps.controller.api.model.UserCredentials; import org.cloudfoundry.multiapps.controller.core.auditlogging.FilesApiServiceAuditLog; +import org.cloudfoundry.multiapps.controller.core.util.LogSanitizer; import org.cloudfoundry.multiapps.controller.core.util.UriUtil; import org.cloudfoundry.multiapps.controller.persistence.model.AsyncUploadJobEntry; import org.cloudfoundry.multiapps.controller.persistence.model.AsyncUploadJobEntry.State; @@ -110,8 +111,8 @@ public ResponseEntity uploadFile(MultipartHttpServletRequest reque FileMetadata file = parseFileEntry(fileEntry); filesApiServiceAuditLog.logUploadFile(SecurityContextUtil.getUsername(), spaceGuid, file); var endTime = LocalDateTime.now(); - LOGGER.trace(Messages.UPLOADED_FILE, file.getId(), file.getName(), file.getSize(), file.getSpace(), file.getDigest(), - file.getDigestAlgorithm(), ChronoUnit.MILLIS.between(startTime, endTime)); + LOGGER.trace(Messages.UPLOADED_FILE, file.getId(), LogSanitizer.sanitize(file.getName()), file.getSize(), file.getSpace(), + file.getDigest(), file.getDigestAlgorithm(), ChronoUnit.MILLIS.between(startTime, endTime)); return ResponseEntity.status(HttpStatus.CREATED) .body(file); } catch (Exception e) { diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java index d41dd1c727..7fc4a38d56 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java @@ -25,6 +25,7 @@ import org.cloudfoundry.multiapps.controller.core.helpers.DescriptorParserFacadeFactory; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; import org.cloudfoundry.multiapps.controller.core.util.FileUtils; +import org.cloudfoundry.multiapps.controller.core.util.LogSanitizer; import org.cloudfoundry.multiapps.controller.persistence.model.AsyncUploadJobEntry; import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; import org.cloudfoundry.multiapps.controller.persistence.model.ImmutableAsyncUploadJobEntry; @@ -80,7 +81,7 @@ public AsyncUploadJobOrchestrator(ExecutorService asyncFileUploadExecutor, Execu public AsyncUploadJobEntry executeUploadFromUrl(String spaceGuid, String namespace, String urlWithoutUserInfo, String decodedUrl, UserCredentials userCredentials) { var entry = createJobEntry(spaceGuid, namespace, urlWithoutUserInfo); - LOGGER.info(Messages.CREATING_ASYNC_UPLOAD_JOB, urlWithoutUserInfo, entry.getId()); + LOGGER.info(Messages.CREATING_ASYNC_UPLOAD_JOB, LogSanitizer.sanitize(urlWithoutUserInfo), entry.getId()); asyncUploadJobService.add(entry); try { deployFromUrlExecutor.submit(() -> deployFromUrl(entry, decodedUrl, userCredentials)); @@ -107,7 +108,7 @@ private AsyncUploadJobEntry createJobEntry(String spaceGuid, String namespace, S } private void deployFromUrl(AsyncUploadJobEntry jobEntry, String fileUrl, UserCredentials userCredentials) { - LOGGER.info(Messages.STARTING_DOWNLOAD_OF_MTAR_WITH_JOB_ID, jobEntry.getUrl(), jobEntry.getId()); + LOGGER.info(Messages.STARTING_DOWNLOAD_OF_MTAR_WITH_JOB_ID, LogSanitizer.sanitize(jobEntry.getUrl()), jobEntry.getId()); var startTime = LocalDateTime.now(); Lock lock = new ReentrantLock(); AtomicLong counterRef = new AtomicLong(); @@ -187,7 +188,7 @@ private FileEntry doUploadMtarFromUrl(UploadFromUrlContext uploadFromUrlContext, // JClods library: https://issues.apache.org/jira/browse/JCLOUDS-1623 try (CountingInputStream source = new CountingInputStream(fileFromUrlData.fileInputStream(), uploadFromUrlContext.getCounterRef()); BufferedInputStream bufferedContent = new BufferedInputStream(source, INPUT_STREAM_BUFFER_SIZE)) { - LOGGER.debug(Messages.UPLOADING_MTAR_STREAM_FROM_REMOTE_ENDPOINT_WITH_JOB_ID, fileFromUrlData.uri(), + LOGGER.debug(Messages.UPLOADING_MTAR_STREAM_FROM_REMOTE_ENDPOINT_WITH_JOB_ID, LogSanitizer.sanitize(fileFromUrlData.uri()), uploadFromUrlContext.getJobEntry() .getId()); return fileService.addFile(ImmutableFileEntry.builder() diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/client/DeployFromUrlRemoteClient.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/client/DeployFromUrlRemoteClient.java index 75f714116c..ac43dd6fb9 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/client/DeployFromUrlRemoteClient.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/client/DeployFromUrlRemoteClient.java @@ -20,6 +20,7 @@ import org.cloudfoundry.multiapps.controller.client.util.CheckedSupplier; import org.cloudfoundry.multiapps.controller.client.util.ResilientOperationExecutor; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; +import org.cloudfoundry.multiapps.controller.core.util.LogSanitizer; import org.cloudfoundry.multiapps.controller.core.util.UriUtil; import org.cloudfoundry.multiapps.controller.web.Constants; import org.cloudfoundry.multiapps.controller.web.Messages; @@ -82,7 +83,7 @@ private HttpResponse callRemoteEndpointWithRetry(String decodedUrl, LOGGER.debug(Messages.CALLING_REMOTE_MTAR_ENDPOINT_FOR_JOB_WITH_ID, getMaskedUri(urlDecodeUrl(decodedUrl)), jobId); var response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream()); if (response.statusCode() / 100 != 2) { - String error = readErrorBodyFromResponse(response); + String error = LogSanitizer.sanitize(readErrorBodyFromResponse(response)); LOGGER.error(error); if (response.statusCode() == HttpStatus.UNAUTHORIZED.value()) { String errorMessage = MessageFormat.format(Messages.DEPLOY_FROM_URL_WRONG_CREDENTIALS_FOR_JOB_WITH_ID,