domainaware / checkdmarc

A parser for SPF and DMARC DNS records
https://domainaware.github.io/checkdmarc
Apache License 2.0
251 stars 77 forks source link

Fix ipv4 validation and add support for ipv6 #52

Closed akondas closed 4 years ago

akondas commented 4 years ago

In this PR:

seanthegeek commented 4 years ago

Awesome. Thank you!

seanthegeek commented 4 years ago

@akondas After I merged this PR and released it in 4.3.0, I ran into a problem that don't know how to deal with.

The SPF record for microsoft.com contains ipv4:207.46.22.98/29

This CIDR blocks causes ipaddress.ip_network() to raise a ValueError exception:

ValueError: 207.46.22.98/29 has host bits set

I honestly don't know enough about CIDR notation to understand this message. The SPF record for microsoft.com passes every other third-part SPF check I could find.

The exception can be avoided by passing strict=False to ipaddress.ip_network(), but that causes your testSPFInvalidIPv6Range unit test to fail.

I've thought about doing something like this, but it feels like a sloppy hack

        try:
            if mechanism in ["ip4", "ip6"]:
                try:
                    if ":" in value:
                        ipaddress.ip_network(value)
                    else:
                        ipaddress.ip_network(value, strict=False)
                except ValueError:
                    raise SPFSyntaxError("{0} is not a valid ipv4/ipv6 "
                                         "value".format(value))

Any suggestions?

akondas commented 4 years ago

Hmm, looks like 207.46.22.98/29 should be written as 207.46.22.96/29. I think we can disable strict mode since people use wrong notation.

akondas commented 4 years ago

53 should fix this problem :wink: