Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add html sanitization enhancement for email message #694

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,16 @@ class MessageContent(

init {
require(!Strings.isNullOrEmpty(title)) { "title is null or empty" }
require(!Strings.isNullOrEmpty(textDescription)) { "text message part is null or empty" }
require(!Strings.isNullOrEmpty(textDescription) || !Strings.isNullOrEmpty(htmlDescription)) {
"both text message part and html message part are null or empty"
}
}

fun buildMessageWithTitle(): String {
return "$title\n\n$textDescription"
return if (!Strings.isNullOrEmpty(htmlDescription)) {
htmlDescription!!
} else {
"$title\n\n$textDescription"
}
}
}
2 changes: 2 additions & 0 deletions notifications/core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ dependencies {
implementation "com.sun.mail:javax.mail:1.6.2"
implementation "javax.activation:activation:1.1"
implementation "org.slf4j:slf4j-api:${versions.slf4j}" //Needed for httpclient5
implementation "com.googlecode.owasp-java-html-sanitizer:owasp-java-html-sanitizer:20220608.1"
implementation 'com.google.guava:guava:32.1.1-jre'
testImplementation(
'org.assertj:assertj-core:3.16.1',
'org.junit.jupiter:junit-jupiter-api:5.6.2',
Expand Down
2 changes: 2 additions & 0 deletions notifications/core/src/main/config/notifications-core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ opensearch.notifications.core:
email:
size_limit: 10000000
minimum_header_length: 160
enable_html_sanitization: true
html_sanitization_allow_list: ["blocks_group", "formatting_group", "images_group", "links_group", "styles_group", "tables_group"]
http:
max_connections: 60
max_connection_per_route: 20
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@

package org.opensearch.notifications.core.client

import org.opensearch.notifications.core.setting.PluginSettings
import org.opensearch.notifications.core.utils.logger
import org.opensearch.notifications.spi.model.MessageContent
import org.owasp.html.AttributePolicy
import org.owasp.html.HtmlChangeListener
import org.owasp.html.HtmlPolicyBuilder
import org.owasp.html.PolicyFactory
import java.util.Base64
import javax.activation.DataHandler
import javax.mail.Message
Expand All @@ -19,6 +25,86 @@ import javax.mail.util.ByteArrayDataSource
* Object for creating mime mimeMessage from the channel mimeMessage for sending mail.
*/
internal object EmailMimeProvider {
private val log by logger(EmailMimeProvider::class.java)

// org.owasp.html.Sanitizers provide some default sanitizers which can be used directly,
// we define these html elements groups and map them to the default sanitizers, users can specify these groups in the allow list,
// and users can still specify individual html tags or elements in the allow list or deny list
private const val HTML_ELEMENTS_GROUP_BLOCKS = "blocks_group"
private const val HTML_ELEMENTS_GROUP_FORMATTING = "formatting_group"
private const val HTML_ELEMENTS_GROUP_IMAGES = "images_group"
private const val HTML_ELEMENTS_GROUP_LINKS = "links_group"
private const val HTML_ELEMENTS_GROUP_STYLES = "styles_group"
private const val HTML_ELEMENTS_GROUP_TABLES = "tables_group"

// copied from org.owasp.html.Sanitizers.INTEGER
private val INTEGER = AttributePolicy { _, _, value ->
val n = value.length
if (n == 0) {
return@AttributePolicy null
}
for (i in 0 until n) {
val ch = value[i]
if (ch == '.') {
return@AttributePolicy if (i == 0) {
null
} else value.substring(0, i)
// truncate to integer.
} else if (ch !in '0'..'9') {
return@AttributePolicy null
}
}
value
}
private val htmlSanitizationPolicy: PolicyFactory?
get() {
if (PluginSettings.enableEmailHtmlSanitization) {
var policyBuilder = HtmlPolicyBuilder()
if (PluginSettings.emailHtmlSanitizationAllowList.isNotEmpty()) {
PluginSettings.emailHtmlSanitizationAllowList.forEach { e ->
when (e) {
// we do not use the pre-defined sanitizers directly but copy the definition here,
// because found that deny list takes no effect if we use the pre-defined sanitizers
HTML_ELEMENTS_GROUP_BLOCKS -> policyBuilder = policyBuilder.allowCommonBlockElements()
HTML_ELEMENTS_GROUP_FORMATTING -> policyBuilder = policyBuilder.allowCommonInlineFormattingElements()
HTML_ELEMENTS_GROUP_IMAGES -> policyBuilder = policyBuilder.allowUrlProtocols("http", "https").allowElements("img")
.allowAttributes("alt", "src").onElements("img")
.allowAttributes("border", "height", "width").matching(INTEGER)
.onElements("img")
HTML_ELEMENTS_GROUP_LINKS -> policyBuilder = policyBuilder.allowStandardUrlProtocols().allowElements("a")
.allowAttributes("href").onElements("a").requireRelNofollowOnLinks()
HTML_ELEMENTS_GROUP_STYLES -> policyBuilder = policyBuilder.allowStyling()
HTML_ELEMENTS_GROUP_TABLES -> policyBuilder = policyBuilder.allowStandardUrlProtocols()
.allowElements(
"table", "tr", "td", "th",
"colgroup", "caption", "col",
"thead", "tbody", "tfoot"
)
.allowAttributes("summary").onElements("table")
.allowAttributes("align", "valign")
.onElements(
"table", "tr", "td", "th",
"colgroup", "col",
"thead", "tbody", "tfoot"
)
.allowTextIn("table")
else -> policyBuilder = policyBuilder.allowElements(e)
}
}
}

if (PluginSettings.emailHtmlSanitizationDenyList.isNotEmpty()) {
// deny list only accepts individual html tags or elements
PluginSettings.emailHtmlSanitizationDenyList.forEach { e ->
policyBuilder = policyBuilder.disallowElements(e)
}
}

return policyBuilder.toFactory()
}
return null
}

/**
* Create and prepare mime mimeMessage to send mail
* @param session The mail session to use to create mime mimeMessage
Expand Down Expand Up @@ -60,7 +146,33 @@ internal object EmailMimeProvider {
// Define the HTML part
if (messageContent.htmlDescription != null) {
val htmlPart = MimeBodyPart()
htmlPart.setContent(messageContent.htmlDescription, "text/html; charset=UTF-8")
if (PluginSettings.enableEmailHtmlSanitization && htmlSanitizationPolicy != null) {
log.info(
"html sanitization for email enabled, allow list: [" + PluginSettings.emailHtmlSanitizationAllowList.joinToString() + "], deny list: [" +
PluginSettings.emailHtmlSanitizationDenyList.joinToString() + "], will sanitize the html body of the email from $fromAddress to $recipient"
)
val sanitizedHtml = htmlSanitizationPolicy!!.sanitize(
messageContent.htmlDescription,
object : HtmlChangeListener<String> {
override fun discardedTag(context: String?, elementName: String) {
log.debug("html sanitization for email, discard tag: $elementName")
}

override fun discardedAttributes(
context: String?,
elementName: String,
vararg attributeNames: String
) {
log.debug("html sanitization for email, discard attributes: $attributeNames")
}
},
null
)
htmlPart.setContent(sanitizedHtml, "text/html; charset=UTF-8")
} else {
htmlPart.setContent(messageContent.htmlDescription, "text/html; charset=UTF-8")
}

// Add the HTML part to the child container
msgBody.addBodyPart(htmlPart)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,21 @@ internal object PluginSettings {
*/
private const val TOOLTIP_SUPPORT_KEY = "$KEY_PREFIX.tooltip_support"

/**
* Setting to enable email html sanitization
*/
private const val ENABLE_EMAIL_HTML_SANITIZATION_KEY = "$EMAIL_KEY_PREFIX.enable_html_sanitization"

/**
* Setting for allow list of email html sanitization
*/
private const val EMAIL_HTML_SANITIZATION_ALLOW_LIST_KEY = "$EMAIL_KEY_PREFIX.html_sanitization_allow_list"

/**
* Setting for deny list of email html sanitization
*/
private const val EMAIL_HTML_SANITIZATION_DENY_LIST_KEY = "$EMAIL_KEY_PREFIX.html_sanitization_deny_list"

/**
* Default email size limit as 10MB.
*/
Expand Down Expand Up @@ -169,6 +184,23 @@ internal object PluginSettings {
*/
private val DEFAULT_DESTINATION_SETTINGS = emptyMap<String, SecureDestinationSettings>()

/**
* Default enable email html sanitization
*/
private const val DEFAULT_ENABLE_EMAIL_HTML_SANITIZATION = true

/**
* Default email html sanitization allow list
*/
private val DEFAULT_EMAIL_HTML_SANITIZATION_ALLOW_LIST = listOf(
"blocks_group", "formatting_group", "images_group", "links_group", "styles_group", "tables_group"
)

/**
* Default email html sanitization deny list
*/
private val DEFAULT_EMAIL_HTML_SANITIZATION_DENY_LIST = emptyList<String>()

/**
* list of allowed config types.
*/
Expand Down Expand Up @@ -229,6 +261,24 @@ internal object PluginSettings {
@Volatile
var destinationSettings: Map<String, SecureDestinationSettings>

/**
* Enable email html sanitization
*/
@Volatile
var enableEmailHtmlSanitization: Boolean

/**
* Email html sanitization allow list
*/
@Volatile
var emailHtmlSanitizationAllowList: List<String>

/**
* Email html sanitization deny list
*/
@Volatile
var emailHtmlSanitizationDenyList: List<String>

private const val DECIMAL_RADIX: Int = 10

private val log by logger(javaClass)
Expand Down Expand Up @@ -259,6 +309,18 @@ internal object PluginSettings {
tooltipSupport = settings?.getAsBoolean(TOOLTIP_SUPPORT_KEY, true) ?: DEFAULT_TOOLTIP_SUPPORT
hostDenyList = settings?.getAsList(HOST_DENY_LIST_KEY, null) ?: DEFAULT_HOST_DENY_LIST
destinationSettings = if (settings != null) loadDestinationSettings(settings) else DEFAULT_DESTINATION_SETTINGS
enableEmailHtmlSanitization =
settings?.getAsBoolean(ENABLE_EMAIL_HTML_SANITIZATION_KEY, true) ?: DEFAULT_ENABLE_EMAIL_HTML_SANITIZATION
emailHtmlSanitizationAllowList = settings?.getAsList(EMAIL_HTML_SANITIZATION_ALLOW_LIST_KEY, null)
?: DEFAULT_EMAIL_HTML_SANITIZATION_ALLOW_LIST
emailHtmlSanitizationDenyList = settings?.getAsList(EMAIL_HTML_SANITIZATION_DENY_LIST_KEY, null)
?: DEFAULT_EMAIL_HTML_SANITIZATION_DENY_LIST
// html sanitization deny list can be set only when allow list is not empty
// this behavior aligns with the owasp html sanitizer, which disallow any elements by default
// but can use deny list to make an exception based on the allow list
if (emailHtmlSanitizationAllowList.isEmpty() && emailHtmlSanitizationDenyList.isNotEmpty()) {
throw IllegalArgumentException("html_sanitization_deny_list cannot be set if html_sanitization_allow_list is empty!")
}

defaultSettings = mapOf(
EMAIL_SIZE_LIMIT_KEY to emailSizeLimit.toString(DECIMAL_RADIX),
Expand All @@ -267,7 +329,8 @@ 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),
TOOLTIP_SUPPORT_KEY to tooltipSupport.toString()
TOOLTIP_SUPPORT_KEY to tooltipSupport.toString(),
ENABLE_EMAIL_HTML_SANITIZATION_KEY to enableEmailHtmlSanitization.toString(),
)
}

Expand Down Expand Up @@ -342,6 +405,26 @@ internal object PluginSettings {
NodeScope, Dynamic
)

val ENABLE_EMAIL_HTML_SANITIZATION: Setting<Boolean> = Setting.boolSetting(
ENABLE_EMAIL_HTML_SANITIZATION_KEY,
defaultSettings[ENABLE_EMAIL_HTML_SANITIZATION_KEY]!!.toBoolean(),
NodeScope, Final
)

val EMAIL_HTML_SANITIZATION_ALLOW_LIST: Setting<List<String>> = Setting.listSetting(
EMAIL_HTML_SANITIZATION_ALLOW_LIST_KEY,
DEFAULT_EMAIL_HTML_SANITIZATION_ALLOW_LIST,
{ it },
NodeScope, Final
)

val EMAIL_HTML_SANITIZATION_DENY_LIST: Setting<List<String>> = Setting.listSetting(
EMAIL_HTML_SANITIZATION_DENY_LIST_KEY,
DEFAULT_EMAIL_HTML_SANITIZATION_DENY_LIST,
{ it },
NodeScope, Final
)

private val LEGACY_EMAIL_USERNAME: Setting.AffixSetting<SecureString> = Setting.affixKeySetting(
LEGACY_EMAIL_DESTINATION_SETTING_PREFIX,
"username",
Expand Down Expand Up @@ -393,24 +476,42 @@ internal object PluginSettings {
TOOLTIP_SUPPORT,
HOST_DENY_LIST,
EMAIL_USERNAME,
EMAIL_PASSWORD
EMAIL_PASSWORD,
ENABLE_EMAIL_HTML_SANITIZATION,
EMAIL_HTML_SANITIZATION_ALLOW_LIST,
EMAIL_HTML_SANITIZATION_DENY_LIST
)
}
/**
* Update the setting variables to setting values from local settings
* @param clusterService cluster service instance
*/
private fun updateSettingValuesFromLocal(clusterService: ClusterService) {
allowedConfigTypes = ALLOWED_CONFIG_TYPES.get(clusterService.settings)
val localAllowedConfigTypes = clusterService.settings.get(ALLOWED_CONFIG_TYPE_KEY)
if (localAllowedConfigTypes != null) {
allowedConfigTypes = clusterService.settings.getAsList(ALLOWED_CONFIG_TYPE_KEY)
}
emailSizeLimit = EMAIL_SIZE_LIMIT.get(clusterService.settings)
emailMinimumHeaderLength = EMAIL_MINIMUM_HEADER_LENGTH.get(clusterService.settings)
maxConnections = MAX_CONNECTIONS.get(clusterService.settings)
maxConnectionsPerRoute = MAX_CONNECTIONS_PER_ROUTE.get(clusterService.settings)
connectionTimeout = CONNECTION_TIMEOUT_MILLISECONDS.get(clusterService.settings)
socketTimeout = SOCKET_TIMEOUT_MILLISECONDS.get(clusterService.settings)
tooltipSupport = TOOLTIP_SUPPORT.get(clusterService.settings)
hostDenyList = HOST_DENY_LIST.get(clusterService.settings)
val localHostDenyList = clusterService.settings.get(HOST_DENY_LIST_KEY)
if (localHostDenyList != null) {
hostDenyList = clusterService.settings.getAsList(HOST_DENY_LIST_KEY)
}
destinationSettings = loadDestinationSettings(clusterService.settings)
enableEmailHtmlSanitization = ENABLE_EMAIL_HTML_SANITIZATION.get(clusterService.settings)
val localEmailHtmlSanitizationAllowList = clusterService.settings.get(EMAIL_HTML_SANITIZATION_ALLOW_LIST_KEY)
if (localEmailHtmlSanitizationAllowList != null) {
emailHtmlSanitizationAllowList = clusterService.settings.getAsList(EMAIL_HTML_SANITIZATION_ALLOW_LIST_KEY)
}
val localEmailHtmlSanitizationDenyList = clusterService.settings.get(EMAIL_HTML_SANITIZATION_DENY_LIST_KEY)
if (localEmailHtmlSanitizationDenyList != null) {
emailHtmlSanitizationDenyList = clusterService.settings.getAsList(EMAIL_HTML_SANITIZATION_DENY_LIST_KEY)
}
}

/**
Expand Down Expand Up @@ -570,5 +671,8 @@ internal object PluginSettings {
allowedConfigTypes = DEFAULT_ALLOWED_CONFIG_TYPES
tooltipSupport = DEFAULT_TOOLTIP_SUPPORT
hostDenyList = DEFAULT_HOST_DENY_LIST
enableEmailHtmlSanitization = DEFAULT_ENABLE_EMAIL_HTML_SANITIZATION
emailHtmlSanitizationAllowList = DEFAULT_EMAIL_HTML_SANITIZATION_ALLOW_LIST
emailHtmlSanitizationDenyList = DEFAULT_EMAIL_HTML_SANITIZATION_DENY_LIST
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ internal class ChimeDestinationTests {
val exception = Assertions.assertThrows(IllegalArgumentException::class.java) {
MessageContent("title", "")
}
assertEquals("text message part is null or empty", exception.message)
assertEquals("both text message part and html message part are null or empty", exception.message)
}

@ParameterizedTest
Expand Down
Loading