On Mon, Jul 9, 2018 at 3:01 PM, Jim Fenton <[email protected]> wrote:
> 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. > Terrific - thanks for going through it. Responses to your questions inline below. > 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). > While I understand your question, is the combination of the two terms not self-evident? Is it something that needs to be further expanded upon? If so, can you suggest some language which might have satisfied your uncertainty? > 2.1: "modify message content": Might not be clear whether this includes > the message header or only the body. > Intended in the most broad sense: header, body, or both. We can add a parenthetical comment to clarify. > 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? > Because A-R is ephemeral and, per 7601 both can and should be deleted at will. > - 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. > The ordering is clearly defined - when "h=" cites (using abbreviated header names) "DKIM:DKIM:Date:AAR:AMS:AAR:AMS:AAR:AMS" that would be AAR[1]:AMS[1]:AAR[2]:AMS[2]" per the definition. - 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? > Because the AS, signing only the ARC chain headers, provides a less fragile/more resilient mechanism for chain continuity. By scoping to just the ARC chain, we expect less collateral breakage than any signature which covers arbitrary headers and body. > In the ABNF for the AMS header field (and others), where is "instance" > defined? > instance is defined in section 3.6 > "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? > Yes, and it goes back to the "ephemerality" of A-R headers. We want to avoid signing any content which downstream handlers can arbitrarily delete. > 4.2.1: Instance tag value range: Why 50? That seems like an arbitrary > choice; is there some other basis for it? > The choice of 50 is somewhat arbitrary, in that it could equally well be 49 or 75. Early versions specified values up to 1024 would be legal, but since the working group felt that handling chains over 10 ADMDs to be fairly unlikely, we thought that a 5x margin should be sufficient without allowing completely ridiculous header bloat. > 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. > Yes, it's the ARC-MULTI doc. We split them so that we can get the basic protocol through WGLC before wrestling through multiple algorithms and algorithm migration. > 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). > Will review the text to clarify. > 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. > The colon is a typo, should be a period. > 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. > Not really, but the process is not clearly spelled out. If an incoming message fails to have a passing DMARC result and the ARC chain does not provide sufficient information (or trusted information) to mitigate a resulting "REJECT" policy, then a message can be REJECTed during the SMTP transaction with this extended result. No ARC chain would have the same result as a failing one in such a case. > 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. > I agree that this is not really a security issue - but it is a potential operational one. Where should this caution be moved do? > 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). > Yes, the "s" tag is the "header.s" ptype when more fully specified. I'll clarify the note to the editor. > 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)? > The previous versions are cited because some of the implementations listed in section 12 were built to earlier versions and have not necessarily been vetted against the current version. With the removal of section 12 upon publication, those earlier versions can also be dropped. The references to 7601bis will be normative, but only once the updated document number is issued. I don't think that we can cite a "bis" as a normative reference. > Appendix B: It would be really, really helpful to have at least one > example here. > Yes, planning to...up until version -13, we had carried some very old examples which were now causing more confusion as people looked at the examples rather than the text. > ===== > Nits: > <eliding here - updates noted for the next version of the spec> Thanks again! --Kurt
_______________________________________________ dmarc mailing list [email protected] https://www.ietf.org/mailman/listinfo/dmarc
