Subject: [Emu] review of PP-EAP

From: Charles Clancy <[email-masked]>
Date: Tue, 24 Jul 2007 12:05:09 -0400

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 [username]@[REALM]"
"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-masked]  <>  eng.umd.edu/~tcc
adjunct professor, electrical engineering, university of maryland

_______________________________________________
Emu mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/emu