search

pimlie / authres_status

Roundcube plugin that checks the Authentication-Results headers (rfc5451) of your emails and displays the verification status.
Other
34 stars 20 forks source link

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

Closed listerr closed 2 months ago

listerr commented 3 months ago

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.

pimlie commented 2 months ago

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 commented 2 months ago

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.

https://github.com/pimlie/authres_status/pull/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 :)