Verifying, modifying and then signing emails results broken chain

[abeverley]

If OpenARC is used to verify an inbound email (with no existing chain), then if the email is modified and re-signed, then the chain is broken. This is because OpenARC carries forward the existing none verification status, rather than changing it to a pass as would be expected from this scenario functioning correctly.

As an example:

• an email arrives without any chain
• OpenARC initially verifies this and adds Authentication-Results as none (correct)
• Then the content of the email is modified locally and resent via the same milter. At this point, OpenARC identifies none in the Authentication-Results header and carries this forward.
• At the receiving end, the email is received with an ARC status of none and thus fails.

A simple change to fix this would be to carry forward a none as a pass, on the basis of the Authentication-Results header being trusted as being generated on the local machine and the chain not existing at that point.

Interestingly it looks like this was implemented as such in commit 94c7639b. What I do not understand is that this was then reverted in commit c210d04c with no reason why.

Can anyone see any reason why the behavior cannot be retained as per 94c7639b?

[flowerysong]

It needs to be conditional, I think. It's possible for a prior processing step to add Authentication-Results without sealing the message, in which case the correct result to carry forward remains none.

It might be reasonable to do something like this in arc_set_cv():

--- a/libopenarc/arc.c
+++ b/libopenarc/arc.c
@@ -3115,6 +3115,9 @@ arc_set_cv(ARC_MESSAGE *msg, ARC_CHAIN cv)
               cv == ARC_CHAIN_FAIL ||
               cv == ARC_CHAIN_PASS);

+       if ((cv == ARC_CHAIN_NONE) && (msg->arc_nsets != 0))
+               cv = ARC_CHAIN_PASS;
+
        msg->arc_cstate = cv;
 }

It's not valid to set this to none for anything other than i=1, so it makes sense to me for the library to upgrade that to pass and avoid putting itself in an invalid state.

[abeverley]

Thanks @flowerysong makes sense. I've opened #170.