Return-Path: X-Original-To: apps-discuss@ietfa.amsl.com Delivered-To: apps-discuss@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id A179321F9021 for ; Fri, 22 Mar 2013 12:21:25 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -2.599 X-Spam-Level: X-Spam-Status: No, score=-2.599 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, HTML_MESSAGE=0.001, NO_RELAYS=-0.001] Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pxYukPuRzyDM for ; Fri, 22 Mar 2013 12:21:24 -0700 (PDT) Received: from mail-we0-x235.google.com (mail-we0-x235.google.com [IPv6:2a00:1450:400c:c03::235]) by ietfa.amsl.com (Postfix) with ESMTP id 78E7121F9019 for ; Fri, 22 Mar 2013 12:21:23 -0700 (PDT) Received: by mail-we0-f181.google.com with SMTP id p43so3588816wea.40 for ; Fri, 22 Mar 2013 12:21:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=alyKuNajeZCXFbqvIiTiEGRC2NYbbwJMv7bKyje9NkA=; b=GtoLucK0e29eutiBc8FgQtHX+ZZUYX6XuTi693bq6vI5soK6+VG6TwhNLe8OqtVyhb 6c8fQ/WkkRDXxejaDWl9OVMFf58wK0kzWM0r9NB5HdM+5rjRWjIeIgj9onxWqmFE9Rrb ffgvI+8A0JrAzLtRa5eReikX+MGzEbGlSuO8QMBw2gHQIx0duMz2QK0BSzJYfVIIrcRO xkgv8k5edDlf84H6djUcV0bUnb4rICeLdz5J79cE4fIILZCPXKIi0xqWVgPsWpQSplWV W6o+FCFNwDWuM5FCDavuD7z4tM/VbkZ17thlMR5PDygd3xLfJ8I8ruGrRODx4AoCq6P1 6v6w== MIME-Version: 1.0 X-Received: by 10.194.178.9 with SMTP id cu9mr5229687wjc.39.1363980082053; Fri, 22 Mar 2013 12:21:22 -0700 (PDT) Received: by 10.180.13.71 with HTTP; Fri, 22 Mar 2013 12:21:21 -0700 (PDT) In-Reply-To: References: Date: Fri, 22 Mar 2013 12:21:21 -0700 Message-ID: From: "Murray S. Kucherawy" To: Dave Cridland Content-Type: multipart/alternative; boundary=089e013d1f284586ff04d8885c09 Cc: IETF Apps Discuss Subject: Re: [apps-discuss] draft-kucherawy-rfc5451bis X-BeenThere: apps-discuss@ietf.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: General discussion of application-layer protocols List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Mar 2013 19:21:25 -0000 --089e013d1f284586ff04d8885c09 Content-Type: text/plain; charset=ISO-8859-1 Hi Dave, thanks for the review. Comments below. On Fri, Mar 15, 2013 at 4:34 AM, Dave Cridland wrote: > For the most part, this document looks fine and solid. > Hooray! > > Two items concern me in a major way; one further item has me mildly > nervous. > > First, the minor one: > > The header includes definition of a version ABNF production optionally > following the authentication server's identity. It took me some time to > find the note of how to treat this; I didn't see any examples with it > present either. > I've updated one of the examples to include a version. > > Second, the first major one: > > The examples tend to imply, for the most part, a canonical form to the > header, which places CFWS is only particular places. In particular, all the > examples are of the form auth-srv-id "; " method "=" result, with some > optional properties and/or reason, with one single exception (which > includes a comment in lieu of a reason). It's not immediately apparent to a > comparatively novice reader that the following header field is valid: > > Authentication-Results: foo.example.net (My mailserver) / (I am number) 1 > (You see) ; dkim (Because I like it) /2 (Two yay) = (wait for it) pass ; > policy (And a dot can go here) . (Like that) fruit (which surprised me) = > (because I was not expecting it) banana > > I appreciate it's always nicer to only write pretty examples, but given > the specification, providing a nasty one is probably needed, lest people > expect to take a whitespace delimited token and split it on '='. For some > reason I particularly wasn't expecting the CFWS around the dot. (Maybe > that's because I'm out of practise with header parsing). > > In any case, highlighting this with an example seems appropriate and > useful. > Copied to the draft, with some adjustments to make it compliant and fit the width limit. > > Thirdly, one more major concern: > > You'll note that I included a version for the method in my example above. > The definition for this, from the comments in the ABNF, suggests that this > relates to the version of RFC 5451 and successors used, but this seems > unlikely - I'm also at a loss as to what a receiving implementation should > do with such a method, since the handling of version seems to be specified > only for the version number after the auth-srv-id. > > I have a vague suspicion that the correct thing to do here may be to > deprecate the method version entirely, but I might be missing something - > still, it doesn't appear to relate to the method, but the header field. > > > The method version refers to the method itself, which is specified elsewhere; the authserv-id version refers to this draft and thus the syntax of this header field. The idea is that if you find a version after an authserv-id that you can't handle, you stop trying to parse right away because what follows might not be what you expect. In terms of a method version, the parser should ignore a method result if the version of the version is not supported in case the semantics of the result have a different meaning. If, for example, DKIM v2 (were such a thing to exist) yielded a "pass" for different reasons than v1 did, a consumer of this field might not want the altered semantics to apply. This is a way to indicate this and let the consumer decide. Is your reply to this "You should say that?" :-) -MSK --089e013d1f284586ff04d8885c09 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
Hi Dave, thanks for the review.=A0=A0 Comments below.
<= div class=3D"gmail_extra">
On Fri, Mar 15, 20= 13 at 4:34 AM, Dave Cridland <dave@cridland.net> wrote:
For the most part, this = document looks fine and solid.

