Opened 5 years ago

Last modified 20 months ago

#7 infoneeded defect

ABNF for dmarc-record is slightly wrong

Reported by: tim@… Owned by: todd.herr@…
Priority: major Milestone: Deliverable #3 (changes to DMARC base spec + DMARC Usage Guide
Component: dmarc-bis Version:
Severity: - Keywords:
Cc: kboth+ietf@…

Description

ABNF contains (in section 6.4):

dmarc-record = dmarc-version dmarc-sep

[dmarc-request]

The "dmarc-request" part shouldn't be encapsulated as an "optional sequence" (which is what the square brackets mean). If the dmarc-request is truly optional, then its possible to have valid DMARC records of:

v=DMARC1; ; rua=BLAH p=none

..which is messed up!

Change History (13)

comment:1 Changed 5 years ago by tim@…

[comment via email from George Thessalonikefs]
Hi Tim,

I just read the ticket (https://trac.ietf.org/trac/dmarc/ticket/7) you
opened for the ABNF notation.

Making the dmarc-request not optional will contradict section 7.1
Verifying External Destinations (step 5) which states:

..In particular, the "v=DMARC1" tag is mandatory and MUST
appear first in the list.

in regards to third-party reporting records.

If I don't miss anything I would propose to change the ABNF notation for
dmarc-record from:

dmarc-record = dmarc-version dmarc-sep

[dmarc-request]
[dmarc-sep dmarc-srequest]
[dmarc-sep dmarc-auri]
[dmarc-sep dmarc-furi]
[dmarc-sep dmarc-adkim]
[dmarc-sep dmarc-aspf]
[dmarc-sep dmarc-ainterval]
[dmarc-sep dmarc-fo]
[dmarc-sep dmarc-rfmt]
[dmarc-sep dmarc-percent]
[dmarc-sep]
; components other than dmarc-version and
; dmarc-request may appear in any order

to:

dmarc-record = dmarc-version dmarc-sep

[

dmarc-request
[dmarc-sep dmarc-srequest]
[dmarc-sep dmarc-auri]
[dmarc-sep dmarc-furi]
[dmarc-sep dmarc-adkim]
[dmarc-sep dmarc-aspf]
[dmarc-sep dmarc-ainterval]
[dmarc-sep dmarc-fo]
[dmarc-sep dmarc-rfmt]
[dmarc-sep dmarc-percent]
[dmarc-sep]

]
; components other than dmarc-version and
; dmarc-request may appear in any order

This way I read it as "If you want your dmarc record to include any tags
other than the version tag (thus making it a dmarc policy record), you
should at least include the dmarc-request as the second tag and all
other optional tags can be in any order."

Or even:

dmarc-record = dmarc-policy-record / dmarc-reporting-record

dmarc-reporting-record = dmarc-version dmarc-sep

dmarc-policy-record = dmarc-version dmarc-sep dmarc-request

[dmarc-sep dmarc-srequest]
[dmarc-sep dmarc-auri]
[dmarc-sep dmarc-furi]
[dmarc-sep dmarc-adkim]
[dmarc-sep dmarc-aspf]
[dmarc-sep dmarc-ainterval]
[dmarc-sep dmarc-fo]
[dmarc-sep dmarc-rfmt]
[dmarc-sep dmarc-percent]
[dmarc-sep]

If you want to make the separation between policy and third-party
reporting records clearer.

-- George

comment:2 Changed 5 years ago by kboth+ietf@…

  • Cc kboth+ietf@… added
  • Component changed from arc-multi to dmarc-future-notes

comment:3 Changed 4 years ago by tim@…

Just a note to make sure "section 7.1 Verifying External Destinations (step 5)" is cleaned up to say "v=DMARC1;" with the trailing semi-colon.

comment:4 Changed 2 years ago by vesely@…

I'd prefer an OR grammar, like so:

    dmarc-record    = dmarc-version *(dmarc-sep dmarc-tag)

    dmarc-tag       = dmarc-request /
                      dmarc-srequest /
                      dmarc-auri /
                      dmarc-furi /
                      dmarc-adkim /
                      dmarc-aspf /
                      dmarc-ainterval /
                      dmarc-fo /
                      dmarc-rfmt /
                      dmarc-percent

That syntax would make repeated and missing tags syntactically valid. The spec should still specify what to do with invalid tags in a valid record (cfr. bullet 6 of Section 6.6.3 "Policy Discovery").

--Ale

comment:5 Changed 21 months ago by todd.herr@…

  • Component changed from dmarc-future-notes to dmarc-bis

comment:6 Changed 21 months ago by todd.herr@…

  • Owner changed from draft-ietf-dmarc-arc-multi@… to todd.herr@…
  • Status changed from new to accepted

comment:7 Changed 21 months ago by todd.herr@…

I like Ale's version, but with a slight tweak to reflect that "v=DMARC1;" is the valid record:

    dmarc-record    = dmarc-version dmarc-sep *(dmarc-tag dmarc-sep)

    dmarc-tag       = dmarc-request /
                      dmarc-srequest /
                      dmarc-auri /
                      dmarc-furi /
                      dmarc-adkim /
                      dmarc-aspf /
                      dmarc-ainterval /
                      dmarc-fo /
                      dmarc-rfmt /
                      dmarc-percent

comment:8 Changed 21 months ago by todd.herr@…

ABNF in last comment pushed and merged to main branch.

comment:9 Changed 21 months ago by todd.herr@…

  • Status changed from accepted to started

comment:10 Changed 21 months ago by todd.herr@…

  • Resolution set to fixed
  • Status changed from started to closed

comment:11 Changed 20 months ago by todd.herr@…

  • Resolution fixed deleted
  • Status changed from closed to new

comment:12 Changed 20 months ago by todd.herr@…

  • Status changed from new to assigned

comment:13 Changed 20 months ago by todd.herr@…

  • Status changed from assigned to infoneeded
Note: See TracTickets for help on using tickets.