Tunnel Extensible Authentication Protocol (TEAP) Version 1
draft-ietf-emu-rfc7170bis-22

These are all pretty picky comments.  I won't be upset if you choose to ignore some/all of them.


Section 3.11, para 2:  Part of the authentication process is to show that the server presenting the certificate holds the private key.  Presentation of a certificate is insufficient, it is the certificate validate (in TLS language) that matters.  I'm not asking you to change anything here, just pointing out a pet peeve.

Section 3.11, para 4: (because of my PKI background): "This ensures that the authenticated TEAP peer is in possession of the private key used to sign the certification request."  This is stated backwards, maybe "This ensures that the signed certificate request is being presented by an authenticated TEAP peer which is in possession of the private key."  But, this is very pedantic on my part.  

Section 3.11.2, para 2: "Alternatively, the supplied information could contain private data which should not be sent over a TLS 1.2 connection where that data would be exposed."  Since you have specified that TLS cipher suites that do not provide confidentiality MUST NOT be used (Section 3.2), this seems like a weird statement.  If you remove it, you can combine this paragraph with the next one.

Section 3.11.2, para 4:  It is not uncommon for a (private) CA to ignore some fields in the CSR and instead populate the certificate fields based on local policy.  I would add 'or modify' in addition to 'can refuse'.

Section 3.11.2, para 2-4:  Personally, I'd make this one paragraph.  It would tighten up the section a bunch and possibly make it easier to understand.

Section 3.11.2, para 5 and beyond:  Well it could be used for other purposes, except that you have specified in Section 3.3 that private CA SHOULD be used.  Personally, I'd remove the rest of this section, unless you are attempting a tutorial (and then I'd have other comments).

Section 4.2.12, para 2:  There is a lower case 'should' in first sentence, is that correct?  Second sentence, if a Result TLV w/ failure is sent, is it possible to ignore the deprecated TLV?  These two options appear to be at odds.  

Section 7.1, last sentence:  This would be clearer if you said 'both server and peer authentication' instead of 'mutual authentication'.  The normal assumption for 'mutual auth' is that the peer has to authenticate.  In this case, we need the server AND the peer to authenticate.

Section 7.5.1:  General comment:  Requiring (with a SHOULD) a private CA makes it harder for an 'on-path attacker' to impersonate a server - a good thing.

