Subject: Re: [ietf-dkim] [Technical Errata Reported] RFC6376 (4810)

From: "John R Levine" <[email-masked]>
Date: 26 Sep 2016 22:30:28 -0400

tl;dr the proposed correction does the right thing

>> Section: 3.5
>>
>> Original Text
>> -------------
>> x-sig-q-tag-args = qp-hdr-value
>>
>> Corrected Text
>> --------------
>> x-sig-q-tag-args = dkim-quoted-printable  ; with ":" encoded

>... Section 2.10 shows:
>
>     qp-hdr-value    =  dkim-quoted-printable    ; with "|" encoded
>
>so the suggested change doesn't seem to accomplish the stated goal,
>since the two rules are equivalent.
>
>Nor does dkim-safe-char get us there.
>
>I think the rule should exclude WSP, ":", "/" and "=", and I'm not
>seeing an existing one that gets us there.  Am I missing it?

I also don't see any ABNF term that does the trick.  The
DKIM-signature is a tag-list which is a list of tag=value separated by
semicolons.  The q= tag in a signature is a list of query methods
separated by colons.  Each query method can either be a token or token
/ args where the args is x-sig-q-tag-args.  In those args, you have to
quote a semicolon to avoid starting a new tag, you have to quote a
colon to avoid starting a new method, and quote whitespace which is
otherwise ignored.  A slash or equal sign isn't a problem since you
can't have multiple args per method or multiple values for a tag.

The closest we have is dkim-quoted-printable which already requires
that you quote white space and semicolons, so I think the simplest
non-wrong change would be what Juan proposed, dkim-quoted-printable
with colons also encoded.

R's,
John

PS: For people who don't know him, Juan is the author of the widely
used Port25 MTA, so I expect he ran into this while writing its DKIM
parser.
_______________________________________________
NOTE WELL: This list operates according to
http://mipassoc.org/dkim/ietf-list-rules.html

---

From: Stephen Farrell <[email-masked]>
Date: Tue, 27 Sep 2016 07:24:05 +0100

Thanks folks. I plan to accept this as-is later today
unless someone proposes better text that gets a better
reaction.

S

---

From: "HANSEN, TONY L" <[email-masked]>
Date: Tue, 27 Sep 2016 12:42:45 +0000

tl;dr: I agree with the change suggested

*) I agree with John that "/" and "=" do not need to be encoded because there’s no ambiguity if those were to be present.
*) I also agree with John that WS is already covered by the production.
*) But ":" DOES need to be encoded for sig-q-tag-method. 
*) For sig-q-tag-method, "|" does NOT need to be encoded, but it doesn’t hurt if it is.

An alternate solution would have been to change the definition of qp-hdr-value to add ":" to the list of encoded characters:

       qp-hdr-value = dkim-quoted-printable ; with "|" and ":" encoded

For that matter, dkim-quoted-printable is used in so few places, that it might be even better to just change the list of dkim-safe-char to not include any of these characters. So that is another alternate solution:

dkim-safe-char        =  %x21-39 / %x3C / %x3E-7B / %x7D-7E
                                     ; '!' - '9', '<', '>' - '}', '}', '~'

But the least damage to the document and protocol seems to be to follow the suggestion as given.

	Tony

On 9/27/16, 2:24 AM, "ietf-dkim on behalf of Stephen Farrell" <[email-masked] on behalf of [email-masked]> wrote:

    S

---

From: "John R. Levine" <[email-masked]>
Date: 27 Sep 2016 09:51:25 -0400

> tl;dr: I agree with the change suggested

Yeah, if we were opening this up, the ABNF could be considerably 
clarified.  But since we're just patching, one line will have to do.

R's,
John

>    > used Port25 MTA, so I expect he ran into this while writing its DKIM

Regards,
John Levine, [email-masked], Primary Perpetrator of "The Internet for Dummies",
Please consider the environment before reading this e-mail. https://jl.ly

---

From: Juan Altmayer Pizzorno <[email-masked]>
Date: Tue, 27 Sep 2016 13:26:08 -0400

I had formulated the errata as a quick patch, along the
same lines of had been done elsewhere.  It’s so good to
feel understood! :)

On a bit of a side note, I wonder how much appetite there
is for a document update?  Besides this, we have no way to
include non-ASCII UTF-8 local parts in i=.

.. Juan