GlobalCyberAlliance / domain-security-scanner

Scan domains and receive advice based on their BIMI, DKIM, DMARC, and SPF records
https://dmarcguide.globalcyberalliance.org/
Apache License 2.0
132 stars 26 forks source link

GCA Intern: Improve DMARC and SPF advice #22

Open SeanM-temp opened 2 months ago

SeanM-temp commented 2 months ago

Adds additional checks for DMARC and SPF records. DMARC -Check for v=DMARC1 -Check for p= tag -Verify akim and aspf values SPF -Warning for ptr mechanism -Check for exceeding dns lookup limit

wolveix commented 2 months ago

Hey @SeanM-temp, great first PR!

Your DMARC improvements are appreciated; I used your PR as an opportunity to flesh out the existing DMARC tests :)

Your SPF improvements are a good start, though I think the existing functionality can be improved (use the DMARC function as a starting point). Additionally, your "10 DNS lookup limit" advice is actually slightly misunderstood. This limit refers to the DNS lookups needed to validate the SPF record, not the quantity of keys found within the record.

If you're able to improve this validation further, I'll happily merge this :)

wolveix commented 2 months ago

Thanks for the changes, @SeanM-temp!

The existing separation of the advisor and scanner packages does make validating the number of SPF host lookups difficult. The advisor package shouldn't import scanner; however, the custom DNS implementation being outside of the scanner package is difficult. I'll find some time to properly look over your implementation, and see whether we can improve it :)

wolveix commented 3 weeks ago

Hey @SeanM-temp, apologies that this hasn't been properly reviewed and/or merged yet! I intend to look over it soon :)