Hao, Madjid, Hannes,
Below is my review of PP-EAP. Being on the design team, perhaps my
response is a little late, but better late than never.
------
Section 0:
I still think the name/acronym needs work.
Section 1:
I don't think it's good to compare PP-EAP to the deficiencies of other
similar EAP methods to justify why it was developed. I think the
discussion of the design requirements in section 3 sufficiently
justifies it.
Section 2:
(editorial) The line spacing has problems. There should be line feeds
after the indented descriptions, before the next term.
Section 3:
I think many of the introduced protocols and terms need references,
including OTP, LDAP, *CHAP, etc. Some of the documents also need
references, including RFC 4017 and EAP key management. I think the AAA
key management might need to be referenced as well (RFC 4962).
Section 4.1:
(editorial) Paragraph starting "The protocol consists of..."
s/phases, during/phases. During/
Use of both "EAP server" and "PP-EAP" seems confusing -- stick with just
one.
Section 4.2:
I think we need to discuss what changes to the protocol would warrant a
new version number, with respect to the extensibility options already
available.
Section 4.3:
What happens if the client cert is used and the client cert validation
fails? Should we still check the password? What if the password is
correct but client cert validation fails? Vice-versa? Is a client cert
validation failure equivalent to simply not sending a client cert, or is
it more catostrphic? Personally, I think we should make things simple
and ignore client certs.
I think we need more disucssion about when the phase-2 TLVs can be sent.
Can they be sent immediately after (and in the same packet) as the
ChangeCipherSpec, or do we need to wait until after both
ChangeCipherSpec's? Can we add an ASCII-art protocol diagram or forward
reference to the examples section?
Section 4.4:
The example "RESPONSE=test\0205" is confusing. Some may think "\0205"
is unicode character 0x0205, or something. At least "\0" should be
defined somewhere.
I think we need to think further about different authentication
scenarios with multiple password credential sets. It seems we could
have any combination of:
[username]
[password]
[username]\0[password]
... and any composition of the above in sequence for a variety of
chained authentications. I think we may need to explicitly define the
formatting, either by expanding upon the "REQUEST" / "RESPONSE" labels,
or, for example, requiring a password to always begin with a "\0". So
for example, imagine a common PAM authentication with multiple
authentication modules. You might have something like:
"REQUEST=Please enter your username and UNIX password"
"RESPONSE=[username]\0[password1]"
"REQUEST=Kerberos Password for [EMAIL PROTECTED]"
"RESPONSE=\0[password2]"
... another problem is that the user GUI needs to know how many blanks
to put on the screen (username and/or password) and whether they should
be *'d out or not. I guess my overall point is that I don't think the
REQUEST/RESPONSE semantics are detailed enough to cover every scenario.
Section 4.4.1:
(editorial) Section 4.4 should be 4.4.1 and 4.4.1 should be 4.4.2, and
4.4 should be an overall heading.
Where did these error codes come from? They look copy/pasted from
somewhere else. Do we need a reference? If not, why not start them from 1?
I'm confused about the challenges in the error responses. Is this
specific to MSCHAPv2? If so, we should have broader section discussing
on MSCHAPv2. Could these be "REQUEST=*" values? They're also referred
to as a "hexadecimal challenge value". At what point are these values
formatted in hexadecimal? Just because something is binary data doesn't
mean it's formatted in "hexadecimal".
Section 4.5:
I think we should forbid inner methods, and get rid of the
crypto-bindings discussion.
Discussion of how PEAPv2 and EAP-FAST derived keys is unnecessary.
Section 4.10:
More confusion about MSCHAP... was discussed in the error handling
section...
Section 4.11:
(editorial) There are a variety of instances where the list formatting
is missing line feeds, as in the terminology section.
Section 4.11.1:
Much of this seems to duplicate the EAP header. Is it necessary inside
the tunnel as well?
Why is there an optional message length field? Is this to optimize
things with fragmentation? Why not just make it simple and always have it?
ACK is a zero-length data field? Why not use that extra bit in the
flags. Zero-length data field seems like it could lead to problems
(what those are, I'm not sure).
Section 4.11.2 / 4.11.5:
The behavior of the NACK TLV seems underspecified. What should the
server do if it receives a NACK for a TLV? Is that a catostrophic
error? Should each TLV detail what should happen if a NACK is received
in response?
Section 4.11.3:
(editorial) indenting issues
Section 4.11.6:
This statement needs help: "Vendor TLVs may be optional or mandatory."
What are the implications of a mandatory vendor-specific TLV?
Isn't the SMI space increasing to 4 bytes? I know we needed to recently
make changes to EAP-GPSK.
Section 4.11.7:
Again, I think inner methods, and also crypto-bindings, should go away.
Section 6.1:
I think we need a paragraph on each describing it in more detail.
Section 7:
Do we need IANA space for the error codes? Are they extensible?
Section 10:
I think 3748 should be a normative reference.
--
t. charles clancy, ph.d. <> [EMAIL PROTECTED] <> eng.umd.edu/~tcc
adjunct professor, electrical engineering, university of maryland
_______________________________________________
Emu mailing list
Emu@ietf.org
https://www1.ietf.org/mailman/listinfo/emu
