Skip to content

opendmarc_spf qualifier evaluation missing/incorrect #169

@sp-andwei

Description

@sp-andwei

Hey,

because libspf2 seems to be no longer maintained and ignores maximum recursion depth, I was evaluating if libopendmarc's SPF implementation could be used as a replacement. I also found it easier to use.

However, I stumbled upon an issue with an "-ipv4" mechanism, which -- unless I am very much mistaken -- is done wrong in opendmarc_spf. Regardless of type of mechanism or if the mechanism matches, it always returns SPF_RETURN_DASH_FORCED_HARD_FAIL if encountered at the toplevel.

There's a testcase, which expects a wrong result: {"149.20.68.145", "<foo@gushi.org>", "gushi.org", "v=spf1 -ip4:1.2.3.4 ip4:149.20.68.145 -all", DMARC_POLICY_SPF_OUTCOME_FAIL}, /* fail before success */. This test should not fail, because -ip4:1.2.3.4 is not matched by the given IPv4 address.

In a broader scope, the implementation in general does not handle qualifiers for matching mechanisms at all (with the exception of the "all" mechanism). Citing RFC 7208:

When a mechanism is evaluated, one of three things can happen: it can
match, not match, or return an exception.

If it matches, processing ends and the qualifier value is returned as
the result of that record. If it does not match, processing
continues with the next mechanism. If it returns an exception,
mechanism processing ends and the exception value is returned.

Thus, if a mechanism matches on the toplevel, regardless of the mechanism, the qualifier should be checked and the result returned. If stack depth (i.e., include) is greater than 0, any qualifier other than +/none should abort processing of the included record -- this is currently not the case as far as I can see.

While I admit that in >90% of cases qualifiers appear only with "all", I think it would be nice to handle them correctly with other mechanisms.

I'm currently trying to figure out how to best fix those issues (and put together a pull request) but would also be happy for opinions on this (also wouldn't be the first time if I seriously overlooked something).

Andreas

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions