From f0668a7d50be271f05423f7f007cf7d5efda1fc5 Mon Sep 17 00:00:00 2001 From: Amardeepsingh Siglani Date: Wed, 25 Sep 2024 11:50:07 -0700 Subject: [PATCH] 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 fixed build Signed-off-by: Amardeepsingh Siglani added test Signed-off-by: Amardeepsingh Siglani added try catch Signed-off-by: Amardeepsingh Siglani refactored logic to validate host first Signed-off-by: Amardeepsingh Siglani fixed ktlint Signed-off-by: Amardeepsingh Siglani * fixed integ tests Signed-off-by: Amardeepsingh Siglani * throw unknownhostexception instead of illegal argument exception Signed-off-by: Amardeepsingh Siglani --------- Signed-off-by: Amardeepsingh Siglani --- .../spi/utils/ValidationHelpers.kt | 34 +++++++++++-- .../spi/utils/ValidationHelpersTests.kt | 14 ++++-- .../core/utils/ValidationHelpers.kt | 48 +++++++++++++++---- .../destinations/ChimeDestinationTests.kt | 2 + .../CustomWebhookDestinationTests.kt | 2 + .../MicrosoftTeamsDestinationTests.kt | 2 + .../destinations/SlackDestinationTests.kt | 2 + .../core/utils/ValidationHelpersTests.kt | 23 +++++++++ 8 files changed, 112 insertions(+), 15 deletions(-) diff --git a/notifications/core-spi/src/main/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpers.kt b/notifications/core-spi/src/main/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpers.kt index 4cdaab82..29784eb8 100644 --- a/notifications/core-spi/src/main/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpers.kt +++ b/notifications/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.hc.client5.http.classic.methods.HttpPatch import org.apache.hc.client5.http.classic.methods.HttpPost import org.apache.hc.client5.http.classic.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 { + 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): 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/notifications/core-spi/src/test/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpersTests.kt b/notifications/core-spi/src/test/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpersTests.kt index bfc0290b..e3b0a3ac 100644 --- a/notifications/core-spi/src/test/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpersTests.kt +++ b/notifications/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/notifications/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt b/notifications/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt index 720f57b4..b6b06152 100644 --- a/notifications/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt +++ b/notifications/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt @@ -12,7 +12,10 @@ import org.apache.hc.client5.http.classic.methods.HttpPost import org.apache.hc.client5.http.classic.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" } @@ -20,7 +23,13 @@ fun validateUrl(urlString: String) { } fun validateUrlHost(urlString: String, hostDenyList: List) { - 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): 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/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt index 1ef6fa99..af42ed58 100644 --- a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt +++ b/notifications/core/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 @@ -50,6 +51,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/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt index 26aac9e8..fcce063d 100644 --- a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt +++ b/notifications/core/src/test/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 @@ -62,6 +63,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/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/MicrosoftTeamsDestinationTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/MicrosoftTeamsDestinationTests.kt index 54ec06c5..464a050d 100644 --- a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/MicrosoftTeamsDestinationTests.kt +++ b/notifications/core/src/test/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.mockk import io.mockk.mockkStatic @@ -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/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt index 0175bf06..a010cac7 100644 --- a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt +++ b/notifications/core/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.mockk import io.mockk.mockkStatic @@ -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/notifications/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt index 848fca8d..05406a36 100644 --- a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt +++ b/notifications/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)) + } }