From 1d8da7cd96a810bc6119a8f604e934235e68f9cf Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Tue, 5 Jun 2012 02:39:21 +0400 Subject: [PATCH] * 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'. --- src/cfg_utils.c | 48 ++++++++++++++++------ src/cfg_xml.c | 5 +++ src/cfg_xml.h | 1 + src/dkim.c | 88 ++++++++++++++++++++++------------------ src/plugins/dkim_check.c | 70 ++++++++++++++++++++++++-------- 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); -- 2.39.5