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 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 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 callback) { this.webhook = webhook; this.event = event; + this.payloadUri = uri; this.callback = callback; } + public WebhookDeliveryThread(Webhook webhook, Event event, + AsyncCompletionCallback 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 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 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 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 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 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 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 context = new WebhookDeliveryThread.WebhookDeliveryContext<>(null, event.getEventId(), webhook.getId()); AsyncCallbackDispatcher 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 webhooks = webhookDao.listByEnabledForDelivery(event.getResourceAccountId(), domainIds); - Map> domainConfigs = new HashMap<>(); + Map 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 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 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 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 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 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,