Subject: [Emu] Benjamin Kaduk's Discuss on draft-ietf-emu-eap-tls13-20: (with DISCUSS and COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Wed, 06 Oct 2021 19:33:55 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-emu-eap-tls13-20: Discuss

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/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

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]".

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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.