On Jan 9, 2023, at 2:43 AM, Alexander Clouter <alex+i...@coremem.com> wrote:
....

  Fixed, unless otherwise noted / discussed.

> Section 3.3.1 - EAP Sequences
> 
> * "Upon completion of each EAP method in the tunnel, the server MUST send an 
> Intermediate-Result TLV indicating the result of the inner EAP method. The 
> peer MUST respond to the Intermediate-Result TLV indicating its result. If 
> the result indicates success, the Intermediate-Result TLV MUST be accompanied 
> by a Crypto-Binding TLV."
> 
> This could be interpreted as the peer sends the Crypto-Binding TLV (without 
> reading further) and not the server. Can we amend this to something more 
> front loaded such as:

  Or maybe just "the servers Intermediate-Result TLV MUST be accompanied..."

> Section 3.7 - Fragmentation
> 
> I think this section should be removed as it adds nothing over RFC5216 
> section 2.1.5 (EAP Fragmentation) and doing a word diff it is mostly just 
> word smithing changes that (at least for me) does nothing to improve 
> understanding.
> 
> If we want to keep it to minimise changes to RFC7170, I suggest we just 
> truncate the section and state "go read RFC5216 section 2.1.5".

  I think that's reasonable.

> Section 3.8.1 - PAC Provisioning 
> 
> "The peer MUST send separate PAC TLVs for each type of PAC it wants to be 
> provisioned. Multiple PAC TLVs can be sent in the same packet or in different 
> packets. The EAP server will send the PACs after its internal policy has been 
> satisfied, or it MAY ignore the request or request additional authentications 
> if its policy dictates."
> 
> It is not explicitly stated that a server must respond to PAC TLVs in the 
> order requested, it would though be hard to see how this could be implemented 
> otherwise. Of course as PAC-Type (section 4.2.12.6) only supports the single 
> value of 1 for 'Tunnel PAC', we may want to state for TEAP version one (1) 
> *only* a single PAC may be requested whilst the server will only optionally 
> service the first and ignore the rest?
> 
> Afterall, the statement does say the client can send three requests and the 
> server can ignore any of them, even arguably reply to request 1 and 3 and 
> ignore 2. No idea how the peer could figure out what happened here without 
> more signalling.
> 
> I would recommend to reword this section to something more like:
> 
> "The peer MAY only request a single Tunnel PAC it wants to be provisioned. 
> Additional PAC TLVs MUST be ignored by the server. The EAP server will send 
> the PAC after its internal policy has been satisfied, or it MAY ignore the 
> request or request additional authentications if its policy dictates."

  While that's a technical change, I think it's entirely reasonable.  Since 
there is no way to handle multiple PACs correctly, the only possible behavior 
is to support only one PAC.

  And all the PAC issues are moot once TLS 1.3 is supported.

> Section 3.8.3 - Server Unauthenticated Provisioning Mode
> 
> "Phase 2 EAP methods used in Server Unauthenticated Provisioning Mode MUST 
> provide mutual authentication, provide key generation, and be resistant to 
> dictionary attack. Example inner methods include EAP-pwd and EAP-EKE."
> 
> As EAP-FAST-MSCHAPv2 is not resistant to dictionary attack[2] we need to tell 
> people this.
> 
> In practice, as anyone seen anything other than EAP-FAST-MSCHAPv2 actually be 
> used for this? Win10/11 does not; and EAP-AKA/EAP-SIM is not exactly 
> available to non-telcos, right? The other methods supported you would have 
> the server (inner) identity available (ie. EAP-TLS) which opens the question 
> why you would not also know the outer server identity.
> 
> No idea what to do here.

  Note that EAP-MSCHAPv2 is not resistant to dictionary attacks, and run away.

> Section 4.2.3 - Identity-Type TLV 
> 
> Earlier in the parent section 4.2 states this TLV must be supported by all 
> implementations but then the 'M' flag is set to optional (M=0). The wording 
> in the section 
> 
> I do not think we should set this to M=1 (as Win10/11 does not) but maybe we 
> need some language here to cover that though you must support it and respond 
> to it, we mark it optional still?

  Arg.  I think it's best to mark it Mandatory.  There's no issue if 
implementations don't set that bit.

> Section 4.2.5 - NAK TLV
> 
> I cannot find the why it was changed (happened in 
> draft-dekok-emu-rfc7170bis-00) so I probably missed the memo, but the length 
> is >=6 in RFC7170 and here is =6 which seems wrong as there is a (albeit 
> unused) TLV section in the attribute.

  That's likely a markdown issue.  The source has ">= 6", so perhaps the ">" is 
being interpreted as HTML.  :)

> Section 4.2.12 - PAC TLV Format

  I'll leave this as a "todo".  I've added a Github issue to track it.

> Section 4.2.12.5 - PAC-Acknowledgement TLV

  I've added a GitHub issue to track it.

> Section 4.2.13 - Crypto-Binding TLV 
> ...
> "Flags -> TODO: What if there is just Basic-Password-Auth, and no EAP 
> method?" - I would suggest "use a zero-MSK". It was suggested by Eliot[3] 
> that this would be a good thing to mark under 'Security Considerations' to 
> inform the reader that if they use a zero-MSK thing it opens a MitM 
> possibility. We would be describe what I called makes an 'effective' 
> Crypto-Binding TLV and what that buys you.

  I think that's covered by other text which says "the MSK for basic password 
auth is zero".  I've pushed a fix.

> Section 4.2.1[45] - Basic-Password-Auth-{Req,Resp} TLV 

