Subject: [secdir] SECDIR review of draft-melnikov-sasl-scram-ldap-03

From: Chris Lonvick <[email-masked]>
Date: Tue, 13 Oct 2009 14:57:20 -0700 (PDT)

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

---

From: Barry Leiba <[email-masked]>
Date: Thu, 15 Oct 2009 16:44:27 -0400

>   mechanism in the Simple Authentication and Security Layer (SASL)

I agree that strings of attributive nouns and noun-phrases can be
confusing, especially when they're long and also shown as acronyms.  I
think the second half of your suggested change is good.  But I think
the first half actually makes it worse, by making it look like there's
some attribute of authPassword that's called "LDAP".  The best way to
clarify that part is just to put the attribute name in quotes:
SUGGESTED++:
  This memo describes how the "authPassword" LDAP attribute can be used
  for storing secrets used by the Salted Challenge Response (SCRAM)
  mechanism in the Simple Authentication and Security Layer (SASL)
  framework.

Barry

---

From: Chris Lonvick <[email-masked]>
Date: Thu, 15 Oct 2009 14:39:24 -0700 (PDT)

Hi Barry,

I like that better as well.

Best regards,
Chris

---

From: Alexey Melnikov <[email-masked]>
Date: Sat, 31 Oct 2009 18:28:09 +0000

>  mechanism in the Simple Authentication and Security Layer (SASL)
Ok, I've done this change in my copy.

---

From: Alexey Melnikov <[email-masked]>
Date: Sat, 31 Oct 2009 19:10:33 +0000

> Hi,

Hi Chris,
Thank you for the review.

> 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.
>
> 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've updated comments in the ABNF to clearly indicate where various 
productions are coming from.

> I think that mostly all you need to do there is to copy the ABNF from 
> RFC 3112 and insert your values.

I don't think copying all productions defined elsewhere into this 
document is a good idea.

> 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.)

Ok, I've added a clarifying comment.

> Perhaps:
> 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.

I thought all three sentences are related to each other.

> 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.

Ok, I will try to rework this.

> 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.

Right. I will let RFC Editor handle that. I would like to use symbolic 
references, but xml2rfc doesn't make it easier without cut and pasting 
reference descriptions.