[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Updated DIGEST-MD5 document (draft-ietf-sasl-rfc2831bis-10.txt)



Alexey Melnikov wrote:

> I need more reviews

1 - Please use RFC 4234 ABNF, not the RFC 822 version.  There are
    various tools that can handle 4234-ABNF, readers are familiar
    with it (after some time), and I've no clue if "implicit LWS"
    allows several adjacent CRLF SP.

    Here's an example how to translate <qop-options> to 4234-ABNF:

-   qop-options       = "qop" "=" DQUOTE qop-list DQUOTE
-   qop-list          = 1#qop-value
-   qop-value         = "auth" | "auth-int" | "auth-conf" |
-                       qop-token
+   qop-options       = "qop" EQU DQUOTE qop-list DQUOTE
+   qop-list          = [LWS] qop-value *( SEP qop-value ) [LWS]
+   qop-value         = "auth" / "auth-int" / "auth-conf" /
+                       qop-token
+   EQU               = [LWS] "=" [LWS]
+   SEP               = [LWS] "," [LWS]
+   LWS               = [*WSP CRLF] 1*WSP

With EQU recycled in all other name=value pairs, SEP used in all
1# constructs, and LWS derived from RFC 2822 FWS excl. obs-FWS.

If all goes well RFC 4234 is at STD before 2831bis.  It's really
confusing if alternatives sometimes use "|" as in 2068 and 2616,
but at other times "/" as in 822 and 4234.

2 - Is the introduction of the new "backslash-canonicalization"
    necessary ?  RFC 2617 says:

| However, the "unq" notation indicates that surrounding quotation
| marks are removed in forming the string A1.

It also mumbles something about not using a DQUOTE within <nonce>,
apparently clients weren't supposed to look for an escaped DQUOTE.

It's almost exactly the same idea in RFC 2069:

| The "username-value" field is a "quoted-string" as specified in
| section 2.2 of the HTTP/1.1 specification [2].  However, the
| surrounding quotation marks are removed in forming the string A1.

Nothing about looking at backslashes within that string.  RFC 2831:

| quoted-string  = ( <"> qdstr-val <"> )
| qdstr-val      = *( qdtext | quoted-pair )
| qdtext         = <any TEXT except <">>

A <qdstr-val> is apparently used as is including any <quoted-pair>.

Are you sure that existing implementations got this wrong and
removed the backslashes introducing a <quoted-pair> ?  Otherwise
I'd think it's an incompatibility with 2831 / 2617 / 2069, and it
should be listed in appendices A and B.

> opinions on the open issues

A.3 is as you say in progress, see above for an idea how to finish
the transition to 4234-ABNF.  This would also eliminate chapter 7.

Generally 2617 and 2831 is a horrible scheme with its at least 10
parameters, some can be omitted, or empty, or repeated, or they are
required exactly once, in any order.  More ranting removed, it's
not your fault.  Please get rid of the "improvement over CRAM-MD5"
hype in the abstract.

C.1: s/2052/2782/ and move to informative, maybe replace [Unicode]
     by [3629] + [UTR #36], s/2279/3629/ for [UTF-8], s/3548/4648/

     The URL in the informative [MD5] reference is broken. try this:
     <ftp://ftp.rsasecurity.com/pub/cryptobytes/crypto1n1.pdf> (and
     maybe add 375 KB in the <format> element if you use xml2rfc ;-)

C.4: Seconded, one example for any major modification, e.g. something
     with the new unq function if you keep that, and something where
     prep has an effect (the 2195bis Aladin example is nice), etc.

Frank