On 6/24/18 9:27 PM, Kurt Andersen wrote:
I've now posted draft 15 of the ARC protocol. It should be ready for
last call - please review with that in mind. Note that the XML was a
little wacky in the Implementation Status section. I'll fix that
formatting in the next rev.
I'm very late in reviewing the ARC protocol, and I apologize for that,
as well as for not keeping up with the discussions on this list. So
please consider the below to be a "external review" -- I suspect that
other reviewers outside the WG will have many of the same
questions/comments.
Substantive issues:
General: I see lots of references to "authentication state", beginning
with two references in the abstract, but I don't see the term defined
anywhere. Perhaps add to the general concepts (section 2).
2.1: "modify message content": Might not be clear whether this includes
the message header or only the body.
4. Protocol elements: It's not immediately evident to me why we need new
header fields here rather than extensions to existing header fields:
- AAR is basically an Authentication-Results with an added instance tag;
why not just do that?
- AMS says that it's basically a DKIM-Signature header field with an
instance tag and no version. Why not just add an optional instance tag
to DKIM-Signature? But I do see a semantic difference that isn't listed
in 4.1.2. The order of inclusion of "instanced" header fields is based
on the instance number, not simply bottom-up as in DKIM. I can see why
this is done, since header field ordering isn't guaranteed to be
preserved. But I haven't found anything that fully orders the header
fields in this case; do the "instanced" header fields go before the
others, after the others, or some other way? Note that even if distinct
header field names are used, the h= value in the DKIM-Signature
specifies what's included, not the order.
- AS: I'm having trouble understanding what this is for. Why do we need
another layer of signature on top of that in the AMS?
In the ABNF for the AMS header field (and others), where is "instance"
defined?
"Authentication-Results header fields MUST NOT be included in AMS
signatures": This seems harsh, especially since Section 5 of 7601bis
(cited, also RFC 7601) make inclusion of Authentication-Results a MAY.
Is there a reason for the much more restrictive wording here?
4.2.1: Instance tag value range: Why 50? That seems like an arbitrary
choice; is there some other basis for it?
Informational paragraph: Since the cited DCRUP work has passed IESG
approval (just got a state update to "Waiting on RFC Editor" as I type
this), it sounds like the ARC-MULTI stuff ought to be here rather than
in a separate document. But there are other reasons for having multiple
DKIM-Signatures on a message, such as key rotation. ARC needs to be able
to deal with this; perhaps it's all covered in ARC-MULTI which I haven't
read yet.
5.1.1: As noted above, needs to completely specify the order in which
header fields are supplied to the hash function (both ARC and non-ARC
header fields).
5.2 step 5 et seq: Are some of the following steps sub-steps to 5 (done
repeatedly for each ARC set)? There's a colon there but not sure how far
it goes.
5.2.2 conflicts with the second to last paragraph in 5.2 that a message
with a failing ARC MUST be treated the same as one with no ARC.
9.1 is not a security consideration, or at least it's not in expressed
in terms of how an attacker might exploit ARC to cause problems with
header size.
10. I don't see any other reference to header.s so I'm not sure what the
bracketed comment at the beginning is referring to.
10.2 I'm not clear on why there is an IANA consideration for the "s"
signature tag (perhaps this is the header.s above).
13.2 Not clear to me why previous revisions of this draft are included
as informative references. ARC-MULTI (IMO) should be included in this
draft. ENHANCED-STATUS looks like a normative reference to me. Some of
the references to 7601bis might also be normative; can these be changed
to 7601 (non-bis)?
Appendix B: It would be really, really helpful to have at least one
example here.
=====
Nits:
1. paragraph 3, "each individual processing results" -> "each individual
processing result"
3. paragraph 3, "is directly relevant" -> "are directly relevant"
3.5 (imported ABNF tokens) looks like it would go better being adjacent
to the imported terms in section 3.
4.1.2 second bullet "AMS header" -> "AMS header field"
4.1.2 first bullet of second group "should" -> "SHOULD"
4.1.3 second bullet "Section" is repeated
4.1.3 fifth bullet "h" header -> "h" tag
Might want to run idnits on the draft; it especially doesn't like the
URIs in 13.3 (perhaps just include needed URIs inline in the text).
=====
-Jim
_______________________________________________
dmarc mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/dmarc
