Support host deny list for Destinations (#353)

* Support deny list for destinations

* Support deny list for destinations

* Addressed comments and added integ tests

Author: skkosuri-amzn

Diff for: alerting/build.gradle

@@ -73,6 +73,7 @@ dependencies {
 
     compile project(":alerting-core")
     compile project(":alerting-notification")
+    implementation "com.github.seancfoley:ipaddress:5.3.3"
 
     testImplementation "org.jetbrains.kotlin:kotlin-test:${kotlin_version}"
 }
@@ -121,6 +122,10 @@ testClusters.integTest {
     }
 }
 
+testClusters.integTest.nodes.each { node ->
+    node.setting("opendistro.destination.host.deny_list", "[\"10.0.0.0/8\", \"127.0.0.1\"]")
+}
+
 integTest {
     systemProperty 'tests.security.manager', 'false'
     systemProperty 'java.io.tmpdir', es_tmp_dir.absolutePath

Diff for: alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/AlertingPlugin.kt

@@ -254,7 +254,8 @@ internal class AlertingPlugin : PainlessExtension, ActionPlugin, ScriptPlugin, R
                 AlertingSettings.FILTER_BY_BACKEND_ROLES,
                 DestinationSettings.EMAIL_USERNAME,
                 DestinationSettings.EMAIL_PASSWORD,
-                DestinationSettings.ALLOW_LIST
+                DestinationSettings.ALLOW_LIST,
+                DestinationSettings.HOST_DENY_LIST
             )
     }
 

Diff for: alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunner.kt

@@ -52,6 +52,7 @@ import com.amazon.opendistroforelasticsearch.alerting.settings.AlertingSettings.
 import com.amazon.opendistroforelasticsearch.alerting.settings.AlertingSettings.Companion.MOVE_ALERTS_BACKOFF_COUNT
 import com.amazon.opendistroforelasticsearch.alerting.settings.AlertingSettings.Companion.MOVE_ALERTS_BACKOFF_MILLIS
 import com.amazon.opendistroforelasticsearch.alerting.settings.DestinationSettings.Companion.ALLOW_LIST
+import com.amazon.opendistroforelasticsearch.alerting.settings.DestinationSettings.Companion.HOST_DENY_LIST
 import com.amazon.opendistroforelasticsearch.alerting.settings.DestinationSettings.Companion.loadDestinationSettings
 import com.amazon.opendistroforelasticsearch.alerting.util.IndexUtils
 import com.amazon.opendistroforelasticsearch.alerting.util.addUserBackendRolesFilter
@@ -121,6 +122,8 @@ class MonitorRunner(
         BackoffPolicy.exponentialBackoff(MOVE_ALERTS_BACKOFF_MILLIS.get(settings), MOVE_ALERTS_BACKOFF_COUNT.get(settings))
     @Volatile private var allowList = ALLOW_LIST.get(settings)
 
+    @Volatile private var hostDenyList = HOST_DENY_LIST.get(settings)
+
     @Volatile private var destinationSettings = loadDestinationSettings(settings)
     @Volatile private var destinationContextFactory = DestinationContextFactory(client, xContentRegistry, destinationSettings)
 
@@ -537,7 +540,8 @@ class MonitorRunner(
                     actionOutput[MESSAGE_ID] = destination.publish(
                         actionOutput[SUBJECT],
                         actionOutput[MESSAGE]!!,
-                        destinationCtx
+                        destinationCtx,
+                        hostDenyList
                     )
                 }
             }

Diff for: alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/model/destination/Destination.kt

@@ -29,6 +29,7 @@ import com.amazon.opendistroforelasticsearch.alerting.elasticapi.optionalUserFie
 import com.amazon.opendistroforelasticsearch.alerting.model.destination.email.Email
 import com.amazon.opendistroforelasticsearch.alerting.util.DestinationType
 import com.amazon.opendistroforelasticsearch.alerting.util.IndexUtils.Companion.NO_SCHEMA_VERSION
+import com.amazon.opendistroforelasticsearch.alerting.util.isHostInDenylist
 import com.amazon.opendistroforelasticsearch.commons.authuser.User
 import org.apache.logging.log4j.LogManager
 import org.elasticsearch.common.io.stream.StreamInput
@@ -236,7 +237,13 @@ data class Destination(
     }
 
     @Throws(IOException::class)
