[Gen-art] Gen-ART Last Call review of draft-baeuerle-netnews-cancel-lock-06

[Paul Kyzivat <[e-mail]>]

I am the assigned Gen-ART reviewer for this draft. The General Area 
Review Team (Gen-ART) reviews all IETF documents being processed by the 
IESG for the IETF Chair. Please wait for direction from your document 
shepherd or AD before posting a new version of the draft. For more 
information, please see the FAQ at 
<​http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-baeuerle-netnews-cancel-lock-06
Reviewer: Paul Kyzivat
Review Date: 2017-09-21
IETF LC End Date: 2017-06-28
IESG Telechat date: 2017-09-26

Summary:

This draft is on the right track but has open issues, described in the 
review.

General Comments:

I have not attempted to validate the security properties of this 
document - leaving that to a security review.

I have also not attempted to verify the operational suitability of this 
mechanism because I don't have the experience needed to do so.

Issues:

Major: 1
Minor: 0
Nits:  0

(1) MAJOR:

In Section 2, the ABNF syntax provided does not do what you want it to. 
You supply:

        fields =/ *( cancel-lock / cancel-key )

as an extension to the definition in RFC5536:

     fields          =/ *( approved /
                           archive /
                           control /
                           distribution /
                           expires /
                           followup-to /
                           injection-date /
                           injection-info /
                           lines /
                           newsgroups /
                           organization /
                           path /
                           summary /
                           supersedes /
                           user-agent /
                           xref )

and that in turn extends RFC5322:

     fields          =   *(trace
                           *optional-field /
                           *(resent-date /
                            resent-from /
                            resent-sender /
                            resent-to /
                            resent-cc /
                            resent-bcc /
                            resent-msg-id))
                         *(orig-date /
                         from /
                         sender /
                         reply-to /
                         to /
                         cc /
                         bcc /
                         message-id /
                         in-reply-to /
                         references /
                         subject /
                         comments /
                         keywords /
                         optional-field)

     message         =   (fields / obs-fields)
                         [CRLF body]

RFC5536 got this wrong, and the new draft continues the mistake. The 
problem is with the way things are grouped. Let me give a simpler example:

     foo = *("a" / "b") / *("c" / "d")

This means the following are valid: ab aabb bab cd ccdd dcd
But the following are not: abcd ac

The latter is what you want, for which the syntax would be:

     foo = *("a" / "b" / "c" / "d")

It isn't easy to do a valid syntax extension like this because of way 
the ABNF of RFC5322 is structured. However, after offline discussion, we 
realized that RFC5322 already has an extension point for new headers via 
the <optional-field> rule. Along with that, RFC3864 established a 
process for registering header fields with IANA.

So, my recommendation is that to fix this, remove from section 2 the 
extension of the <fields> rule:

       fields =/ *( cancel-lock / cancel-key )

Instead, simply rely on the text to specify the newly defined header 
fields, and the IANA registration to link them to RFC5322. This will 
probably require some minor tweaking of the text. I won't try to do the 
wordsmithing here.