Resovle host to all ips and check against the deny list (#964)

* resovle host to all ips and check against the deny list Signed-off-by: Amardeepsingh Siglani <[email protected]> fixed build Signed-off-by: Amardeepsingh Siglani <[email protected]> added test Signed-off-by: Amardeepsingh Siglani <[email protected]> added try catch Signed-off-by: Amardeepsingh Siglani <[email protected]> refactored logic to validate host first Signed-off-by: Amardeepsingh Siglani <[email protected]> fixed ktlint Signed-off-by: Amardeepsingh Siglani <[email protected]> * fixed integ tests Signed-off-by: Amardeepsingh Siglani <[email protected]> * throw unknownhostexception instead of illegal argument exception Signed-off-by: Amardeepsingh Siglani <[email protected]> --------- Signed-off-by: Amardeepsingh Siglani <[email protected]> (cherry picked from commit f0668a7)
opensearch-project · Sep 25, 2024 · 4313eb8 · 4313eb8
1 parent cb60d20
commit 4313eb8
Show file tree

Hide file tree

Showing 8 changed files with 112 additions and 15 deletions.
diff --git a/...ions/core-spi/src/main/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpers.kt b/...ions/core-spi/src/main/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpers.kt
@@ -5,13 +5,16 @@
 
 package org.opensearch.notifications.spi.utils
 
+import inet.ipaddr.HostName
 import inet.ipaddr.IPAddressString
 import org.apache.commons.validator.routines.DomainValidator
 import org.apache.http.client.methods.HttpPatch
 import org.apache.http.client.methods.HttpPost
 import org.apache.http.client.methods.HttpPut
+import org.apache.logging.log4j.LogManager
 import org.opensearch.core.common.Strings
 import org.opensearch.notifications.spi.utils.ValidationHelpers.FQDN_REGEX
+import java.lang.Exception
 import java.net.InetAddress
 import java.net.URL
 
@@ -49,13 +52,38 @@ fun isValidUrl(urlString: String): Boolean {
     }
 }
 
