Last Call Review of draft-melnikov-sasl-scram-ldap-
review-melnikov-sasl-scram-ldap-secdir-lc-lonvick-2009-10-16-00

Request Review of draft-melnikov-sasl-scram-ldap
Requested rev. no specific revision (document currently at 04)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2009-10-26
Requested 2009-09-30
Authors Alexey Melnikov
Draft last updated 2009-10-16
Completed reviews Secdir Last Call review of -?? by Chris Lonvick
Assignment Reviewer Chris Lonvick
State Completed
Review review-melnikov-sasl-scram-ldap-secdir-lc-lonvick-2009-10-16
Review completed: 2009-10-16

Review
review-melnikov-sasl-scram-ldap-secdir-lc-lonvick-2009-10-16

Hi,

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written primarily for the benefit of the
security area directors.  Document editors and WG chairs should treat
these comments just like any other last call comments.



Althought this isn't a WG document, it does state that discussion may take 


place on the sasl WG list, so I'm cc'ing the Chairs of that WG.






Overall, the concept is pretty straightforward and the description is 


succinct.  However, I do have some items that I would like to see 


addressed before I would recommend that this become an RFC.






Your ABNF is not complete.  You are using values taken from the complete 


ABNF in RFC 3112 so your ABNF is not going to properly parse.  I think 


that mostly all you need to do there is to copy the ABNF from RFC 3112 and 


insert your values.  You'll also need to define iter-count in the document 


somewhere.  (draft-ietf-sasl-scram-07 doesn't reference "iter-count"; only 


iteration count.)  Perhaps:



CURRENT:
      The "authInfo" part of the authPassword attribute is the iteration
      count, followed by ":" and base-64 [BASE64] encoded salt.
SUGGESTED:
      The "authInfo" part of the authPassword attribute is the iteration
      count [SCRAM] (identified here as the iter-count), followed by ":"
      and base-64 [BASE64] encoded salt [SCRAM].



An example is needed and I see that you have an anchor for that.  Please 


complete that.






Your Security Considerations section needs some work.  Each sentence you 


have there is actually a separate paragraph.  Rather than reworking that, 


I'd suggest that you start the section by stating that this specification 


utilizes the framework of RFC 4422 and the security concerns expressed 


there apply.  If needed, you could call out individual concerns from that 


Section 6.  Then you could call out any specific concerns that apply 


specifically to this document.






Just as a nit, you're mixing reference types.  RFC 3112 is referenced as 


[AUTHTYPE] whereas RFC 2119 is referenced as [RFC2119].  These should be 


consistent.




I'd also recommend that you revise the abstract a bit for clarity.
CURRENT:
   This memo describes how authPassword LDAP attribute can be used for
   storing secrets used by Salted Challenge Response (SCRAM) Simple
   Authentication and Security Layer (SASL) Mechanism.
SUGGESTED:
   This memo describes how the LDAP attribute of authPassword can be used
   for storing secrets used by the Salted Challenge Response (SCRAM)
   mechanism in the Simple Authentication and Security Layer (SASL)
   framework.

Best regards,
Chris