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
#876)

* Adding max http response string length as a setting, and capping http response string based on that setting (manual backport to 2.x)

Signed-off-by: Dennis Toepker <[email protected]>

* misc fix in plugin settings

Signed-off-by: Dennis Toepker <[email protected]>

---------

Signed-off-by: Dennis Toepker <[email protected]>
Co-authored-by: Dennis Toepker <[email protected]>
Co-authored-by: Hailong Cui <[email protected]>
(cherry picked from commit 9feaf33)
  • Loading branch information
toepkerd authored and github-actions[bot] committed Apr 16, 2024
1 parent d7f36a4 commit a14d3a5
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import org.apache.http.client.methods.HttpPatch
import org.apache.http.client.methods.HttpPost
import org.apache.http.client.methods.HttpPut
import org.apache.http.client.methods.HttpRequestBase
import org.apache.http.entity.ContentType
import org.apache.http.entity.StringEntity
import org.apache.http.impl.client.CloseableHttpClient
import org.apache.http.impl.client.DefaultHttpRequestRetryHandler
import org.apache.http.impl.client.HttpClientBuilder
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager
import org.apache.http.util.CharArrayBuffer
import org.apache.http.util.EntityUtils
import org.opensearch.common.xcontent.XContentType
import org.opensearch.core.rest.RestStatus
Expand All @@ -35,6 +37,10 @@ import org.opensearch.notifications.spi.model.destination.MicrosoftTeamsDestinat
import org.opensearch.notifications.spi.model.destination.SlackDestination
import org.opensearch.notifications.spi.model.destination.WebhookDestination
import java.io.IOException
import java.io.InputStream
import java.io.InputStreamReader
import java.lang.IllegalArgumentException
import java.nio.charset.Charset
import java.nio.charset.StandardCharsets
import java.util.Collections
import kotlin.collections.HashSet
Expand All @@ -58,6 +64,9 @@ class DestinationHttpClient {
companion object {
private val log by logger(DestinationHttpClient::class.java)

private const val DEFAULT_CHAR_BUFFER_SIZE = 1024
private const val DEFAULT_BYTE_BUFFER_SIZE = 4096

/**
* all valid response status
*/
Expand All @@ -76,6 +85,21 @@ class DestinationHttpClient {
)
)

private val CONTENT_TYPE_MAP = Collections.unmodifiableMap(
mapOf(
ContentType.APPLICATION_ATOM_XML.mimeType to ContentType.APPLICATION_ATOM_XML,
ContentType.APPLICATION_FORM_URLENCODED.mimeType to ContentType.APPLICATION_FORM_URLENCODED,
ContentType.APPLICATION_JSON.mimeType to ContentType.APPLICATION_JSON,
ContentType.APPLICATION_SVG_XML.mimeType to ContentType.APPLICATION_SVG_XML,
ContentType.APPLICATION_XHTML_XML.mimeType to ContentType.APPLICATION_XHTML_XML,
ContentType.APPLICATION_XML.mimeType to ContentType.APPLICATION_XML,
ContentType.MULTIPART_FORM_DATA.mimeType to ContentType.MULTIPART_FORM_DATA,
ContentType.TEXT_HTML.mimeType to ContentType.TEXT_HTML,
ContentType.TEXT_PLAIN.mimeType to ContentType.TEXT_PLAIN,
ContentType.TEXT_XML.mimeType to ContentType.TEXT_XML
)
)

private fun createHttpClient(): CloseableHttpClient {
val config: RequestConfig = RequestConfig.custom()
.setConnectTimeout(PluginSettings.connectionTimeout)
Expand Down Expand Up @@ -148,7 +172,7 @@ class DestinationHttpClient {
@Throws(IOException::class)
fun getResponseString(response: CloseableHttpResponse): String {
val entity: HttpEntity = response.entity ?: return "{}"
val responseString = EntityUtils.toString(entity)
val responseString = httpResponseToString(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 Expand Up @@ -182,4 +206,44 @@ class DestinationHttpClient {
.endObject()
return builder.string()
}

private fun httpResponseToString(entity: HttpEntity, maxResultLength: Int): String? {
if (entity == null) throw IllegalArgumentException("HttpEntity received was null")
val contentType = if (entity.contentType != null) ContentType.parse(entity.contentType.value) else null
val contentLength = toContentLength(checkContentLength(entity).toInt())
val inStream = entity.content ?: return null
var charset: Charset? = null
if (contentType != null) {
charset = contentType.charset
if (charset == null) {
charset = CONTENT_TYPE_MAP[contentType.mimeType]?.charset
}
}
return toCharArrayBuffer(inStream, contentLength, charset, maxResultLength).toString()
}

private fun checkContentLength(entity: HttpEntity): Long {
if (entity.contentLength < -1 || entity.contentLength > Int.MAX_VALUE) {
throw IllegalArgumentException("HTTP entity too large to be buffered in memory: ${entity.contentLength} is out of range [-1, ${Int.MAX_VALUE}]")
}
return entity.contentLength
}

private fun toContentLength(contentLength: Int): Int {
return if (contentLength < 0) DEFAULT_BYTE_BUFFER_SIZE else contentLength
}

private fun toCharArrayBuffer(inStream: InputStream, contentLength: Int, charset: Charset?, maxResultLength: Int): CharArrayBuffer {
require(maxResultLength > 0)
val actualCharSet = charset ?: StandardCharsets.UTF_8
val buf = CharArrayBuffer(minOf(maxResultLength, if (contentLength > 0) contentLength else DEFAULT_CHAR_BUFFER_SIZE))
val reader = InputStreamReader(inStream, actualCharSet)
val tmp = CharArray(DEFAULT_CHAR_BUFFER_SIZE)
var chReadCount: Int
while (reader.read(tmp).also { chReadCount = it } != -1 && buf.length < maxResultLength) {
buf.append(tmp, 0, chReadCount)
}
buf.setLength(minOf(buf.length, maxResultLength))
return buf
}
}
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")
maxHttpResponseSize = clusterMaxHttpResponseSize
}
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
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 a14d3a5

Please sign in to comment.