+fun getResolvedIps(host: String): List<IPAddressString> {
+    try {
+        val resolvedIps = InetAddress.getAllByName(host)
+        return resolvedIps.map { inetAddress -> IPAddressString(inetAddress.hostAddress) }
+    } catch (e: Exception) {
+        LogManager.getLogger().error("Unable to resolve host ips")
+    }
+
+    return listOf()
+}
+
 fun isHostInDenylist(urlString: String, hostDenyList: List<String>): Boolean {
     val url = URL(urlString)
     if (url.host != null) {
-        val ipStr = IPAddressString(InetAddress.getByName(url.host).hostAddress)
+        val resolvedIpStrings = getResolvedIps(url.host)
+        val hostStr = HostName(url.host)
+
         for (network in hostDenyList) {
-            val netStr = IPAddressString(network)
-            if (netStr.contains(ipStr)) {
+            val denyIpStr = IPAddressString(network)
+            val denyHostStr = HostName(network)
+            val hostInDenyList = denyHostStr.equals(hostStr)
+            var ipInDenyList = false
+
+            for (ipStr in resolvedIpStrings) {
+                if (denyIpStr.contains(ipStr)) {
+                    ipInDenyList = true
+                    break
+                }
+            }
+
+            if (hostInDenyList || ipInDenyList) {
+                LogManager.getLogger().error("${url.host} is denied")
                 return true
             }
         }

diff --git a/...core-spi/src/test/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpersTests.kt b/...core-spi/src/test/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpersTests.kt
@@ -51,13 +51,21 @@ internal class ValidationHelpersTests {
 
     @Test
     fun `test hostname gets resolved to ip for denylist`() {
-        val invalidHost = "invalid.com"
+        val expectedAddressesForInvalidHost = arrayOf(
+            InetAddress.getByName("174.120.0.0"),
+            InetAddress.getByName("10.0.0.1")
+        )
+        val expectedAddressesForValidHost = arrayOf(
+            InetAddress.getByName("174.12.0.0")
+        )
+
         mockkStatic(InetAddress::class)
-        every { InetAddress.getByName(invalidHost).hostAddress } returns "10.0.0.1" // 10.0.0.0/8
+        val invalidHost = "invalid.com"
+        every { InetAddress.getAllByName(invalidHost) } returns expectedAddressesForInvalidHost
         assertEquals(true, isHostInDenylist("https://$invalidHost", hostDenyList))
 
         val validHost = "valid.com"
-        every { InetAddress.getByName(validHost).hostAddress } returns "174.12.0.0"
+        every { InetAddress.getAllByName(validHost) } returns expectedAddressesForValidHost
         assertEquals(false, isHostInDenylist("https://$validHost", hostDenyList))
     }
 

diff --git a/...cations/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt b/...cations/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt
@@ -12,15 +12,24 @@ import org.apache.http.client.methods.HttpPost
 import org.apache.http.client.methods.HttpPut
 import org.apache.logging.log4j.LogManager
 import org.opensearch.core.common.Strings
+import java.lang.Exception
+import java.net.InetAddress
 import java.net.URL
+import java.net.UnknownHostException
 
 fun validateUrl(urlString: String) {
     require(!Strings.isNullOrEmpty(urlString)) { "url is null or empty" }
     require(isValidUrl(urlString)) { "Invalid URL or unsupported" }
 }
 
 fun validateUrlHost(urlString: String, hostDenyList: List<String>) {
-    require(!isHostInDenylist(urlString, hostDenyList)) {
+    val url = URL(urlString)
+
+    if (org.opensearch.notifications.spi.utils.getResolvedIps(url.host).isEmpty()) {
+        throw UnknownHostException("Host could not be resolved to a valid Ip address")
+    }
+
+    require(!org.opensearch.notifications.spi.utils.isHostInDenylist(urlString, hostDenyList)) {
         "Host of url is denied, based on plugin setting [notification.core.http.host_deny_list]"
     }
 }
@@ -35,18 +44,39 @@ fun isValidUrl(urlString: String): Boolean {
     return ("https" == url.protocol || "http" == url.protocol) // Support only http/https, other protocols not supported
 }
 
+@Deprecated("This function is not maintained, use org.opensearch.notifications.spi.utils.isHostInDenylist instead.")
 fun isHostInDenylist(urlString: String, hostDenyList: List<String>): Boolean {
     val url = URL(urlString)
     if (url.host != null) {
-        val ipStr = IPAddressString(url.host)
-        val hostStr = HostName(url.host)
-        for (network in hostDenyList) {
-            val denyIpStr = IPAddressString(network)
-            val denyHostStr = HostName(network)
-            if (denyIpStr.contains(ipStr) || denyHostStr.equals(hostStr)) {
-                LogManager.getLogger().error("${url.host} is denied")
-                return true
+        try {
+            val resolvedIps = InetAddress.getAllByName(url.host)
+            val resolvedIpStrings = resolvedIps.map { inetAddress -> IPAddressString(inetAddress.hostAddress) }
+            val hostStr = HostName(url.host)
+
+            for (network in hostDenyList) {
+                val denyIpStr = IPAddressString(network)
+                val denyHostStr = HostName(network)
+                val hostInDenyList = denyHostStr.equals(hostStr)
+                var ipInDenyList = false
+
+                for (ipStr in resolvedIpStrings) {
+                    if (denyIpStr.contains(ipStr)) {
+                        ipInDenyList = true
+                        break
+                    }
+                }
+
+                if (hostInDenyList || ipInDenyList) {
+                    LogManager.getLogger().error("${url.host} is denied")
+                    return true
+                }
             }
+        } catch (e: UnknownHostException) {
+            LogManager.getLogger().error("Error checking denylist: Unknown host")
+            return false
+        } catch (e: Exception) {
+            LogManager.getLogger().error("Error checking denylist: ${e.message}", e)
+            return false
         }
     }
 

diff --git a/...e/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt b/...e/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt
@@ -5,6 +5,7 @@
 
 package org.opensearch.notifications.core.destinations
 
+import inet.ipaddr.IPAddressString
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkStatic
@@ -52,6 +53,7 @@ internal class ChimeDestinationTests {
         // Stubbing isHostInDenylist() so it doesn't attempt to resolve hosts that don't exist in the unit tests
         mockkStatic("org.opensearch.notifications.spi.utils.ValidationHelpersKt")
         every { org.opensearch.notifications.spi.utils.isHostInDenylist(any(), any()) } returns false
+        every { org.opensearch.notifications.spi.utils.getResolvedIps(any()) } returns listOf(IPAddressString("174.0.0.0"))
     }
 
     @Test

diff --git a/...st/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt b/...st/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt
@@ -5,6 +5,7 @@
 
 package org.opensearch.notifications.core.destinations
 
+import inet.ipaddr.IPAddressString
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkStatic
@@ -63,6 +64,7 @@ internal class CustomWebhookDestinationTests {
         // Stubbing isHostInDenylist() so it doesn't attempt to resolve hosts that don't exist in the unit tests
         mockkStatic("org.opensearch.notifications.spi.utils.ValidationHelpersKt")
         every { org.opensearch.notifications.spi.utils.isHostInDenylist(any(), any()) } returns false
+        every { org.opensearch.notifications.spi.utils.getResolvedIps(any()) } returns listOf(IPAddressString("174.0.0.0"))
     }
 
     @ParameterizedTest(name = "method {0} should return corresponding type of Http request object {1}")

diff --git a/...t/kotlin/org/opensearch/notifications/core/destinations/MicrosoftTeamsDestinationTests.kt b/...t/kotlin/org/opensearch/notifications/core/destinations/MicrosoftTeamsDestinationTests.kt
@@ -5,6 +5,7 @@
 
 package org.opensearch.notifications.core.destinations
 
+import inet.ipaddr.IPAddressString
 import io.mockk.every
 import io.mockk.mockkStatic
 import org.apache.http.client.methods.CloseableHttpResponse
@@ -52,6 +53,7 @@ internal class MicrosoftTeamsDestinationTests {
         // Stubbing isHostInDenylist() so it doesn't attempt to resolve hosts that don't exist in the unit tests
         mockkStatic("org.opensearch.notifications.spi.utils.ValidationHelpersKt")
         every { org.opensearch.notifications.spi.utils.isHostInDenylist(any(), any()) } returns false
+        every { org.opensearch.notifications.spi.utils.getResolvedIps(any()) } returns listOf(IPAddressString("174.0.0.0"))
     }
 
     @Test

diff --git a/...e/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt b/...e/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt
@@ -5,6 +5,7 @@
 
 package org.opensearch.notifications.core.destinations
 
+import inet.ipaddr.IPAddressString
 import io.mockk.every
 import io.mockk.mockkStatic
 import org.apache.http.client.methods.CloseableHttpResponse
@@ -52,6 +53,7 @@ internal class SlackDestinationTests {
         // Stubbing isHostInDenylist() so it doesn't attempt to resolve hosts that don't exist in the unit tests
         mockkStatic("org.opensearch.notifications.spi.utils.ValidationHelpersKt")
         every { org.opensearch.notifications.spi.utils.isHostInDenylist(any(), any()) } returns false
+        every { org.opensearch.notifications.spi.utils.getResolvedIps(any()) } returns listOf(IPAddressString("174.0.0.0"))
     }
 
     @Test

diff --git a/...ns/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt b/...ns/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt
@@ -5,8 +5,11 @@
 
 package org.opensearch.notifications.core.utils
 
+import io.mockk.every
+import io.mockk.mockkStatic
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
+import java.net.InetAddress
 
 internal class ValidationHelpersTests {
 
@@ -44,4 +47,24 @@ internal class ValidationHelpersTests {
             assertEquals(false, isHostInDenylist("https://$url", hostDenyList), "address $url was not supposed to be identified as in the deny list, but was")
         }
     }
+
+    @Test
+    fun `test hostname gets resolved to ip for denylist`() {
+        val expectedAddressesForInvalidHost = arrayOf(
+            InetAddress.getByName("174.120.0.0"),
+            InetAddress.getByName("10.0.0.1")
+        )
+        val expectedAddressesForValidHost = arrayOf(
+            InetAddress.getByName("174.12.0.0")
+        )
+
+        mockkStatic(InetAddress::class)
+        val invalidHost = "invalid.com"
+        every { InetAddress.getAllByName(invalidHost) } returns expectedAddressesForInvalidHost
+        assertEquals(true, isHostInDenylist("https://$invalidHost", hostDenyList))
+
+        val validHost = "valid.com"
+        every { InetAddress.getAllByName(validHost) } returns expectedAddressesForValidHost
+        assertEquals(false, isHostInDenylist("https://$validHost", hostDenyList))
+    }
 }