Re: [Emu] draft-ietf-emu-tls-eap-types-06 comments

[Alan DeKok <[e-mail]>]

On Jun 20, 2022, at 11:20 AM, Heikki Vatiainen <[e-mail]> 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.