Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verifying, modifying and then signing emails results broken chain #169

Open
abeverley opened this issue Feb 17, 2024 · 2 comments
Open

Verifying, modifying and then signing emails results broken chain #169

abeverley opened this issue Feb 17, 2024 · 2 comments

Comments

@abeverley
Copy link

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 94c7639. What I do not understand is that this was then reverted in commit c210d04 with no reason why.

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

@flowerysong
Copy link
Contributor

flowerysong commented Feb 17, 2024

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
Copy link
Author

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

flowerysong added a commit to flowerysong/OpenARC that referenced this issue Apr 10, 2024
This is mainly an issue when a single administrative domain extends
the chain multiple times, e.g. on initial receipt and after an
internal modification of the message.

Fixes trusteddomainproject/OpenARC#169
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants