Skip to content

Add specification for testing SPF#1146

Merged
marc-vanderwal merged 7 commits into
zonemaster:developfrom
marc-vanderwal:feature/spec-spf
Apr 18, 2023
Merged

Add specification for testing SPF#1146
marc-vanderwal merged 7 commits into
zonemaster:developfrom
marc-vanderwal:feature/spec-spf

Conversation

@marc-vanderwal

Copy link
Copy Markdown
Contributor

Purpose

This PR adds a simple test case for SPF on a domain under test.

Context

Following up on, and simplifying, PR #1100.

Changes

Adds one test case which tests SPF.

How to test this PR

This is a documentation update.

Take the draft specification in the state it was in for pull request
\zonemaster#1100 before that PR was closed.
Reduce the scope of this test case by removing all of the tests that
involve semantics. In this test case, we will care about the syntax
only, in order to simplify the specification.
@marc-vanderwal marc-vanderwal added T-Feature Type: New feature in software or test case description A-TestCase Area: Test case specification or implementation of test case A-Documentation Area: Documentation only. labels Apr 6, 2023
@marc-vanderwal marc-vanderwal added this to the v2023.2 milestone Apr 6, 2023
@marc-vanderwal marc-vanderwal requested review from a user, hannaeko, matsduf, mattias-p and tgreenx April 6, 2023 09:16
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated

@matsduf matsduf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good but I have some minor comments.

Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
 * Output extra messages if SPF is
   inconsistent (Z11_DIFFERENT_SPF_POLICIES_FOUND);

 * Align the message ID for Z11_SPF1_MULTIPLE_RECORDS with @matsduf’s
   suggestion for Z11_DIFFERENT_SPF_POLICIES_FOUND.
@marc-vanderwal marc-vanderwal requested a review from matsduf April 14, 2023 07:57

@matsduf matsduf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks fine, but I have a few comments. Then I can approve.

Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
Comment thread docs/specifications/tests/Zone-TP/zone11.md Outdated
matsduf
matsduf previously approved these changes Apr 18, 2023

@matsduf matsduf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one comment, but that could be considered later. It would not prevent the implementation.

:--------------------------------|:--------|:-------------|:--------------------------------------------
Z11_INCONSISTENT_SPF_POLICIES | WARNING | | The *Child Zone* publishes different SPF policies on different name servers.
Z11_DIFFERENT_SPF_POLICIES_FOUND | NOTICE | ns_ip_list | The following name servers returned the same SPF version 1 policy, but other name servers returned a different policy. Name servers: {ns_ip_list}.
Z11_NO_SPF_FOUND | INFO | | The *Child Zone* does not publish an SPF policy.

@matsduf matsduf Apr 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again, is it a problem if SPF policy is missing? Is that against recommendations? Should we raise the level to NOTICE when there is no policy?

Do you see a connection between the existence of MX and existence of SPF for a domain?

@marc-vanderwal marc-vanderwal Apr 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, not publishing an SPF policy is similar to not signing a zone with DNSSEC: not an error in itself, but maybe something a domain name owner should consider implementing.

The same can be said for DKIM (unless the SPF policy is a so-called “null SPF”, such as v=spf1 -all) and DMARC.

According to my data, on the .fr zone, 62.6% of zones have both MX records and an SPF policy, and 33.2% of zones have no MX at all but an SPF policy. I assume most of these would be “null SPF” policies (i.e. v=spf -all).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to be consistent with the level of the message Zonemaster outputs if there is no DNSSEC signature, I think we can raise the message level for Z11_NO_SPF_FOUND to NOTICE.

Regardless of whether the domain originates mail, it is a good practice to implement SPF:

  • if a domain originates mail, specifying the mail servers authorized to send mail on the domain’s behalf is a basic defense against spoofing and even though SPF alone won’t stop all forms of spoofing, it’s required by DMARC;
  • if a domain does not originate mail (for example, when it’s a parked domain), then it’s a good practice to publish a “null SPF” policy.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* if a domain does not originate mail (for example, when it’s a parked domain), then it’s a [good practice](https://www.m3aawg.org/sites/default/files/m3aawg_parked_domains_bcp-2022-06.pdf) to publish a “null SPF” policy.

In ZONE09 the test case detects Null MX. Is it easy to detect a Null SPF? Should this test case report Null SPF and update ZONE09 to report Null MX?

@matsduf matsduf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for implementation. My comment can be handled later.

@marc-vanderwal

Copy link
Copy Markdown
Contributor Author

Either way, let’s merge this!

@marc-vanderwal marc-vanderwal merged commit 051fef8 into zonemaster:develop Apr 18, 2023
@matsduf

matsduf commented Apr 18, 2023

Copy link
Copy Markdown
Contributor

@marc-vanderwal, could you move the specification to the new path?

marc-vanderwal added a commit to marc-vanderwal/zonemaster that referenced this pull request Apr 19, 2023
@marc-vanderwal

Copy link
Copy Markdown
Contributor Author

Sure, I’ve just created a pull request for that.

@tgreenx tgreenx removed this from the v2023.2 milestone Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Documentation Area: Documentation only. A-TestCase Area: Test case specification or implementation of test case T-Feature Type: New feature in software or test case description

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add SPF and possibly other mail related test cases [Feature Request]

3 participants