Subject: [Emu] AD review of draft-ietf-emu-eap-tunnel-method

From: Sean Turner <[email-masked]>
Date: Mon, 24 Jun 2013 10:56:40 -0400

Here's my first pass (sitting at SFO killing time).  I'd like to like to 
discuss a couple of things before issuing the IETF LC:

0) s1.2: Is section 1.2 still required?  Didn't we publish a 
requirements draft with all of this information in it?

1) s2: Does phase one include:

       Authenticating Peer     Authenticator
        -------------------     -------------
+-                              <- EAP-Request/
|                               Identity
|this?  EAP-Response/
|       Identity (MyID1) ->
+-

or does it start here:
                                <- EAP-Request/
                                EAP-Type=TEAP, V=1
                           (TEAP Start, S bit set, Authority-ID)

Id it's the latter, then maybe:

OLD:

   TEAP authentication occurs in two phases.

NEW:

   TEAP authentication occurs in two phases after the initial EAP
   request/response exchange.

Not sure that my suggestion is right maybe it's between initial eap 
request and

2) s2.2: I find the figure kind of confusing.  Is this same thing:

+----------------------------------------------------------+
|    +----------------------------------------------------+|
|    |     +------+----+---------------------------------+||
|    |     |      |    |     +--------------------------+|||
|    |     |      |    |     |      +------------------+||||
| CP | EAP | code | id | len | type | type data = TEAP |||||
|    |     |      |    |     |      +------------------+||||
|    |     |      |    |     +--------------------------+|||
|    |     +------+----+---------------------------------+||
|    +----------------------------------------------------+|
+----------------------------------------------------------+

Where CP is Carrier Protocol, c = EAP code, i = EAP id, l = EAP length, 
t = EAP type (and it = TEAP), td = EAP type data

and then TEAP is in s4.1?

If it's just layers:

+------+
| TLS  |
+------+
| TEAP |
+------+
| EAP  |
+------+
| CP   |
+------+

mixing the EAP/TEAP wrappers in looks odd to me.  I'm not hard over on 
making this change I'm just trying to understand it.

3) s3.1: I haven't yet found if this is else where in the draft, but if 
this this check fails then what?:

  The receiver of the Crypto-Binding TLV MUST verify
  that the version received in the Crypto-Binding TLV matches the
  version sent by the receiver in the TEAP version negotiation.

Section 3.6.1?

4) s3.2: Okay so any chance we could at least SHOULD a cipher suite with 
SHA-256?

5) s3.2: This confused me a bit:

  TEAP implementations MUST support peer authentication during tunnel
  establishment using the TLS ciphersuites specified in Section 3.2.
  The EAP peer does not need to authenticate as part of the TLS
  exchange, but can alternatively be authenticated through additional
  exchanges carried out in Phase 2.

Is this about the TEAP clients supporting TLS server side 
authentication?  I read that and took it mean that both sides had to 
support certificate-based authentication and if that's the case:

  TEAP implementations MUST support mutual peer authentication
  during tunnel establishment using the TLS ciphersuites specified
  in Section 3.2.

if it's just server side:

  TEAP client's MUST support server-side certificate authentication
  during tunnel establishment using the TLS ciphersuites specified
  in Section 3.2.

In the next sentence it's the EAP peer is that supposed to be the TEAP peer?

6) s3.2: This section introduces the concept that other TLS extensions 
can be used.  Should this section also include MUST support statements 
for TLS session tickets and TLS unique?

7) s3.2.2: Paque opaque MUST NOT????  What about the other MUSTs in this 
section?

8) s3.8.2: Couple of questions:

8.1) How is the pkcs7/10 encoded; is it encapsulated in an media-type 
(i.e., does it include the content-type, etc headers)?  The PKCS#10 is 
covered in 4.2.17, but what about the PKCS#7.

8.2) Need a reference to where the challenge-password attribute is 
defined (EST authors did this as well): request challenge-password field 
([RFC2985], Section 5.4.1).

8.3) that attribute has a length limit and there's a small chance the 
tls-unique might be longer.  How about adding the same kind of text the 
EST authors did:

  The
  challenge-password field is limited to 255 bytes.  If the TLS cipher
  suite in use produces a longer verify_data than that, then an
  associated hash algorithm will have to be selected to reduce the
  verify_data to fit within the challenge password length limit.
  (Section 7.4.9 of [RFC5246] indicates that no existing cipher suite
  pose such an issue.)

8.4) What do you think about the following:

  If tls-unique information is not embedded
  within the certification request the challenge-password field MUST be
  empty to indicate that the client did not include the optional
  channel-binding information (any value submitted is verified by the
  server as tls-unique information).

8.5) Need some text about verifying the returned certificate back to an 
authorized TA.  Don't want the client to willy-nilly accept the returned 
  certificate.

8.6) Are the CRLs for the issuing CA returned as part of the certs-only 
message or will the client use the CRLs from the TLS handshake?  Should 
we have some kind of guidance about if the client is going to do this 
that it SHOULD pull the CRLs/OCSP responses for the CA?

9) s4.2.*: Should the M bits be "OPTiONAL" and "REQUIRED" as opposed to 
"Optional" and "Mandatory" in order to use RFC 2119 language?

10) s4.2.2: Mandatory is == 1 right?
     r/Reserved, set to zero (0)
      /Reserved MUST be set to 1

NITS:

0) abstract: by using the Transport Layer Security (TLS)
              by using the Transport Layer Security (TLS) protocol

1) replace reference to RFC 2560 with RFC 6960.  An updated OCSP RFC was 
published.

2) s1: this is a little close to marketing:

  Since the introduction of EAP-FAST [RFC4851] a few years ago, it has
  been widely adopted in variety of devices and platforms due to its
  strong security, flexibility and ease of deployment.

How about this:

  Since the introduction of EAP-FAST [RFC4851] a few years ago, it has
  been widely adopted in variety of devices and platforms.

3) s1.3: r/... may distributed using RFC
           /... may be distributed using RFC

4) s2: r/TEAP makes use of the TLS enhancements in Ticket Extension
          [RFC5077] to enable an optimized TLS tunnel session resume
          while minimizing server state.
         /TEAP makes use of the TLS SessionTicket Extension [RFC5077],
          which supports TLS session resumption without requiring
          session-specific state at the TLS.

5) s2: r/The ticket is referred to as the Protected Access
          Credential opaque data (or PAC-Opaque).
         /In this document, the SessionTicket is referred to as the
          Protected Access Credential opaque data (or PAC-Opaque).

6) s2: r/NewSessionTicket message is being used to
         /NewSessionTicket message is used to

7) s2.2: r/requires a carrier protocol for transport
           /requires a transport protocol

8) s2.2: Question: Is there any other way to do this?

  All conversations in the TEAP protected tunnel MUST be
  encapsulated in a TLV layer.

If it's always going to be encapsulated and that's the only way to do it:

  All conversations in the TEAP protected tunnel are
  encapsulated in a TLV layer.

9) s3/s2: The intro in s3 is the same s2 maybe just shorten s3 intro to:

  The operation of the protocol, including
  Phase 1 and Phase 2, is the topic of this section.  The format of
  TEAP messages is given in Section 4 and the cryptographic
  calculations are given in Section 5.

10) s3.1: Again if this is how it is don't need the MUST:

  If the EAP peer supports this version of the protocol, it
  responds with an EAP-Response of EAP type=TEAP, and the version
  number proposed by the TEAP server.

11) s3.2: r/TEAP is based on the TLS handshake
            /TEAP relies on the TLS handshake

12) s3.2: Are there any other TLS cipher suites that shouldn't be used? 
  NULL?

13) s3.2: How about also pointing to RFC 6961 for the multi-ocsp 
extension too:

  For instance, Certificate
  Status Request extension [RFC6066] and multiple certificate
  status request extension [RFC6961] can be used to leverage a
  certificate-status protocol such as OCSP [RFC6960] to check the
  status of server certificates.

14) s3.2: Could we say this instead to not confuse it with the TLS 
record protocol:

OLD:

  This message
  encapsulates one or more TLS records containing the TLS handshake
  messages.

NEW:

  This message
  encapsulates one or more TLS handshake
  messages.

15) s3.2: r/TLS ciphersuites specified in Section 3.2.
            /TLS ciphersuites specified in this section.

16) s3.2.2:r/from a previous handshake in its ClientHello message
             /from a previous TLS handshake in its ClientHello message

17) s3.2.3: r/peer can request for a new PAC to be provisioned after
              /peer can request that a new PAC be provisioned after
             r/for a new PAC to be provisioned
              /that a new PAC be provisioned

18) s3.3/3.3.3: Can you add a couple of words about why the two MUST 
NOTs?  It's obvious to me but it might not be to another AD ;)

19) s3.3.2: Should "not recommended" be NOT RECOMMENDED?

20) s3.4.4:

OLD:

   The subject field identifies the entity associated with the public
   key stored in the subject public key field.  The subject name MAY
   be carried in the subject field and/or the subjectAltName
   extension....  If subject naming information is present only in
   the subjectAltName extension (e.g., a key bound only to an email
   address or URI), then the subject name MUST be an empty sequence
   and the subjectAltName extension MUST be critical.

NEW:

   The subject field identifies the entity associated with the public
   key stored in the subject public key field.  The subject name MAY
   be carried in the subject field and/or the subjectAltName
   extension.

   If subject naming information is present only in
   the subjectAltName extension (e.g., a key bound only to an email
   address or URI), then the subject name MUST be an empty sequence
   and the subjectAltName extension MUST be critical.

21) s3.5: Need to say || means concatenation

22) s3.6.2: SHOULD here?

  If the TEAP peer detects an error at any point in the TLS layer, the
  TEAP peer should send a a TEAP response encapsulating a TLS record
  containing the appropriate TLS alert message.

23) s3.7: You've never seen the 100Mbyte CRLs ;)  Note that I know the 
TSV ADs will probably key on the fragmentation section so I gave them a 
heads ups so they won't be surprised later.

24) s3.7: Assume this throws off an error?  Is there some kind of catch 
all sentence someplace that I missed that says all MUST NOTs result in 
an error being thrown?

25) s3.8: Mutual authentication is when both parties are authenticated 
to each other.  I guess the point in the following is that the 
server-side certificate authentication has already occurred?

   Mutual authentication results if the peer trusts the provided server
   certificate belongs to the server; typically this involves both
   validating the certificate to a trust anchor and confirming the
   entity named by the certificate is the intended server.

26) s3.8: Is "mutually" the right word shouldn't it just be previously 
authenticate?

  Mutual
  authentication also results when the procedures of Section 3.3 are
  used to resume a session in which the server was previously mutually
  authenticated.

  or should it be:

   ... a previous session in which the peer and server were mutually
  authenticated.

27) s3.8: r/EMSK compound MAC present
            /EMSK compound MAC is present

28) s4.1: r/Reserved (MUST be zero)
            /Reserved (MUST be zero and ignored upon receipt)

30) s4.2.3/7: r/(Optional)/(OPTIONAL)

31) s4.2.12: r/
              0 - Non-mandatory TLV
              1 - Mandatory TLV
              /0 or 1
     matches earlier section or to change s4.2.8

32) s4.2.14: r/optional/OPTIONAL

33) s4.2.15: Need reference for UTF-8: RFC 3629

34) s4.2.16: r/The PKCS#7 TLV is always marked as optional
               /The PKCS#7 TLV is always marked as OPTIONAL

35) s7.3: Add NOT RECOMMENDED to s1.1.

---

From: "Hao Zhou (hzhou)" <[email-masked]>
Date: Thu, 27 Jun 2013 20:36:14 +0000

Sean:

Thanks for the detailed review. Please see my comments inline below.

>requirements draft with all of this information in it?
[HZ], We will remove it.
>       Authenticating Peer     Authenticator
[HZ] Your suggestion seems to be fine.
>2) s2.2: I find the figure kind of confusing.  Is this same thing:
[HZ] Your interpretation is correct and we are using the same layering
format as in PEAP and TTLS RFC.
>  version sent by the receiver in the TEAP version negotiation.
[HZ] Good catch. How about we add a sentence in the end:
"If the Crypto-Binding TLV fails to be validated, then it is a fatal error
and is handled as
   described in Section 3.6.3."?

>4) s3.2: Okay so any chance we could at least SHOULD a cipher suite with
[HZ] Ok, Will add a SHOULD for CipherSuite TLS_RSA_WITH_AES_256_CBC_SHA

>  during tunnel establishment using the TLS ciphersuites specified
[HZ] That's what we mean. Will change to that.
>In the next sentence it's the EAP peer is that supposed to be the TEAP
[HZ] Will be changed to TEAP Peer.
>for TLS session tickets and TLS unique?
[HZ] Sure. We mentioned in Section 2 already about the mandatory support
for TicketExtension. Will add.
>7) s3.2.2: Paque opaque MUST NOT????  What about the other MUSTs in this
[HZ] Not quite understand what you mean. Please explain.

>covered in 4.2.17, but what about the PKCS#7.
[HZ] We will look into this and get back you.
>([RFC2985], Section 5.4.1).
[HZ] Will add that.

>  (Section 7.4.9 of [RFC5246] indicates that no existing cipher suite
[HZ] Sounds good. Thanks for the text.
>  server as tls-unique information).
[HZ] Sounds good.
>authorized TA.  Don't want the client to willy-nilly accept the returned
[HZ] Good suggestion. Do you have any text to suggest or from EST?
>that it SHOULD pull the CRLs/OCSP responses for the CA?
[HZ] We are looking into and will get back to you.

>"Optional" and "Mandatory" in order to use RFC 2119 language?
[HZ] The mandatory and optional but in TEAP are bit different than the
OPTIONAL and REQUIRED in RFC2119. If the mandatory bit is set, the other
peer receiving it MUST understand and handle it, not necessary required to
be present in every packet exchange. That's why we keep them in lower
case. 
>10) s4.2.2: Mandatory is == 1 right?
[HZ] Yes. There is a typo, should be set to (1).

>      /Reserved MUST be set to 1
[HZ] Actually, The Reserved bit should be set to 0, not used.
>              by using the Transport Layer Security (TLS) protocol
[HZ] Will do.
>1) replace reference to RFC 2560 with RFC 6960.  An updated OCSP RFC was
[HZ] Will do
>  been widely adopted in variety of devices and platforms.
[HZ] Ok.
>           /... may be distributed using RFC
[HZ] Ok.
>          session-specific state at the TLS.
[HZ] Ok.
>          Protected Access Credential opaque data (or PAC-Opaque).
[HZ] Ok.
>         /NewSessionTicket message is used to
[HZ] Ok.
>           /requires a transport protocol
[HZ] Ok.
>8) s2.2: Question: Is there any other way to do this?
[HZ] No.
>  encapsulated in a TLV layer.
[HZ] Will do.
>  calculations are given in Section 5.
[HZ] Ok.
>  number proposed by the TEAP server.
[HZ] Ok.
>            /TEAP relies on the TLS handshake
[HZ] Ok.
>12) s3.2: Are there any other TLS cipher suites that shouldn't be used?
[HZ] Ok, Will add.
>  status of server certificates.
[HZ} Ok.
>  encapsulates one or more TLS handshake
[HZ] ok.
>            /TLS ciphersuites specified in this section.
[HZ] Ok.
>             /from a previous TLS handshake in its ClientHello message
[HZ] Ok.
>              /that a new PAC be provisioned
[HZ] Ok.
>NOTs?  It's obvious to me but it might not be to another AD ;)
[HZ] Will do.

>19) s3.3.2: Should "not recommended" be NOT RECOMMENDED?
[HZ] Yes.
>   and the subjectAltName extension MUST be critical.
[HZ] Ok.
>21) s3.5: Need to say || means concatenation
[HZ] Ok.
>  containing the appropriate TLS alert message.
[HZ] Yes.
>all sentence someplace that I missed that says all MUST NOTs result in
[HZ] What is "this?"
>   entity named by the certificate is the intended server.
[HZ] I guess so. It is contributed text. I think it is talking about
making sure server side authentication is happened.
>   ... a previous session in which the peer and server were mutually

[HZ] I like the later sentence.
>            /EMSK compound MAC is present
[HZ] Ok.
>            /Reserved (MUST be zero and ignored upon receipt)
[HZ] Ok.
>30) s4.2.3/7: r/(Optional)/(OPTIONAL)
[HZ] See Comment #10.
>     matches earlier section or to change s4.2.8
[HZ] Ok.
>32) s4.2.14: r/optional/OPTIONAL
[HZ] See Comment #10.
>33) s4.2.15: Need reference for UTF-8: RFC 3629
[HZ[ Ok.
>               /The PKCS#7 TLV is always marked as OPTIONAL
[HZ] See Comment #10.
>35) s7.3: Add NOT RECOMMENDED to s1.1.
[HZ] ok.
>_______________________________________________
>Emu mailing list
>[email-masked]
>https://www.ietf.org/mailman/listinfo/emu

---

From: Sean Turner <[email-masked]>
Date: Fri, 28 Jun 2013 12:40:53 -0400

I trimmed this down to the ones that weren't resolved.  Let me know how 
those two items end getting resolved.

> [HZ] Not quite understand what you mean. Please explain.

Yeah this one got sent out early ;)  Sorry about that.

What I meant to write was please add some text about why there are MUST 
NOTs.  If it's just a MUST NOT to make things easier then you'll get 
push back later but if there's a reason for the MUST NOTs and you 
include it now it'll smooth the way later.

> [HZ] We will look into this and get back you.

ack

> [HZ] Good suggestion. Do you have any text to suggest or from EST?

I was thinking simply "The peer MUST verify the returned certificate to 
an authorized Trust Anchor."

> [HZ] We are looking into and will get back to you.

ack

> [HZ] Yes. There is a typo, should be set to (1).

wfm

> [HZ] Actually, The Reserved bit should be set to 0, not used.

yeah got that one wrong leave it as is

>> all sentence someplace that I missed that says all MUST NOTs result in

This bit:

  The L
  flag is set to indicate the presence of the four-octet TLS Message
  Length field, and MUST be set for the first fragment of a fragmented
  TLS message or set of messages.  It MUST NOT be present for any other
  message.

It's back to providing text about the reason for the MUST NOT.

> making sure server side authentication is happened.

yeah okay this seems fine after another read.