[Resending with corrected review date]
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-05
Reviewer: Paul Kyzivat
Review Date: 2017-06-23
IETF LC End Date: 2017-06-28
IESG Telechat date: TBD
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: 3
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")
This isn't easy to fix because of way the ABNF of RFC5322 is structured.
What you need is for RFC5322 to be restructured as follows:
fields = *trace-prefix
*normal-field
trace-prefix = trace
*optional-field /
*resent
*(resent-date /
resent-from /
resent-sender /
resent-to /
resent-cc /
resent-bcc /
resent-msg-id)
normal-field = orig-date /
from /
sender /
reply-to /
to /
cc /
bcc /
message-id /
in-reply-to /
references /
subject /
comments /
keywords /
optional-field
(Note: I've focused on the normal-field part. Additional work is
probably required on the trace-prefix to get proper extensibility.)
Once that is done, then RFC5536 could be revised as follows:
normal-field =/ approved /
archive /
control /
distribution /
expires /
followup-to /
injection-date /
injection-info /
lines /
newsgroups /
organization /
path /
summary /
supersedes /
user-agent /
xref
And your ABNF can then be:
normal-field =/ cancel-lock / cancel-key
Then you will have a syntax that reflects your intent. Unfortunately
this is a big deal because it requires revisions to both RFC5322 and
RFC5536. The revision to RFC5322 would be entirely backward compatible
in that it will accept exactly the same inputs. The revision to RFC5536
is *not* backward compatible, but IIUC it is compatible with the
*intent* and presumably what has been deployed. (Obviously
implementations of RFC5536 have not been generated directly from the
ABNF.) I'm surprised this hasn't ever been reported.
Another way to handle this would be to revise RFC5322 to simply include
the generic syntax, such as:
fields = *trace-prefix
*optional-field
and then rely on the IANA Permanent Message Header Field Repository to
define the allowable fields.
(2) Minor:
In Section 3.5, step 1 says to hash the key using the algorithm from its
scheme. But IIUC the scheme describes the algorithm that has already
been used when constructing the Cancel-Key header. IIUC the serving
agent need not hash the provided key. Rather it only uses the scheme to
select the locks to compare the hash against.
(3) Minor:
IMO the examples in Section 5 seem incomplete. They don't make clear
what is being done by each participating entity, and at what stage. And
because all the examples use sha256 they don't make it clear that the
algorithm used to create the hashed key could be different from that
used to construct the corresponding lock.
(4) Minor:
In Section 8.1/8.2 the syntax/semantics of the Owner/Change controller
is unspecified. But IANA is charged with allowing/rejecting change
requests only from the "same" owner. How is IANA expected to verify
this? What if a change is required but the original owner is no longer
available?
E.g., the owner might be specified by email address. Later the mail
server for that email address may no longer exist.
_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art