-    fun publish(compiledSubject: String?, compiledMessage: String, destinationCtx: DestinationContext): String {
+    fun publish(
+        compiledSubject: String?,
+        compiledMessage: String,
+        destinationCtx: DestinationContext,
+        denyHostRanges: List<String>
+    ): String {
+
         val destinationMessage: BaseMessage
         val responseContent: String
         val responseStatusCode: Int
@@ -285,6 +292,7 @@ data class Destination(
             }
         }
 
+        validateDestinationUri(destinationMessage, denyHostRanges)
         val response = Notification.publish(destinationMessage) as DestinationResponse
         responseContent = response.responseContent
         responseStatusCode = response.statusCode
@@ -307,4 +315,10 @@ data class Destination(
         }
         return content
     }
+
+    private fun validateDestinationUri(destinationMessage: BaseMessage, denyHostRanges: List<String>) {
+        if (destinationMessage.isHostInDenylist(denyHostRanges)) {
+            throw IOException("The destination address is invalid.")
+        }
+    }
 }

Diff for: alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/settings/DestinationSettings.kt

@@ -57,6 +57,14 @@ class DestinationSettings {
             Function { key: String -> SecureSetting.secureString(key, null) }
         )
 
+        val HOST_DENY_LIST: Setting<List<String>> = Setting.listSetting(
+                "opendistro.destination.host.deny_list",
+                emptyList<String>(),
+                Function.identity(),
+                Setting.Property.NodeScope,
+                Setting.Property.Final
+        )
+
         fun loadDestinationSettings(settings: Settings): Map<String, SecureDestinationSettings> {
             // Only loading Email Destination settings for now since those are the only secure settings needed.
             // If this logic needs to be expanded to support other Destinations, different groups can be retrieved similar

Diff for: alerting/src/main/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/AlertingUtils.kt

@@ -15,12 +15,16 @@
 
 package com.amazon.opendistroforelasticsearch.alerting.util
 
+import com.amazon.opendistroforelasticsearch.alerting.destination.message.BaseMessage
 import com.amazon.opendistroforelasticsearch.alerting.model.destination.Destination
 import com.amazon.opendistroforelasticsearch.alerting.settings.DestinationSettings
 import com.amazon.opendistroforelasticsearch.commons.authuser.User
 import org.elasticsearch.ElasticsearchStatusException
 import org.elasticsearch.action.ActionListener
 import org.elasticsearch.rest.RestStatus
+import inet.ipaddr.IPAddressString
+import java.net.InetAddress
+import org.elasticsearch.transport.TransportChannel.logger
 
 /**
  * RFC 5322 compliant pattern matching: https://www.ietf.org/rfc/rfc5322.txt
@@ -41,6 +45,18 @@ fun isValidEmail(email: String): Boolean {
 /** Allowed Destinations are ones that are specified in the [DestinationSettings.ALLOW_LIST] setting. */
 fun Destination.isAllowed(allowList: List<String>): Boolean = allowList.contains(this.type.value)
 
+fun BaseMessage.isHostInDenylist(networks: List<String>): Boolean {
+    val ipStr = IPAddressString(this.uri.host)
+    for (network in networks) {
+        val netStr = IPAddressString(network)
+        if (netStr.contains(ipStr)) {
+            logger.error("Host: {} resolves to: {} which is in denylist: {}.", uri.getHost(), InetAddress.getByName(uri.getHost()), netStr)
+            return true
+        }
+    }
+    return false
+}
+
 /**
   1. If filterBy is enabled
      a) Don't allow to create monitor/ destination (throw error) if the logged-on user has no backend roles configured.

Diff for: alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/MonitorRunnerIT.kt

@@ -27,6 +27,7 @@ import com.amazon.opendistroforelasticsearch.alerting.model.Monitor
 import com.amazon.opendistroforelasticsearch.alerting.core.model.SearchInput
 import com.amazon.opendistroforelasticsearch.alerting.model.ActionExecutionResult
 import com.amazon.opendistroforelasticsearch.alerting.model.action.Throttle
+import com.amazon.opendistroforelasticsearch.alerting.model.destination.CustomWebhook
 import com.amazon.opendistroforelasticsearch.alerting.model.destination.Destination
 import com.amazon.opendistroforelasticsearch.alerting.model.destination.email.Email
 import com.amazon.opendistroforelasticsearch.alerting.model.destination.email.Recipient
@@ -653,6 +654,58 @@ class MonitorRunnerIT : AlertingRestTestCase() {
         verifyAlert(alerts.single(), monitor, ACTIVE)
     }
 
+    fun `test execute monitor with custom webhook destination`() {
+        val customWebhook = CustomWebhook("http://15.16.17.18", null, null, 80, null, "PUT", emptyMap(), emptyMap(), null, null)
+        val destination = createDestination(
+                Destination(
+                        type = DestinationType.CUSTOM_WEBHOOK,
+                        name = "testDesination",
+                        user = randomUser(),
+                        lastUpdateTime = Instant.now(),
+                        chime = null,
+                        slack = null,
+                        customWebhook = customWebhook,
+                        email = null
+                ))
+        val action = randomAction(destinationId = destination.id)
+        val trigger = randomTrigger(condition = ALWAYS_RUN, actions = listOf(action))
+        val monitor = createMonitor(randomMonitor(triggers = listOf(trigger)))
+        executeMonitor(adminClient(), monitor.id)
+
+        val alerts = searchAlerts(monitor)
+        assertEquals("Alert not saved", 1, alerts.size)
+        verifyAlert(alerts.single(), monitor, ERROR)
+        Assert.assertTrue(alerts.single().errorMessage?.contains("Connect timed out") as Boolean)
+    }
+
+    fun `test execute monitor with custom webhook destination and denied host`() {
+
+        listOf("http://10.1.1.1", "127.0.0.1").forEach {
+            val customWebhook = CustomWebhook(it, null, null, 80, null, "PUT", emptyMap(), emptyMap(), null, null)
+            val destination = createDestination(
+                    Destination(
+                            type = DestinationType.CUSTOM_WEBHOOK,
+                            name = "testDesination",
+                            user = randomUser(),
+                            lastUpdateTime = Instant.now(),
+                            chime = null,
+                            slack = null,
+                            customWebhook = customWebhook,
+                            email = null
+                    ))
+            val action = randomAction(destinationId = destination.id)
+            val trigger = randomTrigger(condition = ALWAYS_RUN, actions = listOf(action))
+            val monitor = createMonitor(randomMonitor(triggers = listOf(trigger)))
+            executeMonitor(adminClient(), monitor.id)
+
+            val alerts = searchAlerts(monitor)
+            assertEquals("Alert not saved", 1, alerts.size)
+            verifyAlert(alerts.single(), monitor, ERROR)
+
+            Assert.assertTrue(alerts.single().errorMessage?.contains("The destination address is invalid") as Boolean)
+        }
+    }
+
     fun `test execute AD monitor doesn't return search result without user`() {
         // TODO: change to REST API call to test security enabled case
         if (!securityEnabled()) {

Diff for: alerting/src/test/kotlin/com/amazon/opendistroforelasticsearch/alerting/util/AlertingUtilsTests.kt

@@ -0,0 +1,60 @@
+package com.amazon.opendistroforelasticsearch.alerting.util
+
+import com.amazon.opendistroforelasticsearch.alerting.destination.message.BaseMessage
+import com.amazon.opendistroforelasticsearch.alerting.destination.message.CustomWebhookMessage
+import org.elasticsearch.test.ESTestCase
+import java.util.HashMap
+
+class AlertingUtilsTests : ESTestCase() {
+
+    private val HOST_DENY_LIST = listOf(
+            "127.0.0.0/8",
+            "10.0.0.0/8",
+            "172.16.0.0/12",
+            "192.168.0.0/16",
+            "0.0.0.0/8",
+            "9.9.9.9" // ip
+    )
+
+    fun `test ips in denylist`() {
+        val ips = listOf(
+                "127.0.0.1", // 127.0.0.0/8
+                "10.0.0.1", // 10.0.0.0/8
+                "10.11.12.13", // 10.0.0.0/8
+                "172.16.0.1", // "172.16.0.0/12"
+                "192.168.0.1", // 192.168.0.0/16"
+                "0.0.0.1", // 0.0.0.0/8
+                "9.9.9.9"
+        )
+        for (ip in ips) {
+            val bm = createMessageWithHost(ip)
+            assertEquals(true, bm.isHostInDenylist(HOST_DENY_LIST))
+        }
+    }
+
+    fun `test url in denylist`() {
+        val urls = listOf("https://www.amazon.com", "https://mytest.com", "https://mytest.com")
+        for (url in urls) {
+            val bm = createMessageWithURl(url)
+            assertEquals(false, bm.isHostInDenylist(HOST_DENY_LIST))
+        }
+    }
+
+    private fun createMessageWithHost(host: String): BaseMessage {
+        return CustomWebhookMessage.Builder("abc")
+                .withHost(host)
+                .withPath("incomingwebhooks/383c0e2b-d028-44f4-8d38-696754bc4574")
+                .withMessage("{\"Content\":\"Message test\"}")
+                .withMethod("POST")
+                .withQueryParams(HashMap<String, String>()).build()
+    }
+
+    private fun createMessageWithURl(url: String): BaseMessage {
+        return CustomWebhookMessage.Builder("abc")
+                .withUrl(url)
+                .withPath("incomingwebhooks/383c0e2b-d028-44f4-8d38-696754bc4574")
+                .withMessage("{\"Content\":\"Message test\"}")
+                .withMethod("POST")
+                .withQueryParams(HashMap<String, String>()).build()
+    }
+}

Diff for: notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/client/DestinationHttpClient.java

@@ -110,8 +110,7 @@ private CloseableHttpResponse getHttpResponse(BaseMessage message) throws Except
         HttpRequestBase httpRequest;
         if (message instanceof CustomWebhookMessage) {
             CustomWebhookMessage customWebhookMessage = (CustomWebhookMessage) message;
-            uri = buildUri(customWebhookMessage.getUrl(), customWebhookMessage.getScheme(), customWebhookMessage.getHost(),
-                    customWebhookMessage.getPort(), customWebhookMessage.getPath(), customWebhookMessage.getQueryParams());
+            uri = customWebhookMessage.getUri();
             httpRequest = constructHttpRequest(((CustomWebhookMessage) message).getMethod());
             // set headers
             Map<String, String> headerParams = customWebhookMessage.getHeaderParams();
@@ -124,7 +123,7 @@ private CloseableHttpResponse getHttpResponse(BaseMessage message) throws Except
             }
         } else {
              httpRequest = new HttpPost();
-             uri = buildUri(message.getUrl().trim(), null, null, -1, null, null);
+             uri = message.getUri();
         }
 
         httpRequest.setURI(uri);
@@ -149,29 +148,6 @@ private HttpRequestBase constructHttpRequest(String method) {
         }
     }
 
-    private URI buildUri(String endpoint, String scheme, String host,
-                         int port, String path, Map<String, String> queryParams)
-            throws Exception {
-        try {
-            if(Strings.isNullOrEmpty(endpoint)) {
-                logger.info("endpoint empty. Fall back to host:port/path");
-                if (Strings.isNullOrEmpty(scheme)) {
-                    scheme = "https";
-                }
-                URIBuilder uriBuilder = new URIBuilder();
-                if(queryParams != null) {
-                    for (Map.Entry<String, String> e : queryParams.entrySet())
-                        uriBuilder.addParameter(e.getKey(), e.getValue());
-                }
-                return uriBuilder.setScheme(scheme).setHost(host).setPort(port).setPath(path).build();
-            }
-            return new URIBuilder(endpoint).build();
-        } catch (URISyntaxException exception) {
-            logger.error("Error occured while building Uri");
-            throw new IllegalStateException("Error creating URI");
-        }
-    }
-
     public String getResponseString(CloseableHttpResponse response) throws IOException {
             HttpEntity entity = response.getEntity();
             if (entity == null)

Diff for: notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/message/BaseMessage.java

@@ -15,8 +15,13 @@
 
 package com.amazon.opendistroforelasticsearch.alerting.destination.message;
 
+import org.apache.http.client.utils.URIBuilder;
 import org.elasticsearch.common.Strings;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Map;
+
 /**
  * This class holds the generic parameters required for a
  * message.
@@ -68,4 +73,28 @@ public String getUrl() {
         return url;
     }
 
+    public URI getUri() {
+        return buildUri(getUrl().trim(), null, null, -1, null, null);
+    }
+
+    protected URI buildUri(String endpoint, String scheme, String host,
+                         int port, String path, Map<String, String> queryParams) {
+        try {
+            if(Strings.isNullOrEmpty(endpoint)) {
+                if (Strings.isNullOrEmpty(scheme)) {
+                    scheme = "https";
+                }
+                URIBuilder uriBuilder = new URIBuilder();
+                if(queryParams != null) {
+                    for (Map.Entry<String, String> e : queryParams.entrySet())
+                        uriBuilder.addParameter(e.getKey(), e.getValue());
+                }
+                return uriBuilder.setScheme(scheme).setHost(host).setPort(port).setPath(path).build();
+            }
+            return new URIBuilder(endpoint).build();
+        } catch (URISyntaxException exception) {
+            throw new IllegalStateException("Error creating URI");
+        }
+    }
+
 }

Diff for: notification/src/main/java/com/amazon/opendistroforelasticsearch/alerting/destination/message/CustomWebhookMessage.java

@@ -20,6 +20,7 @@
 import org.apache.http.client.methods.HttpPut;
 import org.elasticsearch.common.Strings;
 
+import java.net.URI;
 import java.util.Map;
 
 /**
@@ -217,4 +218,7 @@ public Map<String, String> getHeaderParams() {
         return headerParams;
     }
 
+    public URI getUri() {
+        return buildUri(getUrl(), getScheme(), getHost(), getPort(), getPath(), getQueryParams());
+    }
 }