From: Jan Schär <jan@jschaer.ch>
Date: Sat, 19 Oct 2024 22:08:36 +0000 (+0200)
Subject: [CritFix] Fix ARC-Seal signing
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=bfd833c73432dd3e178e773f9f02deb43200bd33;p=rspamd.git

[CritFix] Fix ARC-Seal signing

Signing of ARC-Seal headers was recently broken; the created signatures
failed to validate. Most likely, this was caused by commit 1e661a2fc6e3,
which changed the way signatures are created in lua_rsa_sign_memory
without adding the calls to EVP_PKEY_CTX_set_rsa_padding and
EVP_PKEY_CTX_set_signature_md needed with the new interface.

After fixing this, some existing tests failed, because the test values
passed to the hash parameter did not have the correct size for a sha256
hash. I fixed these by adjusting the length of the test values.
Additionally, I extended the "RSA sign" unit test to compare the created
signature against the expected one. This is possible because RSA signing
is deterministic, and should prevent the same bug from occuring again.

Fixes: https://github.com/rspamd/rspamd/issues/5173
---

diff --git a/src/lua/lua_rsa.c b/src/lua/lua_rsa.c
index 4b9aa0354..5f7db606f 100644
--- a/src/lua/lua_rsa.c
+++ b/src/lua/lua_rsa.c
@@ -716,6 +716,8 @@ lua_rsa_verify_memory(lua_State *L)
 		EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(pkey, NULL);
 		g_assert(pctx != NULL);
 		g_assert(EVP_PKEY_verify_init(pctx) == 1);
+		g_assert(EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PADDING) == 1);
+		g_assert(EVP_PKEY_CTX_set_signature_md(pctx, EVP_sha256()) == 1);
 
 		ret = EVP_PKEY_verify(pctx, signature->str, signature->len, data, sz);
 
@@ -766,6 +768,8 @@ lua_rsa_sign_memory(lua_State *L)
 		g_assert(pctx != NULL);
 
 		g_assert(EVP_PKEY_sign_init(pctx) == 1);
+		g_assert(EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PADDING) == 1);
+		g_assert(EVP_PKEY_CTX_set_signature_md(pctx, EVP_sha256()) == 1);
 		size_t slen = signature->allocated;
 
 		ret = EVP_PKEY_sign(pctx, signature->str, &slen, data, sz);
diff --git a/test/lua/unit/rsa.lua b/test/lua/unit/rsa.lua
index 019212df4..bc4113ae4 100644
--- a/test/lua/unit/rsa.lua
+++ b/test/lua/unit/rsa.lua
@@ -10,6 +10,7 @@ context("RSA signature verification test", function()
   local privkey = 'testkey.sec'
   local data = 'test.data'
   local signature = 'test.sig'
+  local signature_bytes = 'test.sig_bytes'
   local test_dir = string.gsub(debug.getinfo(1).source, "^@(.+/)[^/]+$", "%1")
   local rsa_key, rsa_sig
 
@@ -23,7 +24,10 @@ context("RSA signature verification test", function()
     h:update(d)
     local sig = rsa.sign_memory(rsa_key, h:bin())
     assert_not_nil(sig)
-    sig:save(string.format('%s/%s', test_dir, signature), true)
+    sig:save(string.format('%s/%s', test_dir, signature_bytes), true)
+    local sig_actual = string.format('%s\n', sig:base64(80, 'lf'))
+    local sig_expected = io.open(string.format('%s/%s', test_dir, signature), "rb"):read "*a"
+    assert_equal(sig_actual, sig_expected)
   end)
 
   test("RSA verify", function()
@@ -33,28 +37,28 @@ context("RSA signature verification test", function()
     h:update(d)
     rsa_key = rsa_pubkey.load(string.format('%s/%s', test_dir, pubkey))
     assert_not_nil(rsa_key)
-    rsa_sig = rsa_signature.load(string.format('%s/%s', test_dir, signature))
+    rsa_sig = rsa_signature.load(string.format('%s/%s', test_dir, signature_bytes))
     assert_not_nil(rsa_sig)
     assert_true(rsa.verify_memory(rsa_key, rsa_sig, h:bin()))
   end)
 
   test("RSA keypair + sign + verify", function()
     local sk, pk = rsa.keypair()
-    local sig = rsa.sign_memory(sk, "test")
-    assert_true(rsa.verify_memory(pk, sig, "test"))
-    assert_false(rsa.verify_memory(pk, sig, "test1"))
+    local sig = rsa.sign_memory(sk, "test_012345678901234567890123456")
+    assert_true(rsa.verify_memory(pk, sig, "test_012345678901234567890123456"))
+    assert_false(rsa.verify_memory(pk, sig, "blah_012345678901234567890123456"))
     -- Overwrite
     sk, pk = rsa.keypair()
-    assert_false(rsa.verify_memory(pk, sig, "test"))
+    assert_false(rsa.verify_memory(pk, sig, "test_012345678901234567890123456"))
   end)
 
   test("RSA-2048 keypair + sign + verify", function()
     local sk, pk = rsa.keypair(2048)
-    local sig = rsa.sign_memory(sk, "test")
-    assert_true(rsa.verify_memory(pk, sig, "test"))
-    assert_false(rsa.verify_memory(pk, sig, "test1"))
+    local sig = rsa.sign_memory(sk, "test_012345678901234567890123456")
+    assert_true(rsa.verify_memory(pk, sig, "test_012345678901234567890123456"))
+    assert_false(rsa.verify_memory(pk, sig, "blah_012345678901234567890123456"))
     -- Overwrite
     sk, pk = rsa.keypair(2048)
-    assert_false(rsa.verify_memory(pk, sig, "test"))
+    assert_false(rsa.verify_memory(pk, sig, "test_012345678901234567890123456"))
   end)
 end)
diff --git a/test/lua/unit/test.sig b/test/lua/unit/test.sig
new file mode 100644
index 000000000..6bf4f48a3
--- /dev/null
+++ b/test/lua/unit/test.sig
@@ -0,0 +1,5 @@
+D3IZyIpD0dzfEG0JCZ53BWQLgkRkek7V6JxeGRod3QqNzbGFbbisOkRUW3m3tYL4J7m29taRPT8Ki+RN
+	NdaPPylijID3E7vdjSY2+c3eajUvlgOCGjEl5kkpYEZeBsO/wJGrS+lucsx/QC/nWJFDGFbiMhbb5HJ/
+	fKguRXIqnIh6Dbp3VonP9k7DjgP0yRz6B9BBUBE/z01SeSfM7Knx83ZUsiAN3U8JEudVO9ahLArwFXST
+	pZDfS3Mn3zbghdXfmwmEFbtaN/SrmBvnEbhvsUfrbChy4Rk4d6wMYa3M83/DcVgxh4yaydlCHhctYBcP
+	gDQg2BrLzVkPCeWOyLicHg==