Re: [Emu] TEAP - RFC 7170 - Errata ID 5768

2020-06-29 Thread Joseph Salowey
On Mon, Jun 29, 2020 at 6:49 AM Oleg Pekar 
wrote:

> Jorge, Joe, thank you for your comments. So the updated proposal should be:
>
> 1) In Section "4.2.13.  Crypto-Binding TLV" make "EMSK Compound MAC" and
> "MSK Compound MAC" fields variable length depending on the number of output
> bits of the MAC function negotiated by TLS protocol. A field "Compound MAC
> Length" of size 1 octet is added after the "Nonce" field to represent the
> length of each of "EMSK Compound MAC" and "MSK Compound MAC" fields. The
> minimum length for each Compound MAC field is 20 octets (to honor the
> original RFC fixed size and to allow usage of SHA-1 - that is still used in
> TLS 1.2 - to provide MAC without padding). The length should never be less
> than 20 octets since there are no ciphers supported by TLS 1.2 with a
> digest of less than 160 bits output size. Therefore if "Compound MAC
> Length" field value is less than 20 then the Crypto-binding TLV should be
> considered of erroneous format and handled as described in section 3.6.
> Error Handling. The value of the length field of the whole Crypto-Binding
> TLV should be calculated accordingly (currently fix 76 octets).
>
> [Joe] I'm not sure I see the need to for a length here.  Isn't the MAC
algorithm known before this message is processed and therefore the length
should be known?  The document should clarify how the MAC algorithm is
determined for TLS 1.2 and TLS 1.3. The length could then be defined as the
length of the output of the MAC function.


> The updated structure of Crypto-Binding TLV:
>
> The Crypto-Binding TLV is defined as follows:
>
> 0   1   2   3
> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>|M|R| TLV Type  |Length |
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>|Reserved   |Version|  Received Ver.| Flags|Sub-Type|
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>|   |
>~ Nonce ~
>|   |
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>|  Comp MAC Len |
>
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>|   |
>~   EMSK Compound MAC   ~
>|   |
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>|   |
>~MSK Compound MAC   ~
>|   |
>+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>
> If we would not bump the TEAP protocol version - we still can change the 
> Crypto-Binding TLV version to 2.
>
>
> 2) [This was accepted and stays unchanged]
>
> 3) In Section "5.3.  Computing the Compound MAC" when specifying the list of 
> fields to be placed in the BUFFER" should say "...2  A single octet contains 
> TEAP EAP method type 0x37". However to let this byte play a role in 
> protection of Crypto-Binding TLV I would suggest to place in it the inner EAP 
> method type related to the calculation or 0x0 if no inner EAP method was 
> executed. What are your opinions on this?
>
>
[Joe] I believe the original intent was that the same Compound MAC
construction could be used in multiple protocols (EAP-FAST, PEAP, TEAP).
Using the outer EAP method type would help protect against cross protocol
attacks if this TLV were reused.  If we are going to change the
TLV/protocol version we could revisit this and other aspects of the
compound TLV to validate their security goals and implementation.


>
> Thanks
> Oleg
>
>
> On Tue, May 26, 2020 at 12:29 PM Jorge Vergara 
> wrote:
>
>>
>>
>> *From:* Joseph Salowey 
>> *Sent:* Monday, May 25, 2020 9:27 PM
>> *To:* Jorge Vergara 
>> *Cc:* Oleg Pekar ; Jouni Malinen ;
>> EMU WG 
>> *Subject:* Re: [Emu] TEAP - RFC 7170 - Errata ID 5768
>>
>>
>>
>>
>>
>>
>>
>> On Fri, May 22, 2020 at 1:18 PM Jorge Vergara > 40microsoft@dmarc.ietf.org> wrote:
>>
>> My review of this errata and the current responses from Oleg:
>>
>>
>>
>>1. I agree with this proposed resolution. I do think this is an
>>important omission that needs to be clarified in the RFC. Otherwise it is
>>somewhat guesswork that truncation is the right action. I think the 
>> current
>>wording leans toward truncation, but I definitely asked this question
>>myself while implementing.
>>
>> [Joe]  Why not just change the TLV to be variable length?  It seems if we
>> hardcode the length to

Re: [Emu] TEAP errata 5770

2020-06-29 Thread Oleg Pekar
Joe, nice proposal. Few questions:
1. We have a case of Basic Password Authentication instead of inner method
thus we should also use Crypto-Binding TLV based on Zero-MSK after it
2. As Eliot mentioned, we have a case of no inner method at all - we should
use Crypto-Binding TLV based on Zero-MSK after it
3. Since it is not clear whether MSK or Zero-MSK is used - we should
introduce a new flag value for Crypto-Binding TLV: "0" - Zero-MSK
based Compound MAC is used in place of MSK Compound MAC field.
4. We have a TLS-PRF usage ambiguity here (Errata 5127 and 5128):
TLS 1.2 RFC 5246 in section "5.  HMAC and the Pseudorandom Function"
defines PRF(secret, label, seed) = P_(secret, label + seed)

TEAP RFC uses TLS-PRF several times:
* IMSK = First 32 octets of TLS-PRF(EMSK, "teapbind...@ietf.org" | "\0" |
64)
  The call refers to RFC 5295 that defines USRK = KDF(EMSK, key label |
"\0" | optional data | length) but that RFC says "USRKs MUST be at least 64
octets in length" and IMSK is 32 octets
* IMCK[j] = TLS-PRF(S-IMCK[j-1], "Inner Methods Compound Keys", IMSK[j], 60)
* MSK  = TLS-PRF(S-IMCK[j], "Session Key Generating Function", 64) and
similar for EMSK

We can see that all three times the parameters are different and don't
exactly fit the definition and this creates some confusion.

In the other earlier thread you mentioned that truncation of 64-octets USRK
to 32 octet is valid. In yet another thread you mentioned that we should
define TLS-PRF function with the output size as a parameter and we should
just concatenate for all TLS-PRF calls all the parameters starting from the
second. Is that understanding correct? This should be also helpful when
mapping to TLS1.3 HKDF extract and expand functions.

Thanks
Oleg

On Thu, Oct 10, 2019 at 11:00 AM Eliot Lear  wrote:

> Just one comment:
>
> TEAP does not *require* an inner method, and indeed for IoT it is not
> necessary.
>
> Eliot
>
> On 9 Oct 2019, at 07:46, Joseph Salowey  wrote:
>
> Based on Jouni's analysis the derivation of the S-IMCKs is not well
> specified.  To me it looks like the solution to handle an arbitrary number
> of methods that may or may not make an EMSK available would be fairly
> complex.
>
> I think the most straight forward solution is to either always assume that
> for an EMSK generating method the EMSK is either always available or always
> unavailable.  It seems that it is safer to always assume that the EMSK is
> unavailable and will therefore never be used.  I think this has the
> following implications:
>
> -  The S-IMCK is always generated from the inner method MSK or the
> all-zero value if the method does not generate key material.
> -  The EMSK compound MAC is not used
> -  Implementations must have a policy that accepts MSK MACs
>
> It would appear that from a cryptographic analysis point of view the MSK
> and EMSK are equivalent since these keys will only be used in these
> TEAP calculations and not for other purposes..  There are documented
> drawbacks of using the MSK described in RFC7029 (
> https://tools.ietf.org/html/rfc7029#page-12).   If the properties of the
> EMSK binding are needed in the use cases currently under consideration then
> I think we could redefine the way the EMSK MAC to be computed from the EMSK
> of the inner method changing the above to
>
> -  The S-IMCK is always generated from the inner method MSK or the
> all-zero value if the method does not generate key material.
> - Compute EMSK MAC from the inner-method EMSK instead of the S-IMCK
>  Compound-MAC = MAC( EMSK[J], BUFFER )
> -  Implementations have a policy that prevents EMSK downgrade to MSK
>
> Hopefully there is a more elegant solution. Any ideas?
>
> Joe
> ___
> Emu mailing list
> Emu@ietf.org
> https://www.ietf.org/mailman/listinfo/emu
>
>
> ___
> Emu mailing list
> Emu@ietf.org
> https://www.ietf.org/mailman/listinfo/emu
>
___
Emu mailing list
Emu@ietf.org
https://www.ietf.org/mailman/listinfo/emu


Re: [Emu] TEAP - RFC 7170 - Errata

2020-06-29 Thread Oleg Pekar
Jorge, thanks for correction: Intermediate-Result TLV must be sent at the
end of Basic Password Authentication. I think we also need to list all four
inner method cases that I mention in above explicitly, for example in
"3.3.3.  Protected Termination and Acknowledged Result Indication" section.

Since every Intermediate-Result TLV indicating success must be accompanied
with Crypto-Binding TLV (section "4.2.11.  Intermediate-Result TLV") -
please refer to the Zero-MSK addition discussed in the other thread with
subject "[Emu] draft-dekok-emu-tls-eap-types discussion". Zero-MSK based
Crypto-Binding TLV should be used when we have no inner method or after
Basic Password Authentication exchange. Per Joe's suggestion and Jorge's
confirmation we should not support downgrading from MSK to Zero MSK if the
inner method supports MSK.

Thanks
Oleg

On Fri, May 22, 2020 at 11:56 PM Jorge Vergara 
wrote:

> I am going to consider these 3 errata separately as I think it is clearest:
>
>
>
> Errata ID 5767:
>
>
>
> I believe Jouni’s differentiation of an “EAP authentication method” vs
> “EAP method” is that an “EAP authentication method” has a type >= 4. Jouni
> mentions this in his errata filing and it is also mentioned in RFC 3748
> Section 2.1  (possibly
> other places also). Jouni, I apologize if I have misinterpreted – please
> correct me if so.
>
>
>
> So, an EAP-Identity (type 1) request is an “EAP method”, but there is
> obviously no intermediate result TLV or crypto binding TLV exchanged after
> an EAP-Identity exchange.
>
>
>
> Jouni has done an excellent job in this errata of identifying where the
> terminology should be updated, and I agree with Jouni’s changes. I think
> the intent of the RFC in the four instances Jouni identified was fairly
> clear.
>
>
>
> Errata ID 5844:
>
>
>
> Oleg, your final proposal seems to indicate that an Intermediate-Result
> TLV should NOT be sent after Basic Password Authentication – I’m not sure I
> follow this. Section 3.3.2 outlines that an Intermediate-Result TLV SHOULD
> be sent after Basic Password Authentication.
>
>
>
> So I find this errata valid and agree with Jouni that section C.1 and C.2
> should be updated to include the Intermediate-Result TLV in the diagrams.
>
>
>
> I agree with the rest of the proposals Jouni has made as well that this
> should be made clearer throughout the document. Most of the time where the
> Intermediate-Result TLV is mentioned, only EAP is discussed and the Basic
> Password Authentication is forgotten. I don’t believe Jouni’s proposals
> change any timings – just clarify the language.
>
>
>
> Errata ID 5845:
>
>
>
> I find the errata valid and agree with Jouni. The direction in the rest of
> the document seems to be that an Intermediate-Result TLV is always
> exchanged (after both EAP authentication methods and Basic Password
> Authentications) and the text in 3.3.1 is confusing.
>
>
>
> *
>
>
>
> In summary – I find all of these errata valid and appreciate Jouni’s
> suggestions for clarifications. I find them all to be language
> clarifications and don’t believe any exchanges need to be altered..
>
>
>
> Oleg, your final proposal seems to be removing the exchange of
> Intermediate-Result TLVs in the Basic Password Authentication case – I am
> not sure I follow this but if you believe this is needed for the resolution
> of this errata I would like to better understand why.
>
>
>
> Thanks,
>
> Jorge
>
>
>
> *From:* Emu  *On Behalf Of * Oleg Pekar
> *Sent:* Sunday, May 3, 2020 10:02 AM
> *To:* Jouni Malinen ; EMU WG 
> *Subject:* [Emu] TEAP - RFC 7170 - Errata
>
>
>
> Hi Jouni,
>
> You filed Errata ID: 5767, 5844, 5845 regarding sending of
> Intermediate-Result TLV. Can we clarify a general scheme of
> sending Intermediate-Result TLV and Crypto-Binding TLV in all cases? It is
> not exactly clear what is "EAP authentication method" and what is its
> different from "EAP method" (you referred to appendix C.3 as an example of
> "EAP Method" but it is not clear why it is not an "EAP authentication
> method" - could you please clarify).
>
>
>
> Here's the list of cases with the current RFC instructions (please add if
> something is missing):
>
>
>
> 1. A single inner EAP method is executed inside the tunnel.
>
> *** RFC says ***
>
> Section 4.2.11 "An Intermediate-Result TLV indicating success MUST be
> accompanied by a Crypto-Binding TLV". Section 3.3 "Phase 2 MUST always end
> with a Crypto-Binding TLV exchange"
>
>
>
> 2. Multiple inner EAP methods are executed inside the tunnel.
>
> *** RFC says ***
>
> Send Intermediate-Result TLV if more than one method is going to be
> executed in the tunnel. Send Crypto-Binding TLV if Intermediate-Result
> TLV indicates success.
>
> Section 3.3.1 "If more than one method is going to be executed in the
> tunnel, then upon method completion, the server MUST send an
> Intermediate-Result TLV indicating the result" - wrong
>
> Section 3.3.3 "The Crypto-Bin

Re: [Emu] TEAP - RFC 7170 - Errata ID 5768

2020-06-29 Thread Oleg Pekar
Jorge, Joe, thank you for your comments. So the updated proposal should be:

1) In Section "4.2.13.  Crypto-Binding TLV" make "EMSK Compound MAC" and
"MSK Compound MAC" fields variable length depending on the number of output
bits of the MAC function negotiated by TLS protocol. A field "Compound MAC
Length" of size 1 octet is added after the "Nonce" field to represent the
length of each of "EMSK Compound MAC" and "MSK Compound MAC" fields. The
minimum length for each Compound MAC field is 20 octets (to honor the
original RFC fixed size and to allow usage of SHA-1 - that is still used in
TLS 1.2 - to provide MAC without padding). The length should never be less
than 20 octets since there are no ciphers supported by TLS 1.2 with a
digest of less than 160 bits output size. Therefore if "Compound MAC
Length" field value is less than 20 then the Crypto-binding TLV should be
considered of erroneous format and handled as described in section 3.6.
Error Handling. The value of the length field of the whole Crypto-Binding
TLV should be calculated accordingly (currently fix 76 octets).

