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

["Murray S. Kucherawy" <[e-mail]>]

> -----Original Message-----
> From: Aaron Stone [mailto:[e-mail]]
> Sent: Friday, March 16, 2012 4:11 AM
> To: [e-mail]; [e-mail]
> Cc: The IESG
> Subject: APPSDIR review of draft-ietf-marf-dkim-reporting-15
> 
> 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.

Hi Aaron,

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

> === 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.)

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:
> 
>    3.   If the DNS query returns anything other than RCODE 0 (NOERROR),
>         or if multiple TXT records are returned, terminate.

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.

> === Minor Issues:
> 
> Introduction:
> 
> UPDATED
> [...]

OK.

> Sections 2.2 and 2.3 - switch the order, so that it goes: Keywords,
> 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.

> 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 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.

>    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].

OK.

> 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".

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
>    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.

OK.

> Section 3.2: Confusing use of "tags" and "tokens". Edited to use "tokens".
> 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.

> 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.

Right, fixed.

> 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)

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

> 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 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.

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

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

> 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].

Fixed.

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

Fixed.

> 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. ...

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.

> 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:

Fixed.

> 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.

OK.

> 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.

OK.

> 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,

OK.

> 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.

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
> throughout.

OK.

Thanks for the review!

-MSK