Subject: [apps-discuss] APPSDIR review of draft-ietf-marf-dkim-reporting-15

From: Aaron Stone <[email-masked]>
Date: Fri, 16 Mar 2012 04:11:17 -0700

Document: draft-ietf-marf-dkim-reporting-15
Title: Extensions to DKIM for Failure Reporting
Reviewer: Aaron Stone
Review Date: 3/15/2012
IESG Telechat Date: 3/15/2012

Summary: This draft is generally in good shape; the major issues here
should be easy to resolve.
I hope the minor issues notes are helpful and not too pedantic.

=== Major Issues:

Should the well-known domain components "_report._domainkey" be
registered with IANA?
(Hmm, I don't see any such registry. In that case, there is an edit
below suggesting text to more clearly state a MUST around this domain
record name.)

Please add a note about DKIM selectors. The name "_report._domainkey" is in the
same namespace as RFC 6376, Section 3.1 Selectors, which take the form,
"yourselector._domainkey". This effectively states that a domain implementing
both strategies MUST NOT use "_report" as a selector, because of this
requirement of the algorithm described in Section 3.3:

   3.   If the DNS query returns anything other than RCODE 0 (NOERROR),
        or if multiple TXT records are returned, terminate.

=== Minor Issues:

Introduction:

UPDATED
   DomainKeys Identified Mail [DKIM] introduced a mechanism for message
   signing and authentication.  It uses digital signing to associate a
   domain name with a message in a reliable manner.
   The verified domain name can then be evaluated before
   the message is delivered, e.g., checking advertised sender
   policy, comparing to a known-good sender list, submitting to a reputation
   service, and so on.

   A message sender employing [DKIM] may want to know when and why
   messages received by others fail verification. This applies equally to
   verification failures in a message containing DKIM headers, as well as
   published signing practices (e.g., Author Domain Signing
   Practices, [ADSP]) in an Administrative Management Domain
   (ADMD; see [EMAIL-ARCH]).

   This document extends [DKIM] and [ADSP] to add an optional reporting
   address and some reporting parameters.  Reports are generated using
   the format defined in [I-D.IETF-MARF-AUTHFAILURE-REPORT].

Sections 2.2 and 2.3 - switch the order, so that it goes: Keywords,
Notation, Imported Definitions, Other Definitions.

Section 2.4 - report generator has an awkward use of "also". Remove
the world "also":

OLD
    ... also designed to ...
NEW
   ... designed to ...

Section 3:

OLD
   A domain name owner employing [DKIM] for email signing and
   authentication might want to know when signatures in use by that ought to be
   verifiable with specific public keys are not successfully verifying.
   Currently there is no such mechanism defined.

   This document adds optional "tags" (as defined in [DKIM]) to the
   DKIM-Signature header field and the DKIM key record in the DNS, using
   the formats defined in that specification.
NEW
   A domain name owner employing [DKIM] for email signing and
   authentication might want to know when message recipients are unable
   to verify a message due to problems in the keys published for a domain.
   Currently there is no such mechanism defined.

   This section adds optional tags to the DKIM-Signature header field
   and the DKIM key record in the DNS, using the formats defined in [DKIM].

Section 3.1: mismatch between prose and ABNF. Prose says "upper or
lower case", ABNF is lower-case only. Edit the prose to match the
ABNF:

OLD
    At present, the only legal value is the single character "y"
    (in either upper or lower case).

NEW
    At present, the only legal value is the single character "y".

Section 3.2: I'm confused by the following paragraph, and took a while to
understand which DNS record would be used. I suggest this edit:

OLD
   When a Signer wishes to advertise that it wants to receive failed
   verification reports, it also places in the DNS a TXT resource record
   (RR).  The RR is made up of a sequence of tag-value objects (much
   like DKIM key records, as defined in Section 3.6.1 of [DKIM]), but it
   is entirely independent of those key records and is found at a
   different name.  In the case of a record advertising the desire for
   authentication failure reports, the tags and values comprise the
   parameters to be used when generating the reports.  A report
   generator will request the content of this record when it sees an
   "r=" tag in a DKIM-Signature header field.
