Skip to content

Commit 1790275

Browse files
committed
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 <abhishek.mrt22@gmail.com>
1 parent e8f200a commit 1790275

9 files changed

Lines changed: 568 additions & 51 deletions

File tree

plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookApiServiceImpl.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
import com.cloud.user.AccountManager;
6262
import com.cloud.utils.Pair;
6363
import com.cloud.utils.Ternary;
64-
import com.cloud.utils.UriUtils;
6564
import com.cloud.utils.component.ManagerBase;
6665
import com.cloud.utils.db.Filter;
6766
import com.cloud.utils.db.SearchBuilder;
@@ -221,10 +220,17 @@ protected void validateWebhookOwnerPayloadUrl(Account owner, String payloadUrl,
221220
return;
222221
}
223222
String error = String.format("Payload URL: %s is already in use by another webhook", payloadUrl);
224-
logger.error(String.format("%s: %s for Account [%s]", error, webhookVO, owner));
223+
logger.error("{}: {} for Account [{}]", error, webhookVO, owner);
225224
throw new InvalidParameterValueException(error);
226225
}
227226

227+
protected URI validatePayloadUrlByDeliverySecurityPolicy(String payloadUrl, long domainId) {
228+
return WebhookUrlValidator.validateWebhookDestinationUrl(payloadUrl,
229+
WebhookService.WebhookDeliveryAllowHttp.valueIn(domainId),
230+
WebhookService.WebhookDeliveryBlocklist.valueIn(domainId),
231+
WebhookService.WebhookDeliveryBlockLocalAddresses.value());
232+
}
233+
228234
@Override
229235
public ListResponse<WebhookResponse> listWebhooks(ListWebhooksCmd cmd) {
230236
final CallContext ctx = CallContext.current();
@@ -338,7 +344,7 @@ public WebhookResponse createWebhook(CreateWebhookCmd cmd) throws CloudRuntimeEx
338344
throw new InvalidParameterValueException("Invalid state specified");
339345
}
340346
}
341-
UriUtils.validateUrl(payloadUrl);
347+
validatePayloadUrlByDeliverySecurityPolicy(payloadUrl, owner.getDomainId());
342348
validateWebhookOwnerPayloadUrl(owner, payloadUrl, null);
343349
URI uri = URI.create(payloadUrl);
344350
if (sslVerification && !HttpConstants.HTTPS.equalsIgnoreCase(uri.getScheme())) {
@@ -421,7 +427,7 @@ public WebhookResponse updateWebhook(UpdateWebhookCmd cmd) throws CloudRuntimeEx
421427
}
422428
URI uri = URI.create(webhook.getPayloadUrl());
423429
if (StringUtils.isNotEmpty(payloadUrl)) {
424-
UriUtils.validateUrl(payloadUrl);
430+
validatePayloadUrlByDeliverySecurityPolicy(payloadUrl, owner.getDomainId());
425431
validateWebhookOwnerPayloadUrl(owner, payloadUrl, webhook);
426432
uri = URI.create(payloadUrl);
427433
webhook.setPayloadUrl(payloadUrl);
@@ -540,8 +546,13 @@ public WebhookDeliveryResponse executeWebhookDelivery(ExecuteWebhookDeliveryCmd
540546
}
541547
webhook = webhookDao.findById(existingDelivery.getWebhookId());
542548
}
549+
URI uri = null;
543550
if (StringUtils.isNotBlank(payloadUrl)) {
544-
UriUtils.validateUrl(payloadUrl);
551+
long domainId = owner.getDomainId();
552+
if (webhook != null) {
553+
domainId = webhook.getDomainId();
554+
}
555+
uri = validatePayloadUrlByDeliverySecurityPolicy(payloadUrl, domainId);
545556
}
546557
if (webhookId != null) {
547558
webhook = webhookDao.findById(webhookId);
@@ -562,7 +573,7 @@ public WebhookDeliveryResponse executeWebhookDelivery(ExecuteWebhookDeliveryCmd
562573
webhook = new WebhookVO(owner.getDomainId(), owner.getId(), payloadUrl, secretKey,
563574
Boolean.TRUE.equals(sslVerification));
564575
}
565-
WebhookDelivery webhookDelivery = webhookService.executeWebhookDelivery(existingDelivery, webhook, payload);
576+
WebhookDelivery webhookDelivery = webhookService.executeWebhookDelivery(existingDelivery, webhook, payload, uri);
566577
if (webhookDelivery.getId() != WebhookDelivery.ID_DUMMY) {
567578
return createWebhookDeliveryResponse(webhookDeliveryJoinDao.findById(webhookDelivery.getId()));
568579
}

plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookDeliveryThread.java

Lines changed: 93 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
import java.io.IOException;
2121
import java.net.URI;
22-
import java.net.URISyntaxException;
2322
import java.nio.charset.StandardCharsets;
2423
import java.security.InvalidKeyException;
2524
import java.security.KeyManagementException;
@@ -40,11 +39,11 @@
4039
import org.apache.cloudstack.storage.command.CommandResult;
4140
import org.apache.commons.codec.DecoderException;
4241
import org.apache.commons.codec.binary.Base64;
43-
import org.apache.commons.httpclient.HttpStatus;
4442
import org.apache.commons.lang3.StringUtils;
4543
import org.apache.http.Header;
4644
import org.apache.http.HttpEntity;
4745
import org.apache.http.HttpHeaders;
46+
import org.apache.http.HttpStatus;
4847
import org.apache.http.client.config.RequestConfig;
4948
import org.apache.http.client.methods.CloseableHttpResponse;
5049
import org.apache.http.client.methods.HttpPost;
@@ -63,13 +62,16 @@
6362
import org.json.JSONException;
6463
import org.json.JSONObject;
6564

65+
import com.cloud.exception.InvalidParameterValueException;
66+
6667
public class WebhookDeliveryThread implements Runnable {
6768
protected static Logger LOGGER = LogManager.getLogger(WebhookDeliveryThread.class);
6869

6970
private static final String HEADER_X_CS_EVENT_ID = "X-CS-Event-ID";
7071
private static final String HEADER_X_CS_EVENT = "X-CS-Event";
7172
private static final String HEADER_X_CS_SIGNATURE = "X-CS-Signature";
7273
private static final String PREFIX_HEADER_USER_AGENT = "CS-Hookshot/";
74+
private static final int MAX_REDIRECT_HOPS = 5;
7375
private final Webhook webhook;
7476
private final Event event;
7577
private CloseableHttpClient httpClient;
@@ -79,6 +81,11 @@ public class WebhookDeliveryThread implements Runnable {
7981
private Date startTime;
8082
private int deliveryTries = 3;
8183
private int deliveryTimeout = 10;
84+
private String destinationBlocklist;
85+
private boolean blockLocalAddresses = true;
86+
private boolean allowRedirects;
87+
private boolean allowHttp;
88+
private URI payloadUri;
8289

8390
AsyncCompletionCallback<WebhookDeliveryResult> callback;
8491

@@ -97,19 +104,19 @@ protected boolean isValidJson(String json) {
97104

98105
protected void setHttpClient() throws NoSuchAlgorithmException, KeyStoreException, KeyManagementException {
99106
if (webhook.isSslVerification()) {
100-
httpClient = HttpClients.createDefault();
107+
httpClient = HttpClients.custom().disableRedirectHandling().build();
101108
return;
102109
}
103110
httpClient = HttpClients
104111
.custom()
112+
.disableRedirectHandling()
105113
.setSSLContext(new SSLContextBuilder().loadTrustMaterial(null,
106114
TrustAllStrategy.INSTANCE).build())
107115
.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE)
108116
.build();
109117
}
110118

111-
protected HttpPost getBasicHttpPostRequest() throws URISyntaxException {
112-
final URI uri = new URI(webhook.getPayloadUrl());
119+
protected HttpPost getBasicHttpPostRequest(final URI uri) {
113120
HttpPost request = new HttpPost();
114121
RequestConfig.Builder requestConfig = RequestConfig.custom();
115122
requestConfig.setConnectTimeout(deliveryTimeout * 1000);
@@ -143,13 +150,19 @@ protected void updateRequestHeaders(HttpPost request) throws DecoderException, N
143150
this.headers = StringUtils.join(headers, "\n");
144151
}
145152

146-
public WebhookDeliveryThread(Webhook webhook, Event event,
153+
public WebhookDeliveryThread(Webhook webhook, Event event, URI uri,
147154
AsyncCompletionCallback<WebhookDeliveryResult> callback) {
148155
this.webhook = webhook;
149156
this.event = event;
157+
this.payloadUri = uri;
150158
this.callback = callback;
151159
}
152160

161+
public WebhookDeliveryThread(Webhook webhook, Event event,
162+
AsyncCompletionCallback<WebhookDeliveryResult> callback) {
163+
this(webhook, event, null, callback);
164+
}
165+
153166
public void setDeliveryTries(int deliveryTries) {
154167
this.deliveryTries = deliveryTries;
155168
}
@@ -158,13 +171,29 @@ public void setDeliveryTimeout(int deliveryTimeout) {
158171
this.deliveryTimeout = deliveryTimeout;
159172
}
160173

174+
public void setDestinationBlocklist(String destinationBlocklist) {
175+
this.destinationBlocklist = destinationBlocklist;
176+
}
177+
178+
public void setBlockLocalAddresses(boolean blockLocalAddresses) {
179+
this.blockLocalAddresses = blockLocalAddresses;
180+
}
181+
182+
public void setAllowRedirects(boolean allowRedirects) {
183+
this.allowRedirects = allowRedirects;
184+
}
185+
186+
public void setAllowHttp(boolean allowHttp) {
187+
this.allowHttp = allowHttp;
188+
}
189+
161190
@Override
162191
public void run() {
163-
LOGGER.debug("Delivering event: {} for {}", event.getEventType(), webhook);
164192
if (event == null) {
165193
LOGGER.warn("Invalid event received for delivering to {}", webhook);
166194
return;
167195
}
196+
LOGGER.debug("Delivering event: {} for {}", event.getEventType(), webhook);
168197
payload = event.getDescription();
169198
LOGGER.trace("Payload: {}", payload);
170199
int attempt = 0;
@@ -176,6 +205,10 @@ public void run() {
176205
callback.complete(new WebhookDeliveryResult(headers, payload, success, response, new Date()));
177206
return;
178207
}
208+
if (payloadUri == null) {
209+
payloadUri = WebhookUrlValidator.validateWebhookDestinationUrl(webhook.getPayloadUrl(), allowHttp,
210+
destinationBlocklist, blockLocalAddresses);
211+
}
179212
while (attempt < deliveryTries) {
180213
attempt++;
181214
if (delivery(attempt)) {
@@ -188,7 +221,7 @@ public void run() {
188221

189222
protected void updateResponseFromRequest(HttpEntity entity) {
190223
try {
191-
this.response = EntityUtils.toString(entity, StandardCharsets.UTF_8);
224+
this.response = EntityUtils.toString(entity, StandardCharsets.UTF_8);
192225
} catch (IOException e) {
193226
LOGGER.error("Failed to parse response for event: {} for {}",
194227
event.getEventType(), webhook);
@@ -199,30 +232,66 @@ protected void updateResponseFromRequest(HttpEntity entity) {
199232
protected boolean delivery(int attempt) {
200233
startTime = new Date();
201234
try {
202-
HttpPost request = getBasicHttpPostRequest();
203-
StringEntity input = new StringEntity(payload,
204-
isValidJson(payload) ? ContentType.APPLICATION_JSON : ContentType.TEXT_PLAIN);
205-
request.setEntity(input);
206-
updateRequestHeaders(request);
207-
LOGGER.trace("Delivering event: {} for {} with timeout: {}, " +
208-
"attempt #{}", event.getEventType(), webhook,
209-
deliveryTimeout, attempt);
210-
final CloseableHttpResponse response = httpClient.execute(request);
211-
updateResponseFromRequest(response.getEntity());
212-
if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
213-
LOGGER.trace("Successfully delivered event: {} for {}",
214-
event.getEventType(), webhook);
215-
return true;
235+
int redirectsFollowed = 0;
236+
URI currentUri = payloadUri;
237+
while (true) {
238+
HttpPost request = getBasicHttpPostRequest(currentUri);
239+
StringEntity input = new StringEntity(payload,
240+
isValidJson(payload) ? ContentType.APPLICATION_JSON : ContentType.TEXT_PLAIN);
241+
request.setEntity(input);
242+
updateRequestHeaders(request);
243+
LOGGER.trace("Delivering event: {} for {} with timeout: {}, attempt #{}, uri={}",
244+
event.getEventType(), webhook, deliveryTimeout, attempt, currentUri);
245+
try (CloseableHttpResponse response = httpClient.execute(request)) {
246+
updateResponseFromRequest(response.getEntity());
247+
int statusCode = response.getStatusLine().getStatusCode();
248+
if (statusCode == HttpStatus.SC_OK) {
249+
LOGGER.trace("Successfully delivered event: {} for {}",
250+
event.getEventType(), webhook);
251+
return true;
252+
}
253+
if (!isRedirectStatus(statusCode)) {
254+
return false;
255+
}
256+
if (!allowRedirects) {
257+
this.response = String.format(
258+
"Delivery failed due to redirect status code %s while redirects are disabled",
259+
statusCode);
260+
return false;
261+
}
262+
Header locationHeader = response.getFirstHeader(HttpHeaders.LOCATION);
263+
if (locationHeader == null || StringUtils.isBlank(locationHeader.getValue())) {
264+
this.response = "Delivery failed due to redirect response without location header";
265+
return false;
266+
}
267+
redirectsFollowed++;
268+
if (redirectsFollowed > MAX_REDIRECT_HOPS) {
269+
this.response = String.format(
270+
"Delivery failed due to too many redirect hops (>%s)",
271+
MAX_REDIRECT_HOPS);
272+
return false;
273+
}
274+
currentUri = WebhookUrlValidator.validateWebhookDestinationURI(
275+
currentUri.resolve(locationHeader.getValue()),
276+
allowHttp,destinationBlocklist, blockLocalAddresses);
277+
}
216278
}
217-
} catch (URISyntaxException | IOException | DecoderException | NoSuchAlgorithmException |
218-
InvalidKeyException e) {
279+
} catch (IOException | DecoderException | NoSuchAlgorithmException | InvalidKeyException e) {
219280
LOGGER.warn("Failed to deliver {}, in attempt #{} due to: {}",
220281
webhook, attempt, e.getMessage());
221282
response = String.format("Failed due to : %s", e.getMessage());
283+
} catch (InvalidParameterValueException e) {
284+
LOGGER.warn("Failed to deliver {}, in attempt #{} due to security policy: {}",
285+
webhook, attempt, e.getMessage());
286+
response = String.format("Failed due to : %s", e.getMessage());
222287
}
223288
return false;
224289
}
225290

291+
protected boolean isRedirectStatus(final int statusCode) {
292+
return statusCode >= 300 && statusCode < 400;
293+
}
294+
226295
public static String generateHMACSignature(String data, String key)
227296
throws InvalidKeyException, NoSuchAlgorithmException, DecoderException {
228297
Mac mac = Mac.getInstance("HMACSHA256");

plugins/event-bus/webhook/src/main/java/org/apache/cloudstack/mom/webhook/WebhookService.java

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package org.apache.cloudstack.mom.webhook;
1919

20+
import java.net.URI;
21+
2022
import org.apache.cloudstack.framework.config.ConfigKey;
2123
import org.apache.cloudstack.framework.config.Configurable;
2224
import org.apache.cloudstack.framework.events.Event;
@@ -42,6 +44,38 @@ public interface WebhookService extends PluggableService, Configurable {
4244
"Size of the thread pool for webhook deliveries",
4345
false, ConfigKey.Scope.Global);
4446

47+
ConfigKey<String> WebhookDeliveryBlocklist = new ConfigKey<>("Advanced", String.class,
48+
"webhook.delivery.blocklist",
49+
"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,"
50+
+ "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,"
51+
+ "198.51.100.0/24,203.0.113.0/24,224.0.0.0/4,240.0.0.0/4,"
52+
+ "::1/128,::/128,::ffff:0:0/96,64:ff9b::/96,64:ff9b:1::/48,100::/64,"
53+
+ "2001::/32,2001:db8::/32,2002::/16,fc00::/7,fe80::/10,ff00::/8",
54+
"Comma-separated list of IPv4/IPv6 CIDR ranges where webhook deliveries are prohibited "
55+
+ "from accessing. Validation is performed against the resolved destination IP "
56+
+ "addresses.",
57+
true, ConfigKey.Scope.Domain);
58+
59+
ConfigKey<Boolean> WebhookDeliveryBlockLocalAddresses = new ConfigKey<>("Hidden", Boolean.class,
60+
"webhook.delivery.block.local.addresses", "true",
61+
"Whether webhook deliveries are prohibited from accessing IP addresses assigned to "
62+
+ "the local management server. Validation is performed against resolved "
63+
+ "destination IP addresses.",
64+
true, ConfigKey.Scope.Global);
65+
66+
ConfigKey<Boolean> WebhookDeliveryAllowRedirects = new ConfigKey<>("Advanced", Boolean.class,
67+
"webhook.delivery.allow.redirects", "false",
68+
"Whether webhook deliveries are allowed to follow HTTP redirects. If enabled, "
69+
+ "each redirect target is validated against the destination blocklist and "
70+
+ "local management server address restrictions, when enabled.",
71+
true, ConfigKey.Scope.Domain);
72+
73+
ConfigKey<Boolean> WebhookDeliveryAllowHttp = new ConfigKey<>("Advanced", Boolean.class,
74+
"webhook.delivery.allow.http", "false",
75+
"Whether unencrypted HTTP URLs are allowed as webhook destinations. When false, "
76+
+ "only HTTPS URLs are permitted.",
77+
true, ConfigKey.Scope.Domain);
78+
4579
ConfigKey<Integer> WebhookDeliveriesLimit = new ConfigKey<>("Advanced", Integer.class,
4680
"webhook.deliveries.limit", "10",
4781
"Limit for the number of deliveries to keep in DB per webhook",
@@ -57,7 +91,9 @@ public interface WebhookService extends PluggableService, Configurable {
5791
"Interval (in seconds) for cleaning up webhook deliveries",
5892
false, ConfigKey.Scope.Global);
5993

94+
95+
6096
void handleEvent(Event event) throws EventBusException;
61-
WebhookDelivery executeWebhookDelivery(WebhookDelivery delivery, Webhook webhook, String payload)
97+
WebhookDelivery executeWebhookDelivery(WebhookDelivery delivery, Webhook webhook, String payload, URI uri)
6298
throws CloudRuntimeException;
6399
}

0 commit comments

Comments
 (0)