You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by sh...@apache.org on 2026/08/18 13:55:51 UTC
(cloudstack) 17/23: webhook: additional check for payload url
This is an automated email from the ASF dual-hosted git repository.
shwstppr pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 1790275babdfc4eadd86d71517c36a0ef0097d31
Author: Abhishek Kumar <ab...@gmail.com>
AuthorDate: Mon Jun 8 14:11:01 2026 +0530
webhook: additional check for payload url
Adds new global configurations to give more flexibility in controlling
payload URL for the webhooks. New domain-scope configs:
- webhook.delivery.blocklist
- webhook.delivery.allow.redirects
- webhook.delivery.allow.http
- webhook.delivery.block.local.addresses (Hidden)
Signed-off-by: Abhishek Kumar <ab...@gmail.com>
---
.../mom/webhook/WebhookApiServiceImpl.java | 23 +++-
.../mom/webhook/WebhookDeliveryThread.java | 117 ++++++++++++++++----
.../cloudstack/mom/webhook/WebhookService.java | 38 ++++++-
.../cloudstack/mom/webhook/WebhookServiceImpl.java | 61 ++++++++--
.../mom/webhook/WebhookUrlValidator.java | 123 +++++++++++++++++++++
.../mom/webhook/WebhookDeliveryThreadTest.java | 7 ++
.../mom/webhook/WebhookUrlValidatorTest.java | 100 +++++++++++++++++
test/integration/smoke/test_webhook_delivery.py | 67 ++++++++++-
test/integration/smoke/test_webhook_lifecycle.py | 83 ++++++++++++--
9 files changed, 568 insertions(+), 51 deletions(-)
diff --git a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookApiServiceImpl.java b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookApiServiceImpl.java
index d119c7d181a..37f4b3d0a92 100644
--- a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookApiServiceImpl.java
+++ b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookApiServiceImpl.java
@@ -61,7 +61,6 @@ import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.utils.Pair;
import com.cloud.utils.Ternary;
-import com.cloud.utils.UriUtils;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.SearchBuilder;
@@ -221,10 +220,17 @@ public class WebhookApiServiceImpl extends ManagerBase implements WebhookApiServ
return;
}
String error = String.format("Payload URL: %s is already in use by another webhook", payloadUrl);
- logger.error(String.format("%s: %s for Account [%s]", error, webhookVO, owner));
+ logger.error("{}: {} for Account [{}]", error, webhookVO, owner);
throw new InvalidParameterValueException(error);
}
+ protected URI validatePayloadUrlByDeliverySecurityPolicy(String payloadUrl, long domainId) {
+ return WebhookUrlValidator.validateWebhookDestinationUrl(payloadUrl,
+ WebhookService.WebhookDeliveryAllowHttp.valueIn(domainId),
+ WebhookService.WebhookDeliveryBlocklist.valueIn(domainId),
+ WebhookService.WebhookDeliveryBlockLocalAddresses.value());
+ }
+
@Override
public ListResponse<WebhookResponse> listWebhooks(ListWebhooksCmd cmd) {
final CallContext ctx = CallContext.current();
@@ -338,7 +344,7 @@ public class WebhookApiServiceImpl extends ManagerBase implements WebhookApiServ
throw new InvalidParameterValueException("Invalid state specified");
}
}
- UriUtils.validateUrl(payloadUrl);
+ validatePayloadUrlByDeliverySecurityPolicy(payloadUrl, owner.getDomainId());
validateWebhookOwnerPayloadUrl(owner, payloadUrl, null);
URI uri = URI.create(payloadUrl);
if (sslVerification && !HttpConstants.HTTPS.equalsIgnoreCase(uri.getScheme())) {
@@ -421,7 +427,7 @@ public class WebhookApiServiceImpl extends ManagerBase implements WebhookApiServ
}
URI uri = URI.create(webhook.getPayloadUrl());
if (StringUtils.isNotEmpty(payloadUrl)) {
- UriUtils.validateUrl(payloadUrl);
+ validatePayloadUrlByDeliverySecurityPolicy(payloadUrl, owner.getDomainId());
validateWebhookOwnerPayloadUrl(owner, payloadUrl, webhook);
uri = URI.create(payloadUrl);
webhook.setPayloadUrl(payloadUrl);
@@ -540,8 +546,13 @@ public class WebhookApiServiceImpl extends ManagerBase implements WebhookApiServ
}
webhook = webhookDao.findById(existingDelivery.getWebhookId());
}
+ URI uri = null;
if (StringUtils.isNotBlank(payloadUrl)) {
- UriUtils.validateUrl(payloadUrl);
+ long domainId = owner.getDomainId();
+ if (webhook != null) {
+ domainId = webhook.getDomainId();
+ }
+ uri = validatePayloadUrlByDeliverySecurityPolicy(payloadUrl, domainId);
}
if (webhookId != null) {
webhook = webhookDao.findById(webhookId);
@@ -562,7 +573,7 @@ public class WebhookApiServiceImpl extends ManagerBase implements WebhookApiServ
webhook = new WebhookVO(owner.getDomainId(), owner.getId(), payloadUrl, secretKey,
Boolean.TRUE.equals(sslVerification));
}
- WebhookDelivery webhookDelivery = webhookService.executeWebhookDelivery(existingDelivery, webhook, payload);
+ WebhookDelivery webhookDelivery = webhookService.executeWebhookDelivery(existingDelivery, webhook, payload, uri);
if (webhookDelivery.getId() != WebhookDelivery.ID_DUMMY) {
return createWebhookDeliveryResponse(webhookDeliveryJoinDao.findById(webhookDelivery.getId()));
}
diff --git a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThread.java b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThread.java
index ac840c00be3..e68fcc2f30c 100644
--- a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThread.java
+++ b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThread.java
@@ -19,7 +19,6 @@ package org.apache.cloudstack.mom.webhook;
import java.io.IOException;
import java.net.URI;
-import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.security.InvalidKeyException;
import java.security.KeyManagementException;
@@ -40,11 +39,11 @@ import org.apache.cloudstack.framework.events.Event;
import org.apache.cloudstack.storage.command.CommandResult;
import org.apache.commons.codec.DecoderException;
import org.apache.commons.codec.binary.Base64;
-import org.apache.commons.httpclient.HttpStatus;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpHeaders;
+import org.apache.http.HttpStatus;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpPost;
@@ -63,6 +62,8 @@ import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
+import com.cloud.exception.InvalidParameterValueException;
+
public class WebhookDeliveryThread implements Runnable {
protected static Logger LOGGER = LogManager.getLogger(WebhookDeliveryThread.class);
@@ -70,6 +71,7 @@ public class WebhookDeliveryThread implements Runnable {
private static final String HEADER_X_CS_EVENT = "X-CS-Event";
private static final String HEADER_X_CS_SIGNATURE = "X-CS-Signature";
private static final String PREFIX_HEADER_USER_AGENT = "CS-Hookshot/";
+ private static final int MAX_REDIRECT_HOPS = 5;
private final Webhook webhook;
private final Event event;
private CloseableHttpClient httpClient;
@@ -79,6 +81,11 @@ public class WebhookDeliveryThread implements Runnable {
private Date startTime;
private int deliveryTries = 3;
private int deliveryTimeout = 10;
+ private String destinationBlocklist;
+ private boolean blockLocalAddresses = true;
+ private boolean allowRedirects;
+ private boolean allowHttp;
+ private URI payloadUri;
AsyncCompletionCallback<WebhookDeliveryResult> callback;
@@ -97,19 +104,19 @@ public class WebhookDeliveryThread implements Runnable {
protected void setHttpClient() throws NoSuchAlgorithmException, KeyStoreException, KeyManagementException {
if (webhook.isSslVerification()) {
- httpClient = HttpClients.createDefault();
+ httpClient = HttpClients.custom().disableRedirectHandling().build();
return;
}
httpClient = HttpClients
.custom()
+ .disableRedirectHandling()
.setSSLContext(new SSLContextBuilder().loadTrustMaterial(null,
TrustAllStrategy.INSTANCE).build())
.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE)
.build();
}
- protected HttpPost getBasicHttpPostRequest() throws URISyntaxException {
- final URI uri = new URI(webhook.getPayloadUrl());
+ protected HttpPost getBasicHttpPostRequest(final URI uri) {
HttpPost request = new HttpPost();
RequestConfig.Builder requestConfig = RequestConfig.custom();
requestConfig.setConnectTimeout(deliveryTimeout * 1000);
@@ -143,13 +150,19 @@ public class WebhookDeliveryThread implements Runnable {
this.headers = StringUtils.join(headers, "\n");
}
- public WebhookDeliveryThread(Webhook webhook, Event event,
+ public WebhookDeliveryThread(Webhook webhook, Event event, URI uri,
AsyncCompletionCallback<WebhookDeliveryResult> callback) {
this.webhook = webhook;
this.event = event;
+ this.payloadUri = uri;
this.callback = callback;
}
+ public WebhookDeliveryThread(Webhook webhook, Event event,
+ AsyncCompletionCallback<WebhookDeliveryResult> callback) {
+ this(webhook, event, null, callback);
+ }
+
public void setDeliveryTries(int deliveryTries) {
this.deliveryTries = deliveryTries;
}
@@ -158,13 +171,29 @@ public class WebhookDeliveryThread implements Runnable {
this.deliveryTimeout = deliveryTimeout;
}
+ public void setDestinationBlocklist(String destinationBlocklist) {
+ this.destinationBlocklist = destinationBlocklist;
+ }
+
+ public void setBlockLocalAddresses(boolean blockLocalAddresses) {
+ this.blockLocalAddresses = blockLocalAddresses;
+ }
+
+ public void setAllowRedirects(boolean allowRedirects) {
+ this.allowRedirects = allowRedirects;
+ }
+
+ public void setAllowHttp(boolean allowHttp) {
+ this.allowHttp = allowHttp;
+ }
+
@Override
public void run() {
- LOGGER.debug("Delivering event: {} for {}", event.getEventType(), webhook);
if (event == null) {
LOGGER.warn("Invalid event received for delivering to {}", webhook);
return;
}
+ LOGGER.debug("Delivering event: {} for {}", event.getEventType(), webhook);
payload = event.getDescription();
LOGGER.trace("Payload: {}", payload);
int attempt = 0;
@@ -176,6 +205,10 @@ public class WebhookDeliveryThread implements Runnable {
callback.complete(new WebhookDeliveryResult(headers, payload, success, response, new Date()));
return;
}
+ if (payloadUri == null) {
+ payloadUri = WebhookUrlValidator.validateWebhookDestinationUrl(webhook.getPayloadUrl(), allowHttp,
+ destinationBlocklist, blockLocalAddresses);
+ }
while (attempt < deliveryTries) {
attempt++;
if (delivery(attempt)) {
@@ -188,7 +221,7 @@ public class WebhookDeliveryThread implements Runnable {
protected void updateResponseFromRequest(HttpEntity entity) {
try {
- this.response = EntityUtils.toString(entity, StandardCharsets.UTF_8);
+ this.response = EntityUtils.toString(entity, StandardCharsets.UTF_8);
} catch (IOException e) {
LOGGER.error("Failed to parse response for event: {} for {}",
event.getEventType(), webhook);
@@ -199,30 +232,66 @@ public class WebhookDeliveryThread implements Runnable {
protected boolean delivery(int attempt) {
startTime = new Date();
try {
- HttpPost request = getBasicHttpPostRequest();
- StringEntity input = new StringEntity(payload,
- isValidJson(payload) ? ContentType.APPLICATION_JSON : ContentType.TEXT_PLAIN);
- request.setEntity(input);
- updateRequestHeaders(request);
- LOGGER.trace("Delivering event: {} for {} with timeout: {}, " +
- "attempt #{}", event.getEventType(), webhook,
- deliveryTimeout, attempt);
- final CloseableHttpResponse response = httpClient.execute(request);
- updateResponseFromRequest(response.getEntity());
- if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
- LOGGER.trace("Successfully delivered event: {} for {}",
- event.getEventType(), webhook);
- return true;
+ int redirectsFollowed = 0;
+ URI currentUri = payloadUri;
+ while (true) {
+ HttpPost request = getBasicHttpPostRequest(currentUri);
+ StringEntity input = new StringEntity(payload,
+ isValidJson(payload) ? ContentType.APPLICATION_JSON : ContentType.TEXT_PLAIN);
+ request.setEntity(input);
+ updateRequestHeaders(request);
+ LOGGER.trace("Delivering event: {} for {} with timeout: {}, attempt #{}, uri={}",
+ event.getEventType(), webhook, deliveryTimeout, attempt, currentUri);
+ try (CloseableHttpResponse response = httpClient.execute(request)) {
+ updateResponseFromRequest(response.getEntity());
+ int statusCode = response.getStatusLine().getStatusCode();
+ if (statusCode == HttpStatus.SC_OK) {
+ LOGGER.trace("Successfully delivered event: {} for {}",
+ event.getEventType(), webhook);
+ return true;
+ }
+ if (!isRedirectStatus(statusCode)) {
+ return false;
+ }
+ if (!allowRedirects) {
+ this.response = String.format(
+ "Delivery failed due to redirect status code %s while redirects are disabled",
+ statusCode);
+ return false;
+ }
+ Header locationHeader = response.getFirstHeader(HttpHeaders.LOCATION);
+ if (locationHeader == null || StringUtils.isBlank(locationHeader.getValue())) {
+ this.response = "Delivery failed due to redirect response without location header";
+ return false;
+ }
+ redirectsFollowed++;
+ if (redirectsFollowed > MAX_REDIRECT_HOPS) {
+ this.response = String.format(
+ "Delivery failed due to too many redirect hops (>%s)",
+ MAX_REDIRECT_HOPS);
+ return false;
+ }
+ currentUri = WebhookUrlValidator.validateWebhookDestinationURI(
+ currentUri.resolve(locationHeader.getValue()),
+ allowHttp,destinationBlocklist, blockLocalAddresses);
+ }
}
- } catch (URISyntaxException | IOException | DecoderException | NoSuchAlgorithmException |
- InvalidKeyException e) {
+ } catch (IOException | DecoderException | NoSuchAlgorithmException | InvalidKeyException e) {
LOGGER.warn("Failed to deliver {}, in attempt #{} due to: {}",
webhook, attempt, e.getMessage());
response = String.format("Failed due to : %s", e.getMessage());
+ } catch (InvalidParameterValueException e) {
+ LOGGER.warn("Failed to deliver {}, in attempt #{} due to security policy: {}",
+ webhook, attempt, e.getMessage());
+ response = String.format("Failed due to : %s", e.getMessage());
}
return false;
}
+ protected boolean isRedirectStatus(final int statusCode) {
+ return statusCode >= 300 && statusCode < 400;
+ }
+
public static String generateHMACSignature(String data, String key)
throws InvalidKeyException, NoSuchAlgorithmException, DecoderException {
Mac mac = Mac.getInstance("HMACSHA256");
diff --git a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookService.java b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookService.java
index 5a5aced288d..ed8defd27c0 100644
--- a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookService.java
+++ b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookService.java
@@ -17,6 +17,8 @@
package org.apache.cloudstack.mom.webhook;
+import java.net.URI;
+
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.framework.config.Configurable;
import org.apache.cloudstack.framework.events.Event;
@@ -42,6 +44,38 @@ public interface WebhookService extends PluggableService, Configurable {
"Size of the thread pool for webhook deliveries",
false, ConfigKey.Scope.Global);
+ ConfigKey<String> WebhookDeliveryBlocklist = new ConfigKey<>("Advanced", String.class,
+ "webhook.delivery.blocklist",
+ "0.0.0.0/8,10.0.0.0/8,100.64.0.0/10,127.0.0.0/8,169.254.0.0/16,172.16.0.0/12,"
+ + "192.0.0.0/24,192.0.2.0/24,192.88.99.0/24,192.168.0.0/16,198.18.0.0/15,"
+ + "198.51.100.0/24,203.0.113.0/24,224.0.0.0/4,240.0.0.0/4,"
+ + "::1/128,::/128,::ffff:0:0/96,64:ff9b::/96,64:ff9b:1::/48,100::/64,"
+ + "2001::/32,2001:db8::/32,2002::/16,fc00::/7,fe80::/10,ff00::/8",
+ "Comma-separated list of IPv4/IPv6 CIDR ranges where webhook deliveries are prohibited "
+ + "from accessing. Validation is performed against the resolved destination IP "
+ + "addresses.",
+ true, ConfigKey.Scope.Domain);
+
+ ConfigKey<Boolean> WebhookDeliveryBlockLocalAddresses = new ConfigKey<>("Hidden", Boolean.class,
+ "webhook.delivery.block.local.addresses", "true",
+ "Whether webhook deliveries are prohibited from accessing IP addresses assigned to "
+ + "the local management server. Validation is performed against resolved "
+ + "destination IP addresses.",
+ true, ConfigKey.Scope.Global);
+
+ ConfigKey<Boolean> WebhookDeliveryAllowRedirects = new ConfigKey<>("Advanced", Boolean.class,
+ "webhook.delivery.allow.redirects", "false",
+ "Whether webhook deliveries are allowed to follow HTTP redirects. If enabled, "
+ + "each redirect target is validated against the destination blocklist and "
+ + "local management server address restrictions, when enabled.",
+ true, ConfigKey.Scope.Domain);
+
+ ConfigKey<Boolean> WebhookDeliveryAllowHttp = new ConfigKey<>("Advanced", Boolean.class,
+ "webhook.delivery.allow.http", "false",
+ "Whether unencrypted HTTP URLs are allowed as webhook destinations. When false, "
+ + "only HTTPS URLs are permitted.",
+ true, ConfigKey.Scope.Domain);
+
ConfigKey<Integer> WebhookDeliveriesLimit = new ConfigKey<>("Advanced", Integer.class,
"webhook.deliveries.limit", "10",
"Limit for the number of deliveries to keep in DB per webhook",
@@ -57,7 +91,9 @@ public interface WebhookService extends PluggableService, Configurable {
"Interval (in seconds) for cleaning up webhook deliveries",
false, ConfigKey.Scope.Global);
+
+
void handleEvent(Event event) throws EventBusException;
- WebhookDelivery executeWebhookDelivery(WebhookDelivery delivery, Webhook webhook, String payload)
+ WebhookDelivery executeWebhookDelivery(WebhookDelivery delivery, Webhook webhook, String payload, URI uri)
throws CloudRuntimeException;
}
diff --git a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookServiceImpl.java b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookServiceImpl.java
index 97d00c45e4d..002916fef9d 100644
--- a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookServiceImpl.java
+++ b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookServiceImpl.java
@@ -17,6 +17,7 @@
package org.apache.cloudstack.mom.webhook;
+import java.net.URI;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -83,7 +84,26 @@ public class WebhookServiceImpl extends ManagerBase implements WebhookService, W
@Inject
AccountManager accountManager;
- protected WebhookDeliveryThread getDeliveryJob(Event event, Webhook webhook, Pair<Integer, Integer> configs) {
+ static class DeliveryConfig {
+ final int tries;
+ final int timeout;
+ final String blocklist;
+ final boolean blockLocalAddresses;
+ final boolean allowRedirects;
+ final boolean allowHttp;
+
+ DeliveryConfig(int tries, int timeout, String blocklist, boolean blockLocalAddresses,
+ boolean allowRedirects, boolean allowHttp) {
+ this.tries = tries;
+ this.timeout = timeout;
+ this.blocklist = blocklist;
+ this.blockLocalAddresses = blockLocalAddresses;
+ this.allowRedirects = allowRedirects;
+ this.allowHttp = allowHttp;
+ }
+ }
+
+ protected WebhookDeliveryThread getDeliveryJob(Event event, Webhook webhook, DeliveryConfig config) {
WebhookDeliveryThread.WebhookDeliveryContext<WebhookDeliveryThread.WebhookDeliveryResult> context =
new WebhookDeliveryThread.WebhookDeliveryContext<>(null, event.getEventId(), webhook.getId());
AsyncCallbackDispatcher<WebhookServiceImpl, WebhookDeliveryThread.WebhookDeliveryResult> caller =
@@ -92,8 +112,12 @@ public class WebhookServiceImpl extends ManagerBase implements WebhookService, W
.setContext(context);
WebhookDeliveryThread job = new WebhookDeliveryThread(webhook, event, caller);
job = ComponentContext.inject(job);
- job.setDeliveryTries(configs.first());
- job.setDeliveryTimeout(configs.second());
+ job.setDeliveryTries(config.tries);
+ job.setDeliveryTimeout(config.timeout);
+ job.setDestinationBlocklist(config.blocklist);
+ job.setBlockLocalAddresses(config.blockLocalAddresses);
+ job.setAllowRedirects(config.allowRedirects);
+ job.setAllowHttp(config.allowHttp);
return job;
}
@@ -113,21 +137,26 @@ public class WebhookServiceImpl extends ManagerBase implements WebhookService, W
}
List<WebhookVO> webhooks =
webhookDao.listByEnabledForDelivery(event.getResourceAccountId(), domainIds);
- Map<Long, Pair<Integer, Integer>> domainConfigs = new HashMap<>();
+ Map<Long, DeliveryConfig> domainConfigs = new HashMap<>();
for (WebhookVO webhook : webhooks) {
if (!domainConfigs.containsKey(webhook.getDomainId())) {
domainConfigs.put(webhook.getDomainId(),
- new Pair<>(WebhookDeliveryTries.valueIn(webhook.getDomainId()),
- WebhookDeliveryTimeout.valueIn(webhook.getDomainId())));
+ new DeliveryConfig(
+ WebhookDeliveryTries.valueIn(webhook.getDomainId()),
+ WebhookDeliveryTimeout.valueIn(webhook.getDomainId()),
+ WebhookDeliveryBlocklist.valueIn(webhook.getDomainId()),
+ WebhookDeliveryBlockLocalAddresses.value(),
+ WebhookDeliveryAllowRedirects.valueIn(webhook.getDomainId()),
+ WebhookDeliveryAllowHttp.valueIn(webhook.getDomainId())));
}
- Pair<Integer, Integer> configs = domainConfigs.get(webhook.getDomainId());
- WebhookDeliveryThread job = getDeliveryJob(event, webhook, configs);
+ DeliveryConfig config = domainConfigs.get(webhook.getDomainId());
+ WebhookDeliveryThread job = getDeliveryJob(event, webhook, config);
jobs.add(job);
}
return jobs;
}
- protected Runnable getManualDeliveryJob(WebhookDelivery existingDelivery, Webhook webhook, String payload,
+ protected Runnable getManualDeliveryJob(WebhookDelivery existingDelivery, Webhook webhook, String payload, URI uri,
AsyncCallFuture<WebhookDeliveryThread.WebhookDeliveryResult> future) {
if (StringUtils.isBlank(payload)) {
payload = "{ \"CloudStack\": \"works!\" }";
@@ -160,9 +189,13 @@ public class WebhookServiceImpl extends ManagerBase implements WebhookService, W
AsyncCallbackDispatcher.create(this);
caller.setCallback(caller.getTarget().manualDeliveryCompleteCallback(null, null))
.setContext(context);
- WebhookDeliveryThread job = new WebhookDeliveryThread(webhook, event, caller);
+ WebhookDeliveryThread job = new WebhookDeliveryThread(webhook, event, uri, caller);
job.setDeliveryTries(WebhookDeliveryTries.valueIn(webhook.getDomainId()));
job.setDeliveryTimeout(WebhookDeliveryTimeout.valueIn(webhook.getDomainId()));
+ job.setDestinationBlocklist(WebhookDeliveryBlocklist.valueIn(webhook.getDomainId()));
+ job.setBlockLocalAddresses(WebhookDeliveryBlockLocalAddresses.value());
+ job.setAllowRedirects(WebhookDeliveryAllowRedirects.valueIn(webhook.getDomainId()));
+ job.setAllowHttp(WebhookDeliveryAllowHttp.valueIn(webhook.getDomainId()));
return job;
}
@@ -246,6 +279,10 @@ public class WebhookServiceImpl extends ManagerBase implements WebhookService, W
WebhookDeliveryTimeout,
WebhookDeliveryTries,
WebhookDeliveryThreadPoolSize,
+ WebhookDeliveryBlocklist,
+ WebhookDeliveryBlockLocalAddresses,
+ WebhookDeliveryAllowRedirects,
+ WebhookDeliveryAllowHttp,
WebhookDeliveriesLimit,
WebhookDeliveriesCleanupInitialDelay,
WebhookDeliveriesCleanupInterval
@@ -271,10 +308,10 @@ public class WebhookServiceImpl extends ManagerBase implements WebhookService, W
}
@Override
- public WebhookDelivery executeWebhookDelivery(WebhookDelivery delivery, Webhook webhook, String payload)
+ public WebhookDelivery executeWebhookDelivery(WebhookDelivery delivery, Webhook webhook, String payload, URI uri)
throws CloudRuntimeException {
AsyncCallFuture<WebhookDeliveryThread.WebhookDeliveryResult> future = new AsyncCallFuture<>();
- Runnable job = getManualDeliveryJob(delivery, webhook, payload, future);
+ Runnable job = getManualDeliveryJob(delivery, webhook, payload, uri, future);
webhookJobExecutor.submit(job);
WebhookDeliveryThread.WebhookDeliveryResult result = null;
WebhookDeliveryVO webhookDeliveryVO;
diff --git a/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookUrlValidator.java b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookUrlValidator.java
new file mode 100644
index 00000000000..c0fea96b665
--- /dev/null
+++ b/plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookUrlValidator.java
@@ -0,0 +1,123 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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.apache.cloudstack.mom.webhook;
+
+import java.net.InetAddress;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.UnknownHostException;
+import java.util.Arrays;
+
+import org.apache.commons.lang3.StringUtils;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.rest.HttpConstants;
+
+public final class WebhookUrlValidator {
+
+ private WebhookUrlValidator() {
+ }
+
+ public static URI validateWebhookDestinationUrl(final String payloadUrl, final boolean allowHttp,
+ final String blocklist, final boolean blockLocalAddresses) {
+ return validateWebhookDestinationURI(URI.create(payloadUrl), allowHttp, blocklist, blockLocalAddresses);
+ }
+
+ public static URI validateWebhookDestinationURI(final URI uri, final boolean allowHttp,
+ final String blocklist, final boolean blockLocalAddresses) {
+ validateScheme(uri, allowHttp);
+ InetAddress[] resolved = validateResolvedAddresses(uri, blocklist, blockLocalAddresses);
+ return buildResolvedUri(uri, resolved[0]);
+ }
+
+ private static URI buildResolvedUri(final URI uri, final InetAddress address) {
+ try {
+ return new URI(uri.getScheme(), uri.getUserInfo(), address.getHostAddress(), uri.getPort(),
+ uri.getPath(), uri.getQuery(), uri.getFragment());
+ } catch (URISyntaxException e) {
+ throw new InvalidParameterValueException(
+ String.format("Failed to build resolved webhook payload URL from [%s]", uri));
+ }
+ }
+
+ public static void validateScheme(final URI uri, final boolean allowHttp) {
+ final String scheme = uri.getScheme();
+ if (HttpConstants.HTTPS.equalsIgnoreCase(scheme)) {
+ return;
+ }
+ if (allowHttp && "http".equalsIgnoreCase(scheme)) {
+ return;
+ }
+ if (allowHttp) {
+ throw new InvalidParameterValueException(
+ String.format("Unsupported URL scheme [%s], only HTTP/HTTPS are supported", scheme));
+ }
+ throw new InvalidParameterValueException(
+ String.format("Only HTTPS webhook payload URLs are allowed, got: %s", uri));
+ }
+
+ public static InetAddress[] validateResolvedAddresses(final URI uri, final String blocklist,
+ final boolean blockLocalAddresses) {
+ final String host = uri.getHost();
+ if (StringUtils.isBlank(host)) {
+ throw new InvalidParameterValueException(
+ String.format("Invalid webhook payload URL host in [%s]", uri));
+ }
+
+ final InetAddress[] resolved;
+ try {
+ resolved = InetAddress.getAllByName(host);
+ } catch (UnknownHostException e) {
+ throw new InvalidParameterValueException(
+ String.format("Failed to resolve webhook payload URL host [%s]", host));
+ }
+
+ if (resolved.length == 0) {
+ throw new InvalidParameterValueException(
+ String.format("Failed to resolve webhook payload URL host [%s]", host));
+ }
+
+ final String[] blockedCidrs = getNormalizedBlocklist(blocklist);
+ for (InetAddress address : resolved) {
+ if ((blockLocalAddresses && isLocalManagementServerAddress(address)) ||
+ NetUtils.isIpInCidrList(address, blockedCidrs)) {
+ throw new InvalidParameterValueException(
+ String.format("Webhook payload URL [%s] resolves to a blocked IP address", uri));
+ }
+ }
+ return resolved;
+ }
+
+ static boolean isLocalManagementServerAddress(final InetAddress address) {
+ return address != null
+ && (address.isAnyLocalAddress()
+ || address.isLoopbackAddress()
+ || NetUtils.isLocalAddress(address));
+ }
+
+ public static String[] getNormalizedBlocklist(final String blocklist) {
+ if (StringUtils.isBlank(blocklist)) {
+ return new String[0];
+ }
+ return Arrays.stream(blocklist.split(","))
+ .map(String::trim)
+ .filter(StringUtils::isNotBlank)
+ .toArray(String[]::new);
+ }
+}
diff --git a/plugins/event-bus/webhook/src/test/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThreadTest.java b/plugins/event-bus/webhook/src/test/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThreadTest.java
index 3be8dee5c2e..68c6ce63cda 100644
--- a/plugins/event-bus/webhook/src/test/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThreadTest.java
+++ b/plugins/event-bus/webhook/src/test/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThreadTest.java
@@ -59,4 +59,11 @@ public class WebhookDeliveryThreadTest {
webhookDeliveryThread.setDeliveryTries(tries);
Assert.assertEquals(tries, ReflectionTestUtils.getField(webhookDeliveryThread, "deliveryTries"));
}
+
+ @Test
+ public void testIsRedirectStatus() {
+ Assert.assertTrue(webhookDeliveryThread.isRedirectStatus(301));
+ Assert.assertTrue(webhookDeliveryThread.isRedirectStatus(308));
+ Assert.assertFalse(webhookDeliveryThread.isRedirectStatus(200));
+ }
}
diff --git a/plugins/event-bus/webhook/src/test/java/org/apache/cloudstack/mom/webhook/WebhookUrlValidatorTest.java b/plugins/event-bus/webhook/src/test/java/org/apache/cloudstack/mom/webhook/WebhookUrlValidatorTest.java
new file mode 100644
index 00000000000..8ea84733d03
--- /dev/null
+++ b/plugins/event-bus/webhook/src/test/java/org/apache/cloudstack/mom/webhook/WebhookUrlValidatorTest.java
@@ -0,0 +1,100 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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.apache.cloudstack.mom.webhook;
+
+import java.net.InetAddress;
+import java.net.URI;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.net.NetUtils;
+
+public class WebhookUrlValidatorTest {
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testValidateWebhookDestinationUrlRejectsHttpWhenDisallowed() {
+ WebhookUrlValidator.validateWebhookDestinationUrl("http://8.8.8.8/hook", false, "", true);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testValidateWebhookDestinationUrlRejectsBlockedAddress() {
+ WebhookUrlValidator.validateWebhookDestinationUrl("https://127.0.0.1/hook", true, "127.0.0.0/8", true);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testValidateWebhookDestinationUrlRejectsLoopbackWhenNotInBlocklist() {
+ WebhookUrlValidator.validateWebhookDestinationUrl("https://127.0.0.1/hook", true, "", true);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void testValidateWebhookDestinationUrlFailsClosedOnResolutionFailure() {
+ WebhookUrlValidator.validateWebhookDestinationUrl("https://nonexistent.invalid/hook", true, "", true);
+ }
+
+ @Test
+ public void testValidateWebhookDestinationUrlRejectsLocalManagementServerAddressWhenNotInBlocklist() throws Exception {
+ InetAddress address = InetAddress.getByName("8.8.8.8");
+
+ try (MockedStatic<NetUtils> netUtilsMock = Mockito.mockStatic(NetUtils.class, Mockito.CALLS_REAL_METHODS)) {
+ netUtilsMock.when(() -> NetUtils.isLocalAddress(address)).thenReturn(true);
+
+ try {
+ WebhookUrlValidator.validateWebhookDestinationUrl("https://8.8.8.8/hook", false, "", true);
+ Assert.fail("Expected InvalidParameterValueException");
+ } catch (InvalidParameterValueException e) {
+ Assert.assertTrue(e.getMessage().contains("blocked IP address"));
+ Assert.assertTrue(e.getMessage().contains("8.8.8.8"));
+ }
+ }
+ }
+
+ @Test
+ public void testValidateWebhookDestinationUrlAllowsLocalManagementServerAddressWhenDisabled() throws Exception {
+ InetAddress address = InetAddress.getByName("8.8.8.8");
+
+ try (MockedStatic<NetUtils> netUtilsMock = Mockito.mockStatic(NetUtils.class, Mockito.CALLS_REAL_METHODS)) {
+ netUtilsMock.when(() -> NetUtils.isLocalAddress(address)).thenReturn(true);
+
+ URI resolvedUri = WebhookUrlValidator.validateWebhookDestinationUrl("https://8.8.8.8/hook", false, "", false);
+ Assert.assertEquals(URI.create("https://8.8.8.8/hook"), resolvedUri);
+ }
+ }
+
+ @Test
+ public void testValidateWebhookDestinationUrlAcceptsAllowedHttpsIpLiteral() {
+ URI resolvedUri = WebhookUrlValidator.validateWebhookDestinationUrl("https://8.8.8.8/hook", false,
+ "127.0.0.0/8,::1/128", true);
+ Assert.assertEquals(URI.create("https://8.8.8.8/hook"), resolvedUri);
+ }
+
+ @Test
+ public void testValidateWebhookDestinationUrlAllowsLoopbackWhenLocalAddressBlockingDisabled() {
+ URI resolvedUri = WebhookUrlValidator.validateWebhookDestinationUrl("https://127.0.0.1/hook", true, "", false);
+ Assert.assertEquals(URI.create("https://127.0.0.1/hook"), resolvedUri);
+ }
+
+ @Test
+ public void testGetNormalizedBlocklist() {
+ String[] cidrs = WebhookUrlValidator.getNormalizedBlocklist(" 127.0.0.0/8, ::1/128 ,, 192.168.0.0/16 ");
+ Assert.assertArrayEquals(new String[] {"127.0.0.0/8", "::1/128", "192.168.0.0/16"}, cidrs);
+ }
+}
diff --git a/test/integration/smoke/test_webhook_delivery.py b/test/integration/smoke/test_webhook_delivery.py
index 00178bd03ff..f361d2b7760 100644
--- a/test/integration/smoke/test_webhook_delivery.py
+++ b/test/integration/smoke/test_webhook_delivery.py
@@ -21,7 +21,8 @@ from marvin.cloudstackTestCase import cloudstackTestCase
from marvin.lib.base import (Account,
Domain,
Webhook,
- SSHKeyPair)
+ SSHKeyPair,
+ Configurations)
from marvin.lib.common import (get_domain,
get_zone)
from marvin.lib.utils import (random_gen)
@@ -67,6 +68,8 @@ class WebhookReceiver(BaseHTTPRequestHandler):
class TestWebhookDelivery(cloudstackTestCase):
+ original_config_values = {}
+
@classmethod
def setUpClass(cls):
testClient = super(TestWebhookDelivery, cls).getClsTestClient()
@@ -103,6 +106,7 @@ class TestWebhookDelivery(cloudstackTestCase):
except Exception: pass
cls.server = HTTPServer(('0.0.0.0', cls.server_port), WebhookReceiver)
_thread.start_new_thread(startMgmtServer, ("webhook-receiver", cls.server,))
+ cls.manage_webhook_test_configurations()
cls._cleanup = []
@@ -112,8 +116,69 @@ class TestWebhookDelivery(cloudstackTestCase):
cls.server.socket.close()
global deliveries_received
deliveries_received = []
+ cls.manage_webhook_test_configurations(restore=True)
super(TestWebhookDelivery, cls).tearDownClass()
+ @classmethod
+ def manage_webhook_test_configurations(cls, restore=False):
+ """
+ Manage configuration values required for the webhook integration tests.
+
+ During class setup, stores original values and applies test-specific overrides.
+ During class teardown, restores the original values.
+
+ More configurations can be easily added here later by extending the
+ configuration_updates dictionary.
+ """
+ configuration_updates = {
+ "webhook.delivery.allow.http": "true",
+ "webhook.delivery.blocklist": "1.2.3.4/32"
+ }
+
+ if restore:
+ for config_name, original_value in cls.original_config_values.items():
+ if original_value is None:
+ continue
+ try:
+ Configurations.update(
+ cls.apiclient,
+ name=config_name,
+ value=original_value
+ )
+ cls.logger.debug("Restored configuration %s to original value: %s" % (config_name, original_value))
+ except Exception as e:
+ cls.logger.warning("Error restoring configuration %s: %s" % (config_name, str(e)))
+ cls.original_config_values.clear()
+ return
+
+ cls.original_config_values.clear()
+ for config_name, config_value in configuration_updates.items():
+ try:
+ configs = Configurations.list(
+ cls.apiclient,
+ name=config_name
+ )
+ if configs:
+ original_value = configs[0].value
+ cls.original_config_values[config_name] = original_value
+ cls.logger.debug("Stored original value for %s: %s" % (config_name, original_value))
+ else:
+ cls.logger.debug("Configuration %s not found" % config_name)
+ cls.original_config_values[config_name] = None
+ except Exception as e:
+ cls.logger.debug("Error retrieving configuration %s: %s" % (config_name, str(e)))
+ cls.original_config_values[config_name] = None
+
+ try:
+ Configurations.update(
+ cls.apiclient,
+ name=config_name,
+ value=config_value
+ )
+ cls.logger.debug("Updated configuration %s to %s" % (config_name, config_value))
+ except Exception as e:
+ cls.logger.warning("Error updating configuration %s: %s" % (config_name, str(e)))
+
def setUp(self):
self.cleanup = []
self.domain1 = Domain.create(
diff --git a/test/integration/smoke/test_webhook_lifecycle.py b/test/integration/smoke/test_webhook_lifecycle.py
index 2d1f322be6b..7ff9d18b94a 100644
--- a/test/integration/smoke/test_webhook_lifecycle.py
+++ b/test/integration/smoke/test_webhook_lifecycle.py
@@ -22,7 +22,8 @@ from marvin.cloudstackAPI import (listEvents)
from marvin.lib.base import (Account,
Domain,
Webhook,
- SSHKeyPair)
+ SSHKeyPair,
+ Configurations)
from marvin.lib.common import (get_domain,
get_zone)
from marvin.lib.utils import (random_gen)
@@ -40,6 +41,8 @@ HTTPS_PAYLOAD_URL = "https://smee.io/C9LPa7Ei3iB6Qj2"
class TestWebhooks(cloudstackTestCase):
+ original_config_values = {}
+
@classmethod
def setUpClass(cls):
testClient = super(TestWebhooks, cls).getClsTestClient()
@@ -53,11 +56,73 @@ class TestWebhooks(cloudstackTestCase):
cls._cleanup = []
cls.logger = logging.getLogger('TestWebhooks')
cls.logger.setLevel(logging.DEBUG)
+ cls.manage_webhook_test_configurations()
+ time.sleep(30)
@classmethod
def tearDownClass(cls):
+ cls.manage_webhook_test_configurations(restore=True)
super(TestWebhooks, cls).tearDownClass()
+ @classmethod
+ def manage_webhook_test_configurations(cls, restore=False):
+ """
+ Manage configuration values required for the webhook integration tests.
+
+ During class setup, stores original values and applies test-specific overrides.
+ During class teardown, restores the original values.
+
+ More configurations can be easily added here later by extending the
+ configuration_updates dictionary.
+ """
+ configuration_updates = {
+ "webhook.delivery.allow.http": "true"
+ }
+
+ if restore:
+ for config_name, original_value in cls.original_config_values.items():
+ if original_value is None:
+ continue
+ try:
+ Configurations.update(
+ cls.apiclient,
+ name=config_name,
+ value=original_value
+ )
+ cls.logger.debug("Restored configuration %s to original value: %s" % (config_name, original_value))
+ except Exception as e:
+ cls.logger.warning("Error restoring configuration %s: %s" % (config_name, str(e)))
+ cls.original_config_values.clear()
+ return
+
+ cls.original_config_values.clear()
+ for config_name, config_value in configuration_updates.items():
+ try:
+ configs = Configurations.list(
+ cls.apiclient,
+ name=config_name
+ )
+ if configs:
+ original_value = configs[0].value
+ cls.original_config_values[config_name] = original_value
+ cls.logger.debug("Stored original value for %s: %s" % (config_name, original_value))
+ else:
+ cls.logger.debug("Configuration %s not found" % config_name)
+ cls.original_config_values[config_name] = None
+ except Exception as e:
+ cls.logger.debug("Error retrieving configuration %s: %s" % (config_name, str(e)))
+ cls.original_config_values[config_name] = None
+
+ try:
+ Configurations.update(
+ cls.apiclient,
+ name=config_name,
+ value=config_value
+ )
+ cls.logger.debug("Updated configuration %s to %s" % (config_name, config_value))
+ except Exception as e:
+ cls.logger.warning("Error updating configuration %s: %s" % (config_name, str(e)))
+
def setUp(self):
self.cleanup = []
self.domain1 = Domain.create(
@@ -326,12 +391,16 @@ class TestWebhooks(cloudstackTestCase):
for event in events:
if event.type == "REGISTER.SSH.KEYPAIR":
register_sshkeypair_event_count = register_sshkeypair_event_count + 1
- time.sleep(5)
- list_deliveries = self.webhook.list_deliveries(
- self.userapiclient,
- page=1,
- pagesize=20
- )
+ list_deliveries = None
+ for _ in range(3):
+ list_deliveries = self.webhook.list_deliveries(
+ self.userapiclient,
+ page=1,
+ pagesize=20
+ )
+ if list_deliveries is not None and len(list_deliveries) > 0:
+ break
+ time.sleep(10)
self.assertNotEqual(
list_deliveries,
None,