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:
>>
>>
>>

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

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

2020-05-26 Thread Jorge Vergara 

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 
mailto: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.  However an update to TEAP 
should address these.


  1.
  2.  Agree with Jouni that I don’t see the point of the 0x37 octet, but 
regardless this clarification of how it is encoded is positive (minor) change.
[Joe] I think the original reason to include the TEAP method ID in the 
specification was to make sure that we differentiated between similar crypto 
binding implementations in other protocols such as EAP-FAST.   I don't think 
there is much ambiguity here, but I am OK with including 0x37 in the 
description.


Thanks,
Jorge

From: Emu mailto:emu-boun...@ietf.org>> On Behalf Of Oleg 
Pekar
Sent: Tuesday, May 5, 2020 6:27 AM
To: Jouni Malinen mailto:j...@w1.fi>>; EMU WG 
mailto:emu@ietf.org>>
Subject: [Emu] TEAP - RFC 7170 - Errata ID 5768

Hi Jouni,
I propose the following fix for the issues described in this errata id:
1) In Section "4.2.13.  Crypto-Binding TLV" make "EMSK Compound MAC" and "MSK 
Compound MAC" fields 32 octets long (currently 20 octets). The MAC value is 
truncated at 32 octets if it is longer than 32 octets or padded to a length of 
32 octets with zeros to the right if it is less than 32 octets. The length of 
the TLV should be changed to 100 bytes (currently 76).

The motivation is to keep collision-resistance strength of MAC on 128 bit. Hash 
value truncation is described in "NIST Special Publication 800-107 Revision 1: 
Recommendation for Applications Using Approved Hash 
Algorithms"<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnvlpubs.nist.gov%2Fnistpubs%2FLegacy%2FSP%2Fnistspecialpublication800-107r1.pdf=02%7C01%7Cjovergar%40microsoft.com%7C6102b418ffd64be20f0b08d8012d1ade%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637260640995993815=MIOfT9tSEHTmbuP2oTjAxlgJdBOke3V4uAqQq7YDzN0%3D=0>

2) In Section "5.3.  Computing the Compound MAC" specify that "MAC is the MAC 
function negotiated in TLS of TEAP Phase 1" (currently it says TLS 1.2)

The motivation is to support TLS 1.2, 1.3 and possibly later TLS versions.

3) In Section "5.3.  Computing the Compound MAC" when specifying the list of 
field to be placed in the BUFFER" should say "...2  A single octet contains 
TEAP EAP method type 0x37". Alternatively it could be "...2  A single octet 
contains EAP Type of the inner EAP method related to the calculation or 0 if no 
inner EAP method was executed" (currently "...2  The EAP Type sent by the other 
party in the first TEAP message")

Please note that there's still a discussion on sending Crypto-Binding TLV on 
"Authentication inner EAP method" or "Inner EAP method that exports MSK" only.

Thanks
Oleg
___
Emu mailing list
Emu@ietf.org<mailto:Emu@ietf.org>
https://www.ietf.org/mailman/listinfo/emu<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ietf.org%2Fmailman%2Flistinfo%2Femu=02%7C01%7Cjovergar%40microsoft.com%7C6102b418ffd64be20f0b08d8012d1ade%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637260640996003807=kBRkWM2GADS2iZP%2F6A9Ygr61JO6hcEe8cNK2Z6p9Pg0%3D=0>
___
Emu mailing list
Emu@ietf.org
https://www.ietf.org/mailman/listinfo/emu

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

2020-05-25 Thread Joseph Salowey 
On Fri, May 22, 2020 at 1:18 PM Jorge Vergara  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?


>
>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.  However an update to
TEAP should address these.


>
>1.
>2. Agree with Jouni that I don’t see the point of the 0x37 octet, but
>regardless this clarification of how it is encoded is positive (minor)
>change.
>
> [Joe] I think the original reason to include the TEAP method ID in the
specification was to make sure that we differentiated between similar
crypto binding implementations in other protocols such as EAP-FAST.   I
don't think there is much ambiguity here, but I am OK with including 0x37
in the description.


>
>
> Thanks,
>
> Jorge
>
>
>
> *From:* Emu  *On Behalf Of * Oleg Pekar
> *Sent:* Tuesday, May 5, 2020 6:27 AM
> *To:* Jouni Malinen ; EMU WG 
> *Subject:* [Emu] TEAP - RFC 7170 - Errata ID 5768
>
>
>
> Hi Jouni,
>
> I propose the following fix for the issues described in this errata id:
>
> 1) In Section "4.2.13.  Crypto-Binding TLV" make "EMSK Compound MAC" and
> "MSK Compound MAC" fields 32 octets long (currently 20 octets). The MAC
> value is truncated at 32 octets if it is longer than 32 octets or padded to
> a length of 32 octets with zeros to the right if it is less than 32 octets.
> The length of the TLV should be changed to 100 bytes (currently 76).
>
>
>
> The motivation is to keep collision-resistance strength of MAC on 128 bit..
> Hash value truncation is described in "NIST Special Publication 800-107
> Revision 1: Recommendation for Applications Using Approved Hash Algorithms"
> <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnvlpubs.nist.gov%2Fnistpubs%2FLegacy%2FSP%2Fnistspecialpublication800-107r1.pdf=02%7C01%7Cjovergar%40microsoft.com%7C42c90b35b93f4261402008d7f0f817c4%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637242821105169728=MS6AaYWPCm377ZvleMjVIfLCkaCaq6E24NQXSYK%2FNls%3D=0>
>
>
>
>
> 2) In Section "5.3.  Computing the Compound MAC" specify that "MAC is the
> MAC function negotiated in TLS of TEAP Phase 1" (currently it says TLS
> 1.2)
>
>
>
> The motivation is to support TLS 1.2, 1.3 and possibly later TLS versions..
>
>
>
> 3) In Section "5.3.  Computing the Compound MAC" when specifying the list
> of field to be placed in the BUFFER" should say "...2  A single octet
> contains TEAP EAP method type 0x37". Alternatively it could be "...2  A
> single octet contains EAP Type of the inner EAP method related to the
> calculation or 0 if no inner EAP method was executed" (currently "...2  The
> EAP Type sent by the other party in the first TEAP message")
>
>
>
> Please note that there's still a discussion on sending Crypto-Binding TLV
> on "Authentication inner EAP method" or "Inner EAP method that exports MSK"
> only.
>
>
>
> Thanks
>
> Oleg
> ___
> 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 ID 5768

