Re: [Emu] AD review of draft-ietf-emu-eap-tls13-09

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