From: Vsevolod Stakhov Date: Thu, 21 May 2026 15:56:18 +0000 (+0100) Subject: [Fix] arc: emit ARC headers in a deterministic order X-Git-Tag: 4.1.0~37^2 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=152c773e8f0ad48dc8fa1d5154ce8dc1990ac0f6;p=thirdparty%2Frspamd.git [Fix] arc: emit ARC headers in a deterministic order lua_mime.modify_headers accepted an `order` list but it had no effect: the headers passed through a string-keyed Lua table and were serialised to the milter reply in arbitrary hash order. arc.lua relied on `order` to lay out an ARC set, so the three ARC headers were emitted in a non-deterministic order. Some validators (e.g. O365) reject ARC sets that are not in the conventional ARC-Seal, ARC-Message-Signature, ARC-Authentication-Results layout. When `order` is given, emit one milter reply per header in that order (set_milter_reply merges replies cumulatively, so a single-key reply has no ambiguous iteration order) and apply the internal modify_header calls in the same order. Issue: #6045 --- diff --git a/lualib/lua_mime.lua b/lualib/lua_mime.lua index cfee81f85e..80c8cd2681 100644 --- a/lualib/lua_mime.lua +++ b/lualib/lua_mime.lua @@ -872,6 +872,9 @@ end --[[[ -- @function lua_mime.modify_headers(task, {add = {hname = {value = 'value', order = 1}}, remove = {hname = {1,2}}}) -- Adds/removes headers both internal and in the milter reply +-- An optional `order` list of header names makes the resulting header sequence +-- deterministic in both the milter reply and the internal message (without it +-- the headers are emitted in arbitrary hash order) -- Mode defines to be compatible with Rspamd <=3.2 and is the default (equal to 'compat') --]] exports.modify_headers = function(task, hdr_alterations, mode) @@ -884,6 +887,8 @@ exports.modify_headers = function(task, hdr_alterations, mode) local add_headers = {} -- For Milter reply local hdr_flattened = {} -- For C API + local add_order = {} -- Ordered, de-duplicated list of added header names + local add_seen = {} -- Tracks header names already recorded in add_order local function flatten_add_header(hname, hdr) if not add_headers[hname] then @@ -918,14 +923,27 @@ exports.modify_headers = function(task, hdr_alterations, mode) add_headers[hname] = add_headers[hname][1] end end + -- Record the order in which `add` headers are processed. With an explicit + -- `hdr_alterations.order` list this order is honoured for both the milter + -- reply and the internal header path, so callers such as arc.lua get a + -- deterministic header layout. + local function record_and_flatten(hname, hdr) + if hdr == nil then + return + end + if not add_seen[hname] then + add_seen[hname] = true + add_order[#add_order + 1] = hname + end + flatten_add_header(hname, hdr) + end if hdr_alterations.order then - -- Get headers alterations ordered for _, hname in ipairs(hdr_alterations.order) do - flatten_add_header(hname, add[hname]) + record_and_flatten(hname, add[hname]) end else for hname, hdr in pairs(add) do - flatten_add_header(hname, hdr) + record_and_flatten(hname, hdr) end end @@ -958,13 +976,47 @@ exports.modify_headers = function(task, hdr_alterations, mode) hdr_alterations.remove = nil end end - task:set_milter_reply({ - add_headers = add_headers, - remove_headers = hdr_alterations.remove - }) + -- Emit the milter reply. When an explicit order was requested, send one + -- reply per header in that order: set_milter_reply merges replies + -- cumulatively, and a single-key reply has no ambiguous iteration order, so + -- the merged add_headers object keeps the requested sequence. A plain + -- multi-key Lua table would otherwise be serialised in arbitrary hash order. + if add_headers and hdr_alterations.order and #add_order > 1 then + local pending_remove = hdr_alterations.remove + for _, hname in ipairs(add_order) do + local hdr = add_headers[hname] + if hdr ~= nil then + task:set_milter_reply({ + add_headers = { [hname] = hdr }, + remove_headers = pending_remove, + }) + pending_remove = nil + end + end + if pending_remove then + task:set_milter_reply({ remove_headers = pending_remove }) + end + else + task:set_milter_reply({ + add_headers = add_headers, + remove_headers = hdr_alterations.remove + }) + end + -- Apply internal header modifications in a deterministic order: added + -- headers first, following add_order (the caller-requested order when given, + -- arbitrary otherwise), then any headers that only have removals. + local applied = {} + for _, hname in ipairs(add_order) do + if hdr_flattened[hname] and not applied[hname] then + applied[hname] = true + task:modify_header(hname, hdr_flattened[hname]) + end + end for hname, flat_rules in pairs(hdr_flattened) do - task:modify_header(hname, flat_rules) + if not applied[hname] then + task:modify_header(hname, flat_rules) + end end end diff --git a/src/plugins/lua/arc.lua b/src/plugins/lua/arc.lua index 91e3f52282..acd9a77201 100644 --- a/src/plugins/lua/arc.lua +++ b/src/plugins/lua/arc.lua @@ -680,7 +680,10 @@ local function arc_sign_seal(task, params, header) local folded_sig = rspamd_util.encode_base64(rspamd_util.decode_base64(sig_b64), 70, nl_type) cur_arc_seal = cur_arc_seal .. folded_sig - -- Add all ARC headers in a single call with explicit ordering + -- Add all ARC headers with a deterministic order. Each header is prepended + -- (order = 1), and the `order` list controls the emission sequence, so the + -- resulting message has ARC-Seal, ARC-Message-Signature, ARC-Authentication-Results + -- top to bottom. Some validators (e.g. O365) reject sets in any other layout. lua_util.debugm(N, task, 'adding ARC-Authentication-Results: %s', cur_auth_results) lua_util.debugm(N, task, 'adding ARC-Message-Signature: %s', header) lua_util.debugm(N, task, 'adding ARC-Seal: %s', cur_arc_seal) @@ -696,7 +699,8 @@ local function arc_sign_seal(task, params, header) { structured = true, encode = false }) } }, - -- RFC 8617 requires strict ordering of ARC headers + -- Emission order: each header is prepended, so listing AAR, AMS, AS here + -- yields AS, AMS, AAR in the final message (newest seal on top) order = { 'ARC-Authentication-Results', 'ARC-Message-Signature', 'ARC-Seal' }, }) task:insert_result(settings.sign_symbol, 1.0, diff --git a/test/functional/cases/320_arc_signing/005_header_order.robot b/test/functional/cases/320_arc_signing/005_header_order.robot new file mode 100644 index 0000000000..230731238c --- /dev/null +++ b/test/functional/cases/320_arc_signing/005_header_order.robot @@ -0,0 +1,55 @@ +*** Settings *** +Suite Setup Rspamd Setup +Suite Teardown Rspamd Teardown +Library Collections +Library ${RSPAMD_TESTDIR}/lib/rspamd.py +Resource ${RSPAMD_TESTDIR}/lib/rspamd.robot +Variables ${RSPAMD_TESTDIR}/lib/vars.py + +*** Variables *** +${CONFIG} ${RSPAMD_TESTDIR}/configs/arc_signing/simple.conf +${MESSAGE} ${RSPAMD_TESTDIR}/messages/dmarc/fail_none.eml +${REDIS_SCOPE} Suite +${RSPAMD_SCOPE} Suite +${RSPAMD_URL_TLD} ${RSPAMD_TESTDIR}/../lua/unit/test_tld.dat +@{EXPECTED_ARC_ORDER} ARC-Authentication-Results ARC-Message-Signature ARC-Seal + +*** Test Cases *** +ARC SET MILTER HEADER ORDER + # arc.lua passes an explicit `order` list to lua_mime.modify_headers. The + # milter add_headers block must be emitted in that order + # (ARC-Authentication-Results, ARC-Message-Signature, ARC-Seal): a milter + # inserts each header at index 1, so this emission order makes the final + # message read ARC-Seal, ARC-Message-Signature, ARC-Authentication-Results + # top to bottom. Without an explicit order the headers were serialised in + # arbitrary hash order (issue #6045). + Scan File ${MESSAGE} User=bob@cacophony.za.org + Expect Symbol ARC_SIGNED + ${arc_order} = Arc Add Headers Order + Should Be Equal ${arc_order} ${EXPECTED_ARC_ORDER} + +ARC SET HEADER ORDER IS DETERMINISTIC + # The ARC header order must not depend on hash iteration order: repeated + # scans of the same message must yield an identical layout. + Scan File ${MESSAGE} User=bob@cacophony.za.org + ${first} = Arc Add Headers Order + Scan File ${MESSAGE} User=bob@cacophony.za.org + ${second} = Arc Add Headers Order + Should Be Equal ${first} ${second} + +*** Keywords *** +Arc Add Headers Order + # ARC header field names, in the order they appear in the milter add_headers block + Dictionary Should Contain Key ${SCAN_RESULT} milter + ... msg=milter block was not present in protocol response + Dictionary Should Contain Key ${SCAN_RESULT}[milter] add_headers + ... msg=add_headers block was not present in protocol response + ${keys} = Get Dictionary Keys ${SCAN_RESULT}[milter][add_headers] sort_keys=${False} + ${arc} = Create List + FOR ${k} IN @{keys} + ${is_arc} = Run Keyword And Return Status Should Start With ${k} ARC- + IF ${is_arc} + Append To List ${arc} ${k} + END + END + RETURN ${arc}