NEW
   When a Signer wants to receive reports of failed DKIM verifications,
   it adds a TXT resource record (RR) in the DNS, advertising how to
   notify the Signer of such failures.  The RR contains a sequence of
   tag-value objects in a similar format as DKIM key records, specifying
   parameters to be used when generating failure reports.

   The TXT record containing failure reporting information MUST be
   "_report._domainkey" within the relevant DNS domains.  A report
   generator will request the contents of this record when it sees an
   "r=" tag in a DKIM-Signature header field.

Section 3.2: Confusing use of "tags" and "tokens". Edited to use "tokens".
HOWEVER - maybe the word "values" would be better?

OLD
   rr=  Requested Reports (plain-text; OPTIONAL; default is "all").  The
      value MUST be a colon-separated list of tokens representing those
      conditions under which a report is desired.  See Section 5.1 for a
      list of valid tags.
NEW
   rr=  Requested Reports (plain-text; OPTIONAL; default is "all").  The
      value MUST be a colon-separated list of tokens representing those
      conditions under which a report is desired.  See Section 5.1 for a
      list of valid tokens.

Section 3.2: Use of dkim-quoted-printable in prose, but qp-section in ABNF.
Please edit this as apprpriate.

ERR
   rs=  Requested SMTP Error String (text; OPTIONAL; no default).  The
      value is a dkim-quoted-printable string that the publishing ADMD
                 ^^^^^^^^^^^^^^^^^^^^^
      requests be included in [SMTP] error strings if messages are
      rejected during the delivery SMTP session.  ABNF:

       rep-rs-tag = %x72.73 *WSP "=" qp-section
                                     ^^^^^^^^^^
                  ; "rs=..." (lower-case only for the tag name)

Section 4: Confusing language, similar edits as intro to Section 3:

OLD
   There also exist cases in which a domain name owner employing [ADSP]
   for announcing signing practises with DKIM may want to know when
   messages are received without valid author domain signatures.
   Currently there is no such mechanism defined.

   This document adds the following optional "tags" (as defined in
   [ADSP]) to the DKIM ADSP records, using the form defined in that
   specification:
NEW
   A domain name owner employing Author Domain Signing Practices [ADSP]
   may also want to know when message recipients are unable to verify
   a message due to errors in the ADSP records.
   Currently there is no such mechanism defined.

   This section adds optional tags to the DKIM ADSP records,
   using the formats defined in [ADSP].
(and [DKIM] ?)

Section 4: Run-on sentence, and dkim-quoted-printable vs. qp-section again.

OLD
   ra=  Reporting Address (plain-text; OPTIONAL; no default).  The value
      MUST be a dkim-quoted-printable string containing the local-part
                ^^^^^^^^^^^^^^^^^^^^^
      of an email address to which a report SHOULD be sent when mail
      claiming to be from this domain failed the verification algorithm
      described in [ADSP], in particular because a message arrived
      without a signature that validates, which contradicts what the
      ADSP record claims.
NEW
   ra=  Reporting Address (plain-text; OPTIONAL; no default).  The value
      MUST be a dkim-quoted-printable string containing the local-part
                ^^^^^^^^^^^^^^^^^^^^^
      of an email address to which a report SHOULD be sent when mail
      claiming to be from this domain failed the verification algorithm
      described in [ADSP].

ERR
       adsp-ra-tag = %x72.61 *WSP "=" qp-section
                                      ^^^^^^^^^^
                   ; "ra=..." (lower-case only for the tag name)

Section 4: remove this sentence, it's not particularly useful?

      ... An exception to this
      might be some out-of-band arrangement between two parties to
      override it with some mutually agreed value. ...

Section 4: tokens vs. tags again. "value" might still be a better word?

OLD
   rr=  Requested Reports (plain-text; OPTIONAL; default is "all").  The
      value MUST be a colon-separated list of tokens representing those
      conditions under which a report is desired.  See Section 5.2 for a
      list of valid tags.  ABNF:
