Subject: [apps-discuss] APPSDIR review of draft-kucherawy-marf-source-ports-01

From: S Moonesamy <[email-masked]>
Date: Thu, 19 Apr 2012 14:14:43 -0700

I have been selected as the Applications Area Directorate reviewer 
for this draft (for background on AppsDir, please see 
http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate ).

Please resolve these comments along with any other Last Call comments 
you may receive. Please wait for direction from your document 
shepherd or AD before posting a new version of the draft.

Document: draft-kucherawy-marf-source-ports-01
Title: Source Ports in ARF Reports
Reviewer: S. Moonesamy
Review Date: April 19, 2012

Summary:  This document is almost ready for publication as a Proposed 
Standard.

This draft defines and registers an additional header field for use in Abuse
Reporting Format reports.  The header field carries source port 
information, which can be useful in IP address sharing scenarios.

Minor issues:

In Section 3:

   "A new ARF reporting field called "Source-Port" is defined.  When
    present in a report, it MUST contain the TCP or UDP source port
    matching the "Source-IP" field in the same report, thereby describing
    completely the origin of the abuse incident."

UDP is not used for SMTP.  It's easier just to remove "TCP or UDP".

   "When any report is generated that includes the "Source-IP" reporting
    field, this field SHOULD also be present."

It's difficult to tell when not to do the above.  I suggest replacing 
SHOULD with RECOMMENDED:

   it is RECOMMENDED to add this header field.

In the Security Considerations section, I suggest referring to RFC 6302.

Nits:

In the Abstract:

   "This document registers an additional header field for use in Abuse
    Reporting Format reports to permit the identification of the source
    port of the connection involved in an abuse incident."

The sentence describes a registration and what the header field 
does.  I suggest breaking the sentence into two parts or keeping it easy:

    This document defines an additional header field for use in Abuse
    Reporting Format reports to permit the identification of the source
    port of the connection involved in an abuse incident.

In the Introduction Section:

   "[ARF] defined the Abuse Reporting Format, a new header message format
    for use in reporting incidents of email abuse."

I suggest removing "new" as it won't be new in a year or 
two.  "header message format" is confusing.  I'll suggest:

    [ARF] defined the Abuse Reporting Format, an extensible format for
    Email Feedback Reports.  These reports are used used to report incidents
    of email abuse.  [ARF] was extended by ...

   "Although those specifications gave the capability to include
    the source IP address in the report, the source port was not
    included

  I suggest:

   These specifications provided for the source IP address to be included
   in a report. As explained in [LOG], the deployment of IP address
   sharing techniques requires the source port values to be included in
   reports if unambiguous identification of the origin of abuse is to be
   achieved.

   "Accordingly, this memo registers an ARF reporting field to contain
    this information and provides guidance for its use."

I suggest:

   This document defines ARF reporting field to specify the source
   port.

I don't see much guidance in the draft.

The reference to I-D.IETF-MARF-AUTHFAILURE-REPORT should be updated 
to RFC 5691.

In Section 3:

   'A new ARF reporting field called "Source-Port" is defined.'

That should be header field (see Section 3.2 of RFC 5965).  I gather 
that the intent is to make this an optional header field.  I suggest 
specifying that Section 3.2 is being updated.  That should also be 
done for Section 3.1 of RFC 6591.

In Section 4:

   "Description:  TCP or UDP source port from which the reported
      connection originated"

I suggest removing "TCP or UDP".

Regards,
S. Moonesamy

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Thu, 19 Apr 2012 23:45:13 +0000

Hi SM, thanks for the review!

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf Of S Moonesamy
> Sent: Thursday, April 19, 2012 2:15 PM
> To: [email-masked]
> Cc: [email-masked]; [email-masked]
> Subject: [apps-discuss] APPSDIR review of draft-kucherawy-marf-source-
> ports-01
> 
> Minor issues:
> 
> In Section 3:
> 

You're right about UDP.  I'd prefer to leave TCP in, however.

>    it is RECOMMENDED to add this header field.

I think these are semantically the same.  We're still left with the question, "When would you not?"  The answer is "When you don't have it," I suppose.  I'll reword accordingly.

> In the Security Considerations section, I suggest referring to RFC 6302.

Good idea; done.

> The sentence describes a registration and what the header field does.
> I suggest breaking the sentence into two parts or keeping it easy:
> 

Done.

> I suggest removing "new" as it won't be new in a year or two.  "header
> message format" is confusing.  I'll suggest:
> 

Done.

>    reports if unambiguous identification of the origin of abuse is to be

OK.

> I don't see much guidance in the draft.

There's some in the next version, based on yours and other feedback.  :-)

> The reference to I-D.IETF-MARF-AUTHFAILURE-REPORT should be updated to
> RFC 5691.

Already done in my copy, but yes.

> specifying that Section 3.2 is being updated.  That should also be done
> for Section 3.1 of RFC 6591.

I haven't seen specific section call-outs done in an updating document before, only the "Updates" stuff on the title page.  Is this necessary?

> I suggest removing "TCP or UDP".

Removed "or UDP".

Thanks again,

-MSK

---

From: S Moonesamy <[email-masked]>
Date: Thu, 19 Apr 2012 17:54:03 -0700

Hi Murray,

>Hi SM, thanks for the review!

It took 74 minutes, including distractions. :-)

>You're right about UDP.  I'd prefer to leave TCP in, however.

Ok.

>I think these are semantically the same.  We're still left with the 
>question, "When would you not?"  The answer is "When you don't have 
>it," I suppose.  I'll reword accordingly.

I used RECOMMENDED to be in line with RFC 6302.  I could not come up 
with suggested text for that "SHOULD" within the deadline.

>There's some in the next version, based on yours and other feedback.  :-)

I'll do a follow-up if the draft goes to Last Call.  BTW, you don't 
need an update in the Abstract ( -02).

>I haven't seen specific section call-outs done in an updating 
>document before, only the "Updates" stuff on the title page.  Is 
>this necessary?

No, as I classified it under nits.  This document may be folded in 
the RFCs it updates at some point.  It's easier if it inherits the 
requirements in those documents.  If I am implementing the 
specifications, it is easier to read and understand.  From an IETF 
perspective, you don't have to worry about all that.

BTW, I didn't get into the RFC 6302 angle altogether.  Section 13.3 
of RFC 6269 discusses about spam in the context of IP address 
sharing.  The issues around IP address sharing are due to the 
deployment of CGNs.

Regards,
S. Moonesamy

---

From: Barry Leiba <[email-masked]>
Date: Fri, 20 Apr 2012 13:08:19 -0400

> I haven't seen specific section call-outs done in an updating document before, only
> the "Updates" stuff on the title page.  Is this necessary?

I've seen it occasionally.  I think that if the sense in which B
updates A is narrow enough that it's easy to specify what's updated,
it's quite useful to say what's updated.  I doubt you'll get any
DISCUSSes if you don't, but anything that will help the reader make
better use of both specs is a good thing.

Barry

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Fri, 20 Apr 2012 17:16:37 +0000

> From: [email-masked]
> [mailto:[email-masked]] On Behalf Of Barry Leiba
> Sent: Friday, April 20, 2012 10:08 AM
> To: Murray S. Kucherawy
> Cc: S Moonesamy; [email-masked]; draft-kucherawy-marf-source-
> [email-masked]; [email-masked]
> Subject: Re: [apps-discuss] APPSDIR review of draft-kucherawy-marf-
> source-ports-01
> 
> > I haven't seen specific section call-outs done in an updating document
> > before, only the "Updates" stuff on the title page.  Is this necessary?
> 

Huh, OK.  References added.

-MSK