Skip to content

Commit f0668a7

Browse files
authored
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 <amardeep7194@gmail.com> fixed build Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> added test Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> added try catch Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> refactored logic to validate host first Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> fixed ktlint Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> * fixed integ tests Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> * throw unknownhostexception instead of illegal argument exception Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com> --------- Signed-off-by: Amardeepsingh Siglani <amardeep7194@gmail.com>
1 parent 47f8a3a commit f0668a7

File tree

8 files changed

+112
-15
lines changed

8 files changed

+112
-15
lines changed

notifications/core-spi/src/main/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpers.kt

+31-3
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@
55

66
package org.opensearch.notifications.spi.utils
77

8+
import inet.ipaddr.HostName
89
import inet.ipaddr.IPAddressString
910
import org.apache.commons.validator.routines.DomainValidator
1011
import org.apache.hc.client5.http.classic.methods.HttpPatch
1112
import org.apache.hc.client5.http.classic.methods.HttpPost
1213
import org.apache.hc.client5.http.classic.methods.HttpPut
14+
import org.apache.logging.log4j.LogManager
1315
import org.opensearch.core.common.Strings
1416
import org.opensearch.notifications.spi.utils.ValidationHelpers.FQDN_REGEX
17+
import java.lang.Exception
1518
import java.net.InetAddress
1619
import java.net.URL
1720

@@ -49,13 +52,38 @@ fun isValidUrl(urlString: String): Boolean {
4952
}
5053
}
5154