NEW
   rr=  Requested Reports (plain-text; OPTIONAL; default is "all").  The
      value MUST be a colon-separated list of tokens representing those
      conditions under which a report is desired.  See Section 5.2 for a
      list of valid tokens.  ABNF:

Section 5: remove extra sentence

OLD
   This memo also includes, as the "ro" "rr" tags defined above, the means by
   which the signer can request reports for specific circumstances of
   interest.  Verifiers MUST NOT generate reports for incidents that do
   not match a requested report, and MUST ignore requests for reports
   not included in this list.
NEW
   The "ro" and "rr" tags allow a Signer to specify the types of errors it
   is interested in receiving reports about. This section defines
   the error types and corresponding token values.

   Verifiers MUST NOT generate reports for incidents that do
   not match a requested report, and MUST ignore requests for reports
   not included in this list.

Section 8.3: missing context clues

OLD
   Some threats caused by deliberate misuse of this mechanism are
   discussed in Section 3.3, but they warrant further discussion here.
NEW
   Some threats caused by deliberate misuse of this error reporting
   mechanism are discussed in Section 3.3, but they warrant further
   discussion here.

Paragraph out of order; this paragraph:
   Negative caching offers some protection against this pattern of
   abuse, although it will work only as long as the negative time-to-
   live on the relevant SOA record in the DNS.

Please move it to just above the paragraph about positive caching, as well
as making some small changes below:

OLD
   Implementers are therefore strongly advised not to advertise that DNS
   record except when reports desired, including the risk of receiving
   this kind of attack.

   Positive caching of this DNS reply also means turning off the flow of
   reports by removing the record is not likely to have immediate
   effect.  A low time-to-live on the record needs to be considered.
NEW
   Domain owners are therefore strongly advised not to advertise the DNS
   records specified in this document except when failure reports are desired.
   Domain owners ought to be prepared to receive unexpected traffic and attacks.

   Negative caching offers some protection against this pattern of
   abuse, although it will work only as long as the negative time-to-
   live on the relevant SOA record in the DNS.

   Positive caching of DNS records also means turning off the flow of
   reports by removing the record is not likely to have immediate
   effect.  A low time-to-live on the record needs to be considered.

=== Nits: [list editorial issues such as typographical errors,
preferably by section number]

Section 3.2: typo at the very last sentence:
OLD
   See Section 8.4 for some considerations regardin limitations of
this mechanism.
NEW
   See Section 8.4 for some considerations regarding limitations of
this mechanism.

Remove the quotes around "tags" -- tags appears as a bare word in [DKIM], so
it's safe to use here, too.

Choose either "document" or "memo" to refer to this document / memo throughout.

END

---

From: Dave Crocker <[email-masked]>
Date: Fri, 16 Mar 2012 07:26:46 -0700

> below suggesting text to more clearly state a MUST around this domain

Currently, every new use of a reserved, underscore-based domain name is creating 
its own registry for the name(s).  So far, there are roughly 20.

I've twice proposed creating a single, consolidated registry for this.  The 
first proposal, in 2006, was incomplete.

The recent version seems complete to me, but the dnsop working group produced no 
responses when I queried it for feedback repeatedly:

    http://tools.ietf.org/html/draft-crocker-dns-attrleaf-06

We still need the registry, as you note.

d/
-- 

   Dave Crocker
   Brandenburg InternetWorking
   bbiw.net

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Fri, 16 Mar 2012 17:42:33 +0000

> -----Original Message-----
> From: Aaron Stone [mailto:[email-masked]]
> Sent: Friday, March 16, 2012 4:11 AM
> To: [email-masked]; [email-masked]
> Cc: The IESG
> Subject: APPSDIR review of draft-ietf-marf-dkim-reporting-15
> 

Hi Aaron,

This arrived after the telechat completed.  I'll talk to our AD about how to handle your comments, modulo my replies below.

> below suggesting text to more clearly state a MUST around this domain

