Skip to content

Commit

Permalink
Adding max http response string length as a setting, and capping http…
Browse files Browse the repository at this point in the history 
… response string based on that setting (opensearch-project#861)

Signed-off-by: Dennis Toepker <[email protected]>
  • Loading branch information
@toepkerd
toepkerd authored Feb 22, 2024
1 parent 39bd3bf commit 8c662a6
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 1 deletion.
2 changes: 1 addition & 1 deletion ...ns/core/src/main/kotlin/org/opensearch/notifications/core/client/DestinationHttpClient.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class DestinationHttpClient {
@Throws(IOException::class)
fun getResponseString(response: CloseableHttpResponse): String {
val entity: HttpEntity = response.entity ?: return "{}"
val responseString = EntityUtils.toString(entity)
val responseString = EntityUtils.toString(entity, PluginSettings.maxHttpResponseSize / 2) // Java char is 2 bytes
// DeliveryStatus need statusText must not be empty, convert empty response to {}
return if (responseString.isNullOrEmpty()) "{}" else responseString
}
Expand Down
38 changes: 38 additions & 0 deletions ...ications/core/src/main/kotlin/org/opensearch/notifications/core/setting/PluginSettings.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import org.opensearch.common.settings.Setting.Property.Final
import org.opensearch.common.settings.Setting.Property.NodeScope
import org.opensearch.common.settings.Settings
import org.opensearch.core.common.settings.SecureString
import org.opensearch.http.HttpTransportSettings.SETTING_HTTP_MAX_CONTENT_LENGTH
import org.opensearch.notifications.core.NotificationCorePlugin.Companion.LOG_PREFIX
import org.opensearch.notifications.core.NotificationCorePlugin.Companion.PLUGIN_NAME
import org.opensearch.notifications.core.utils.OpenForTesting
Expand Down Expand Up @@ -81,6 +82,11 @@ internal object PluginSettings {
*/
private const val SOCKET_TIMEOUT_MILLISECONDS_KEY = "$HTTP_CONNECTION_KEY_PREFIX.socket_timeout"

/**
* Setting for maximum string length of HTTP response, allows protection from DoS
*/
private const val MAX_HTTP_RESPONSE_SIZE_KEY = "$KEY_PREFIX.max_http_response_size"

/**
* Legacy setting for list of host deny list in Alerting
*/
Expand Down Expand Up @@ -146,6 +152,11 @@ internal object PluginSettings {
*/
private const val DEFAULT_SOCKET_TIMEOUT_MILLISECONDS = 50000

/**
* Default maximum HTTP response string length
*/
private val DEFAULT_MAX_HTTP_RESPONSE_SIZE = SETTING_HTTP_MAX_CONTENT_LENGTH.getDefault(Settings.EMPTY).getBytes().toInt()

/**
* Default email header length. minimum value from 100 reference emails
*/
Expand Down Expand Up @@ -223,6 +234,12 @@ internal object PluginSettings {
@Volatile
var socketTimeout: Int

/**
* Maximum HTTP response string length
*/
@Volatile
var maxHttpResponseSize: Int

/**
* Tooltip support
*/
Expand Down Expand Up @@ -273,6 +290,7 @@ internal object PluginSettings {
connectionTimeout = (settings?.get(CONNECTION_TIMEOUT_MILLISECONDS_KEY)?.toInt())
?: DEFAULT_CONNECTION_TIMEOUT_MILLISECONDS
socketTimeout = (settings?.get(SOCKET_TIMEOUT_MILLISECONDS_KEY)?.toInt()) ?: DEFAULT_SOCKET_TIMEOUT_MILLISECONDS
maxHttpResponseSize = (settings?.get(MAX_HTTP_RESPONSE_SIZE_KEY)?.toInt()) ?: DEFAULT_MAX_HTTP_RESPONSE_SIZE
allowedConfigTypes = settings?.getAsList(ALLOWED_CONFIG_TYPE_KEY, null) ?: DEFAULT_ALLOWED_CONFIG_TYPES
tooltipSupport = settings?.getAsBoolean(TOOLTIP_SUPPORT_KEY, true) ?: DEFAULT_TOOLTIP_SUPPORT
hostDenyList = settings?.getAsList(HOST_DENY_LIST_KEY, null) ?: DEFAULT_HOST_DENY_LIST
Expand All @@ -286,6 +304,7 @@ internal object PluginSettings {
MAX_CONNECTIONS_PER_ROUTE_KEY to maxConnectionsPerRoute.toString(DECIMAL_RADIX),
CONNECTION_TIMEOUT_MILLISECONDS_KEY to connectionTimeout.toString(DECIMAL_RADIX),
SOCKET_TIMEOUT_MILLISECONDS_KEY to socketTimeout.toString(DECIMAL_RADIX),
MAX_HTTP_RESPONSE_SIZE_KEY to maxHttpResponseSize.toString(DECIMAL_RADIX),
TOOLTIP_SUPPORT_KEY to tooltipSupport.toString()
)
}
Expand Down Expand Up @@ -333,6 +352,13 @@ internal object PluginSettings {
Dynamic
)

val MAX_HTTP_RESPONSE_SIZE: Setting<Int> = Setting.intSetting(
MAX_HTTP_RESPONSE_SIZE_KEY,
defaultSettings[MAX_HTTP_RESPONSE_SIZE_KEY]!!.toInt(),
NodeScope,
Dynamic
)

val ALLOWED_CONFIG_TYPES: Setting<List<String>> = Setting.listSetting(
ALLOWED_CONFIG_TYPE_KEY,
DEFAULT_ALLOWED_CONFIG_TYPES,
Expand Down Expand Up @@ -420,6 +446,7 @@ internal object PluginSettings {
MAX_CONNECTIONS_PER_ROUTE,
CONNECTION_TIMEOUT_MILLISECONDS,
SOCKET_TIMEOUT_MILLISECONDS,
MAX_HTTP_RESPONSE_SIZE,
ALLOWED_CONFIG_TYPES,
TOOLTIP_SUPPORT,
HOST_DENY_LIST,
Expand All @@ -440,6 +467,7 @@ internal object PluginSettings {
maxConnectionsPerRoute = MAX_CONNECTIONS_PER_ROUTE.get(clusterService.settings)
connectionTimeout = CONNECTION_TIMEOUT_MILLISECONDS.get(clusterService.settings)
socketTimeout = SOCKET_TIMEOUT_MILLISECONDS.get(clusterService.settings)
maxHttpResponseSize = MAX_HTTP_RESPONSE_SIZE.get(clusterService.settings)
tooltipSupport = TOOLTIP_SUPPORT.get(clusterService.settings)
hostDenyList = HOST_DENY_LIST.get(clusterService.settings)
destinationSettings = loadDestinationSettings(clusterService.settings)
Expand Down Expand Up @@ -482,6 +510,11 @@ internal object PluginSettings {
log.debug("$LOG_PREFIX:$SOCKET_TIMEOUT_MILLISECONDS_KEY -autoUpdatedTo-> $clusterSocketTimeout")
socketTimeout = clusterSocketTimeout
}
val clusterMaxHttpResponseSize = clusterService.clusterSettings.get(MAX_HTTP_RESPONSE_SIZE)
if (clusterMaxHttpResponseSize != null) {
log.debug("$LOG_PREFIX:$MAX_HTTP_RESPONSE_SIZE_KEY -autoUpdatedTo-> $clusterMaxHttpResponseSize")
socketTimeout = clusterSocketTimeout
}
val clusterAllowedConfigTypes = clusterService.clusterSettings.get(ALLOWED_CONFIG_TYPES)
if (clusterAllowedConfigTypes != null) {
log.debug("$LOG_PREFIX:$ALLOWED_CONFIG_TYPE_KEY -autoUpdatedTo-> $clusterAllowedConfigTypes")
Expand Down Expand Up @@ -542,6 +575,10 @@ internal object PluginSettings {
socketTimeout = it
log.info("$LOG_PREFIX:$SOCKET_TIMEOUT_MILLISECONDS_KEY -updatedTo-> $it")
}
clusterService.clusterSettings.addSettingsUpdateConsumer(MAX_HTTP_RESPONSE_SIZE) {
maxHttpResponseSize = it
log.info("$LOG_PREFIX:$MAX_HTTP_RESPONSE_SIZE_KEY -updatedTo-> $it")
}
clusterService.clusterSettings.addSettingsUpdateConsumer(TOOLTIP_SUPPORT) {
tooltipSupport = it
log.info("$LOG_PREFIX:$TOOLTIP_SUPPORT_KEY -updatedTo-> $it")
Expand Down Expand Up @@ -605,6 +642,7 @@ internal object PluginSettings {
maxConnectionsPerRoute = DEFAULT_MAX_CONNECTIONS_PER_ROUTE
connectionTimeout = DEFAULT_CONNECTION_TIMEOUT_MILLISECONDS
socketTimeout = DEFAULT_SOCKET_TIMEOUT_MILLISECONDS
maxHttpResponseSize = DEFAULT_MAX_HTTP_RESPONSE_SIZE
allowedConfigTypes = DEFAULT_ALLOWED_CONFIG_TYPES
tooltipSupport = DEFAULT_TOOLTIP_SUPPORT
hostDenyList = DEFAULT_HOST_DENY_LIST
Expand Down
30 changes: 30 additions & 0 deletions ...ns/core/src/test/kotlin/org/opensearch/notifications/core/settings/PluginSettingsTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import org.opensearch.cluster.ClusterName
import org.opensearch.cluster.service.ClusterService
import org.opensearch.common.settings.ClusterSettings
import org.opensearch.common.settings.Settings
import org.opensearch.http.HttpTransportSettings.SETTING_HTTP_MAX_CONTENT_LENGTH
import org.opensearch.notifications.core.NotificationCorePlugin
import org.opensearch.notifications.core.setting.PluginSettings

Expand All @@ -32,6 +33,7 @@ internal class PluginSettingsTests {
private val httpMaxConnectionPerRouteKey = "$httpKeyPrefix.max_connection_per_route"
private val httpConnectionTimeoutKey = "$httpKeyPrefix.connection_timeout"
private val httpSocketTimeoutKey = "$httpKeyPrefix.socket_timeout"
private val maxHttpResponseSizeKey = "$keyPrefix.max_http_response_size"
private val legacyAlertingHostDenyListKey = "opendistro.destination.host.deny_list"
private val alertingHostDenyListKey = "plugins.destination.host.deny_list"
private val httpHostDenyListKey = "$httpKeyPrefix.host_deny_list"
Expand All @@ -48,6 +50,7 @@ internal class PluginSettingsTests {
.put(httpMaxConnectionPerRouteKey, 20)
.put(httpConnectionTimeoutKey, 5000)
.put(httpSocketTimeoutKey, 50000)
.put(maxHttpResponseSizeKey, SETTING_HTTP_MAX_CONTENT_LENGTH.getDefault(Settings.EMPTY).getBytes().toInt())
.putList(httpHostDenyListKey, emptyList<String>())
.putList(
allowedConfigTypeKey,
Expand Down Expand Up @@ -91,6 +94,7 @@ internal class PluginSettingsTests {
PluginSettings.MAX_CONNECTIONS_PER_ROUTE,
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.MAX_HTTP_RESPONSE_SIZE,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.HOST_DENY_LIST
Expand Down Expand Up @@ -118,6 +122,10 @@ internal class PluginSettingsTests {
defaultSettings[httpSocketTimeoutKey],
PluginSettings.socketTimeout.toString()
)
Assertions.assertEquals(
defaultSettings[maxHttpResponseSizeKey],
PluginSettings.maxHttpResponseSize.toString()
)
Assertions.assertEquals(
defaultSettings[allowedConfigTypeKey],
PluginSettings.allowedConfigTypes.toString()
Expand Down Expand Up @@ -145,6 +153,7 @@ internal class PluginSettingsTests {
.put(httpMaxConnectionPerRouteKey, 100)
.put(httpConnectionTimeoutKey, 100)
.put(httpSocketTimeoutKey, 100)
.put(maxHttpResponseSizeKey, 20000000)
.putList(httpHostDenyListKey, listOf("sample"))
.putList(allowedConfigTypeKey, listOf("slack"))
.put(tooltipSupportKey, false)
Expand All @@ -163,6 +172,7 @@ internal class PluginSettingsTests {
PluginSettings.MAX_CONNECTIONS_PER_ROUTE,
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.MAX_HTTP_RESPONSE_SIZE,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.HOST_DENY_LIST,
Expand Down Expand Up @@ -191,6 +201,14 @@ internal class PluginSettingsTests {
100,
clusterService.clusterSettings.get(PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS)
)
Assertions.assertEquals(
100,
clusterService.clusterSettings.get(PluginSettings.SOCKET_TIMEOUT_MILLISECONDS)
)
Assertions.assertEquals(
20000000,
clusterService.clusterSettings.get(PluginSettings.MAX_HTTP_RESPONSE_SIZE)
)
Assertions.assertEquals(
listOf("sample"),
clusterService.clusterSettings.get(PluginSettings.HOST_DENY_LIST)
Expand Down Expand Up @@ -224,6 +242,7 @@ internal class PluginSettingsTests {
PluginSettings.MAX_CONNECTIONS_PER_ROUTE,
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.MAX_HTTP_RESPONSE_SIZE,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.HOST_DENY_LIST,
Expand Down Expand Up @@ -252,6 +271,14 @@ internal class PluginSettingsTests {
defaultSettings[httpConnectionTimeoutKey],
clusterService.clusterSettings.get(PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS).toString()
)
Assertions.assertEquals(
defaultSettings[httpSocketTimeoutKey],
clusterService.clusterSettings.get(PluginSettings.SOCKET_TIMEOUT_MILLISECONDS).toString()
)
Assertions.assertEquals(
defaultSettings[maxHttpResponseSizeKey],
clusterService.clusterSettings.get(PluginSettings.MAX_HTTP_RESPONSE_SIZE).toString()
)
Assertions.assertEquals(
defaultSettings[httpHostDenyListKey],
clusterService.clusterSettings.get(PluginSettings.HOST_DENY_LIST).toString()
Expand Down Expand Up @@ -290,6 +317,7 @@ internal class PluginSettingsTests {
PluginSettings.MAX_CONNECTIONS_PER_ROUTE,
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.MAX_HTTP_RESPONSE_SIZE,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.LEGACY_ALERTING_HOST_DENY_LIST,
Expand Down Expand Up @@ -325,6 +353,7 @@ internal class PluginSettingsTests {
PluginSettings.MAX_CONNECTIONS_PER_ROUTE,
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.MAX_HTTP_RESPONSE_SIZE,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.LEGACY_ALERTING_HOST_DENY_LIST,
Expand Down Expand Up @@ -359,6 +388,7 @@ internal class PluginSettingsTests {
PluginSettings.MAX_CONNECTIONS_PER_ROUTE,
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.MAX_HTTP_RESPONSE_SIZE,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.LEGACY_ALERTING_HOST_DENY_LIST,
Expand Down

0 comments on commit 8c662a6

Please sign in to comment.