Subject: [secdir] SecDir Review of draft-ietf-httpauth-scram-auth-11

From: Russ Housley <[email-masked]>
Date: Tue, 1 Dec 2015 12:54:58 -0500

I 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 authors, document editors, and WG chairs should
treat these comments just like any other IETF Last Call comments.

Version reviewed: draft-ietf-httpauth-scram-auth-11

Summary: Not Ready

Major Concerns:

The document claims that SCRAM is better than authentication a plaintext
password protected by TLS and that it has better deployment properties
than earlier TLS-protected challenge response authentication mechanisms.
Please add a few sentence to the Introduction that justifies these
claims.

Section 5 says: "The "server-first-message" contains the user's
iteration count i, the user's salt, and the nonce with a
concatenation of the client-specified one with a server nonce."
It needs to be more clear that the nonce is a concatenation of the
client nonce from the client-first-message and a freshly generated
nonce by the server.

Section 5 goes on to say: "The latter has the same nonce and a
ClientProof computed using the selected hash function (e.g.  SHA-256)
as explained earlier."  Please be clear that this is the nonce from the
server-first-message (not the one from the client-first-message).

Section 5.1 says: "[[CREF1: Should some counter be added to make
"sr" unique for each reauth?]]".  This needs to be specified.  In my
opinion, something needs to be done to make the AuthMessage different
for each invocation.  One possibility is to include a counter in
AuthMessage as follows:

      AuthMessage := client-first-message-bare + "," +
                     server-first-message + "," + counter + "," +
                     client-final-message-without-proof

Section 8 says: "A hostile server can perform a computational
denial-of-service attack on clients by sending a big iteration count
value."  The document could suggest that a client implementation pick
a maximum iteration count that it is will to perform, and that it
reject any values that exceed that threshold, and of course, failing
the authentication.

Minor Concerns:

I think the Abstract covers some material that belongs in the
Introduction.  For example, the fact that HTTP Digest did not get
wide deployment does not help the reader decide if they want to
read this document.  I suggest:

   This specification describes a family of HTTP authentication
   mechanisms called the Salted Challenge Response Authentication
   Mechanism (SCRAM), which provides a more robust authentication
   mechanism than a plaintext password protected by Transport Layer
   Security (TLS) and avoids the deployment obstacles presented by
   earlier TLS-protected challenge response authentication mechanisms.
   
Then, please add the information that was removed from the Abstract
to a new paragraph in the Introduction.

I think the document would be easier to digest if the Introduction
came before the current Section 1, "Conventions Used in This Document".

Section 7 says: "The "UTF8-2", "UTF8-3" and "UTF8-4" non-terminals are
defined in [RFC3629]."  These non-terminals are not used in Section 7.

Nits:

Section 1.1 says: "For network-level protocols such as PPP or 802.11x,
the use of RADIUS [RFC2865] is more common."  PPP and 802.11 are not
network-level protocols.  To avoid getting too picky, it might be best
to say, "For lower-layer protocols such as ..."

I think it would be more readable if the Hi function were described in
its own section.  Also, since the INT function is only used in this one
place to handle a constant, I wonder if 0x00000001 would be easier to
understand.

The paragraph beginning with "Formally" in Section 5.1 does not need an
extra level of indentation.

In Section 3: s/it's useful/it is useful/

---

From: Alexey Melnikov <[email-masked]>
Date: Tue, 01 Dec 2015 18:27:39 +0000

Hi Russ,
I am quickly replying to your major concerns below.

> Please add a few sentence to the Introduction that justifies these
Ok. Some text exists in RFC 5802, but I can copy it here.
> client nonce from the client-first-message and a freshly generated
This is described in RFC 5802, which is a normative reference.
> server-first-message (not the one from the client-first-message).
Ok.
>                       client-final-message-without-proof
This was fixed in -12.
> reject any values that exceed that threshold, and of course, failing
Ok.

Best Regards,
Alexey

---

From: Russ Housley <[email-masked]>
Date: Tue, 1 Dec 2015 14:13:38 -0500

Alexey:

Continuing the discussion on two of my major concerns.

> This is described in RFC 5802, which is a normative reference.

I looked before sending this comment, and I did not find anything about freshness.
Can you point me to the text I missed?

>>                      client-final-message-without-proof

I just looked at the text in -12, and I think it needs to be more clear about the
number of octets that are used to represent nonce-count.  Perhaps you can
use INT().  If you use INT(), please define it as a bullet of its own in Section
1.2.

Russ

---

From: Alexey Melnikov <[email-masked]>
Date: Sat, 5 Dec 2015 21:30:12 +0000

Hi Russ,

I think I addressed everything from your review other than a couple of nits:

> extra level of indentation.
Can you check the latest version and confirm?

Thank you,
Alexey

---

From: Alexey Melnikov <[email-masked]>
Date: Mon, 07 Dec 2015 11:26:49 +0000

Hi Russ,

> Can you point me to the text I missed?
That was always the intent, but I clarified this directly in the draft.
> number of octets that are used to represent nonce-count.
There is ABNF which says it is a number (in decimal) with no leading zeroes.
> Perhaps you can
Best Regards,
Alexey