* DKIM module now check only first signature as others must be rejected or skipped by definition.

* Add 'trusted_only' and 'skip_multi' options to DKIM module.
* Improve flags parsing to support strings like: 'true', 'false', '1', '0', 'yes', 'no', 'y', 'n'.

5 files changed, 142 insertions, 70 deletions

diff --git a/src/cfg_utils.c b/src/cfg_utils.c
index 28430df14..c1b71e10a 100644
--- a/src/cfg_utils.c
+++ b/src/cfg_utils.c
@@ -449,23 +449,45 @@ cfg_parse_time (const gchar *t, enum time_type default_type)
 gchar
 parse_flag (const gchar *str)
 {
-	if (!str || !*str)
-		return -1;
+	guint							 len;
+	gchar							 c;
 
-	if ((*str == 'Y' || *str == 'y')) {
-		return 1;
-	}
-
-	if ((*str == 'Y' || *str == 'y') && (*(str + 1) == 'E' || *(str + 1) == 'e') && (*(str + 2) == 'S' || *(str + 2) == 's')) {
-		return 1;
+	if (!str || !*str) {
+		return -1;
 	}
 
-	if ((*str == 'N' || *str == 'n')) {
-		return 0;
-	}
+	len = strlen (str);
 
-	if ((*str == 'N' || *str == 'n') && (*(str + 1) == 'O' || *(str + 1) == 'o')) {
-		return 0;
+	switch (len) {
+	case 1:
+		c = g_ascii_tolower (*str);
+		if (c == 'y' || c == '1') {
+			return 1;
+		}
+		else if (c == 'n' || c == '0') {
+			return 0;
+		}
+		break;
+	case 2:
+		if (g_ascii_strncasecmp (str, "no", len) == 0) {
+			return 0;
+		}
+		break;
+	case 3:
+		if (g_ascii_strncasecmp (str, "yes", len) == 0) {
+			return 1;
+		}
+		break;
+	case 4:
+		if (g_ascii_strncasecmp (str, "true", len) == 0) {
+			return 1;
+		}
+		break;
+	case 5:
+		if (g_ascii_strncasecmp (str, "false", len) == 0) {
+			return 0;
+		}
+		break;
 	}
 
 	return -1;
diff --git a/src/cfg_xml.c b/src/cfg_xml.c
index d681ec252..6c21fa14d 100644
--- a/src/cfg_xml.c
+++ b/src/cfg_xml.c
@@ -2224,6 +2224,11 @@ check_module_option (const gchar *mname, const gchar *optname, const gchar *data
 			return FALSE;
 		}
 		break;
+	case MODULE_OPT_TYPE_FLAG:
+		if (parse_flag (data) == -1) {
+			return FALSE;
+		}
+		break;
 	}
 
 	return TRUE;
diff --git a/src/cfg_xml.h b/src/cfg_xml.h
index b78a74fd9..bfe8123a0 100644
--- a/src/cfg_xml.h
+++ b/src/cfg_xml.h
@@ -39,6 +39,7 @@ enum module_opt_type {
 	MODULE_OPT_TYPE_TIME,
 	MODULE_OPT_TYPE_MAP,
 	MODULE_OPT_TYPE_SIZE,
+	MODULE_OPT_TYPE_FLAG,
 	MODULE_OPT_TYPE_ANY
 };
 
diff --git a/src/dkim.c b/src/dkim.c
index d50ddc0eb..7d4d91471 100644
--- a/src/dkim.c
+++ b/src/dkim.c
@@ -854,7 +854,7 @@ rspamd_dkim_relaxed_body_step (GChecksum *ck, const gchar **start, guint remain)
 
 	*start = h;
 
-#if 1
+#if 0
 	msg_debug ("update signature with buffer: %*s", t - buf, buf);
 #endif
 	g_checksum_update (ck, buf, t - buf);
@@ -1087,17 +1087,19 @@ rspamd_dkim_canonize_header_simple (rspamd_dkim_context_t *ctx, const gchar *hea
 		}
 	}
 
-	if (!is_sign) {
+	if (found) {
+		if (!is_sign) {
 
-		for (i = to_sign->len - 1; i >= 0 && count > 0; i --, count --) {
-			elt = &g_array_index (to_sign, struct rspamd_dkim_sign_chunk, i);
-			msg_debug ("update signature with header: %*s", elt->len, elt->begin);
-			g_checksum_update (ctx->headers_hash, elt->begin, elt->len);
+			for (i = to_sign->len - 1; i >= 0 && count > 0; i --, count --) {
+				elt = &g_array_index (to_sign, struct rspamd_dkim_sign_chunk, i);
+				msg_debug ("update signature with header: %*s", elt->len, elt->begin);
+				g_checksum_update (ctx->headers_hash, elt->begin, elt->len);
+			}
+		}
+		else {
+			elt = &g_array_index (to_sign, struct rspamd_dkim_sign_chunk, 0);
+			rspamd_dkim_signature_update (ctx, elt->begin, elt->len);
 		}
-	}
-	else {
-		elt = &g_array_index (to_sign, struct rspamd_dkim_sign_chunk, to_sign->len - 1);
-		rspamd_dkim_signature_update (ctx, elt->begin, elt->len);
 	}
 
 	g_array_free (to_sign, TRUE);
@@ -1119,40 +1121,46 @@ rspamd_dkim_canonize_header (rspamd_dkim_context_t *ctx, struct worker_task *tas
 	else {
 		rh = g_hash_table_lookup (task->raw_headers, header_name);
 		if (rh) {
-			rh_iter = rh;
-			while (rh_iter) {
-				rh_num ++;
-				rh_iter = rh_iter->next;
-			}
+			if (!is_sig) {
+				rh_iter = rh;
+				while (rh_iter) {
+					rh_num ++;
+					rh_iter = rh_iter->next;
+				}
 
-			if (rh_num > count) {
-				/* Set skip count */
-				rh_num -= count;
-			}
-			else {
-				rh_num = 0;
-			}
-			rh_iter = rh;
-			while (rh_num) {
-				rh_iter = rh_iter->next;
-				rh_num --;
-			}
-			/* Now insert required headers */
-			while (rh_iter) {
-				nh = g_list_prepend (nh, rh_iter);
-				rh_iter = rh_iter->next;
-			}
-			cur = nh;
-			while (cur) {
-				rh = cur->data;
-				if (! rspamd_dkim_canonize_header_relaxed (ctx, rh->value, header_name, is_sig)) {
+				if (rh_num > count) {
+					/* Set skip count */
+					rh_num -= count;
+				}
+				else {
+					rh_num = 0;
+				}
+				rh_iter = rh;
+				while (rh_num) {
+					rh_iter = rh_iter->next;
+					rh_num --;
+				}
+				/* Now insert required headers */
+				while (rh_iter) {
+					nh = g_list_prepend (nh, rh_iter);
+					rh_iter = rh_iter->next;
+				}
+				cur = nh;
+				while (cur) {
+					rh = cur->data;
+					if (! rspamd_dkim_canonize_header_relaxed (ctx, rh->value, header_name, is_sig)) {
+						g_list_free (nh);
+						return FALSE;
+					}
+					cur = g_list_next (cur);
+				}
+				if (nh != NULL) {
 					g_list_free (nh);
-					return FALSE;
 				}
-				cur = g_list_next (cur);
 			}
-			if (nh != NULL) {
-				g_list_free (nh);
+			else {
+				/* For signature check just use the first dkim header */
+				rspamd_dkim_canonize_header_relaxed (ctx, rh->value, header_name, is_sig);
 			}
 			return TRUE;
 		}
diff --git a/src/plugins/dkim_check.c b/src/plugins/dkim_check.c
index c5e9d4416..99dccf80b 100644
--- a/src/plugins/dkim_check.c
+++ b/src/plugins/dkim_check.c
@@ -30,10 +30,11 @@
  * - symbol_reject (string): symbol to insert (default: 'R_DKIM_REJECT')
  * - symbol_rempfail (string): symbol to insert in case of temporary fail (default: 'R_DKIM_TEMPFAIL')
  * - whitelist (map): map of whitelisted networks
- * - domains (map): map of domains to check (if absent all domains are checked)
- * - strict_domains (map): map of domains that requires strict score for dkim
+ * - domains (map): map of domains to check
  * - strict_multiplier (number): multiplier for strict domains
  * - time_jitter (number): jitter in seconds to allow time diff while checking
+ * - trusted_only (flag): check signatures only for domains in 'domains' map
+ * - skip_mutli (flag): skip messages with multiply dkim signatures
  */
 
 #include "config.h"
@@ -64,10 +65,11 @@ struct dkim_ctx {
 	memory_pool_t                   *dkim_pool;
 	radix_tree_t                    *whitelist_ip;
 	GHashTable						*dkim_domains;
-	GHashTable						*strict_domains;
 	guint							 strict_multiplier;
 	guint							 time_jitter;
 	rspamd_lru_hash_t               *dkim_hash;
+	gboolean						 trusted_only;
+	gboolean						 skip_multi;
 };
 
 static struct dkim_ctx        *dkim_module_ctx = NULL;
@@ -101,9 +103,10 @@ dkim_module_init (struct config_file *cfg, struct module_ctx **ctx)
 	register_module_opt ("dkim", "dkim_cache_expire", MODULE_OPT_TYPE_TIME);
 	register_module_opt ("dkim", "whitelist", MODULE_OPT_TYPE_MAP);
 	register_module_opt ("dkim", "domains", MODULE_OPT_TYPE_MAP);
-	register_module_opt ("dkim", "strict_domains", MODULE_OPT_TYPE_MAP);
 	register_module_opt ("dkim", "strict_multiplier", MODULE_OPT_TYPE_UINT);
 	register_module_opt ("dkim", "time_jitter", MODULE_OPT_TYPE_TIME);
+	register_module_opt ("dkim", "trusted_only", MODULE_OPT_TYPE_FLAG);
+	register_module_opt ("dkim", "skip_multi", MODULE_OPT_TYPE_FLAG);
 
 	return 0;
 }
@@ -114,6 +117,7 @@ dkim_module_config (struct config_file *cfg)
 	gchar                          *value;
 	gint                            res = TRUE;
 	guint                           cache_size, cache_expire;
+	gboolean						got_trusted = FALSE;
 
 	dkim_module_ctx->whitelist_ip = radix_tree_create ();
 
@@ -159,13 +163,11 @@ dkim_module_config (struct config_file *cfg)
 		}
 	}
 	if ((value = get_module_opt (cfg, "dkim", "domains")) != NULL) {
-		if (! add_map (value, read_host_list, fin_host_list, (void **)&dkim_module_ctx->dkim_domains)) {
-			msg_warn ("cannot load domains list from %s", value);
+		if (! add_map (value, read_kv_list, fin_kv_list, (void **)&dkim_module_ctx->dkim_domains)) {
+			msg_warn ("cannot load dkim domains list from %s", value);
 		}
-	}
-	if ((value = get_module_opt (cfg, "dkim", "strict_domains")) != NULL) {
-		if (! add_map (value, read_kv_list, fin_kv_list, (void **)&dkim_module_ctx->strict_domains)) {
-			msg_warn ("cannot load strict domains list from %s", value);
+		else {
+			got_trusted = TRUE;
 		}
 	}
 	if ((value = get_module_opt (cfg, "dkim", "strict_multiplier")) != NULL) {
@@ -174,13 +176,35 @@ dkim_module_config (struct config_file *cfg)
 	else {
 		dkim_module_ctx->strict_multiplier = 1;
 	}
+	if ((value = get_module_opt (cfg, "dkim", "trusted_only")) != NULL) {
+		dkim_module_ctx->trusted_only = parse_flag (value) == 1;
+	}
+	else {
+		dkim_module_ctx->trusted_only = FALSE;
+	}
+	if ((value = get_module_opt (cfg, "dkim", "skip_multi")) != NULL) {
+		dkim_module_ctx->skip_multi = parse_flag (value) == 1;
+	}
+	else {
+		dkim_module_ctx->skip_multi = FALSE;
+	}
+
+	if (dkim_module_ctx->trusted_only && !got_trusted) {
+		msg_err ("trusted_only option is set and no trusted domains are defined; disabling dkim module completely as it is useless in this case");
+	}
+	else {
+		register_symbol (&cfg->cache, dkim_module_ctx->symbol_reject, 1, dkim_symbol_callback, NULL);
+		register_virtual_symbol (&cfg->cache, dkim_module_ctx->symbol_tempfail, 1);
+		register_virtual_symbol (&cfg->cache, dkim_module_ctx->symbol_allow, 1);
 
-	register_symbol (&cfg->cache, dkim_module_ctx->symbol_reject, 1, dkim_symbol_callback, NULL);
-	register_virtual_symbol (&cfg->cache, dkim_module_ctx->symbol_tempfail, 1);
-	register_virtual_symbol (&cfg->cache, dkim_module_ctx->symbol_allow, 1);
+		dkim_module_ctx->dkim_hash = rspamd_lru_hash_new (rspamd_strcase_hash, rspamd_strcase_equal,
+				cache_size, cache_expire, g_free, (GDestroyNotify)rspamd_dkim_key_free);
 
-	dkim_module_ctx->dkim_hash = rspamd_lru_hash_new (rspamd_strcase_hash, rspamd_strcase_equal,
-			cache_size, cache_expire, g_free, (GDestroyNotify)rspamd_dkim_key_free);
+
+#ifndef HAVE_OPENSSL
+		msg_warn ("openssl is not found so dkim rsa check is disabled, only check body hash, it is NOT safe to trust these results");
+#endif
+	}
 
 	return res;
 }
@@ -230,9 +254,9 @@ dkim_module_check (struct worker_task *task, rspamd_dkim_context_t *ctx, rspamd_
 	msg_debug ("check dkim signature for %s domain from %s", ctx->domain, ctx->dns_key);
 	res = rspamd_dkim_check (ctx, key, task);
 
-	if (dkim_module_ctx->strict_domains != NULL) {
+	if (dkim_module_ctx->dkim_domains != NULL) {
 		/* Perform strict check */
-		if ((strict_value = g_hash_table_lookup (dkim_module_ctx->strict_domains, ctx->domain)) != NULL) {
+		if ((strict_value = g_hash_table_lookup (dkim_module_ctx->dkim_domains, ctx->domain)) != NULL) {
 			if (!dkim_module_parse_strict (strict_value, &score_allow, &score_deny)) {
 				score_allow = dkim_module_ctx->strict_multiplier;
 				score_deny = dkim_module_ctx->strict_multiplier;
@@ -295,6 +319,14 @@ dkim_symbol_callback (struct worker_task *task, void *unused)
 #endif
 			/* Parse signature */
 			msg_debug ("create dkim signature");
+			/* Check only last signature as there is no way to check embeded signatures after resend or something like this */
+			if (dkim_module_ctx->skip_multi) {
+				if (hlist->next != NULL) {
+					msg_info ("<%s> skip dkim check as it has several dkim signatures", task->message_id);
+					return;
+				}
+			}
+			hlist = g_list_last (hlist);
 			ctx = rspamd_create_dkim_context (hlist->data, task->task_pool, dkim_module_ctx->time_jitter, &err);
 			if (ctx == NULL) {
 				msg_info ("cannot parse DKIM context: %s", err->message);
@@ -302,6 +334,10 @@ dkim_symbol_callback (struct worker_task *task, void *unused)
 			}
 			else {
 				/* Get key */
+				if (dkim_module_ctx->trusted_only && (dkim_module_ctx->dkim_domains == NULL || g_hash_table_lookup (dkim_module_ctx->dkim_domains, ctx->domain) == NULL)) {
+					msg_debug ("skip dkim check for %s domain", ctx->domain);
+					return;
+				}
 				key = rspamd_lru_hash_lookup (dkim_module_ctx->dkim_hash, ctx->dns_key, task->tv.tv_sec);
 				if (key != NULL) {
 					debug_task ("found key for %s in cache", ctx->dns_key);