2020-05-22 Thread Jorge Vergara 
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.
  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.
  3.  Agree with Jouni that I don’t see the point of the 0x37 octet, but 
regardless this clarification of how it is encoded is positive (minor) change.

Thanks,
Jorge

From: Emu  On Behalf Of Oleg Pekar
Sent: Tuesday, May 5, 2020 6:27 AM
To: Jouni Malinen ; EMU WG 
Subject: [Emu] TEAP - RFC 7170 - Errata ID 5768

Hi Jouni,
I propose the following fix for the issues described in this errata id:
1) In Section "4.2.13.  Crypto-Binding TLV" make "EMSK Compound MAC" and "MSK 
Compound MAC" fields 32 octets long (currently 20 octets). The MAC value is 
truncated at 32 octets if it is longer than 32 octets or padded to a length of 
32 octets with zeros to the right if it is less than 32 octets. The length of 
the TLV should be changed to 100 bytes (currently 76).

The motivation is to keep collision-resistance strength of MAC on 128 bit. Hash 
value truncation is described in "NIST Special Publication 800-107 Revision 1: 
Recommendation for Applications Using Approved Hash 
Algorithms"<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnvlpubs.nist.gov%2Fnistpubs%2FLegacy%2FSP%2Fnistspecialpublication800-107r1.pdf=02%7C01%7Cjovergar%40microsoft.com%7C42c90b35b93f4261402008d7f0f817c4%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637242821105169728=MS6AaYWPCm377ZvleMjVIfLCkaCaq6E24NQXSYK%2FNls%3D=0>

2) In Section "5.3.  Computing the Compound MAC" specify that "MAC is the MAC 
function negotiated in TLS of TEAP Phase 1" (currently it says TLS 1.2)

The motivation is to support TLS 1.2, 1.3 and possibly later TLS versions.

3) In Section "5.3.  Computing the Compound MAC" when specifying the list of 
field to be placed in the BUFFER" should say "...2  A single octet contains 
TEAP EAP method type 0x37". Alternatively it could be "...2  A single octet 
contains EAP Type of the inner EAP method related to the calculation or 0 if no 
inner EAP method was executed" (currently "...2  The EAP Type sent by the other 
party in the first TEAP message")

Please note that there's still a discussion on sending Crypto-Binding TLV on 
"Authentication inner EAP method" or "Inner EAP method that exports MSK" only.

Thanks
Oleg
___
Emu mailing list
Emu@ietf.org
https://www.ietf.org/mailman/listinfo/emu

[Emu] TEAP - RFC 7170 - Errata ID 5768

2020-05-05 Thread Oleg Pekar 
Hi Jouni,
I propose the following fix for the issues described in this errata id:
1) In Section "4.2.13.  Crypto-Binding TLV" make "EMSK Compound MAC" and
"MSK Compound MAC" fields 32 octets long (currently 20 octets). The MAC
value is truncated at 32 octets if it is longer than 32 octets or padded to
a length of 32 octets with zeros to the right if it is less than 32 octets.
The length of the TLV should be changed to 100 bytes (currently 76).

The motivation is to keep collision-resistance strength of MAC on 128 bit.
Hash value truncation is described in "NIST Special Publication 800-107
Revision 1: Recommendation for Applications Using Approved Hash Algorithms"



2) In Section "5.3.  Computing the Compound MAC" specify that "MAC is the
MAC function negotiated in TLS of TEAP Phase 1" (currently it says TLS 1.2)

The motivation is to support TLS 1.2, 1.3 and possibly later TLS versions.

3) In Section "5.3.  Computing the Compound MAC" when specifying the list
of field to be placed in the BUFFER" should say "...2  A single octet
contains TEAP EAP method type 0x37". Alternatively it could be "...2  A
single octet contains EAP Type of the inner EAP method related to the
calculation or 0 if no inner EAP method was executed" (currently "...2  The
EAP Type sent by the other party in the first TEAP message")

Please note that there's still a discussion on sending Crypto-Binding TLV
on "Authentication inner EAP method" or "Inner EAP method that exports MSK"
only.

Thanks
Oleg
___
Emu mailing list
Emu@ietf.org
https://www.ietf.org/mailman/listinfo/emu