From 5abdde6d8f89b16abe237d54df36a0f7ca48c7f8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 11 Dec 2018 17:46:34 +0100 Subject: [PATCH] SONAR-9919 obfuscate credentials in webhook delivery logs --- .../sonar/server/webhook/HttpUrlHelper.java | 103 ++++++++++++++++++ .../server/webhook/WebhookCallerImpl.java | 13 ++- .../sonar/server/webhook/WebhookDelivery.java | 12 ++ .../webhook/WebhookDeliveryStorage.java | 2 +- .../server/webhook/HttpUrlHelperTest.java | 74 +++++++++++++ .../webhook/WebhookDeliveryStorageTest.java | 15 +++ 6 files changed, 212 insertions(+), 7 deletions(-) create mode 100644 server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java create mode 100644 server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java new file mode 100644 index 00000000000..20f38dcd5a2 --- /dev/null +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/HttpUrlHelper.java @@ -0,0 +1,103 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.webhook; + +import com.google.common.base.Supplier; +import java.util.Objects; +import java.util.stream.Stream; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; +import okhttp3.HttpUrl; + +import static com.google.common.base.Preconditions.checkState; +import static org.apache.commons.lang.StringUtils.repeat; + +final class HttpUrlHelper { + private HttpUrlHelper() { + // prevents instantiation + } + + /** + * According to inline comment in {@link okhttp3.HttpUrl.Builder#parse(HttpUrl base, String input)}: + *
+ * Username, password and port are optional. + * [username[:password]@]host[:port] + *
+ *

+ * This function replaces the chars of the username and the password from the {@code originalUrl} by '*' chars + * based on username and password parsed in {@code parsedUrl}. + */ + public static String toEffectiveUrl(String originalUrl, HttpUrl parsedUrl) { + String username = parsedUrl.username(); + String password = parsedUrl.password(); + if (username.isEmpty() && password.isEmpty()) { + return originalUrl; + } + + if (!username.isEmpty() && !password.isEmpty()) { + String encodedUsername = parsedUrl.encodedUsername(); + String encodedPassword = parsedUrl.encodedPassword(); + return Stream.>of( + () -> replaceOrDie(originalUrl, username, password), + () -> replaceOrDie(originalUrl, encodedUsername, encodedPassword), + () -> replaceOrDie(originalUrl, encodedUsername, password), + () -> replaceOrDie(originalUrl, username, encodedPassword)) + .map(Supplier::get) + .filter(Objects::nonNull) + .findFirst() + .orElse(originalUrl); + } + if (!username.isEmpty()) { + return Stream.>of( + () -> replaceOrDie(originalUrl, username, null), + () -> replaceOrDie(originalUrl, parsedUrl.encodedUsername(), null)) + .map(Supplier::get) + .filter(Objects::nonNull) + .findFirst() + .orElse(originalUrl); + } + checkState(password.isEmpty(), "having a password without a username should never occur"); + return originalUrl; + } + + @CheckForNull + private static String replaceOrDie(String original, String username, @Nullable String password) { + return replaceOrDieImpl(original, authentStringOf(username, password), obfuscatedAuthentStringOf(username, password)); + } + + private static String authentStringOf(String username, @Nullable String password) { + if (password == null) { + return username + "@"; + } + return username + ":" + password + "@"; + } + + private static String obfuscatedAuthentStringOf(String userName, @Nullable String password) { + return authentStringOf(repeat("*", userName.length()), password == null ? null : repeat("*", password.length())); + } + + private static String replaceOrDieImpl(String original, String target, String replacement) { + String res = original.replace(target, replacement); + if (!res.equals(original)) { + return res; + } + return null; + } +} diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java index 4f0624b6117..d6c76864c5f 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookCallerImpl.java @@ -64,7 +64,12 @@ public class WebhookCallerImpl implements WebhookCaller { .setWebhook(webhook); try { - Request request = buildHttpRequest(webhook, payload); + HttpUrl url = HttpUrl.parse(webhook.getUrl()); + if (url == null) { + throw new IllegalArgumentException("Webhook URL is not valid: " + webhook.getUrl()); + } + builder.setEffectiveUrl(HttpUrlHelper.toEffectiveUrl(webhook.getUrl(), url)); + Request request = buildHttpRequest(url, payload); try (Response response = execute(request)) { builder.setHttpStatus(response.code()); } @@ -77,11 +82,7 @@ public class WebhookCallerImpl implements WebhookCaller { .build(); } - private static Request buildHttpRequest(Webhook webhook, WebhookPayload payload) { - HttpUrl url = HttpUrl.parse(webhook.getUrl()); - if (url == null) { - throw new IllegalArgumentException("Webhook URL is not valid: " + webhook.getUrl()); - } + private static Request buildHttpRequest(HttpUrl url, WebhookPayload payload) { Request.Builder request = new Request.Builder(); request.url(url); request.header(PROJECT_KEY_HEADER, payload.getProjectKey()); diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookDelivery.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookDelivery.java index 38194e05d80..12d34c7500c 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookDelivery.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookDelivery.java @@ -34,6 +34,7 @@ public class WebhookDelivery { private final Webhook webhook; private final WebhookPayload payload; + private final String effectiveUrl; private final Integer httpStatus; private final Integer durationInMs; private final long at; @@ -42,6 +43,7 @@ public class WebhookDelivery { private WebhookDelivery(Builder builder) { this.webhook = requireNonNull(builder.webhook); this.payload = requireNonNull(builder.payload); + this.effectiveUrl = builder.effectiveUrl; this.httpStatus = builder.httpStatus; this.durationInMs = builder.durationInMs; this.at = builder.at; @@ -56,6 +58,10 @@ public class WebhookDelivery { return payload; } + public Optional getEffectiveUrl() { + return Optional.ofNullable(effectiveUrl); + } + /** * @return the HTTP status if {@link #getError()} is empty, else returns * {@link Optional#empty()} @@ -101,6 +107,7 @@ public class WebhookDelivery { public static class Builder { private Webhook webhook; private WebhookPayload payload; + private String effectiveUrl; private Integer httpStatus; private Integer durationInMs; private long at; @@ -116,6 +123,11 @@ public class WebhookDelivery { return this; } + public Builder setEffectiveUrl(@Nullable String effectiveUrl) { + this.effectiveUrl = effectiveUrl; + return this; + } + public Builder setHttpStatus(@Nullable Integer httpStatus) { this.httpStatus = httpStatus; return this; diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookDeliveryStorage.java b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookDeliveryStorage.java index 7bf13c484a2..07025d00a91 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookDeliveryStorage.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/webhook/WebhookDeliveryStorage.java @@ -72,7 +72,7 @@ public class WebhookDeliveryStorage { delivery.getWebhook().getCeTaskUuid().ifPresent(dto::setCeTaskUuid); delivery.getWebhook().getAnalysisUuid().ifPresent(dto::setAnalysisUuid); dto.setName(delivery.getWebhook().getName()); - dto.setUrl(delivery.getWebhook().getUrl()); + dto.setUrl(delivery.getEffectiveUrl().orElse(delivery.getWebhook().getUrl())); dto.setSuccess(delivery.isSuccess()); dto.setHttpStatus(delivery.getHttpStatus().orElse(null)); dto.setDurationMs(delivery.getDurationInMs().orElse(null)); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java new file mode 100644 index 00000000000..16ba1a36b93 --- /dev/null +++ b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/HttpUrlHelperTest.java @@ -0,0 +1,74 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.webhook; + +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import okhttp3.HttpUrl; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.apache.commons.lang.StringUtils.repeat; +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(DataProviderRunner.class) +public class HttpUrlHelperTest { + + @Test + @UseDataProvider("toEffectiveUrlUseCases") + public void verify_toEffectiveUrl(String originalUrl, String expectedUrl) { + assertThat(HttpUrlHelper.toEffectiveUrl(originalUrl, HttpUrl.parse(originalUrl))).isEqualTo(expectedUrl); + } + + @DataProvider + public static Object[][] toEffectiveUrlUseCases() { + List rows = new ArrayList<>(); + for (String before : Arrays.asList("http://", "https://")) { + for (String host : Arrays.asList("foo", "127.0.0.1", "[2001:db8:85a3:0:0:8a2e:370:7334]", "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]")) { + for (String port : Arrays.asList("", ":123")) { + for (String after : Arrays.asList("", "/", "/bar", "/bar/", "?", "?a=b", "?a=b&c=d")) { + for (String username : Arrays.asList("", "us", "a b", "a%20b")) { + for (String password : Arrays.asList("", "pwd", "pwd%20k", "pwd k", "c:d")) { + if (username.isEmpty()) { + String url = before + host + port + after; + rows.add(new Object[] {url, url}); + } else if (password.isEmpty()) { + String url = before + username + '@' + host + port + after; + String expected = before + repeat("*", username.length()) + '@' + host + port + after; + rows.add(new Object[] {url, expected}); + } else { + String url = before + username + ':' + password + '@' + host + port + after; + String expected = before + repeat("*", username.length()) + ':' + repeat("*", password.length()) + '@' + host + port + after; + rows.add(new Object[] {url, expected}); + } + } + } + } + } + } + } + return rows.toArray(new Object[0][]); + } + +} diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookDeliveryStorageTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookDeliveryStorageTest.java index f90edd08d8f..5539868867d 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookDeliveryStorageTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/webhook/WebhookDeliveryStorageTest.java @@ -31,6 +31,7 @@ import org.sonar.db.DbTester; import org.sonar.db.webhook.WebhookDeliveryDto; import org.sonar.db.webhook.WebhookDeliveryTesting; +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -101,6 +102,20 @@ public class WebhookDeliveryStorageTest { assertThat(selectAllDeliveryUuids(dbTester, dbSession)).containsOnly("D2", "D3"); } + @Test + public void persist_effective_url_if_present() { + when(uuidFactory.create()).thenReturn(DELIVERY_UUID); + String effectiveUrl = randomAlphabetic(15); + WebhookDelivery delivery = newBuilderTemplate() + .setEffectiveUrl(effectiveUrl) + .build(); + + underTest.persist(delivery); + + WebhookDeliveryDto dto = dbClient.webhookDeliveryDao().selectByUuid(dbSession, DELIVERY_UUID).get(); + assertThat(dto.getUrl()).isEqualTo(effectiveUrl); + } + private static WebhookDelivery.Builder newBuilderTemplate() { return new WebhookDelivery.Builder() .setWebhook(new Webhook("WEBHOOK_UUID_1", "COMPONENT1", "TASK1", RandomStringUtils.randomAlphanumeric(40),"Jenkins", "http://jenkins")) -- 2.39.5