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

[Joseph Salowey <[e-mail]>]

Thanks for your review.  Comments inline below:

On Thu, Oct 7, 2021 at 4:17 PM Benjamin Kaduk via Datatracker <
[e-mail]> 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
>
>
[Joe] I'll let the authors weigh in on this recommendation. It looks fine
to me.


>
> 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
>
> [Joe] Thank you.  I will take a look.


> 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]".
>
>
[Joe]  would changing "this" to "this update" help?


> 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.
>
>
[Joe]  Since TLS doesn't really define an API it can be hard to define a
particular behavior without referencing messages.   I agree the last
sentence is redundant.


>    *  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)?
>
>
[Joe]  Yes, I think it would be good to mention it.  Another approach would
be to say clients and servers must not use post handshake authentication
and leave the message level details out as per the suggestion above.  My
preference would be to add a bit about the client not sending the extension
as this might be more consistent with the rest of the document.


> 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.
>
> [Joe] The TLS server should have no additional data to send after it sends
the 0x00 application data.  Are you referring to the case where there is a
message like keyupdate that it needs to respond to.


> 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).
>
>
[Joe] Thanks, I'll take a look.


>    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.
>
>
[Joe] Yes I think we should make that change.


>    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.
>
>
[Joe] Should say something like "disclosure of the key used for session
resumption"


> 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.
>
>
[Joe] For completeness it's probably good to mention, but I think the
authors should decide.


> 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?
>
> [Joe] Good question.  I don't see a problem with changing to access
control, but I think MUST NOT is appropriate.  Relying on unauthenticated
data for these purposes is problematic.


>    (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.
>
> [Joe] to make sure I understand your suggestion is to make it explicit
that implementations should allow a particular name to be associated with a
specific trust root?


> 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.
>
>
[Joe]  Yes, they are equivalent, although many EAP-TLS implementations do
not rely upon [RFC5705] exporter functionality.   Maybe
"Note that when TLS 1.2 is used with the EAP-TLS exporter [RFC5705] it
generates the same key material as in EAP-TLS [RFC5216]?


> 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.
>
>
[Joe] I think the point the test is trying to make is that they may be sent
in separate messages.

  "After sending TLS Finished, the EAP-TLS server may send any number of
post-handshake
   messages in one or more EAP-Requests."



> 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...)
>
>
[Joe] Hmm. I don't remember, perhaps one of the authors does.


> 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.
>
>
[Joe]  Before we make a wholesale reference we should check to make sure
that there isn't a conflict between the two documents since EAP-TLS cert
usage is slightly different from application TLS cert usage.


> 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.
>
>
[Joe] Yes these are both referring to the EAP peer.


> 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.
>
> [Joe] Thanks, I'll take a look.


>    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?
>
>
[Joe]  Network access policy may take into account more than just the
EAP-TLS outputs.  For example, it might take into account what access
point/network you are attached to or some other network parameters.
Someone else may have better examples.


> 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.
>
>
[Joe] I think "EAP authentication attempt" is more accurate since the
server will send a fatal 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).
>
>
[Joe] Author's should take a look at this.


> 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.
>
>
[Joe] OK


> Why is RFC 8996 normative but RFC 7525 (the other half of BCP 195) only
> informative?
>
> [Joe] Good question.  I'm not sure that either of these need to be
normative.


> 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.
>
>
[Joe]  I need to look into this.