Re: [apps-discuss] APPSDIR review of draft-ietf-marf-authfailure-report-05

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

Hi SM, thanks for the review.

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of S Moonesamy
> Sent: Friday, December 02, 2011 11:03 AM
> To: [e-mail]; [e-mail]
> Cc: [e-mail]
> Subject: [apps-discuss] APPSDIR review of draft-ietf-marf-authfailure-report-05
> 
> This draft is almost ready for publication as a Proposed Standard.
> 
> Major issues:
> 
> None.

Whew!

> Minor Issues:
> 
> In the Abstract Section:
> 
>    "This memo registers an extension report type to ARF for use in
>     reporting messages that fail one or more authentication checks"
> 
> In the Introduction Section:
> 
>    "There is now also a desire to extend the ARF format to include
>     reporting of messages that fail to authenticate using known
>     authentication methods"
> 
>    "Thus, this memo presents such extensions to the Abuse
>     Reporting Format to allow for detailed reporting of message
>     authentication failures."
> 
> In Section 3:
> 
>    "The current report format defined in [ARF] lacks some specific
>     features required to do effective sender authentication reporting."
> 
> The use of "authentication" is not consistent throughout the text
> quoted above.  This drafts builds upon RFC 5451 in which DKIM and SPF
> are referred to as email authentication methods.  I suggest using the
> term "email authentication method" for consistency.

Agreed.

There will probably be a few people concerned about the "authentication vs. authorization" matter, but I think RFC5451 dealt with this reasonably well, and the field in general has come to be known as "email authentication".  But you're right that "sender" is misleading in terms of roles, especially in the case of DKIM.  This draft has been around for a while and I guess this text hasn't matured as fast as its brethren.

If it would help, we could add a reference to RFC5451 Section 1.5.2, which talks about the differences and their effective union in this area.

> The wording "presents such extensions" in the Introduction Section is
> not clear.  Section 3.1 of the draft defines a new feedback type of
> "auth-failure" as an extension to RFC 5965.  Is there more than one
> extension?  This memo should update RFC 5965.

I suppose it's worded that way because we created a new report type but had to make a number of changes to IANA registries to do so, hence the plural.  But it really is one omnibus extension action.  I don't have a preference as to wording; if the plural is confusing, we can change it.

> In Section 3.1:
> 
>    "Original-Envelope-Id:  As specified in [ARF].  This field SHOULD be
>     included exactly once if available to the entity generating the
>     report."
> 
> RFC 5965 defines the field as optional and MUST NOT appear more than
> once".  The above is a rewording of a requirement level.  I suggest
> rewriting the last sentence to remove the key word:
> 
>    As specified in [ARF].  This field is included only once if available to the
>    entity generating the report.

The point of saying this is that RFC5965 allows for the absence or presence of the field, but proscribes multiple instances of it.  We want to say, for this report type, that Original-Envelope-ID should be there, further limiting the "absence" case.

Perhaps changing the SHOULD to a MUST above is better?

>   "Original-Mail-From:"
> 
> Please see the comment above about "Original-Envelope-Id".

Ditto for me too.

>   "Source-IP:  As specified in [ARF].  This field SHOULD be included
>      exactly once for SPF, or for other methods that evaluate
>      authentication during the SMTP phase."
> 
> Please see the comment above about "Original-Envelope-Id".

Ditto for me too.

> Why is there a "SHOULD" for SPF?

The Source-IP is a key piece of information for reconstructing why an SPF evaluation failed.  It needs to be there if it's available, or the failure report is basically incomplete.

>    "Reported-Domain:  As specified in [ARF].  This field MUST appear at
>       least once."
> 
> Is it the format which is being imported from RFC 5965 or is it also
> what is specified for the field?

It is the same, except that RFC5965 makes it entirely optional.  For this report type, we want to see it at least once.

>    'The third MIME part of the message is either of type "message/rfc822"
>     (as defined in [MIME-TYPES]) or "text/rfc822-headers" (as defined in
>     [REPORT]) and contains a copy of the entire header block from the
>     original message.  This part MUST be included (contrary to [REPORT]).'
> 
> I suggest having a reference to draft-ietf-appsawg-rfc3462bis-04
> instead of RFC 3462 and removing the "(contrary to [REPORT])".

I agree with the reference change, however the "contrary" part is correct because [REPORT] says the third MIME part in a report is optional, while for this specific instance of it, we want it to be there.  If it would help for illustration, we could say "(contrary to [REPORT], which makes it optional)".

