From bfd833c73432dd3e178e773f9f02deb43200bd33 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jan=20Sch=C3=A4r?= Date: Sun, 20 Oct 2024 00:08:36 +0200 Subject: [PATCH] [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 --- src/lua/lua_rsa.c | 4 ++++ test/lua/unit/rsa.lua | 24 ++++++++++++++---------- test/lua/unit/test.sig | 5 +++++ 3 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 test/lua/unit/test.sig 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== -- 2.39.5