Subject: [Emu] AD review on draft-ietf-emu-eap-tls13-18

From: Roman Danyliw <[email-masked]>
Date: Thu, 29 Jul 2021 15:39:12 +0000

Hi!

I performed a second AD review on draft-ietf-emu-eap-tls13.  This time on -18.  Thank you to the authors, the responsible co-chair,  and the WG for all of the work to redesign the -13 protocol in response to the initial IESG review in December 2020.  My review feedback consists of my own minimal additional comments and highlights of where previously stated IESG ballot items do not appear to be responded to or addressed.  All items appear to be minor.

** Section 2.4.  Previously, this text referenced TLS 1.3 or higher necessitating more ambiguous reference to version numbers.  However, now the text only notes TLS 1.3.  

OLD

   When EAP-TLS is used with TLS version 1.3, the EAP-TLS peers and EAP-
   TLS servers MUST comply with the compliance requirements (mandatory-
   to-implement cipher suites, signature algorithms, key exchange
   algorithms, extensions, etc.) for the TLS version used.  For TLS 1.3
   the compliance requirements are defined in Section 9 of [RFC8446].

NEW

When EAP-TLS is used with TLS version 1.3, the EAP-TLS peers and EAP-TLS servers MUST comply with the compliance requirements (mandatory-
to-implement cipher suites, signature algorithms, key exchange   algorithms, extensions, etc.) defined in Section 9 of [RFC8446].

==[ COMMENT (Ben Kaduk) 

Section 2.1.4

   The two paragraphs below replaces the corresponding paragraphs in
   Section 2.1.3 of [RFC5216] when EAP-TLS is used with TLS 1.3 or
   higher.  The other paragraphs in Section 2.1.3 of [RFC5216] still
   apply with the exception that SessionID is deprecated.

      If the EAP-TLS peer authenticates successfully, the EAP-TLS server
      MUST send an EAP-Request packet with EAP-Type=EAP-TLS containing
      TLS records conforming to the version of TLS used.  The message
      flow ends with the EAP-TLS server sending an EAP-Success message.

      If the EAP-TLS server authenticates successfully, the EAP-TLS peer
      MUST send an EAP-Response message with EAP-Type=EAP-TLS containing
      TLS records conforming to the version of TLS used.

Just to walk through the various scenarios here; if the last message of
the TLS handshake for a given TLS version is sent by the TLS client, that
will go in an EAP-Response, so the server is then able to send
EAP-Success directly.  On the other hand, if the last TLS handshake
message is sent by the server, that will have to go in an EAP-TLS
EAP-Request, and so the peer will have to send an empty EAP-TLS response
in order for the server to be able to wend EAP-Success?  Do we need to
have any text here to handle that empty EAP-TLS Eap-Request case?

==[ COMMENT(Alvaro Retana) Is the intention of the Appendix to provide additional formal updates to rfc5216?  That is what it looks like to me, but there's no reference to it from the main text.  Please either reference the Appendix when talking about some of the other updates (if appropriate) or specifically include the text somewhere more prominent.

[Roman] As proposed above, please provide some forward reference.

==[ COMMENT (Éric Vyncke) I find "This section updates Section 2.1.1 of [RFC5216]." a little ambiguous as it the 'updated section' is not identified clearly. I.e., as the sections in RFC 5216 are not too long, why not simply providing whole new sections ?

[Roman] I'd propose a simple fix by using (approximate phrases such as) either:

This section updates Section xxx of [RFC5216] by amending it with the following text. (for the cases where the intent is to keep the existing text in rfc5216 but add this new text)

This section updates Section xxx of [RFC5216] by replacing it with the following text. (for the cases where the intent is to replace an entire section in rfc5216)

==[ COMMENT (Éric Vyncke) None of us are native English speaker, but "e.g." as "i.e." are usually followed by a comma while "but" has usually no comma before ;-)

[Roman] A simple s/e.g./e.g.,/ and s/i.e./i.e.,/ would catch this.

Regards,
Roman