]> source.dussan.org Git - rspamd.git/commitdiff
[CritFix] Fix ARC-Seal signing 5193/head
authorJan Schär <jan@jschaer.ch>
Sat, 19 Oct 2024 22:08:36 +0000 (00:08 +0200)
committerJan Schär <jan@jschaer.ch>
Sat, 19 Oct 2024 22:08:36 +0000 (00:08 +0200)
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
src/lua/lua_rsa.c
test/lua/unit/rsa.lua
test/lua/unit/test.sig [new file with mode: 0644]

index 4b9aa03546b2550c2df55fd17e8dcba688c8edf8..5f7db606f6f0c68309042685605140c59bc2f5c6 100644 (file)
@@ -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);
index 019212df41b8adc880fcf34968a40a31231e570b..bc4113ae4ace6f2c6a4c53d4d0f429ce65cec73f 100644 (file)
@@ -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 (file)
index 0000000..6bf4f48
--- /dev/null
@@ -0,0 +1,5 @@
+D3IZyIpD0dzfEG0JCZ53BWQLgkRkek7V6JxeGRod3QqNzbGFbbisOkRUW3m3tYL4J7m29taRPT8Ki+RN
+       NdaPPylijID3E7vdjSY2+c3eajUvlgOCGjEl5kkpYEZeBsO/wJGrS+lucsx/QC/nWJFDGFbiMhbb5HJ/
+       fKguRXIqnIh6Dbp3VonP9k7DjgP0yRz6B9BBUBE/z01SeSfM7Knx83ZUsiAN3U8JEudVO9ahLArwFXST
+       pZDfS3Mn3zbghdXfmwmEFbtaN/SrmBvnEbhvsUfrbChy4Rk4d6wMYa3M83/DcVgxh4yaydlCHhctYBcP
+       gDQg2BrLzVkPCeWOyLicHg==