Skip to content

Fail if multiple DMARC records are present#939

Open
adhilto wants to merge 4 commits intomainfrom
multiple-dmarc
Open

Fail if multiple DMARC records are present#939
adhilto wants to merge 4 commits intomainfrom
multiple-dmarc

Conversation

@adhilto
Copy link
Collaborator

@adhilto adhilto commented Mar 2, 2026

🗣 Description

Make it so that the SCuBA Gmail policies relating to DMARC and SPF will fail if multiple DMARC records are returned.

I also tweaked the DNS table formatting so that it's a little easier to see when a query returns multiple DNS records.

💭 Motivation and context

Closes #849

🧪 Testing

  • Updated the unit tests so that multiple DMARC records are returned in some cases. Those cases are now appropriately handled and the existing test cases still pass.
  • I added a new domain to our test tenant and configured it with multiple DMARC records

Reviewers should:

  • Review code changes to ensure changes look correct
  • Verify that the unit tests are passing
  • Run ScubaGoggles manually to verify that the new domain I added for testing is failing and that the other domains are unaffected by this change.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • If applicable, All future TODOs are captured in issues, which are referenced in the PR description.
  • The relevant issues PR resolves are linked preferably via closing keywords.
  • All relevant type-of-change labels have been added.
  • I have read and agree to the CONTRIBUTING.md document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

✅ Pre-merge Checklist

  • This PR has been smoke tested to ensure main is in a functional state when this PR is merged.
  • Squash all commits into one PR level commit using the Squash and merge button.

✅ Post-merge Checklist

  • Delete the branch to clean up.
  • Close issues resolved by this PR if the closing keywords did not activate.

@adhilto adhilto added this to the Hammerhead milestone Mar 2, 2026
@adhilto adhilto self-assigned this Mar 2, 2026
@adhilto adhilto added the bug This issue or pull request addresses broken functionality label Mar 2, 2026
Copy link
Collaborator

@mitchelbaker-cisa mitchelbaker-cisa left a comment

Choose a reason for hiding this comment

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

Looks good, tested with an additional domain that had two DMARC records. Gmail 4.1-4.4 collectively fail if two DMARC records are set, pass if one DMARC record is set with all required fields.

One addition I think would be helpful is clarifying text in the report details column to indicate a domain is failing because two records are set. E.g., "2 DMARC records are set for somedomain.com. See DMARC table below for more details."

@mitchelbaker-cisa
Copy link
Collaborator

Tested SPF updates as well, fails for two SPF records and passes when one field is correctly set.

@adhilto
Copy link
Collaborator Author

adhilto commented Mar 3, 2026

Looks good, tested with an additional domain that had two DMARC records. Gmail 4.1-4.4 collectively fail if two DMARC records are set, pass if one DMARC record is set with all required fields.

One addition I think would be helpful is clarifying text in the report details column to indicate a domain is failing because two records are set. E.g., "2 DMARC records are set for somedomain.com. See DMARC table below for more details."

I just added this for DMARC:
image

I'll defer the SPF one for the issue we have for improving the report details for SPF.

@FollyBeachGurl FollyBeachGurl requested a review from atuomit March 4, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This issue or pull request addresses broken functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DMARC controls should fail if user has multiple DMARC records

2 participants