The registry doesn't exist.  Another document proposes it, but it hasn't gained much momentum so far (unfortunately).  See Dave's reply.

> Please add a note about DKIM selectors. The name "_report._domainkey"
> is in the same namespace as RFC 6376, Section 3.1 Selectors, which take
> the form, "yourselector._domainkey". This effectively states that a
> domain implementing both strategies MUST NOT use "_report" as a
> selector, because of this requirement of the algorithm described in
> Section 3.3:
> 

A selector, per RFC6376 Section 3.1, is a set of "sub-domain"s concatenated by periods.  A "sub-domain" is defined in Section 4.1.2 of RFC5321 in a way that disallows underscores.  There's therefore no overlap.

> [...]

OK.

> Notation, Imported Definitions, Other Definitions.

OK.

> Section 2.4 - report generator has an awkward use of "also". Remove the
> world "also":
> 
> OLD
>     ... also designed to ...
> NEW
>    ... designed to ...

Actually the intent there is to indicate this is a slightly atypical Verifier in the DKIM sense; it's a DKIM Verifier that also has a report generation capability.  I'll say that explicitly.

>    Currently there is no such mechanism defined.

This isn't quite right, because there could be no problem with the keys, but rather message alteration in transit that the signer wants to know about.

>    and the DKIM key record in the DNS, using the formats defined in [DKIM].

OK.

>     At present, the only legal value is the single character "y".

Removed; a note was already added in the ABNF to indicate that only the lowercase form is allowed.

> Section 3.2: I'm confused by the following paragraph, and took a while
> to understand which DNS record would be used. I suggest this edit:
> 
> OLD

OK.

> HOWEVER - maybe the word "values" would be better?

I used "tags" in this context to refer to the left half of "a=b" objects; "tokens" are the elements in a list of values.

>       conditions under which a report is desired.  See Section 5.1 for a

Right, fixed.

>       value is a dkim-quoted-printable string that the publishing ADMD

Fixed, and a reference added in the Imported Definitions section.

>    Currently there is no such mechanism defined.

This is also not quite right.  The intent is to allow reporting of messages that fail ADSP checks even when there aren't errors in ADSP records.

>    using the formats defined in [ADSP].

ADSP refers to DKIM to get the tag format, so I think this is good enough.

>       MUST be a dkim-quoted-printable string containing the local-part

Fixed.

>        adsp-ra-tag = %x72.61 *WSP "=" qp-section

Fixed.

>       override it with some mutually agreed value. ...

The IESG lately likes to see example cases where one might go against the admonition of a SHOULD NOT, so I think it's reasonable to leave in.

>       list of valid tokens.  ABNF:

Fixed.

>    not included in this list.

OK.

>    mechanism are discussed in Section 3.3, but they warrant further

OK.

> Please move it to just above the paragraph about positive caching,

OK.

> as well as making some small changes below:
> 
> OLD
>    See Section 8.4 for some considerations regardin limitations of this
> mechanism.
> NEW

Fixed.

> Remove the quotes around "tags" -- tags appears as a bare word in
> [DKIM], so it's safe to use here, too.

I prefer it with the quotes at least on first use.  I agreed with almost everything else, so I'll make my stand here.  :-)

> Choose either "document" or "memo" to refer to this document / memo

OK.

Thanks for the review!

-MSK

---

From: Pete Resnick <[email-masked]>
Date: Fri, 16 Mar 2012 13:18:41 -0500

Hi Aaron,

Thanks for the review. Murray has addressed many of the issues, but I 

>     the format defined in [I-D.IETF-MARF-AUTHFAILURE-REPORT].

That's an awful lot of rewrite and, even though it's in the intro, I am 
worried about introducing something that the WG would not agree with. 
What about the current text are you trying to clarify? Is there 
something wrong with the current text, or is this just a stylistic 

>     "r=" tag in a DKIM-Signature header field.

Same issue (but moreso) as in the introduction. What is the 
clarification you are making or the issue that you are trying to fix?

>     not included in this list.

Again, why?

