Hi Roman, Thanks for your usual careful review. I have submitted a new version that hopefully addresses all the issues. Here is the diff for your convenience: https://www.ietf.org/rfcdiff?url2=draft-ietf-emu-eap-tls13-10
Please see in-line for details on how we have handled each issue. --Mohit On 5/28/20 9:31 PM, Roman Danyliw wrote: > Hi! > > I performed an AD review of draft-ietf-emu-eap-tls13-09. Thanks for the work > on modernizing existing guidance to cover TLS 1.3. > > Two high level things caught my attention: > > ** The abstract, introduction and title suggested to me that this document is > only about TLS 1.3+EAP. If I wasn't using TLS 1.3, then there is nothing > here of interest. However, there appears to be text here that updates > non-TLS 1.3 guidance. I recommend being clear about that up front (in the > abstract and introduction). > > ** Section 2.1 explicitly says that "[t]his document only lists additional > and different requirements, restrictions, and processing compared to > [RFC8446] and [RFC5216]." The text tries to match section headers names with > [RFC5216] (which is helpful). However, I didn't always follow the "updates" > without some searching. Editorially, given a particular description of > behavior, I recommend being clearer by consistently making explicit reference > to a particular section in RFC5216 that is being updated. We initially maintained a strict matching between sections of RFC 5216 and this document. However, there were requests for new sections: Identity (Alan DeKok) and Hello Retry Request (Jim Schaad). In some cases, it was also necessary to have a new section. For example, a new section was needed to explain how server issued tickets are a necessary precursor for session resumption in TLS 1.3. Each section now begins by explicitly informing the reader if it updates a specific section of RFC 5216. New sections that didn't exist in RFC 5216 are also clearly labeled. It is true that the guidance on resumption and authorization is not specific to TLS version 1.3. The text in the abstract and introduction now state that "This document also provides guidance on authorization and resumption for EAP-TLS in general (regardless of the underlying TLS version used)". > > More details on the above observations and others below: > > (1) Can you please check the boiler plate language, idnits is complaining is > as follows: > == The document seems to lack the recommended RFC 2119 boilerplate, even if > it appears to use RFC 2119 keywords -- however, there's a paragraph with > a matching beginning. Boilerplate error? > > (The document does seem to have the reference to RFC 2119 which the > ID-Checklist requires). > (Using the creation date from RFC5216, updated by this document, for > RFC5378 checks: 2007-07-01) > > -- The document seems to lack a disclaimer for pre-RFC5378 work, but may > have content which was first submitted before 10 November 2008. If you > have contacted all the original authors and they are all willing to > grant > the BCP78 rights to the IETF Trust, then this is fine, and you can > ignore > this comment. If not, you may need to add the pre-RFC5378 disclaimer. > (See the Legal Provisions document at > https://trustee.ietf.org/license-info for more information.) The boilerplate error showed up because of a missing space. It shouldn't show up anymore. Even though RFC5216 is pre-RFC5378 work, it had granted BCP78 rights to the IETF trust. So we should be fine. (This issue was also noted by the document shepherd (Joe Salowey).) > > (2) Section 1. Editorial. Instead of something working "perfectly", maybe > just "supports" > OLD > EAP-TLS [RFC5216] references TLS 1.0 [RFC2246] and TLS 1.1 [RFC4346], > but works perfectly also with TLS 1.2 [RFC5246]. > > NEW > EAP-TLS [RFC5216] explicitly references TLS 1.0 [RFC2246] and TLS 1.1 > [RFC4346], but it also supports TLS 1.2 [RFC5246]. I am somewhat uncomfortable with the word "support". In my opinion, EAP-TLS uses a version TLS rather than supports a version of TLS. I have updated the text to say "EAP-TLS [RFC5216] references TLS 1.0 [RFC2246] and TLS 1.1 [RFC4346], but can also work with TLS 1.2 [RFC5246]". I am willing to modify if this is not appropriate. > > (3) Section 1. Editorial. "Easy" might be relative. > > OLD > Privacy is mandatory and achieved without any additional round-trips, > revocation checking is mandatory and easy with OCSP stapling, > > NEW > Privacy is mandatory and achieved without any additional round-trips, > revocation checking is mandatory and simplified with OCSP stapling, Fixed as recommended. > > (3) Section 2.1.1. What is the relationship between this section and NAI > guidance in RFC5216 Section 2.1.4? Is it that anonymous NAIs are RECOMMENDED > for TLS 1.3, but stay a MAY in old EAP-TLS? > > (4) Section 2.1.3. Editorial. To prevent the reader from having to line up > which paragraphs are being replaced in Section 2.1.3 of [RFC5216], I would > recommend citing the old text and showing how this text replaces it. > Additionally, since RFC5216 lists the peer guidance before the server, I'd > recommend reversing the order. For example: > > Original TLS guidance from RFC5216: > If the peer authenticates successfully, the EAP server MUST respond > with an EAP-Request packet with EAP-Type=EAP-TLS, which includes, in > the case of a new TLS session, one or more TLS records containing TLS > change_cipher_spec and finished handshake messages. The latter > contains the EAP server's authentication response to the peer. The > peer will then verify the finished message in order to authenticate > the EAP server. > .... > If the EAP server authenticates successfully, the peer MUST send an > EAP-Response packet of EAP-Type=EAP-TLS, and no data. The EAP Server > then MUST respond with an EAP-Success message. > > TLS 1.3 guidance: > [insert new language in the draft now] I am somewhat hesitant to copy old parts of RFC 5216. The document length would quickly explode. The issue here is that RFC5216 was saying that the server/peer must respond with specific TLS records in the message. While the information was correct for earlier versions of TLS, it would have been in conflict with TLS 1.3. So the new text simply updates RFC 5216 to say that the peer/server MUST send an EAP-Response/Request packet containing TLS records conforming to the version of TLS used. > > (5) Section 2.1.5. Is Figure 6 supposed to be read as being inclusive of all > TLS messages? The figure doesn't contain TLS CertificateRequest, TLS > Certificate, or TLS CertificateVerify. It wouldn't be germane to what the > figure is trying to demonstrate but the examples in prior sections are > "complete". Good catch. I have updated the figure to be complete. > > (6) Section 2.1.6. The text here seems right to describe TLS 1.3 behavior. > Recommend being clearer on which behavior this is replacing in TLS-EAP (i.e., > explicitly citing a given section in RFC5216; is it Section 2.1.2?) > > (7) Section 2.1.7. The text here seems right to describe TLS 1.3 behavior. > Recommend being clearer on which behavior this is replacing in TLS EAP (i.e., > explicitly citing a given section in RFC5216) > > (8) Section 2.1.7. Per "When NAI reuse can be done without privacy > implications", is there any guidance that can be provided on where there > would be a privacy issue? I believe that the sentences following this phrase try to explain where there would be a privacy issue. It says "E.g. the NAI @realm can safely be reused, while the NAI ZmxleG8=@realm cannot." So if the peer had use "@realm" during initial full authentication, it can safely reuse that since it does not reveal any additional information. If the peer however used "ZmxleG8=@realm" during initial full authentication, reusing the same NAI would reveal information that it is the same device/user reconnecting. Let us know if this explanation can be improved somehow. > > (9) Section 2.1.9. The text here seems right to describe TLS 1.3 behavior. > Recommend being clearer on which behavior this is replacing in TLS EAP (i.e., > explicitly citing a given section in RFC5216, is it Section 2.1.5?) > > (10) Section 5.1. Editorial. Wrong section reference. s/Section 4.1. of > [RFC5216]/Section 5.1 of [RFC5216]/ Good catch. Fixed. > > (11) Section 5.1. Per "better confidentiality" in [2], this seems imprecise > > (12) Section 5.4. Per "While certificates often have a long validity period > spanning several years, there are ...", as this is changing perhaps just say > "While certificates may have long validity periods, there are ..." Fixed as recommended. > > (13) Section 5.5. Recommend that this section read "No updates to Section > 5.5 of [RFC5216]" Fixed as recommended. > > (14) Section 5.6. The guidance in this section does appear to be TLS 1.3 > specific. If it isn't, it should be noted as such. The text now says "The guidance in this section is relevant for EAP-TLS in general (regardless of the underlying TLS version used)." > > (15) Section 5.7. Per "Since accounting must be tied to an authenticated > identity ...", is there is a reason not to use a normative MUST here? > Especially since later a normative RECOMMENDED is used for "It is RECOMMENDED > that authorization, accounting and policy decisions are reevaluated based on > information given in resumptions". This was more of a high-level statement than a protocol specific requirement. Hence, we used "must" instead of "MUST". Happy to edit if not suitable. > > (16) Section 5.8. Editorial. Per "[RFC6973] suggests that the privacy > considerations of IETF protocols be documented", it isn't clear to me what > this sentence is adding. Removed. > > (17) Section 5.8. Editorial. s/TLS 1.3 offers much better privacy than > .../TLS 1.3 offers better privacy properties than .../ Fixed as recommended. > > (18) Section 5.8. Editorial. Per "If privacy-friendly identifiers with > encrypted usernames need to be generated with care", as written, this is a > only sentence fragment. Some text went missing here. The new sentence says: "If Anonymous NAIs are not used, the privacy-friendly identifiers need to be generated with care." > > (19) Section 5.8. Per "An EAP peer with a policy allowing communication with > EAP servers supporting only TLS 1.2 without privacy and with a static RSA > key exchange is vulnerable to disclosure of the peer username", the "without > privacy" would benefit from being more precisely stated. This attack (and guidance) is somewhat convoluted to explain. Let me try: RFC 5216 says that "an EAP-TLS peer configured for privacy MUST negotiate a TLS ciphersuite supporting confidentiality and MUST provide a client certificate list containing no entries in response to the initial certificate_request from the EAP-TLS server". This allows the peer to not reveal its identity until it has received and verified the server identity. It is a good privacy feature to have. When we wrote "without privacy", we were referring to a peer that does not insist on servers revealing their identity first. The attack scenario we are referring is as follows: - A fake EAP server pretends that it supports TLS 1.2 (even though the honest server allows TLS 1.3). A fake EAP server would naturally be detected during the TLS handshake since it will not have the private key (corresponding to the public key in the certificate). However, if the fake server offers static RSA key exchange and the peer accepts it, the proof-of-possession of the server private key is not demonstrated until the Finished message. So there is a chance that the peer would have revealed its identity before finding out that the server is fake. I have now added the following sentence: "This implies that an EAP peer SHOULD NOT continue the handshake if a TLS 1.2 EAP server responds to an empty certificate message with a TLS alert message.". Thus the peer should insist on server revealing its identity first. Let us know if the text can be improved somehow. > > (20) Section 5.9. Editorial. Per "As required by [RFC7258] ...", I'm not > sure what this sentence adds. Removed. > > (21) Section 5.9. Could you contextualize this "widespread surveillance of > users " more specifically within the EAP context. Added a sentence saying "In the context EAP-TLS, pervasive monitoring attacks can target peer devices for tracking them (and their users) as and when they join a network." > > Regards, > Roman > > > _______________________________________________ > Emu mailing list > Emu@ietf.org > https://www.ietf.org/mailman/listinfo/emu _______________________________________________ Emu mailing list Emu@ietf.org https://www.ietf.org/mailman/listinfo/emu
