Re: [Emu] draft-ietf-emu-eap-tls13-16.txt

Hi Alan,

Response in-line.

On 6/11/21 4:38 PM, Alan DeKok wrote:
>    Some comments have been addressed, others not.  The majority of the issues raised in my review have been silently ignored.  Some issues are nits, some affect interoperability and security.
>
>    Until these issues are addressed, the document is insufficient to guide a reader into creating a secure and inter-operable implementation.
>
> Unaddressed without any comment or discussion:
>
>> Section 1 says:
>>
>>   While this document updates EAP-TLS [RFC5216], it
>>   remains backwards compatible with it and existing implementations of
>>   EAP-TLS.
>>
>> Other than the abstract, this is the only reference to EAP-TLS 1.3
>> being backwards compatible with older versions of EAP-TLS.  This
>> compatibility is simply asserted, with no further explanation given.
>>
>> Q: What does "backwards compatible" mean?  How is it achieved?
>>
>> Suggestion: add text explaining how it is backwards compatible.  How
>> will EAP-TLS 1.3 implementations negotiate EAP-TLS 1.2?  Perhaps
>> update Section 2.1 with text indicating that TLS version negotiation
>> is handled by the TLS layer, and thus outside of the scope of EAP-TLS.
>> Therefore so long as the underlying TLS implementation correctly
>> implements TLS version negotiation, EAP-TLS will automatically
>> leverage that capability.
The comment here says adding text about "TLS version negotiation". There 
is a comment from you below saying: "I don't understand why it's 
necessary to include discussion of TLS negotiation in EAP, when that 
negotiation does not affect EAP in any way." Am I missing something or 
is there conflicting feedback?
>>
>>
>> Section 2.1.1 says:
>>
>>   TLS 1.3 introduced the Post-Handshake KeyUpdate
>>   message which is not useful and not expected in EAP-TLS.
>>
>> Q: What does it mean that the message is "not expected"?  This seems
>> like a source of implementation-defined behavior, which experience
>> shows has been a source of interoperability and security issues.
>>
>> Suggestion: If there is no use for KeyUpdate messages, then mandate
>> that it SHOULD be ignored, or perhaps connections which use KeyUpdate
>> MUST be closed without updating the keys.  OpenSSL as APIs to
>> determine the status of key updates, so this suggestion is
>> implementable.
>    Addressed:
>
>> Section 2.1.3 says this about the session ticket:
>>
>>   ... If the EAP-TLS server
>>   accepts it, then the security context of the new connection is tied
>>   to the original connection and the key derived from the initial
>>   handshake is used to bootstrap the cryptographic state instead of a
>>   full handshake.
>>
>> Nit: This the the only reference to "bootstrap the cryptographic
>> state" in this document.  This text seems like an unnecessary
>> repetition of RFC 8446 Section 2.2.
>>
>> Suggestion: Perhaps say instead
>>
>>   ... If the EAP-TLS server
>>   accepts it, then the resumed session has been deemed to be
>>   authenticated, and securely associated with the prior authentication
>>   or resumption.
>>
>>
>> Section 2.1.4
>>
>>    In TLS 1.3 [RFC8446], error alerts are not mandatory to send after a
>>    fatal error condition.  Failure to send TLS Error alerts means that
>>    the peer or server would have no way of determining what went wrong.
>>    EAP-TLS 1.3 strengthen this requirement.
>>
>> NIT: strengthenS this requirement.
> Unaddressed without any comment or discussion:
>
>> Section 2.1.5 is essentially empty.
>>
>> Is there guidance as to when no peer authentication should /
>> should not be used?  Is it possible for an EAP peer to present a
>> client certificate, but have the EAP server ignore it?  What kind of network access should an EAP peer have if it does not use peer authentication?
>>
>>   Perhaps some of the text from Section 5.6 could be added here.
>>
>> Perhaps suggest that in the normal case, deployments SHOULD use peer
>> authentication.  Also that the "no peer authentication" case be
>> strictly limited in both time, and network access.
>>
>> e.g. The "no peer authentication" situation MUST NOT be used to give
>> normal network access to EAP peers.  Instead, deployments SHOULD
>> provide access which is limited in both time, and in capability.
>> Usually this means a "quarantine" network, or "walled garden", which
>> has only limited capability.
There is text already in the draft which says : "While the EAP server 
SHOULD require peer authentication, this is not mandatory, since there 
are circumstances in which peer authentication will not be needed (e.g., 
emergency services, as described in [UNAUTH]), or where the peer will 
authenticate via some other means.". This is what was said also in RFC 
5216. We also have also have the following text in the draft: "Some 
deployments may permit no peer authentication for some or all 
connections.  When peer authentication is not used, implementations MUST 
take care to limit network access  appropriately for unauthenticated 
peers and implementations MUST use resumption with caution to ensure 
that a resumed session is not granted more privilege than was intended 
for the original session.". I think we have tried to find a careful 
balance between providing sufficient guidance vs. being repetitive. Is 
there something that should be repeated in section 2.1.5?
>>
>> Also, the Security Considurations section has no discussion of the
>> security impact of not authenticating the peer.  As Section 2.1.5 is
>> new, it has major impacts on security, and thus needs to be discussed.
> Unaddressed without any comment or discussion:
>
>    I don't understand why it's necessary to include discussion of TLS negotiation in EAP, when that negotiation does not affect EAP in any way.
See above.
>
>> Section 2.1.6 says:
>>
>>   As defined in TLS 1.3 [RFC8446], EAP-TLS servers can send a
>>   HelloRetryRequest message in response to a ClientHello if the EAP-TLS
>>   server finds an acceptable set of parameters but the initial
>>   ClientHello does not contain all the needed information to continue
>>   the handshake.
>>
>> It's not clear why this section is necessary.  The text here appears
>> to be discussing internals of TLS layer negotiation, which are
>> invisible to EAP-TLS.  That is, this packet flow has no effect on the
>> EAP-TLS state machine, other than a different number of packets are
>> exchanged than with other packet flows.
>>
>> Question: Is it that "EAP-TLS server" does not have sufficient
>> information to continue the handshake, or is it "the TLS layer" ?
>>
>> Question: if the EAP-TLS implementation can do nothing other than ask
>> the TLS layer to continue the handshake, is this section even
>> necessary or relevant?
This section was added based on a review by Jim Schaad 
(https://mailarchive.ietf.org/arch/msg/emu/qO5qgQ-8GzB-jVx2vtrQCe42cCg/). 
If nothing else, I would like to keep it as a homage to him.
>>
>> Section 2.1.9 says:
>>
>>   Some EAP implementations and access networks may limit the number of
>>   EAP packet exchanges that can be handled.
>>
>> This is under-stating the issue rather severely.  We know with
>> absolute certainty that most (if not all) EAP implementations and
>> access networks limit the number of EAP packet exchanges.  Perhaps
>> update the text to reference implementation and interoperability
>> experience.
>     Addressed in part:
>
>    The updated text is clearer, but still does not address some of the questions raised below about calling this out as a new requirement from 5216, and what happens in an HA environment.
Please let us know a good reference to "implementation and 
interoperability experience." about the upper limit on EAP packet 
exchanges. This would also be useful for draft-ietf-emu-eaptlscert.
>
>> Section 2.2 has substantial new text which was not previously discussed on the WG mailing list.
>>
>>    The EAP server identity in the TLS server certificate is typically a
>>    fully qualified domain name (FQDN).  EAP peer implementations SHOULD
>>    allow users to configuring a unique trust root (CA certificate) and a
>>    server name to authenticate the server certificate and match the
>>    subjectAlternativeName (SAN) extension in the server certificate with
>>    the configured server name.
>>
>> Comment: How does this text related to RFC 5216 Section 5.2 ?  There seems to be substantial overlap.
>>
>> What does this text add to / change from RFC 5216 Section 5.2 ?
>>
>> Requiring a supplicant to be configured with a peer name is a new requirement from RFC 5216, and isn't called out as such.
I agree with you that we could benefit from more clarification on 
relationship between section 2.2 and 5.2 of this draft vs. the old RFC 
5216. Do you have a suggestion how best to split the text and reflect 
the updates to RFC5216?
>>
>> What happens in a high availability (HA) environment?  Are all of the EAP servers required to have the same FQDN?
>>
>> While this text is intended to increase security, there are implementation and operational considerations which need addressing.
>>
>>    In the absence of a user-configured root
>>    CA certificate,
>>
>> Comment: I'm not sure what that means.  It seems to assume certain things without explaining them.
>>
>>    The process of configuring a root CA certificate and a server name is
>>    non-trivial and therefore automated methods of provisioning are
>>    RECOMMENDED.  For example, the eduroam federation [RFC7593] provides
>>    a Configuration Assistant Tool (CAT) to automate the configuration
>>    process.  In the absence of a trusted root CA certificate (user
>>    configured or system-wide), EAP peers MAY implement a trust on first
>>    use (TOFU) mechanism where the peer trusts and stores the server
>>    certificate during the first connection attempt.  The EAP peer
>>    ensures that the server presents the same stored certificate on
>>    subsequent interactions.  Use of TOFU mechanism does not allow for
>>    the server certificate to change without out-of-band validation of
>>    the certificate and is therefore not suitable for many deployments.
>>
>> i.e. when there's an HA configuration.
>    Addressed.
>
>> Section 2.3 says:
>>
>>
>>    When EAP-TLS is used with TLS version 1.3 the Key_Material, IV, and
>>    Method-Id SHALL be derived from the exporter_secret using the TLS
>>    exporter interface [RFC5705] (for TLS 1.3 this is defined in
>>    Section 7.5 of [RFC8446]).
>>
>>    Type-Code  = 0x0D
>>    MSK        = TLS-Exporter("EXPORTER_EAP_TLS_MSK",Type-Code,64)
>>    EMSK       = TLS-Exporter("EXPORTER_EAP_TLS_EMSK",Type-Code,64)
>>    Method-Id  = TLS-Exporter("EXPORTER_EAP_TLS_Method-Id",Type-Code,64)
>>    Session-Id = Type-Code || Method-Id
>>
>> All this is nice, but it might be too late.  I'd check with major implementations which have frozen their code, and are shipping.
>>
>> It would be good for the spec and implementations to match.
>>
>>
>> Section 2.3 says:
>>
>>   By using the TLS exporter, EAP-TLS can use any TLS 1.3 implementation
>>   without having to extract the Master Secret, ClientHello.random, and
>>   ServerHello.random in a non-standard way.
>>
>> NIT: the exporters were first defined in TLS 1.2, and have been widely
>> available in TLS library implementations.  Using master secret,
>> etc. has not been necessary for a while.  Further, the "non-standard"
>> use of Master Secret, etc. was first done in the original EAP-TLS RFC
>> [2716], in 1999.  The TLS WG later defined and standardized the
>> exporters in order to meet the needs of EAP-TLS.
>>
>> Perhaps instead say:
>>
>>   By using the TLS exporter, EAP-TLS can use any TLS 1.3 implementation
>>   which provides a public API for the exporter.  There has been no need
>>   to access internal fields in TLS since the public exporters were
>>   defined in [RFC5705].
>>
>>
>> Section 2.4 says:
>>
>>    While EAP-TLS does not protect any application data except for the
>>    Commitment Message, the negotiated cipher suites and algorithms MAY
>>    be used to secure data as done in other TLS-based EAP methods.
>>
>> Comment: This appears to be the only reference to the commitment message in the document.  It may be good to update Section 2.5 to use the same name for the 0x00 byte of application data.
> Unaddressed without any comment or discussion:
>
>> Section 5.1 says:
>>
>>   [4] Cryptographic Negotiation: TLS 1.3 increases the number of
>>   cryptographic parameters that are negotiated in the handshake.  When
>>   EAP-TLS is used with TLS 1.3, EAP-TLS inherits the cryptographic
>>   negotiation of AEAD algorithm, HKDF hash algorithm, key exchange
>>   groups, and signature algorithm, see Section 4.1.1 of [RFC8446].
>>
>> Question: what does this mean in practice for EAP-TLS?  i.e. this text
>> describes a capability.  It does not describe what that capability
>> does, or how it benefits EAP-TLS.
This is what RFC5216 said "EAP-TLS inherits the secure ciphersuite 
negotiation features of TLS, including key derivation function 
negotiation when utilized with TLS v1.2 ". Do you have some suggestion 
on what we should add?
>>
>>
>> Section 5.2 says:
>>
>> No updates to section 5.2 of [RFC5216].
>>
>> This isn't true.  Section 2.2 has substantial new text with new requirements, and new security impacts.
>>
>>
>> Section 5.3 says:
>>
>>    While certificates may have long validity periods,
>>
>> Comment: Certs issued by public CAs are generally short-lived, as in a year or so.  It may be worth discussing this.
>>
>>
>> Section 5.4 says:
>>
>>    Some deployments may permit no peer authentication for some or all
>>    connections.  When peer authentication is not used, implementations
>>    MUST take care to limit network access appropriately for
>>    unauthenticated peers
>>
>> Q: Are these EAP server implementations?  How does an EAP server limit network access for unauthenticated peers?

Fixed in github version.

Could you also highlight if there are some unaddressed comments. It 
wasn't obvious if "Unaddressed without any comment or discussion:" 
refers only to the discussion immediately below or to several 
discussions below.

--Mohit

>>
>> Section 5.7 says:
>>
>>   There are a number of security issues related to resumption that are
>>   not described in [RFC5216].  The problems, guidelines, and
>>   requirements in this section therefore applies to all version of TLS.
>>
>> NIT: These requirements are for EAP-TLS, and not TLS.  This document does not apply new security requirements to the TLS protocol
>>
>> Perhaps instead:
>>
>>   There are a number of security issues related to resumption that are
>>   not described in [RFC5216].  The problems, guidelines, and
>>   requirements in this section therefore applies EAP-TLS when is used
>>   with any version of TLS.
>>
>>
>> Section 5.7 says:
>>
>>    If the EAP-TLS server or EAP client do not apply any authorization
>>    policies,
>>
>> NIT: EAP-TLS servers do not apply authorization policies.  Perhaps explain that the EAP-TLS server is co-located with RADIUS / Diameter etc, and those apply policies.
>>
>> NIT2: It's not clear how an EAP client would apply authorization policies.  Perhaps just remove the reference to the EAP client.
>>
>>
> _______________________________________________
> Emu mailing list
> Emu@ietf.org
> https://www.ietf.org/mailman/listinfo/emu