From 0eef5c63eee8a25c4059d4c0687a79e616a7b4ac Mon Sep 17 00:00:00 2001
From: Vsevolod Stakhov <vsevolod@highsecure.ru>
Date: Tue, 12 Jun 2018 16:42:52 +0100
Subject: [PATCH] [Feature] Add pubkey checks for dkim_signing

Issue: #2277
---
 src/plugins/dkim_check.c         | 42 ++++++++++++++++++++++------
 src/plugins/lua/dkim_signing.lua | 47 +++++++++++++++++++++++++++-----
 2 files changed, 73 insertions(+), 16 deletions(-)

diff --git a/src/plugins/dkim_check.c b/src/plugins/dkim_check.c
index 773f9c56a..74bc4ea54 100644
--- a/src/plugins/dkim_check.c
+++ b/src/plugins/dkim_check.c
@@ -623,7 +623,7 @@ lua_dkim_sign_handler (lua_State *L)
 	rspamd_dkim_sign_context_t *ctx;
 	rspamd_dkim_sign_key_t *dkim_key;
 	gsize rawlen = 0, keylen = 0;
-	gboolean no_cache = FALSE;
+	gboolean no_cache = FALSE, strict_pubkey_check = FALSE;
 
 	luaL_argcheck (L, lua_type (L, 2) == LUA_TTABLE, 2, "'table' expected");
 	/*
@@ -634,10 +634,12 @@ lua_dkim_sign_handler (lua_State *L)
 	 */
 	if (!rspamd_lua_parse_table_arguments (L, 2, &err,
 			"key=V;rawkey=V;*domain=S;*selector=S;no_cache=B;headers=S;"
-					"sign_type=S;arc_idx=I;arc_cv=S;expire=I;pubkey=S",
+			"sign_type=S;arc_idx=I;arc_cv=S;expire=I;pubkey=S;"
+			"strict_pubkey_check=B",
 			&keylen, &key, &rawlen, &rawkey, &domain,
 			&selector, &no_cache, &headers,
-			&sign_type_str, &arc_idx, &arc_cv, &expire, &pubkey)) {
+			&sign_type_str, &arc_idx, &arc_cv, &expire, &pubkey,
+			&strict_pubkey_check)) {
 		msg_err_task ("cannot parse table arguments: %e",
 				err);
 		g_error_free (err);
@@ -775,20 +777,42 @@ lua_dkim_sign_handler (lua_State *L)
 		pk = rspamd_dkim_parse_key (pubkey, &keylen, NULL);
 
 		if (pk == NULL) {
-			msg_warn_task ("cannot parse pubkey from string: %s",
-					pubkey);
+			if (strict_pubkey_check) {
+				msg_err_task ("cannot parse pubkey from string: %s, skip signing",
+						pubkey);
+				lua_pushboolean (L, FALSE);
+
+				return 1;
+			}
+			else {
+				msg_warn_task ("cannot parse pubkey from string: %s",
+						pubkey);
+			}
 		}
 		else {
 			GError *te = NULL;
 
 			/* We have parsed the key, so try to check keys */
 			if (!rspamd_dkim_match_keys (pk, dkim_key, &te)) {
-				msg_warn_task ("public key for %s/%s does not match private key: %e",
-						domain, selector, te);
-				g_error_free (te);
+				if (strict_pubkey_check) {
+					msg_err_task ("public key for %s/%s does not match private "
+								  "key: %e, skip signing",
+							domain, selector, te);
+					g_error_free (te);
+					lua_pushboolean (L, FALSE);
+					rspamd_dkim_key_unref (pk);
 
-				/* TODO: add fatal failure possibility */
+					return 1;
+				}
+				else {
+					msg_warn_task ("public key for %s/%s does not match private "
+								   "key: %e",
+							domain, selector, te);
+					g_error_free (te);
+				}
 			}
+
+			rspamd_dkim_key_unref (pk);
 		}
 	}
 
diff --git a/src/plugins/lua/dkim_signing.lua b/src/plugins/lua/dkim_signing.lua
index 5e057cf9e..623705549 100644
--- a/src/plugins/lua/dkim_signing.lua
+++ b/src/plugins/lua/dkim_signing.lua
@@ -32,7 +32,9 @@ local settings = {
   allow_hdrfrom_mismatch_sign_networks = false,
   allow_hdrfrom_multiple = false,
   allow_username_mismatch = false,
+  allow_pubkey_mismatch = true,
   auth_only = true,
+  check_pubkey = false,
   domain = {},
   path = string.format('%s/%s/%s', rspamd_paths['DBDIR'], 'dkim', '$domain.$selector.key'),
   sign_local = true,
@@ -56,6 +58,41 @@ local function dkim_signing_cb(task)
     return
   end
 
+  local function do_sign()
+    if settings.check_pubkey then
+      local resolve_name = p.selector .. "._domainkey." .. p.domain
+      task:get_resolver():resolve_txt({
+        task = task,
+        name = resolve_name,
+        callback = function(_, _, results, err)
+          task:inc_dns_req()
+          if not err and results and results[1] then
+            p.pubkey = results[1]
+            p.strict_pubkey_check = not settings.allow_pubkey_mismatch
+          elseif not settings.allow_pubkey_mismatch then
+            rspamd_logger.errx('public key for domain %s/%s is not found: %s, skip signing',
+                p.domain, p.selector, err)
+            return
+          else
+            rspamd_logger.infox('public key for domain %s/%s is not found: %s',
+                p.domain, p.selector, err)
+          end
+
+          local sret, _ = sign_func(task, p)
+          if sret then
+            task:insert_result(settings.symbol, 1.0)
+          end
+        end,
+        forced = true
+      })
+    else
+      local sret, _ = sign_func(task, p)
+      if sret then
+        task:insert_result(settings.symbol, 1.0)
+      end
+    end
+  end
+
   if settings.use_redis then
     local function try_redis_key(selector)
       p.key = nil
@@ -68,11 +105,8 @@ local function dkim_signing_cb(task)
         elseif type(data) ~= 'string' then
           rspamd_logger.debugm(N, task, "missing DKIM key for %s", rk)
         else
-        p.rawkey = data
-        local sret, _ = sign_func(task, p)
-        if sret then
-          task:insert_result(settings.symbol, 1.0)
-        end
+          p.rawkey = data
+          do_sign()
         end
       end
       local rret = rspamd_redis_make_request(task,
@@ -127,8 +161,7 @@ local function dkim_signing_cb(task)
         return false
       end
 
-      local sret, _ = sign_func(task, p)
-      return sret
+      do_sign()
     else
       rspamd_logger.infox(task, 'key path or dkim selector unconfigured; no signing')
       return false
-- 
2.39.5