This is an automated email from the ASF dual-hosted git repository.
yuluo-yx pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hertzbeat.git
The following commit(s) were added to refs/heads/master by this push:
new 59c003aaef [fix] prevent masked notice secret replay to attacker URLs
(#4262)
59c003aaef is described below
commit 59c003aaef5404a00890afc3ff7c11f4c0cd4e89
Author: shown <[email protected]>
AuthorDate: Wed Jul 29 20:54:26 2026 +0800
[fix] prevent masked notice secret replay to attacker URLs (#4262)
Signed-off-by: yuluo-yx <[email protected]>
---
.../service/impl/NoticeConfigServiceImpl.java | 19 ++++++-
.../alert/util/NoticeReceiverMaskUtil.java | 61 +++++++++++++++++++-
.../alert/service/NoticeConfigServiceTest.java | 35 ++++++++++++
.../alert/util/NoticeReceiverMaskUtilTest.java | 66 +++++++++++++++++++++-
4 files changed, 175 insertions(+), 6 deletions(-)
diff --git
a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java
b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java
index bfbed03191..eab8ad014e 100644
---
a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java
+++
b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/service/impl/NoticeConfigServiceImpl.java
@@ -190,13 +190,28 @@ public class NoticeConfigServiceImpl implements
NoticeConfigService, CommandLine
* be resolved, and saving would persist
the placeholder
*/
private void resolveMaskedSecrets(NoticeReceiver noticeReceiver) {
+ resolveMaskedSecrets(noticeReceiver, false);
+ }
+
+ /**
+ * Bind stored secrets to their original notification type and destination
for test messages.
+ */
+ private void resolveMaskedSecretsForTest(NoticeReceiver noticeReceiver) {
+ resolveMaskedSecrets(noticeReceiver, true);
+ }
+
+ private void resolveMaskedSecrets(NoticeReceiver noticeReceiver, boolean
testMessage) {
if (noticeReceiver == null || noticeReceiver.getId() == null) {
return;
}
NoticeReceiver existing =
noticeReceiverDao.findById(noticeReceiver.getId())
.orElseThrow(() -> new IllegalArgumentException(
"The receiver with id " + noticeReceiver.getId() + "
does not exist."));
- NoticeReceiverMaskUtil.resolveMask(noticeReceiver, existing);
+ if (testMessage) {
+ NoticeReceiverMaskUtil.resolveMaskForTest(noticeReceiver,
existing);
+ } else {
+ NoticeReceiverMaskUtil.resolveMask(noticeReceiver, existing);
+ }
}
@Override
@@ -339,7 +354,7 @@ public class NoticeConfigServiceImpl implements
NoticeConfigService, CommandLine
@Override
public boolean sendTestMsg(NoticeReceiver noticeReceiver) {
- resolveMaskedSecrets(noticeReceiver);
+ resolveMaskedSecretsForTest(noticeReceiver);
Map<String, String> labels = new HashMap<>(8);
labels.put(CommonConstants.LABEL_INSTANCE, "127.0.0.1");
labels.put(CommonConstants.LABEL_ALERT_NAME, "CPU Usage Alert");
diff --git
a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtil.java
b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtil.java
index db5fe30a43..51da9d0f52 100644
---
a/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtil.java
+++
b/hertzbeat-alerter/src/main/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtil.java
@@ -18,6 +18,7 @@
package org.apache.hertzbeat.alert.util;
import java.util.List;
+import java.util.Objects;
import java.util.function.BiConsumer;
import java.util.function.Function;
import org.apache.commons.lang3.StringUtils;
@@ -92,6 +93,7 @@ public final class NoticeReceiverMaskUtil {
* A re-entered secret or a cleared field is left untouched.
* @param incoming receiver submitted by the ui, modified in place
* @param existing receiver currently stored in the database
+ * @throws IllegalArgumentException if a submitted mask does not match the
stored secret
*/
public static void resolveMask(NoticeReceiver incoming, NoticeReceiver
existing) {
if (incoming == null || existing == null) {
@@ -102,15 +104,72 @@ public final class NoticeReceiverMaskUtil {
String stored = field.getter().apply(existing);
if (isMaskOf(submitted, stored)) {
field.setter().accept(incoming, stored);
+ } else if (isMaskValue(submitted)) {
+ throw new IllegalArgumentException(
+ "The submitted secret mask does not match the stored
secret.");
}
}
}
+ /**
+ * Resolve masked secrets for a test message while binding them to their
persisted destination.
+ * A caller that changes the notification type or a URL receiving
authentication data
+ * must submit the new secret explicitly instead of reusing a stored
secret mask.
+ * @param incoming receiver submitted for a test message, modified in place
+ * @param existing receiver currently stored in the database
+ * @throws IllegalArgumentException if a masked secret is combined with a
changed destination
+ */
+ public static void resolveMaskForTest(NoticeReceiver incoming,
NoticeReceiver existing) {
+ if (incoming == null || existing == null) {
+ return;
+ }
+
+ boolean containsStoredSecretMask = SECRET_FIELDS.stream()
+ .anyMatch(field -> isMaskOf(field.getter().apply(incoming),
field.getter().apply(existing)));
+ if (containsStoredSecretMask && !Objects.equals(incoming.getType(),
existing.getType())) {
+ throw new IllegalArgumentException(
+ "The notification type cannot be changed when reusing a
masked secret.");
+ }
+ rejectChangedSecretDestination(
+ incoming.getHookAuthToken(),
+ existing.getHookAuthToken(),
+ incoming.getHookUrl(),
+ existing.getHookUrl(),
+ "webhook URL");
+ rejectChangedSecretDestination(
+ incoming.getNtfyToken(),
+ existing.getNtfyToken(),
+ incoming.getNtfyServerUrl(),
+ existing.getNtfyServerUrl(),
+ "ntfy server URL");
+ resolveMask(incoming, existing);
+ }
+
+ private static void rejectChangedSecretDestination(
+ String submittedSecret,
+ String storedSecret,
+ String submittedDestination,
+ String storedDestination,
+ String destinationName) {
+ if (isMaskOf(submittedSecret, storedSecret)
+ && !Objects.equals(submittedDestination, storedDestination)) {
+ throw new IllegalArgumentException(
+ "The " + destinationName + " cannot be changed when
reusing a masked secret.");
+ }
+ }
+
private static boolean isMaskOf(String submitted, String stored) {
if (submitted == null || StringUtils.isBlank(stored)) {
return false;
}
- return submitted.equals(SECRET_MASK) ||
submitted.equals(maskValue(stored));
+ return submitted.equals(maskValue(stored));
+ }
+
+ private static boolean isMaskValue(String value) {
+ return value != null
+ && value.startsWith(SECRET_MASK)
+ && (value.length() == SECRET_MASK.length()
+ || value.length() == SECRET_MASK.length() +
VISIBLE_SUFFIX_LENGTH);
}
private static String maskValue(String value) {
diff --git
a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java
b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java
index 72f21bc3c8..2371e2692c 100644
---
a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java
+++
b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/service/NoticeConfigServiceTest.java
@@ -279,6 +279,41 @@ class NoticeConfigServiceTest {
verify(dispatcherAlarm, never()).sendNoticeMsg(any(), any(), any());
}
+ @Test
+ void sendTestMsgRejectsMaskedSecretReplayToChangedWebhookUrl() {
+ final NoticeReceiver stored = new NoticeReceiver();
+ stored.setId(5L);
+ stored.setType((byte) 2);
+ stored.setHookUrl("https://trusted.example/hook";);
+ stored.setHookAuthToken("hook-auth-token-abcd");
+ when(noticeReceiverDao.findById(5L)).thenReturn(Optional.of(stored));
+
+ final NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(stored);
+ incoming.setHookUrl("https://attacker.example/collect";);
+
+ assertThrows(IllegalArgumentException.class, () ->
noticeConfigService.sendTestMsg(incoming));
+ verify(dispatcherAlarm, never()).sendNoticeMsg(any(), any(), any());
+ }
+
+ @Test
+ void sendTestMsgRejectsBareMaskReplayToChangedWebhookUrl() {
+ final NoticeReceiver stored = new NoticeReceiver();
+ stored.setId(5L);
+ stored.setType((byte) 2);
+ stored.setHookUrl("https://trusted.example/hook";);
+ stored.setHookAuthToken("hook-auth-token-abcd");
+ when(noticeReceiverDao.findById(5L)).thenReturn(Optional.of(stored));
+
+ final NoticeReceiver incoming = new NoticeReceiver();
+ incoming.setId(5L);
+ incoming.setType((byte) 2);
+ incoming.setHookUrl("https://attacker.example/collect";);
+ incoming.setHookAuthToken(NoticeReceiverMaskUtil.SECRET_MASK);
+
+ assertThrows(IllegalArgumentException.class, () ->
noticeConfigService.sendTestMsg(incoming));
+ verify(dispatcherAlarm, never()).sendNoticeMsg(any(), any(), any());
+ }
+
@Test
void deleteReceiver() {
final Long receiverId = 23342525L;
diff --git
a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtilTest.java
b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtilTest.java
index 7d42ed6e87..cdac126c46 100644
---
a/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtilTest.java
+++
b/hertzbeat-alerter/src/test/java/org/apache/hertzbeat/alert/util/NoticeReceiverMaskUtilTest.java
@@ -23,6 +23,7 @@ import org.junit.jupiter.api.Test;
import static
org.apache.hertzbeat.alert.util.NoticeReceiverMaskUtil.SECRET_MASK;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
/**
* Test case for {@link NoticeReceiverMaskUtil}
@@ -104,7 +105,6 @@ class NoticeReceiverMaskUtilTest {
NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
incoming.setAccessToken("new-access-token-1234");
incoming.setGotifyToken(null);
- incoming.setNtfyToken(SECRET_MASK);
NoticeReceiverMaskUtil.resolveMask(incoming, existing);
@@ -122,13 +122,73 @@ class NoticeReceiverMaskUtilTest {
}
@Test
- void resolveMaskIgnoresMaskWhenNothingIsStored() {
+ void resolveMaskRejectsMaskWhenNothingIsStored() {
NoticeReceiver existing = new NoticeReceiver();
NoticeReceiver incoming = new NoticeReceiver();
incoming.setAccessToken(SECRET_MASK);
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> NoticeReceiverMaskUtil.resolveMask(incoming, existing));
+ }
+
+ @Test
+ void resolveMaskRejectsBareMaskAsWildcard() {
+ NoticeReceiver existing = buildReceiverWithSecrets();
+ NoticeReceiver incoming = new NoticeReceiver();
+ incoming.setAccessToken(SECRET_MASK);
+
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> NoticeReceiverMaskUtil.resolveMask(incoming, existing));
+ }
+
+ @Test
+ void resolveMaskRestoresBareMaskForShortStoredSecret() {
+ NoticeReceiver existing = new NoticeReceiver();
+ existing.setAccessToken("short-token");
+ NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
+
NoticeReceiverMaskUtil.resolveMask(incoming, existing);
- assertEquals(SECRET_MASK, incoming.getAccessToken());
+ assertEquals("short-token", incoming.getAccessToken());
+ }
+
+ @Test
+ void resolveMaskForTestRejectsMaskedWebhookSecretForChangedUrl() {
+ NoticeReceiver existing = buildReceiverWithSecrets();
+ existing.setType((byte) 2);
+ NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
+ incoming.setHookUrl("https://attacker.example/collect";);
+
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> NoticeReceiverMaskUtil.resolveMaskForTest(incoming,
existing));
+ assertEquals(SECRET_MASK + "abcd", incoming.getHookAuthToken());
+ }
+
+ @Test
+ void resolveMaskForTestRejectsMaskedNtfySecretForChangedServer() {
+ NoticeReceiver existing = buildReceiverWithSecrets();
+ existing.setType((byte) 15);
+ existing.setNtfyServerUrl("https://ntfy.example";);
+ NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
+ incoming.setNtfyServerUrl("https://attacker.example";);
+
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> NoticeReceiverMaskUtil.resolveMaskForTest(incoming,
existing));
+ assertEquals(SECRET_MASK + "NIz2", incoming.getNtfyToken());
+ }
+
+ @Test
+ void resolveMaskForTestRestoresSecretForUnchangedDestination() {
+ NoticeReceiver existing = buildReceiverWithSecrets();
+ existing.setType((byte) 2);
+ NoticeReceiver incoming = NoticeReceiverMaskUtil.mask(existing);
+
+ NoticeReceiverMaskUtil.resolveMaskForTest(incoming, existing);
+
+ assertEquals("hook-auth-token-abcd", incoming.getHookAuthToken());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