The updated structure of Crypto-Binding TLV:

The Crypto-Binding TLV is defined as follows:

0   1   2   3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |M|R| TLV Type  |Length |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |Reserved   |Version|  Received Ver.| Flags|Sub-Type|
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |   |
   ~ Nonce ~
   |   |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   |  Comp MAC Len |

   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   |   |
   ~   EMSK Compound MAC   ~
   |   |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |   |
   ~MSK Compound MAC   ~
   |   |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


If we would not bump the TEAP protocol version - we still can change
the Crypto-Binding TLV version to 2.


2) [This was accepted and stays unchanged]

3) In Section "5.3.  Computing the Compound MAC" when specifying the
list of fields to be placed in the BUFFER" should say "...2  A single
octet contains TEAP EAP method type 0x37". However to let this byte
play a role in protection of Crypto-Binding TLV I would suggest to
place in it the inner EAP method type related to the calculation or
0x0 if no inner EAP method was executed. What are your opinions on
this?

Thanks
Oleg


On Tue, May 26, 2020 at 12:29 PM Jorge Vergara 
wrote:

>
>
> *From:* Joseph Salowey 
> *Sent:* Monday, May 25, 2020 9:27 PM
> *To:* Jorge Vergara 
> *Cc:* Oleg Pekar ; Jouni Malinen ;
> EMU WG 
> *Subject:* Re: [Emu] TEAP - RFC 7170 - Errata ID 5768
>
>
>
>
>
>
>
> On Fri, May 22, 2020 at 1:18 PM Jorge Vergara  40microsoft@dmarc.ietf.org> wrote:
>
> My review of this errata and the current responses from Oleg:
>
>
>
>1. I agree with this proposed resolution. I do think this is an
>important omission that needs to be clarified in the RFC. Otherwise it is
>somewhat guesswork that truncation is the right action. I think the current
>wording leans toward truncation, but I definitely asked this question
>myself while implementing.
>
> [Joe]  Why not just change the TLV to be variable length?  It seems if we
> hardcode the length to 100 we risk having the same problem in the future?
>
>
>
> [Jorge] I have to admit that in my original response my brain skipped over
> the TLV length adjustment and focused only on the truncation. If the TLV
> length is going to be adjusted at all, I am in favor of making the TLV
> variable length. A minimum length could also be added (32 bits seems to be
> the current recommendation). However, I do believe this is a breaking
> change to any TEAP implementation – would the TEAP version need to be
> bumped?
>
>
>1.
>2. This bleeds into Alan’s TLS 1.3 document somewhat, but I agree with
>Jouni that this will need to change when the rest of the document is
>eventually updated to TLS 1.3.. There are enough TLS 1.3 related things to
>address in TEAP that I don’t exactly view this as an errata. I view it as a
>needed update, whether in this document, Alan’s document, or both.
>
> [Joe]  I tend to agree that this is not an errata.  H