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

Ignore A-R: "arc=none" #70

Closed
wants to merge 1 commit into from
Closed

Ignore A-R: "arc=none" #70

wants to merge 1 commit into from

Conversation

listerr
Copy link
Contributor

@listerr listerr commented Apr 8, 2024

When ARC verification is enabled in Exim 4.96, it always adds arc=none to the Authentication-Results: header if the message is not ARC signed, along with the spf and dkim results:

Example:

Authentication-Results: turing.lentil.org;
    iprev=pass (a3-13.smtp-out.eu-west-1.amazonses.com) smtp.remote-ip=54.240.3.13;
    spf=pass smtp.mailfrom=eu-west-1.amazonses.com;
    dkim=pass header.d=opportunityhub.open.ac.uk header.s=kxf57gyzklfaei26rmmnqdgqpjannfyi header.a=rsa-sha256;
    dkim=pass header.d=amazonses.com header.s=uku4taia5b5tsbglxyj6zym32efj7xqv header.a=rsa-sha256;
    arc=none

This causes authres_status to always report an invalid signature.

This patch ignores arc=none, so if the message does not contain any ARC signature but is otherwise valid DKIM etc. It will report a valid signature.

When ARC is enabled in Exim, it always adds "arc=none" to the Authentication-Results: header if the message is not ARC signed, along with the spf and dkim results

This causes authres_status to always report an invalid signature.

This patch ignores arc=none, so if the message does not contain any ARC signature but is otherwise valid DKIM etc. It will report valid signature.
@pimlie
Copy link
Owner

pimlie commented Apr 9, 2024

Thank you very much for the PR / issue report. Unfortunately I think the fix for the arc=none result is not in the correct place (and too specific for arc only) as I don't think we should ignore/remove information from the authentication result.

A better fix would probably be to account for self::STATUS_NOSIG in this block: https://github.com/pimlie/authres_status/blob/master/authres_status.php#L477-L502, more specifically I think we only have to check on L483 whether status = PASS or status = PASS + NONE :)

@listerr
Copy link
Contributor Author

listerr commented Apr 10, 2024

Agreed. I thought this would be better somewhere else, but didn't manage to unpick all the nested if.. elseif .. else .. logic, and wasn't sure if changing it there would break something else. :)

I notice some other fixes have been merged overnight. I've submitted a new PR against current master. Tested it and it seems to do the trick.

#72

A better fix would probably be to account for self::STATUS_NOSIG in this block: https://github.com/pimlie/authres_status/blob/master/authres_status.php#L477-L502, more specifically I think we only have to check on L483 whether status = PASS or status = PASS + NONE :)

@listerr listerr closed this Apr 10, 2024
pimlie pushed a commit that referenced this pull request Apr 10, 2024
Another go at #70

Move check from the rfc5451_extract_authresheader() header parsing to status check logic.
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

Successfully merging this pull request may close these issues.

2 participants