> In Section 3.2.1:
> 
>    "Auth-Failure:  Indicates the type of authentication failure that is
>       being reported.  The list of valid values is enumerated in
>       Section 3.3."
> 
> What is being reported is the failure from an email authentication
> method and not an "authentication failure".

Fair enough.

> In Section 3.2.2:
> 
>    "policy:  The message was not delivered to the intended inbox due
>       to authentication failure."
> 
> Please refer to my comment about the usage of "authentication failure".

Yes.

> Nits:
> 
> In Section 3.2.4:
> 
>    'DKIM-Canonicalized-Body:  A base64 encoding of the canonicalized body
>        of the message as generated by the verifier.  The encoded content
>        MUST be limited to those bytes that contribute to the DKIM body
>        hash (i.e., the value of the "l=" tag; see Section 3.7 of [DKIM].'
> 
> I suggest replacing the word "bytes" with "octets" (RFC 6376).

OK.

>    "If DKIM-Canonicalized-Header and DKIM-Canonicalized-Body encode
>     redacted data, they MUST NOT be included.  Otherwise, they SHOULD be
>     included.  The data presented there have to be exactly the
>     canonicalized header and body as defined by [DKIM] and computed at
>     the verifier.  This is because these fields are intended to aid in
>     identifying message alterations that invalidate DKIM signatures in
>     transit.  Including redacted data in them renders the data unusable.
>    (See also Section 5 and Section 7.6 for further discussion.)"
> 
> It is better to restate the above as "SHOULD be included" and mention
> that it is not applicable if the date has been modified.  If the
> working group wants to mention "redacted data", it can include an
> informative reference to draft-ietf-marf-redaction-03.

We actually had it the other way (as you suggest), and then changed it to this because we thought this description was more effective, i.e., having the strongest requirement first.

> Section 5 is the IANA Considerations Section.  The draft does not
> contain a Section 7.6.

Looks like they're hard references rather than soft ones.  I'll get the author to fix it.

> In Section 3.2.5:
> 
>    "DKIM-ADSP-DNS: Includes the ADSP record discovered and applied by the
>       entity generating this report"
> 
> I suggest:
> 
>    DKIM-ADSP-DNS It is the ADSP record used to obtain the ADSP result.

How about:

	DKIM-ADSP-DNS: Includes the ADSP policy used to obtain the verifier's ADSP result.

("record" suggests an RRTYPE, and there isn't one specific to ADSP)

> I did not include a "MUST" as there is already one in Section 3.3
> (adsp).

We didn't either.  :-)

> In Section 3.3:
> 
>    "The list of defined authentication failure types"
> 
> Please refer to my previous comments about the usage of "authentication
> failure".

Agree.

>    "Supplementary data MAY be included in the form of [MAIL]-compliant
>     comments."
> 
> Why is there a "MAY"?

SHOULD [NOT] and MUST [NOT] don't apply... :-)

Is there a problem with "MAY" there?

> There should be a normative reference to RFC 5234 given the ABNF in
> Section 4.

Wow, I don't know how I missed that one.  We'll add it.

> In Section 6.2:
> 
>    "Perhaps the simplest means of mitigating this threat is to assert
>     that these reports should themselves be signed with something like
>     DKIM."
> 
> I suggest removing the "something like".

I disagree, since one could also in theory use PGP or S/MIME for similar effect.  DKIM is just the most common example, and is actually in practical use in ARF terms.

>  From the example in Appendix B.1:
> 
>   "Received: from mail.example.com (mail.example.com [192.0.2.1])
>      by mx.example.net (8.14.4/8.14.4) with ESMTP id c6cs67945pbm;
>      Sat, 8 Oct 2011 13:16:24 +0000 (GMT)
>    Return-Path: [e-mail]"
> 
> Isn't the Return-Path: mail header inserted before the Received: mail
> headers?

Quite right.

>   "For more information about this format please see
>     http://datatracker.ietf.org/doc/draft-ietf-marf-authfailure-report"
> 
> I suggest adding a comment for the RFC Editor to reference "this RFC".

Yes, that would be the right thing to do.

>   "Policy-Action: none"
> 
> That field has not been defined.  I suggest that the working group
> reviews the example for correctness.

Right, it should be removed.  And given some other comments made within the WG, we'll be revisiting the example before it goes to the IESG.

Thanks again,
-MSK