Appendix C: I like this section.  But there is some terminology that isn't explained.  '()' appears to be the contents of whatever message is being sent.  '[]' I have no idea, in TLS 1.3 it would signify encrypted messages, but clearly that can't be the case here (TLS server_key_exchange can't be encrypted). I like the ASCII art in Section C.1-C.10, not so much in C.11 on because it is harder to see which way the arrows point (less white space).

Deb

I am relying on two directorate reviews for this document:

https://datatracker.ietf.org/doc/review-ietf-emu-rfc7170bis-17-dnsdir-telechat-weber-2024-05-24/ by Ralf Weber 

https://datatracker.ietf.org/doc/review-ietf-emu-rfc7170bis-16-intdir-telechat-song-2024-05-10/ by Haoyu Song (and I have not seen any reply to Haoyu's comments).

Alas, I had no opportunity to check whether all nits from https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-emu-rfc7170bis-17.txt are false positive.

# Gunter Van de Velde, RTG AD, comments for draft-ietf-emu-rfc7170bis-17

Please find https://www.ietf.org/blog/handling-iesg-ballot-positions/ documenting the handling of ballots.


#GENERIC COMMENTS
#================
275	      authentication, or a vendor-specific authentication method.  Where
276	      the TLS connection is authenticated, the inner method could also
277	      be a PKCS exchange.

can the PKCS (Public Key Cryptography Standards) be expanded upon first usage?

291	   As discussed in [RFC9190] Section 2.1.7 and [RFC9427] Section 3.1,
292	   the outer EAP Identity SHOULD be an anonymous NAI Network Access
293	   Identifier (NAI) as descrived in [RFC7542], Section 2.4.  While

Twice usage of NAI in the phrase construct

s/descrived/described/

301	   Any inner identities (EAP or otherwise) SHOULD also follow the
302	   recommendations of [RFC9427] Section 3.1.

The recommendations are slightly tucked away in RFC9427 sec3.1
Maybe the phrase should be more explicit that its about the recommendations about inner identifies from 3.1 as that section handles more as only inner identities

335	   of the TEAP server, and handles the application data (inner methods,
336	   EAP, passwords, etc.) inside of the TLS tunnel.

Maybe my little knowledge about TEAP/EAP, but the application data mentioned here is all about security. Maybe it should be spelled out more explicit that this is data used by EAP?

I have reviewed this document for GEN Area issues.

Thank you for addressing my COMMENT and DISCUSS feedback.

Thank you for the work on this document.

I mostly focused on the diff with RFC 7170: https://author-tools.ietf.org/iddiff?url1=rfc7170&url2=draft-ietf-emu-rfc7170bis-17&difftype=--html and have no objection to publication.

# Orie Steele, ART AD, comments for draft-ietf-emu-rfc7170bis-16 
CC @OR13

https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-emu-rfc7170bis-16.txt&submitcheck=True

Thanks for this document.

I'm not familiar with EAP or TEAP, please keep this in mind when reviewing my comments.

## Comments

### Why SHOULD use private CA?

```
561	   included, but their use is discouraged.  Systems SHOULD use a private
562	   Certification Authority (CA) for EAP in preference to public CAs.
```

Perhaps the answer is obvious to experts, but I wondered what interoperability issue this addresses.

### Why not MUST?

```
896	   Implementations SHOULD limit the permitted inner EAP methods to a
897	   small set such as EAP-TLS, EAP-MSCHAPv2, and perhaps EAP-pwd.  There
898	   are few reasons for allowing all possible EAP methods to be used in
899	   Phase 2.
```

I wonder if there are really any reasons to allow all possible methods, and what impact they have on interop.

### Updates RFC9427?

```
907	   methods which can tunnel other EAP methods.  As a result, this
908	   document updates [RFC9427].
```

Consider updating the abstract to reflect this:

```
20	   server.  This document obsoletes RFC 7170 [and updates RFC 9427].
```

### Can this MAY be strengthened to a SHOULD?

```
1142	      current inner method.  The side receiving a non-fatal Error TLV
1143	      MAY decide to start a new inner method instead or to send back a
1144	      Result TLV to terminate the TEAP authentication session.
```

### NAK TLV Optionality

```
1524	   optional, it can ignore the unsupported TLV.  It MUST NOT send a NAK
1525	   TLV for a TLV that is not marked mandatory.  If all TLVs in a message
1526	   are marked optional and none are understood by the peer, then a NAK
1527	   TLV or Result TLV could be sent to the other side in order to
1528	   continue the conversation.
```

This section might be improved by eliminating the double negative:

MUST NOT ... for .. not marked mandatory.

And also recommending either Result TLV or NAK TLV for the all optional case.

### Request-Action TLV

```
2084	      If multiple Request-Action TLVs are in the request and none of
2085	      them is processed, then the most fatal status should be used in
2086	      the Result TLV returned.  If a status code in the Request-Action
2087	      TLV is not understood by the receiving entity, then it should be
2088	      treated as a fatal error.
```

Since this is not SHOULD or MUST can this advice be ignored?

There are only 2 fatal errors listed so far.

Perhaps there should be a 2000-2999 error for Request-Action TLV status code not understood?, or simply say to use "2002 Unexpected TLVs Exchanged"?


```
2090	      After processing the TLVs or inner method in the request, another
2091	      round of Result TLV exchange would occur to synchronize the final
2092	      status on both sides.
```

I wonder if "would" is must, or MUST here?

### PAC TLV

```
2261	   [RFC7170] defined a Protected Access Credential (PAC) to mirror EAP-
2262	   FAST [RFC4851].  However, implementation experience and analysis
2263	   determined that the PAC was not necessary.  Instead, TEAP performs
2264	   session resumption using the NewSessionTicket message as defined in
2265	   [RFC9190] Section 2.1.2 and Section 2.1.3.  As such, the PAC TLV has
2266	   been deprecated.
```

When deprecated TLV are encountered, should they be handled any differently than non deprecated TLV?

Are there any Error TLV (info, warning, or fatal) that are recommended for processing deprecated TLV?

Is the guidance in Section 4.2.12 of RFC7170 unchanged in this update? 

## Nits

### anonymous NAI

```
291	   As discussed in [RFC9190] Section 2.1.7 and [RFC9427] Section 3.1,
292	   the outer EAP Identity SHOULD be an anonymous NAI Network Access
293	   Identifier (NAI) [RFC7542].  While [RFC3748] Section 5.1 places no
```

Perhaps this could be better phrased as:

"SHOULD be an anonymous Network Access Identifier (NAI) as described in Section 2.4 of RFC7542."

### Remove "with" ?

```

553	   dnsName attribute containing an FQDN string.  Server certificates MAY
554	   also include with a SubjectDN containing a single element, "CN="
```

### Trusted Anchor store

```
571	   *  when the peer has the Server's End Entity (EE) certificate pinned
572	      or loaded directly into it's Trusted Anchor store [RFC4949];
```

"Trusted Anchor store" is not defined in 4949, but "trust anchor information" is.

### Missing have?

```
605	   the authentication process.  It will therefore no way of correlating
```

### EAP-NAK vs NACK TLV

```
810	   Section 4.2.15 that contains the username and password.  If it does
811	   not wish to perform password authentication, then it responds with a
812	   NAK TLV indicating the rejection of the Basic-Password-Auth-Req TLV.
```

Section 3.1 inherits the term "EAP-Nak" from RFC7170.
Consider adding a parenthetical next to EAP-NAK explaining the relationship to NAK TLV.

### EMSK expand on first use

```
885	   an EMSK, and are vulnerable to on-path attacks.  The construction of
```

### use -> be ?

```
1089	   restarted, it is not clear when that would use useful (or used) for
```

### configuration flag : NOT recommended vs NOT RECOMMENDED

```
1176	   Peer implementations MUST be configured so that by default, the
1177	   current authentication session fails if the server cannot be
1178	   authenticated.  However, it is possible to have a configuration flag
1179	   which permits access to networks where the server cannot be
1180	   authenticated.  Such configurations are NOT recommended, and further
1181	   discussion is outside of the scope of this specification.
```

This configuration flag behavior sounds dangerous (or fun, or both).
Just double checking that "NOT recommended" is intentionally not BCP 14 language "NOT RECOMMENDED".
This might perhaps be clarified in security considerations, since this is the only occurrence of "configuration flag" in the document.

### degenerate (i.e. unsigned)

```
1231	   body of a PKCS#10 TLV (see Section 4.2.17).  The TEAP server issues a
1232	   Simple PKI Response using a PKCS#7 [RFC2315] degenerate (i.e.
1233	   unsigned) "Certificates Only" message encoded into the body of a
1234	   PKCS#7 TLV (see Section 4.2.16), only after an inner method has run
1235	   and provided an identity proof on the peer prior to a certificate is
1236	   being issued.
```

Consider reversing the parenthetical: ... issues a ... unsigned (i.e. degenerate) ...

You might also consider providing guidance (possibly in Section 4.2.16) on the ContentInfo value per the Notes in Section 9.1 of RFC2315.


### piggybacking

```
2158	   To allow piggybacking an EAP request or response with other TLVs, the
2159	   EAP-Payload TLV is defined, which includes an encapsulated EAP packet
2160	   and a list of optional TLVs.  The optional TLVs are provided for
```

Consider an alternative word.

### remove against?

```
3426	   TEAP implementations can mitigate against online "brute force"
```

Thank you for writing this document; it is useful and important.

Also, thank you to Bo Wu for the Ops-Dir review (https://datatracker.ietf.org/doc/review-ietf-emu-rfc7170bis-15-opsdir-lc-wu-2024-03-11/) and Ralf Weber for the (multiple) DNSIR reviews (https://datatracker.ietf.org/doc/review-ietf-emu-rfc7170bis-16-dnsdir-telechat-weber-2024-04-29/), and to the author for addressing these.

My primary remaining comment is that Abstract for the document briefly should say *how* it updates RFC9427 - that way when someone reads RFC9427 they need only check this new document's Abstract to know if they need to read the whole document. I don't have great text to suggest, but perhaps something like: "This document obsoletes RFC 7170 and updates RFC 9427 by prohibiting resumption for the inner EAP methods" (*clearly* I'm not a TEAP/EAP/TLS person and so this will likely need to be edited).