On Jun 20, 2022, at 11:20 AM, Heikki Vatiainen <h...@radiatorsoftware.com> wrote: > Please find below my comments. I think the draft can move forward. The > comments below are clarifications and minor fixes. The list is quite long but > I think there aren't any functional changes.
That's fine. Unless otherwise noted below, I've addressed all of your comments by making the requested changes. > Naming things > +++++++++++ > Should the EAP type names named consistently? For example, always use > EAP-FAST and EAP-TTLS because those are the IANA registered names. PEAP and > TEAP already use just one form. Yes. > Using 'EAP peer', and when unambiguous, 'peer', as much as possible: now the > draft uses also 'client' and 'supplicant' sometimes. Client should remain > when the discussion is about general TLS client behaviour, though. > > Similarly 'EAP server' could be used sometimes to clarify which server is > discussed. It's probably worth just using "EAP peer", and "EAP server" consistently. > Paragraph 5: The second sentence in the draft is shwn below with my suggested > text following: > 'When the supplicant attempts to use the ticket, the peer can simply request > a full reauthentication.' > 'When the EAP peer attempts to use the ticket, the EAP server can simply > request a full authentication.' > This clarifies naming, roles and simply says 'authentication' because it's > not going to be reauthentication any longer. I'll put this as: When the EAP peer attempts to use the ticket, the EAP server can instead request a full authentication > 2.5 PEAP > +++++++ > Paragraphs 1 and 2 in the draft are shown below followed by suggested version: > ... > The idea is to clarify what 'here' refers to (PEAP-MPPE) and to use > 'derivation' instead of 'calculation' for matching terminology. My > understanding that 'different' means 'different than what is specified > section 2.1 in this draft', therefore it was switched to PEAP-specific to be > clear what it refers to. Attempt is also made to clarify that what remains > unchanged is 'PRF+' function. Term 'method' is used just once so that PRF > calculation method won't be confused by 'inner EAP method'. I agree. I've made the change. > 3 Application data > ++++++++++++++ > The draft says: > > The NewSessionTicket message SHOULD also be sent along with other > application data, if possible. Sending that message alone prolongs > the packet exchange to no benefit. > > I fully agree with the above. Experimenting shows that, for example, > eapol_test from wpa_supplicant supports a lone NewSessionTicket message by > sending back an PEAP ack when it receives just the tickets instead of > EAP-Identity/Request+keys after TLS handshake completion. However, another > EAP peer implementation responds nothing and stops the authentication process > when it receives just a NewSessionTicket message without the expected > EAP-Identity/Request. I would add one EAP peer implementation also doesn't implement resumption for TTLS / PEAP. That seems very, very, wrong to me. > To summarise, in addition to prolonging the packet exchange, it can also lead > to non-interoperable implementations. Seems that the smallest number of > messages allowed by the TLS and EAP method specifications is the way to go. > Not just with PEAP but with other EAP methods too. I'll add the last bit as an additional explanation, with some rewording. > 4. Resumption > +++++++++++ > The paragraph in the draft could benefit from a small sentence reordering and > rewording. The original is followed by the suggested version: ... > Note that if resumption is performed, then the EAP server MUST send > the protected success result indicator (one octet of 0x00) inside the > TLS tunnel as per [RFC9190]. The EAP peer MUST in turn check for > the existence the protected success result indicator (one octet of > 0x00), and cause authentication to fail if that octet is not > received. If either peer or server instead > initiates an inner tunnel method, then that method MUST be followed, > and inner authentication MUST NOT be skipped. > > The rewording changes 'resumption MUST NOT be used' to what's shown above. My > understanding is that if TLS resumption is done, then the choice is either to: > - use protected success result indication 0x00; or > - do full inner authentication; but not both That's good. > > 6. Security Considerations > ++++++++++++++++++++ > The last sentence of paragraph 3 is shown below with the suggested change > following: > ... > The changes use 'EAP peer' instead of 'clients' and add missing verb 'use'. Changed, thanks. > > 6.1 Protected Success and Failure indicators > ++++++++++++++++++++++++++++++++++ > Paragraph 5 says '... TTLS with inner PAP or CHAP.'. I'd change the inner > protocols to 'PAP, CHAP or MS-CHAP'. > > Paragraph 7 says '... do not provided protected ...'. Change 'provided' to > 'provide'. > > Paragraph 7 also suggests replacements for PAP and CHAP to ensure protected > indicators can be used. A replacement for MS-CHAP could be EAP-MSCHAP-V2 or > possibly EAP-MD5? A replacement for MS-CHAPv1 is MS-CHAPv2, or EAP-MSCHAPv2 > > 8. References > +++++++++++ > [PEAP-MPPE], [PEAP-PRF] and [PEAP-TK] point to different parts of [MSPEAP]. > It might be useful to clarify that this is the case in order to minimise the > problem with broken links. I'll add some clarifying text. Thanks for the comprehensive review. Alan DeKok. _______________________________________________ Emu mailing list Emu@ietf.org https://www.ietf.org/mailman/listinfo/emu