Hooray!
=A0
=

Two items concern me in a ma= jor way; one further item has me mildly nervous.

First, the minor one:

The header includ= es definition of a version ABNF production optionally following the authent= ication server's identity. It took me some time to find the note of how= to treat this; I didn't see any examples with it present either.

I've updated one of = the examples to include a version.
=A0

Second, the first major one:

T= he examples tend to imply, for the most part, a canonical form to the heade= r, which places CFWS is only particular places. In particular, all the exam= ples are of the form auth-srv-id "; " method "=3D" resu= lt, with some optional properties and/or reason, with one single exception = (which includes a comment in lieu of a reason). It's not immediately ap= parent to a comparatively novice reader that the following header field is = valid:

Authentication-Results: foo.example.net (My mailserver) / (I am number) 1= (You see) ; dkim (Because I like it) /2 (Two yay) =3D (wait for it) pass ;= policy (And a dot can go here) . (Like that) fruit (which surprised me) = =3D (because I was not expecting it) banana

I appreciate it's always nicer to only write pretty= examples, but given the specification, providing a nasty one is probably n= eeded, lest people expect to take a whitespace delimited token and split it= on '=3D'. For some reason I particularly wasn't expecting the = CFWS around the dot. (Maybe that's because I'm out of practise with= header parsing).

In any case, highlighting this with an example seems ap= propriate and useful.

Copied to the draft, with some adjustments to make it compliant and = fit the width limit.
=A0

=
Thirdly, one more major concern:

You'll note that I included a version for the metho= d in my example above. The definition for this, from the comments in the AB= NF, suggests that this relates to the version of RFC 5451 and successors us= ed, but this seems unlikely - I'm also at a loss as to what a receiving= implementation should do with such a method, since the handling of version= seems to be specified only for the version number after the auth-srv-id.

I have a vague suspicion that the correct thing to do h= ere may be to deprecate the method version entirely, but I might be missing= something - still, it doesn't appear to relate to the method, but the = header field.



The method version refers to the method itself, which is specified= elsewhere; the authserv-id version refers to this draft and thus the synta= x of this header field.=A0 The idea is that if you find a version after an = authserv-id that you can't handle, you stop trying to parse right away = because what follows might not be what you expect.=A0 In terms of a method = version, the parser should ignore a method result if the version of the ver= sion is not supported in case the semantics of the result have a different = meaning.=A0 If, for example, DKIM v2 (were such a thing to exist) yielded a= "pass" for different reasons than v1 did, a consumer of this fie= ld might not want the altered semantics to apply.=A0 This is a way to indic= ate this and let the consumer decide.

Is your reply to this "You should say that?"=A0 :-= )

-MSK

--089e013d1f284586ff04d8885c09--