DNS add-on: Initial version, checking if there is a SPF record.

[magmax]

Overview

Creating a SPF extension. The original idea was to help to recon the applications used via DNS TXT records, but after some discussion we decided to separate it into different ActiveScanners, where this one is dedicated to detect SPF problems.

Related Issues

None

Checklist

• Update help
• Update changelog
• Run ./gradlew spotlessApply for code formatting
• Write tests
• Check code coverage
• Sign-off commits
• Squash commits
• Use a descriptive title

For more details, please refer to the developer rules and guidelines.

[thc202]

Changed to draft, still a lot of unnecessary/copied code/resources from the example.

[kingthorin]

All of the ES files should be removed

[magmax]

Thank you very much, @kingthorin, for your detailed review.

Thank you too, @thc202 , for having a look.

I believe I addressed all the findings and should be "good enough", despite it still requires some improvements such as adding solutions to problems and the like.

[thc202]

This should be implemented with an active scan rule not passive (passive should act just on existing content). Either way you need to reserve a scan rule ID.

[magmax]

This should be implemented with an active scan rule not passive (passive should act just on existing content). Either way you need to reserve a scan rule ID.

Yeah, I had my doubts.
Ok, I will change it as soon as I can.

Thank you!

[magmax]

Changed according to conversations.

I tried to write some tests, but not sure if they will work.

[kingthorin]

I think we lost or confused something along the way. The add-on should still be dns. The current rule will remain SPF for starters.

[magmax]

@kingthorin Oh, my bad.

So you proposed to keep the dns plugin, but creating several AbstractAppPlugin inside, each one with a different ID. The first one would be the SPF one, righ?

[kingthorin]

Yup.

[kingthorin]

Actually AbstractHostPlugin probably makes the most sense. (At least for the scenarios I foresee.)

[thc202]

Most of the code was removed instead of renamed.

[kingthorin]

Let us know if you need help sorting it out.

[magmax]

@kingthorin sorry, I forgot to add the new files to git after renaming them.

I've been testing it and seem to work fine, but there is an exception. Because the AbstractPlugin requires a HTTP message with its response, in case the plugin acts over a server which doesn't have an HTTP server it will fail silently.

[kingthorin]

Started review, will do more later. (Sorry I ran out of time)

[magmax]

Is there any non-addressed feedback for PR?

[kingthorin]

Thanks, looks good to me.

[magmax]

@thc202 do you believe we could remove the WIP mark from this PR?

[thc202]

You can (and should) remove it once you think it's ready for merge. (Also mark it ready for review.)

[kingthorin]

@magmax do you plan to finish this?

[magmax]

yes, @kingthorin , but latest review included a lot of changes I didn't expected, and I've been out of time this Christmas. I expect to finish it this week.

[kingthorin]

No problem. Holidays and family are important. No rush, just wanted to know it was still planned 🙂

[magmax]

I've addressed most part of the feedback. Still to be done:

• Implement SessionChangeListener to handle DNS cache over sessions. The simplest way is just to remove the current cache (named reviewedDomains).
• Implement tests for SpfScanRule, what requires mocks and exceeds my current Java habilities (around 15 years not typing Java).
• Perhaps changing some getConstantString and other private methods into static.

[kingthorin]

@magmax are you still planning to finish this?

[magmax]

@kingthorin apologies for the delay.

I'm afraid I do not find this funny any more. I'm not a java developer and creating a mock for the DNS service requires me too much time, so... no, I'm not going to continue. Anyway, I learned a lot in the process.

Thank you very much.

[kingthorin]

@magmax no worries. Thanks for what you've done so far!

We can finish it up.

[github-actions]

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

_{You can retrigger this bot by commenting recheck in this Pull Request}

[kingthorin]

Rebased current, addressed a few items. Still needs session handling, example alerts, and rule unit tests. Which I'll work on.

[thc202]

Be good to see a roadmap for the add-on before merging this (and discuss who's going to maintain/improve, as the OP is no longer interested).

[kingthorin]

From zaproxy/zaproxy#8159 (comment)

• Check SPF rules
• Check dmarc rules by checking _dmarc. TXT records
• Check CAA records
• Extract information in the same sense "wappalyzer" add-on does from TXT records