TLS-Based Extensible Authentication Protocol (EAP) Types for Use with TLS 1.3
draft-ietf-emu-tls-eap-types-13

Thanks for this document. I have a few questions and comments I hope may be of use.

### Section 3

```
   Earlier TLS versions did not always send application data along with
   the Finished message.  It was then possible for implementations to
   assume that a receipt of a Finished message also meant that there was
   no application data available, and that another round trip was
   required.
```

This doesn’t quite make sense to me as written. Do you mean that earlier TLS versions always did not send application data along with the Finished message? Note the significant movement of the word "always". The way it’s written right now, "did not always" implies earlier TLS implementations "sometimes did" send application data along with the Finished message, and if that was the case, I don’t see how (non-buggy) implementations could make the assumption you note.


### Section 4

```
   As the packet flows for resumption are essentially identical across
   all TLS-based EAP types, it is technically possible to authenticate
   using EAP-TLS (Type 13), and then perform resumption using another
   EAP type, just as EAP-TTLS (Type 21).
```

Is “just as” in the last line, supposed to be “such as“? If “just as“ is correct, I don’t understand what’s intended.


### Section 6.1

```
   Similarly, when the inner authentication protocol indicates that
   authentication has succeeded, then implementations SHOULD cause
   authentication to succeed for the entire session.  There MAY be
   additional protocol exchanges in order which could cause other
   failures, so success is not required here.
```

That last sentence doesn’t make much sense to me. I am guessing what you mean is something like, “The reason success is not mandatory but only recommended in this case is that we cannot preclude that an inner protocol might have semantics such that authentication can succeed but the overall exchange still can fail.”

Feel free to use those words if they make sense, or not, but as the text stands I had difficulty so I suggest some kind of clarification. At a minimum, it appears to me that the words “in order” should be removed?

In Section 2.1:

   Which define Master Session Key (MSK) and Extended Master Session Key
   (EMSK).

This seems to be part of a larger sentence that's partly missing.

   It is RECOMMENDED that vendor-defined TLS-based EAP methods use the
   above definitions for TLS 1.3.  There is no compelling reason to use
   different definitions.

Why isn't this MUST if there's no compelling reason to do otherwise?

I concur with Roman's comment about the SHOULD NOT in Section 2.4.

In Section 2.2:

   Where || denotes concatenation.

I understand the earlier citation I made above now, but still this should be a complete sentence.  You later have:

   where CMK[n] is taken from the final ("n"th) inner method.

...which is better in that it looks more like a continuation of something, but still it could be better.  Suggest:

   In this context, CMK[n] is taken from the final ("n"th) inner method.

In Section 3.1:

 In other weds, [...]

I think you meant "words".

   The outer identity SHOULD use an anonymous NAI realm.  [...]

I suggest you add a sentence or two explaining why an implementer might legitimately choose not to do this.

   This practice is NOT RECOMMENDED.  User accounts for an organization
   should be qualified as belonging to that organization, and not to an
   unrelated third party.  There is no reason to tie the configuration
   of user systems to public realm routing, that configuration more
   properly belongs in the network.

This formulation is curious; you first give implementers an "out" with NOT RECOMMENDED, and then basically argue that it should be a MUST NOT.  I suggest revisiting this pattern anywhere you've used it.  If you prefer to keep it, I suggest changing the prose a bit to explain why one might choose to deviate from the advice presented.

Thanks for including Section 5.

In Section 6.1:

   There MAY be
   additional protocol exchanges which could still cause failure, so we
   cannot mandate sending success on successful authentication.

This should just be "may", not "MAY".  You're not presenting an implementation choice.

** Thank you to Melinda Shore for the SECDIR review.

** Section 2.4
   It is therefore RECOMMENDED that EAP servers always send a TLS
   NewSessionTicket message, even if resumption is not configured.  When
   the EAP peer attempts to use the ticket, the EAP server can instead
   request a full authentication.  Implementations SHOULD NOT send
   NewSessionTicket messages until the "inner tunnel" authentication has
   completed, in order to take full advantage of the message as a
   protected success indication.

It is my understanding that this SHOULD NOT is based in implementation realities.  Can text be added to establish the basis for this not being “MUST NOT”.  Please also add cross-references as appropriate into the document on the same topic.

Thanks for working on this specification.

Thank you for the work put into this document. I have no specific comments on the content itself, I just want to thank two people:

Special thanks to Joe Salowey for the shepherd's detailed write-up including the WG consensus **and** the justification of the intended status. 

Other thanks to Bob Halley, the Internet directorate reviewer (at my request), please consider this int-dir review:
https://mailarchive.ietf.org/arch/msg/int-dir/MVqh2NfY86uJ8RZzFu5D0iEGGMw (and I have seen that Alan has already replied)

I hope that this review helps to improve the document,

Regards,

-éric

# GEN AD review of draft-ietf-emu-tls-eap-types-11

CC @larseggert

Thanks to Thomas Fossati for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/fNZOEID38s07l1DnsrSiqNpoXBc).

## Comments

### Boilerplate

Document boilerplate does not indicate intended status?

TLP Section 6.a "Submission Compliance for Internet-Drafts" boilerplate text
seems to have issues.

I-D Guidelines boilerplate text seems to have issues.

TLP Section 6.b "Copyright and License Notice" boilerplate text seems to have
issues.

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Boilerplate

Document still refers to the "Simplified BSD License", which was corrected in
the TLP on September 21, 2021. It should instead refer to the "Revised BSD
License".

### Grammar/style

#### Section 2.3, paragraph 5
```
llenge = TLS-Exporter("ttls challenge",, n) There is no "context_value" ([RFC
                                      ^^
```
Two consecutive commas.

#### Section 3.1, paragraph 8
```
tity can then be fully qualified: user name plus realm of the organization.
                                  ^^^^^^^^^
```
It's more common nowadays to write this noun as one word.

#### Section 4, paragraph 2
```
e implemented and tested to be inter-operable with wpa_supplicant 2.10 and W
                               ^^^^^^^^^^^^^^
```
This word is normally spelled as one.

#### Section 6.1, paragraph 8
```
e Requirement Levels", RFC 2119, March, 1997, <http://www.rfc-editor.org/inf
                                 ^^^^^^^^^^^
```
When specifying a month and year, the comma is unnecessary.

#### Section 6.1, paragraph 9
```
0, May 2014. [RFC8126] Cotton, M., et al, "Guidelines for Writing an IANA Co
                                   ^^^^^
```
A period is misplaced or missing. (Should be "et al."; also elsewhere.)

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

Thanks for spending the time to write this specification to improve security, and thanks to Juergen for the OPSDIR review.