>     effect.  A low time-to-live on the record needs to be considered.

And this one too. Please explain.

Nits I've got no concerns about, and some of the re-ordering and the 
like is just nits. But since you put these in the "Minor issues" 
section, I'd like to understand the issues.

Thanks again for your thorough review.

pr

-- 
Pete Resnick<http://www.qualcomm.com/~presnick/>
Qualcomm Incorporated - Direct phone: (858)651-4478, Fax: (858)651-1102

---

From: Aaron Stone <[email-masked]>
Date: Mon, 19 Mar 2012 11:29:18 -0700

[Text clipped to replied-to sections only]

--
> A selector, per RFC6376 Section 3.1, is a set of "sub-domain"s concatenated by periods.  A "sub-domain" is defined in Section 4.1.2 of RFC5321 in a way that disallows underscores.  There's therefor…

Of course! I've been using service records so much lately that I
lapsed on the whole point of using "_".

--
> Actually the intent there is to indicate this is a slightly atypical Verifier in the DKIM sense; it's a DKIM Verifier that also has a report generation capability.  I'll say that explicitly.

Sounds good.

> This isn't quite right, because there could be no problem with the keys, but rather message alteration in transit that the signer wants to know about.

I had a hard time understanding what "signatures that ought to be
verifiable with specific public keys are not successfully verifying"
refers to. It's not bad, just a little confusing at first. I don't
object to your original text.

--
> I used "tags" in this context to refer to the left half of "a=b" objects; "tokens" are the elements in a list of values.

Sounds good.

--
> This is also not quite right.  The intent is to allow reporting of messages that fail ADSP checks even when there aren't errors in ADSP records.

Ok, my misunderstanding. Original text should remain, though "There
also exist cases in which" could be removed :)

--
> The IESG lately likes to see example cases where one might go against the admonition of a SHOULD NOT, so I think it's reasonable to leave in.

Ok.

--
> Thanks for the review!

Most welcome! Per Pete's note, I apologize for mixing minor technical
comments with wordsmithing nits.

Aaron

---

From: Aaron Stone <[email-masked]>
Date: Mon, 19 Mar 2012 11:57:26 -0700

On Fri, Mar 16, 2012 at 11:18 AM, Pete Resnick <[email-masked]> wrote:
> Hi Aaron,
>
> Thanks for the review. Murray has addressed many of the issues, but I had a
> few questions:

I realized that I mixed together some wordsmithing nits with minor
issues. Sorry about that, and thank you for calling it out.

--
> worried about introducing something that the WG would not agree with. What
> about the current text are you trying to clarify? Is there something wrong
> with the current text, or is this just a stylistic change? Similary below:

Mostly style, a little bit of understanding. In this case, I tried to
unwind the sentence so that it reads straight through.

I withdraw the proposed changes to the intro except as to remove the
parentheses.

Severity reduced to nit.

> Same issue (but moreso) as in the introduction. What is the clarification
> you are making or the issue that you are trying to fix?

Severity reduced to nit. This is totally wordsmithing. Here's what I
thought was important:

  The RR is made up of a sequence of tag-value objects (much
  like DKIM key records, as defined in Section 3.6.1 of [DKIM]), but it
  is entirely independent of those key records and is found at a
  different name.
  In the case of a record advertising the desire for
  authentication failure reports, the tags and values comprise the
  parameters to be used when generating the reports.

These two sentences had low signal to noise, so I combined them into
one shorter sentence that got straight to the point.

>>    not included in this list.

Nit. Removed the comma splice in the first sentence. Separated the
sentences because one introduces the section, and the other states
normative requirements.

> And this one too. Please explain.

The reordered paragraph was a problem, because the negative caching
had no context until a few paragraphs later when I got that the topic
was DNS caching.

Tightening up the sentences was a clarity nit.

> Nits I've got no concerns about, and some of the re-ordering and the like is
> just nits. But since you put these in the "Minor issues" section, I'd like
> to understand the issues.
>

Thanks for your review review! :)

Aaron