55+
fun getResolvedIps(host: String): List<IPAddressString> {
56+
try {
57+
val resolvedIps = InetAddress.getAllByName(host)
58+
return resolvedIps.map { inetAddress -> IPAddressString(inetAddress.hostAddress) }
59+
} catch (e: Exception) {
60+
LogManager.getLogger().error("Unable to resolve host ips")
61+
}
62+
63+
return listOf()
64+
}
65+
5266
fun isHostInDenylist(urlString: String, hostDenyList: List<String>): Boolean {
5367
val url = URL(urlString)
5468
if (url.host != null) {
55-
val ipStr = IPAddressString(InetAddress.getByName(url.host).hostAddress)
69+
val resolvedIpStrings = getResolvedIps(url.host)
70+
val hostStr = HostName(url.host)
71+
5672
for (network in hostDenyList) {
57-
val netStr = IPAddressString(network)
58-
if (netStr.contains(ipStr)) {
73+
val denyIpStr = IPAddressString(network)
74+
val denyHostStr = HostName(network)
75+
val hostInDenyList = denyHostStr.equals(hostStr)
76+
var ipInDenyList = false
77+
78+
for (ipStr in resolvedIpStrings) {
79+
if (denyIpStr.contains(ipStr)) {
80+
ipInDenyList = true
81+
break
82+
}
83+
}
84+
85+
if (hostInDenyList || ipInDenyList) {
86+
LogManager.getLogger().error("${url.host} is denied")
5987
return true
6088
}
6189
}

notifications/core-spi/src/test/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpersTests.kt

+11-3
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,21 @@ internal class ValidationHelpersTests {
5151

5252
@Test
5353
fun `test hostname gets resolved to ip for denylist`() {
54-
val invalidHost = "invalid.com"
54+
val expectedAddressesForInvalidHost = arrayOf(
55+
InetAddress.getByName("174.120.0.0"),
56+
InetAddress.getByName("10.0.0.1")
57+
)
58+
val expectedAddressesForValidHost = arrayOf(
59+
InetAddress.getByName("174.12.0.0")
60+
)
61+
5562
mockkStatic(InetAddress::class)
56-
every { InetAddress.getByName(invalidHost).hostAddress } returns "10.0.0.1" // 10.0.0.0/8
63+
val invalidHost = "invalid.com"
64+
every { InetAddress.getAllByName(invalidHost) } returns expectedAddressesForInvalidHost
5765
assertEquals(true, isHostInDenylist("https://$invalidHost", hostDenyList))
5866

5967
val validHost = "valid.com"
60-
every { InetAddress.getByName(validHost).hostAddress } returns "174.12.0.0"
68+
every { InetAddress.getAllByName(validHost) } returns expectedAddressesForValidHost
6169
assertEquals(false, isHostInDenylist("https://$validHost", hostDenyList))
6270
}
6371

notifications/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt

+39-9
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,24 @@ import org.apache.hc.client5.http.classic.methods.HttpPost
1212
import org.apache.hc.client5.http.classic.methods.HttpPut
1313
import org.apache.logging.log4j.LogManager
1414
import org.opensearch.core.common.Strings
15+
import java.lang.Exception
16+
import java.net.InetAddress
1517
import java.net.URL
18+
import java.net.UnknownHostException
1619

1720
fun validateUrl(urlString: String) {
1821
require(!Strings.isNullOrEmpty(urlString)) { "url is null or empty" }
1922
require(isValidUrl(urlString)) { "Invalid URL or unsupported" }
2023
}
2124

2225
fun validateUrlHost(urlString: String, hostDenyList: List<String>) {
23-
require(!isHostInDenylist(urlString, hostDenyList)) {
26+
val url = URL(urlString)
27+
28+
if (org.opensearch.notifications.spi.utils.getResolvedIps(url.host).isEmpty()) {
29+
throw UnknownHostException("Host could not be resolved to a valid Ip address")
30+
}
31+
32+
require(!org.opensearch.notifications.spi.utils.isHostInDenylist(urlString, hostDenyList)) {
2433
"Host of url is denied, based on plugin setting [notification.core.http.host_deny_list]"
2534
}
2635
}
@@ -35,18 +44,39 @@ fun isValidUrl(urlString: String): Boolean {
3544
return ("https" == url.protocol || "http" == url.protocol) // Support only http/https, other protocols not supported
3645
}
3746

47+
@Deprecated("This function is not maintained, use org.opensearch.notifications.spi.utils.isHostInDenylist instead.")
3848
fun isHostInDenylist(urlString: String, hostDenyList: List<String>): Boolean {
3949
val url = URL(urlString)
4050
if (url.host != null) {
41-
val ipStr = IPAddressString(url.host)
42-
val hostStr = HostName(url.host)
43-
for (network in hostDenyList) {
44-
val denyIpStr = IPAddressString(network)
45-
val denyHostStr = HostName(network)
46-
if (denyIpStr.contains(ipStr) || denyHostStr.equals(hostStr)) {
47-
LogManager.getLogger().error("${url.host} is denied")
48-
return true
51+
try {
52+
val resolvedIps = InetAddress.getAllByName(url.host)
53+
val resolvedIpStrings = resolvedIps.map { inetAddress -> IPAddressString(inetAddress.hostAddress) }
54+
val hostStr = HostName(url.host)
55+
56+
for (network in hostDenyList) {
57+
val denyIpStr = IPAddressString(network)
58+
val denyHostStr = HostName(network)
59+
val hostInDenyList = denyHostStr.equals(hostStr)
60+
var ipInDenyList = false
61+
62+
for (ipStr in resolvedIpStrings) {
63+
if (denyIpStr.contains(ipStr)) {
64+
ipInDenyList = true
65+
break
66+
}
67+
}
68+
69+
if (hostInDenyList || ipInDenyList) {
70+
LogManager.getLogger().error("${url.host} is denied")
71+
return true
72+
}
4973
}
74+
} catch (e: UnknownHostException) {
75+
LogManager.getLogger().error("Error checking denylist: Unknown host")
76+
return false
77+
} catch (e: Exception) {
78+
LogManager.getLogger().error("Error checking denylist: ${e.message}", e)
79+
return false
5080
}
5181
}
5282

notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.opensearch.notifications.core.destinations
77

8+
import inet.ipaddr.IPAddressString
89
import io.mockk.every
910
import io.mockk.mockk
1011
import io.mockk.mockkStatic
@@ -50,6 +51,7 @@ internal class ChimeDestinationTests {
5051
// Stubbing isHostInDenylist() so it doesn't attempt to resolve hosts that don't exist in the unit tests
5152
mockkStatic("org.opensearch.notifications.spi.utils.ValidationHelpersKt")
5253
every { org.opensearch.notifications.spi.utils.isHostInDenylist(any(), any()) } returns false
54+
every { org.opensearch.notifications.spi.utils.getResolvedIps(any()) } returns listOf(IPAddressString("174.0.0.0"))
5355
}
5456

5557
@Test

notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.opensearch.notifications.core.destinations
77

8+
import inet.ipaddr.IPAddressString
89
import io.mockk.every
910
import io.mockk.mockk
1011
import io.mockk.mockkStatic
@@ -62,6 +63,7 @@ internal class CustomWebhookDestinationTests {
6263
// Stubbing isHostInDenylist() so it doesn't attempt to resolve hosts that don't exist in the unit tests
6364
mockkStatic("org.opensearch.notifications.spi.utils.ValidationHelpersKt")
6465
every { org.opensearch.notifications.spi.utils.isHostInDenylist(any(), any()) } returns false
66+
every { org.opensearch.notifications.spi.utils.getResolvedIps(any()) } returns listOf(IPAddressString("174.0.0.0"))
6567
}
6668

6769
@ParameterizedTest(name = "method {0} should return corresponding type of Http request object {1}")

notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/MicrosoftTeamsDestinationTests.kt

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.opensearch.notifications.core.destinations
77

8+
import inet.ipaddr.IPAddressString
89
import io.mockk.every
910
import io.mockk.mockk
1011
import io.mockk.mockkStatic
@@ -52,6 +53,7 @@ internal class MicrosoftTeamsDestinationTests {
5253
// Stubbing isHostInDenylist() so it doesn't attempt to resolve hosts that don't exist in the unit tests
5354
mockkStatic("org.opensearch.notifications.spi.utils.ValidationHelpersKt")
5455
every { org.opensearch.notifications.spi.utils.isHostInDenylist(any(), any()) } returns false
56+
every { org.opensearch.notifications.spi.utils.getResolvedIps(any()) } returns listOf(IPAddressString("174.0.0.0"))
5557
}
5658

5759
@Test

notifications/core/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.opensearch.notifications.core.destinations
77

8+
import inet.ipaddr.IPAddressString
89
import io.mockk.every
910
import io.mockk.mockk
1011
import io.mockk.mockkStatic
@@ -52,6 +53,7 @@ internal class SlackDestinationTests {
5253
// Stubbing isHostInDenylist() so it doesn't attempt to resolve hosts that don't exist in the unit tests
5354
mockkStatic("org.opensearch.notifications.spi.utils.ValidationHelpersKt")
5455
every { org.opensearch.notifications.spi.utils.isHostInDenylist(any(), any()) } returns false
56+
every { org.opensearch.notifications.spi.utils.getResolvedIps(any()) } returns listOf(IPAddressString("174.0.0.0"))
5557
}
5658

5759
@Test

notifications/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt

+23
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55

66
package org.opensearch.notifications.core.utils
77

8+
import io.mockk.every
9+
import io.mockk.mockkStatic
810
import org.junit.jupiter.api.Assertions.assertEquals
911
import org.junit.jupiter.api.Test
12+
import java.net.InetAddress
1013

1114
internal class ValidationHelpersTests {
1215

@@ -44,4 +47,24 @@ internal class ValidationHelpersTests {
4447
assertEquals(false, isHostInDenylist("https://$url", hostDenyList), "address $url was not supposed to be identified as in the deny list, but was")
4548
}
4649
}
50+
51+
@Test
52+
fun `test hostname gets resolved to ip for denylist`() {
53+
val expectedAddressesForInvalidHost = arrayOf(
54+
InetAddress.getByName("174.120.0.0"),
55+
InetAddress.getByName("10.0.0.1")
56+
)
57+
val expectedAddressesForValidHost = arrayOf(
58+
InetAddress.getByName("174.12.0.0")
59+
)
60+
61+
mockkStatic(InetAddress::class)
62+
val invalidHost = "invalid.com"
63+
every { InetAddress.getAllByName(invalidHost) } returns expectedAddressesForInvalidHost
64+
assertEquals(true, isHostInDenylist("https://$invalidHost", hostDenyList))
65+
66+
val validHost = "valid.com"
67+
every { InetAddress.getAllByName(validHost) } returns expectedAddressesForValidHost
68+
assertEquals(false, isHostInDenylist("https://$validHost", hostDenyList))
69+
}
4770
}

0 commit comments

Comments
 (0)