> Also seems strange to have Basic-Password-Auth-Resp TLV marked as optional 
> (M=0)...? Want me to spin up Win10/11 and see what it sets?

  It should be marked Mandatory, too.

> Section 4.2.15 - PKCS#7 TLV

  I'm not sure.  I'll open a GitHub issue.

> Section 4.3 - TLV Rules 
> 
> "The order of TLVs in TEAP does not matter, ..."
> 
> We should though *recommend* an ordering to processing those TLVs as it 
> already tripped one implementation[1]. Something like in descending priority:

  How about:

The order in TLVs are encoded in a TEAP packet does not
matter, however there is an order in which TLVs must be processed:

1. Crypto-Binding-TLV
2. Intermediate-Result-TLV
3. Result-TLV
4. Identity-Type TLV
5. EAP-Payload TLV[Identity-Request] or Basic-Password-Auth-Req TLV

That is, cryptographic binding is checked before any result is used,
and identities are checked before proposing authentication methods, as the
identity may influence the chosen authentication method.

> Section 7.3 - Separation of Phase 1 and Phase 2 Servers 
> 
> "If the inner method is deriving EMSK, then this threat is mitigated as TEAP 
> utilizes the mutual crypto-binding based on EMSK as described in [RFC7029]." 
> - Again mentions here that only EMSK will bring you to comfort for 
> crypto-binding which I think has implicatations for section 3.8.1 
> (unauthenticated server provisioning).

  Sure.  I'll add some text.

> Section 7.4 - Mitigation of Known Vulnerabilities and Protocol Deficiencies 
> 
> "To that extent, the TEAP authentication mitigates several vulnerabilities, 
> such as dictionary attacks, by protecting the weak credential-based 
> authentication method" - I think this ('dictionary attacks') is a false 
> statement. Both EAP-FAST-MSCHAPv2 and Basic-Password-Auth can be just 
> hammered. Maybe 'offline attack' would be a better fit here?

  Sure.

> Section 7.4.2 - Dictionary Attack Resistance
> 
> I do not understand that TLS does provide any dictionary attack protection, 
> right?

  Yes.  I'll add some text on online brute-force attempts.

> Section 7.8 - Security Claims
> 
> "Dictionary attack prot.: Yes" - maybe I am missing something, can someone 
> point me to some reading material that explains how TLS protects from 
> dictionary attacks?

  It doesn't.  TEAP (mostly) does.  I'll add some text.

> Appendix C.1 - Successful Authentication 
> 
> We may want to update this (and the other sub-appendix sections):
> 
> "(TEAP Start, S bit set, Authority-ID)"
> 
> To be written as:
> 
> "(TEAP Start, S bit set, O bit set, Authority-ID)"
> 
> For me there is a little confusion caused by "PAC-Opaque in SessionTicket 
> extension" which leads to a resumed session...which then leads to a 
> refreshing of a PAC in a resumed session which conflicts with section 3.2.3 
> stating "A peer SHOULD NOT request that a new PAC be provisioned after the 
> abbreviated handshake, as requesting a new session ticket based on resumed 
> session is not permitted.".
> 
> Now I have been blending to mean the same 'resumed' and 'abbreviated 
> handshake' which probably means other readers will too. Maybe we should 
> explicitly state somewhere:
> 
> 'resumed session' - no inner authentication methods take place
> 'abbreviated handshake' - shorter TLS handshake

  I'll take a note about that.

> Appendix C.5 - Fragmentation and Reassembly 
> 
> I would be included to remove this for the same reasons as section 3.7 
> (Fragmentation) earlier and it adds nothing more that RFC5216 provides 
> already...which you must have implemented as a prerequisite to get as far as 
> doing TEAP anyway.

  I'll leave that for the WG to decide.

> Appendix C.8 - Sequence of EAP Method with Vendor-Specific TLV Exchange
> 
> Not sure this should go in 'EAP Sequences', but looks like we should help the 
> reader and explicitly state somewhere that a sequence can be either a run of 
> EAP-Payload TLV's or Vendor-Specific TLV's terminated with 
> Intermediate-Result TLV (or Result TLV when used not in a sequence)?
> 
> I probably would add a paragraph to the end of section 3.3.1 (EAP Sequences) 
> stating something like:
> 
> "Implementations wishing to use their own proprietary non-EAP based 
> authentication method, may substitute the EAP-Payload TLV for the 
> Vendor-Specific TLV and use this section without amendment; such as the 
> interaction with Intermediate-Result TLV. It is valid to have a mix sequence 
> of EAP-Payload TLV followed by Vendor-Specific TLV or vice versa.

  I'll add some similar text to the "Phase 2" section above the "EAP sequences" 
section.  Discussion of generic "authentication methods" belongs there.

> Appendix C.9 - Peer Requests Inner Method after Server Sends Result TLV
> 
> We probably should include a note that a TEAP implementation probably does 
> not want to send Result-TLV after a successful non-authenticating outer TLS 
> handshake; the diagram shows a simplified exaggerated scenario. Maybe the 
> diagram should have also added to it:
> 
> "TLS client certificate" is sent or something.

  It already has "TLS certificate" on the LHS in the third exchange from the 
client, so I think that's good.

> Appendix C.10 - Channel Binding
> 
> Maybe this is the key to what to do for a Crypto-Binding when doing 
> Basic-Password-Auth and then immediately sending Result TLV indicating 
> Success. The implementation is RECOMMENDED to send a Channel-Binding TLV; 
> which is not a 'MUST be implemented' TLV so optional.

   I'm not sure what changes you're looking for here.

  Alan DeKok.

_______________________________________________
Emu mailing list
Emu@ietf.org
https://www.ietf.org/mailman/listinfo/emu

Reply via email to