[secdir] Secdir last call review of draft-ietf-kitten-krb-spake-preauth-09

[Barry Leiba via Datatracker <[e-mail]>]

Reviewer: Barry Leiba
Review result: Has Issues

Just some minor things I ask you to consider:

The introduction drops in quite abruptly, with no setup and no references.  It
would be better to start with a short paragraph (perhaps a repetition of the
Abstract, which is itself a bit more detailed that the Abstract needs to be)
that tells us where we are and has reference(s) to where the technical terms
are defined.  And then things such as “when a client” become “when a Kerberos
client”, the first time and whenever it’s not clear.  Terms such as “KDC” need
to be expanded on first use.

Section 1.1 talks about PAKE with no citation to any PAKE document.  Section
1.2 says that SPAKE is defined in Section 2, but it is not, so I presume that
means Section 2 of some other, uncited document, or Section 4 of this one.

In Section 1.3, “FAST” needs to be expanded on first use and it needs its own
citation.

— Section 4 —

   Clients and KDCs MUST implement the
   edwards25519 group, but MAY choose not to offer or accept it by
   default.

How can one tell, externally, whether edwards25519 is not implemented, or
simply is being refused?

— Section 4.1 —

   Upon receipt of this AS-REQ, a KDC which
   requires pre-authentication and supports SPAKE SHOULD reply with a
   KDC_ERR_PREAUTH_REQUIRED error, with METHOD-DATA containing an empty
   PA-SPAKE PA-DATA element

SHOULD?  Why might it not?  What happens if it doesn’t?  (So, why isn’t it
MUST, and how can an implementor decide whether it’s OK not to?)

(In contrast, in 4.2 below, “The KDC SHOULD choose a group…” DOES explain the
SHOULD; thanks.)

— Section 4.2 —

   The KDC selects a random integer x in the range [0,h*p),
   which MUST be divisible by h.

Just a note to check this (and other similar places) carefully in AUTH48, to
make sure the RPC doesn’t try to “correct” the brackets.  Alternatively, you
might consider changing it to “in the range 0 <= x < h*p”.

— Section 10 —

Thanks for a very thorough Security Considerations section; I’m especially
happy to see the good discussion of side channels.

I do wonder, in Section 10.5, why “client implementations SHOULD provide a
configuration option to disable encrypted timestamp on a per-realm basis”,
rather than having encrypted timestamp disabled by default, and saying that
they MAY provide an option to enable it on a per-realm basis.  Keep in mind
that I’m not an expert here, so please just tell me if it’s obvious that that
can’t work, or some such.

— Section 12 —

   IANA MUST only accept registry updates from the designated experts
   and SHOULD direct all requests for registration to the review mailing
   list.

It feels odd to use BCP 14 key words in giving instructions to IANA — and the
SHOULD seems especially strange.  I suggest just putting both the MUST and the
SHOULD in lower case, making them plain-English instructions.

I note that the ID Number field in Kerberos SPAKE Groups says “Values should be
assigned in increasing order,” but it does not say that in Kerberos Second
Factor Types.  I’m just checking whether that’s intentional.

— Appendices —

Just noting here that I did not review the appendices.