Re: [Emu] Benjamin Kaduk's No Objection on draft-ietf-emu-eap-tls13-20: (with COMMENT)

Hi Ben,

Thanks for your review. Comments on the git issue tracking your review 
explain the detailed changes we have made: 
https://github.com/emu-wg/draft-ietf-emu-eap-tls13/issues/96.

Let us know if this is sufficient. If necessary, we can submit version 
-22 before the draft deadline closes on Monday. We hope to ship this soon.

--Mohit

On 10/8/21 2:17 AM, Benjamin Kaduk via Datatracker wrote:
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-emu-eap-tls13-20: No Objection
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to https://www.ietf.org/blog/handling-iesg-ballot-positions/
> for more information about how to handle DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-emu-eap-tls13/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Updating my ballot position from Discuss to No Objection in light of the
> discussion that we had at the telechat today.
>
> Previous Discuss position:
> ======================================================================
> Many thanks for the updates since the -13, the last version I reviewed.
> I'm happy to report that the structural issues I noted in that version
> have been addressed, and my new Discuss point is a fairly mundane one.
>
> In several sections, we say that the text "updates Section X of
> [RFC5216] by amending it with the following text", but I'm quite unclear
> on exactly what that is intended to mean.  Are we adding to the end,
> prepending to the beginning, replacing wholesale, replacing in part, or
> doing something else to the indicated text of RFC 5216?  I expect that
> just tweaking a few words can resolve the ambiguity, but am not sure
> which ones yet.
>
> It is also interesting to contrast the "amending" language with what we
> say in Sections 2.1.4 and 2.3 about "replacing" text from RFC 5216 and
> the various places where we report a "new section when compared to
> [RFC5216]".
> ======================================================================
>
> The discussion helped shed some light on the process the WG took to get
> to the current state of having an amalgamation of new and existing text
> where the new text amends the existing text in a way that has the reader
> perform their own synthesis, avoiding a need for specification authors to
> engage in a tedious exercise of going sentence-by-sentence to check all
> the details.
>
> I would suggest to make a change of the form:
>
> OLD:
> updates Section X of [RFC5216] by amending it with the following text
>
> NEW:
> updates Section X of [RFC5216] by amending it in accordance with the following
> discussion
>
>
> Original COMMENT section retained unchanged:
> ======================================================================
> I echo the sentiments of other reviewers that constructing EAP-TLS 1.3
> as something of a diff against RFC 5216 will make it harder to
> eventually deprecate/obsolete/etc RFC 5216 and makes it somewhat
> challenging to read the EAP-TLS 1.3 specification as a whole.  That
> said, this is just the comment section, so I am not strenuously
> objecting to it.
>
> As another general note, in many places the phrasings used to describe
> TLS 1.3 behaviors feel rather un-idiomatic to me, based on my experience
> with TLS and TLS specifications.  That said, the behavior seems
> well-specified as is, so I don't propose to make any changes in response
> to this comment.  If there is demand, I could probably be persuaded to
> suggest alternative text, but I don't expect much demand at this stage
> in the document's lifecycle.
>
> I made a github PR with some editorial suggestions:
> https://github.com/emu-wg/draft-ietf-emu-eap-tls13/pull/92
>
> Section 2.1
>
>     This section updates Section 2.1 of [RFC5216] by amending it with the
>     following text.
>     [...]
>     TLS 1.3 changes both the message flow and the handshake messages
>     compared to earlier versions of TLS.  Therefore, much of Section 2.1
>     of [RFC5216] does not apply for TLS 1.3.  Except for Sections 2.2 and
>     5.7 this document applies only when TLS 1.3 is negotiated.  When TLS
>     1.2 is negotiated, then [RFC5216] applies.
>
> There is perhaps some philosophical question of what "this document"
> means in the context of an updated collection of text that includes RFC
> 5216 and the text that is being amended as directed here.  I hope that
> the RFC Editor will have some thoughts on this matter, but perhaps
> s/this document/[RFCXXXX]/ would reduce ambiguity.  If this change is
> made, there would be similar/corresponding changes later on as well,
> e.g., whenever the amended text includes a section reference to this
> document, it would be "Section 2.1.3 of [RFCXXXX]".
>
> Also, I think that both Sections 5.6 and 5.7 (not just 5.7) are marked
> as applying to EAP-TLS in general.
>
>     *  Early Data MUST NOT be used in EAP-TLS.  EAP-TLS servers MUST NOT
>        send an early_data extension and clients MUST NOT send an
>        EndOfEarlyData message.
>
> Personally, I wouldn't include the last sentence, as both requirements
> follow naturally from the previous requirement.  It feels a little
> surprising to make reference to the specific message-level requirements
> on the TLS stack, though I won't object to keeping this text if the
> authors/WG find it important.
>
>     *  Servers MUST NOT request post-handshake client authentication.
>
> Do we want to make any statement about the client (not) sending the
> "post_handshake_auth" extension (which the client must do as a
> prerequisite of the server requesting post-handshake client
> authentication)?
>
> Section 2.1.1
>
>                                               After the EAP-TLS server
>     has sent an EAP-Request containing the TLS application data 0x00 and
>     received an EAP-Response packet of EAP-Type=EAP-TLS and no data, the
>     EAP-TLS server sends EAP-Success.
>
> I think in some sense the EAP-Server also needs to not have additional
> TLS data do send in order to declare success and send EAP-Success.
>
> Section 2.1.3
>
>     full handshake.  In the case a full handshake is required, the
>     negotiation proceeds as if the session was a new authentication, and
>     resumption had never been requested.  [...]
>
> I put a suggested alternative phrasing in my editorial PR, but wanted to
> note the reasoning for the change in my ballot comment here.
> "Had never been requested" is perhaps problematic in that the request
> for resumption is encoded in the ClientHello, and to say that it had
> never happened seems a little like suggesting that the ClientHello is
> modified (which is not what happens).
>
>     Figure 3 shows an example message flow for a subsequent successful
>     EAP-TLS resumption handshake where both sides authenticate via a PSK
>     provisioned via an earlier NewSessionTicket and where the server
>     provisions a single new ticket.
>
> I would suggest indicating that the TLS ClientHello includes the
> "pre_shared_key" extension to help differentiate the resumption
> handshake from the full handshake depicted in Figures 1/2.
>
>     When using "psk_dhe_ke", TLS 1.3 provides forward secrecy meaning
>     that key leakage does not compromise any earlier connections.  [...]
>
> It's probably worth saying which key is leaked to trigger the "key
> leakage" scenario being described.
>
> Section 2.1.8
>
> I might consider noting that the "hello_request" message doesn't exist
> in TLS 1.3, so the RFC 5216 procedures are inherently inapplicable with
> TLS 1.3.  On the other hand, that might be covered by the blanket
> statement in §2.1, and thus unneeded here.
>
> Section 2.2
>
>     The EAP peer identity provided in the EAP-Response/Identity is not
>     authenticated by EAP-TLS.  Unauthenticated information MUST NOT be
>     used for accounting purposes or to give authorization.  [...]
>
> There's some analogous text in 5216 that talks about use for "accounting
> purposes" or "access control" (at the SHOULD NOT level) -- is there a
> reason to use/not use the same phrasing that 5216 did?
>
>     (SAN) extension in the server certificate.  If any of the configured
>     names match any of the names in the SAN extension then the name check
>     passes.  [...]
>
> It would be "more secure" in a specific technical sense if the
> implementation could tie the configured acceptable name to the specific
> CA (or CAs) that should have issued it, but I don't think current
> implementations make this easy, and I also don't expect it to make a
> practical difference in most real-world scenarios.  So the most I would
> suggest would be to mention the possibility in the security
> considerations section, and I'm not even sure that it's worth doing
> that.
>
> Section 2.3
>
>     which provides a public API for the exporter.  Note that EAP-TLS with
>     TLS 1.2 [RFC5216] can be used with the TLS exporter since the public
>     exporter was defined in [RFC5705].
>
> Is the intent to say that the TLS exporter-based formulae above will
> produce the same result as (and thus, interoperate with) the PRF-based
> formulae from RFC 5216?  I did not thoroughly check the equivalence, but
> a quick check suggests that it exists.  If that is indeed the intent, I
> would strongly suggest rephrasing to explicitly indicate the achieved
> compatibility.
>
> Section 2.5
>
>     is not useful and not expected in EAP-TLS.  After sending TLS
>     Finished, the EAP-TLS server may send any number of post-handshake
>     messages in separate EAP-Requests.
>
> I don't think there's a fundamental requirement that each post-handshake
> message goes in a separate EAP-Request, as this text seems to imply.
> For example, a TLS stack that produces two NewSessionTicket messages
> after the handshake completes would release them at the same time, and
> (size permitting) EAP could easily carry them in a single EAP-Request.
>
> Section 5.1
>
> We might incorporate the security considerations of RFC 8446 by
> reference, though it's probably not critical to do so.
>
>     [3] Cryptographic strength: TLS 1.3 only defines strong algorithms
>     without major weaknesses and EAP-TLS with TLS 1.3 always provides
>     forward secrecy, see [RFC8446].  Weak algorithms such as 3DES, CBC
>     mode, RC4, SHA-1, MD5, P-192, and RSA-1024 cannot be negotiated.
>
> I don't see anything in RFC 8446 to support the claim that 3DES cannot
> be negotiated -- ciphers at the 112-bit strength level are permitted,
> see D.5.  (But I also think I said this last time I balloted, so maybe I
> just forgot what the response to my comment was...)
>
> Section 5.3
>
> The corresponding section of RFC 5216 does not reference RFC 6125 (which
> is not surprising, given that it didn't exist at the time).  While I
> don't see a strong need to provide an extensive analysis of how the RFC
> 6125 procedures relate to EAP usage, it does seem that RFC 6125 provides
> some useful information for performing server certificate validation,
> and that a brief reference might be appropriate.
>
> Section 5.4
>
>     To enable revocation checking in situations where EAP-TLS peers do
>     not implement or use OCSP stapling, and where network connectivity is
>     not available prior to authentication completion, EAP-TLS peer
>     implementations MUST also support checking for certificate revocation
>     after authentication completes and network connectivity is available.
>
> I just want to confirm that both "peer[s]"s are intended to be "peer"s,
> here.  I think it still makes sense this way, but "in cases where X do
> not implement or use Y ... X implementations MUST also" is not a common
> construction with both "X"es the same, so I wanted to check.
>
> Section 5.7
>
>     [RFC8446], where the EAP-TLS server avoids storing information
>     locally and instead encapsulates the information into a ticket or PSK
>     which is sent to the client for storage.  This ticket or PSK is
>     encrypted using a key that only the EAP-TLS server knows.  Note that
>
> I put this into my github PR, but just to note here: I don't think that
> the "or PSK" is accurate in this scenario.
>
>     If any authorization, accounting, or policy decisions were made with
>     information that has changed between the initial full handshake and
>     resumption, and if change may lead to a different decision, such
>     decisions MUST be reevaluated.  It is RECOMMENDED that authorization,
>     accounting, and policy decisions are reevaluated based on the
>     information given in the resumption.  [...]
>
> Up in §2.2 we say that "unauthenticated information MUST NOT be used
> for [...] authorization".  I'm not sure what scenarios you have in mind
> where there is authenticated information that is changing between the
> initial full handshake and the resumption -- when resumption succeeds,
> won't basically all the authenticated information be from the original
> full handshake?
>
> Section 5.8
>
>     static RSA based cipher suites without privacy.  This implies that an
>     EAP-TLS peer SHOULD NOT continue the handshake if a TLS 1.2 EAP-TLS
>     server sends an EAP-TLS/Request with a TLS alert message in response
>     to an empty certificate message from the peer.
>
> Is it really "continue the [TLS] handshake" or "continue the [EAP]
> authentication attempt"?  My understanding was that the vulnerability
> occurs when the peer initiates a new TLS handshake without attempting to
> use privacy, which would otherwise be the behavior in response to the
> described alert.
>
> Section 5.10
>
>     summarizes the attacks that were known at the time of publishing and
>     BCP 195 [RFC7525] provides recommendations for improving the security
>
> I'm not sure what the best XML syntax is for this, but BCP 195 also
> includes RFC 8996, now (which we do cite, separately, at the end of this
> paragraph).
>
> Section 6.1
>
> RFC 8126 could probably be classified as informative -- while we do say
> that what we specify is compliant to it, the reader does not need to
> read 8126 and validate that claim.
>
> Why is RFC 8996 normative but RFC 7525 (the other half of BCP 195) only
> informative?
>
> Appendix A
>
> I agree with the other reviewer that noted the different section
> numbering between the old and new references; some acknowledgment of the
> need to check section numbers seems in order.
>
>
>
> _______________________________________________
> Emu mailing list
> Emu@ietf.org
> https://www.ietf.org/mailman/listinfo/emu