On Mon, May 20, 2024 at 6:24 PM Alan DeKok <al...@deployingradius.com> wrote:
> On May 20, 2024, at 6:30 PM, Orie Steele via Datatracker <nore...@ietf.org> > wrote: > > # 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. > > In short: the main public CAs are *web* CAs. Which for various reasons > are not always suitable for use with non-web protocols. > > I'll clarify the text. > > > ### 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. > > The methods listed above are the most widely implemented, and are thus > expected to work. > > Other EAP methods such as EAP-AKA are intended for use with the 3G+ > telecommunications network. There isn't much point in using them inside of > a TEAP / TLS tunnel. So it's best to suggest that they not be used > > > ### 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]. > > ``` > > Done. > > > ### 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. > > ``` > > Maybe? One issue is that the client could just do endless inner-method > negotiation. Perhaps strengthen that to a "different" inner method, > > Another issue is that there's no way for the client to automatically > figure out what to do. It's usually configured with something like "Do > X". If the server says "No", then the client can't really negotiate > anything else. It just tells the user "IU can't do X". > > > ### 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. > > "It MUST only send a NAK TLV for a TLV which is marked mandatory." > It MUST send a NAK TLV for any TLV which is marked mandatory but not understood? > > > And also recommending either Result TLV or NAK TLV for the all optional > case. > > If all TLVs in a message > are marked optional and none are understood by the peer, then a Result TLV > SHOULD be sent to the other side in order to > continue the conversation. It is also possible to send a NAK TLV when all > TLVs in a message are marked optional. > Seems better to me, perhaps strengthen the second part as well: If all TLVs in a message are marked optional and all are understood by the peer, a NAK TLV SHOULD be sent. Basically, I'm arguing to just tell the peer what to do for both cases. > > > ### 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? > > I think it's safe to say "SHOULD" here. > > > 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"? > > I'll add a suggestion that if it's not a fatal error, then the > receiving entity can send 2002 error. > > ``` > > 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? > > Yes, MUST is correct. > > > ### 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? > > No. It should be handled as an unknown TLV. > Perhaps "deprecated TLV are ignored, similar to Outer TLVs that are invalid or contain unknown values" ``` 1064 1. If Outer TLVs are invalid or contain unknown values, they will be 1065 ignored. ``` > > Are there any Error TLV (info, warning, or fatal) that are recommended > for > > processing deprecated TLV? > > I don't think so. This is the only deprecated TLV, and I don't think > anyone implemented it. > All the more reason to state explicitly that it should be silently ignored? > > Is the guidance in Section 4.2.12 of RFC7170 unchanged in this update? > > That section of 7170 describes how to use the PAC TLV. Since we're no > longer using it, the guidance given previously can be safely ignored. > Because the PAC TLV MUST be treated as invalid or containing unknown values? Seems like you / other experts can suggest more explicit processing here, I'm not attached to any specific text, I just thought it could be clearer. > > > ## 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." > > Sure. > > > ### Remove "with" ? > > > > ``` > > > > 553 dnsName attribute containing an FQDN string. Server > certificates MAY > > 554 also include with a SubjectDN containing a single element, > "CN=" > > ``` > > Done. > > > ### 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. > > Fixed. > > > ### Missing have? > > > > ``` > > 605 the authentication process. It will therefore no way of > correlating > > ``` > > Fixed. > > > ### 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. > > There is no relationship. EAP-Nak is negotiating which EAP type to use > (PEAP, TEAP, etc.). the NAK TLV is a TEAP-specific protocol field. > Thanks for educating me : ) > > > ### EMSK expand on first use > > > > ``` > > 885 an EMSK, and are vulnerable to on-path attacks. The > construction of > > ``` > > Done. > > > ### use -> be ? > > > > ``` > > 1089 restarted, it is not clear when that would use useful (or > used) for > > ``` > > Fixed. > > > ### 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. > > That text is a hold-over from 7170 when EAP implementations allowed for > a configuration flag "don't check server certificate". The consensus since > then is that this configuration is bad. Perhaps: > > Unless the peer requests server unauthenticated provisioning, it MUST > authenticate the server, and fail the current authentication > session fails if the server cannot be authenticated. > That seems better to me. > > > ### 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) ... > > Done. > > > 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. > > OK. I'll take a look. > > > ### 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. > > Perhaps "coalesced" with > Including an EAP request or response with other TLVs... ? My comment was in consideration of readers whose first language is not english, simpler is better, if it's possible. > > > ### remove against? > > > > ``` > > 3426 TEAP implementations can mitigate against online "brute force" > > ``` > > Done. > > Alan DeKok. > Thanks for your responses. OS, ART AD
_______________________________________________ Emu mailing list -- emu@ietf.org To unsubscribe send an email to emu-le...@ietf.org
