lake-wg / edhoc

Ephemeral Diffie-Hellman Over COSE (EDHOC)
Other
7 stars 12 forks source link

Simpler more secure MAC calculation #121

Closed emanjon closed 3 years ago

emanjon commented 3 years ago

COSE has discussed and are are planning to standardize new encryption algorithms for the AEAD interface after a request from FIDO. The new algorithms would not provide any integrity protection.

The current version of the Group OSCORE draft would not be secure at all with these new algorithms due to the unusual design choice to let the sender countersign it's own AEAD. This could easily be addressed by using a traditional MAC-then-Sign approach.

EDHOC currently uses the inner COSE_Encrypt as a MAC function, this would of course be unsecure if the encryption function is AES-CTR.

emanjon commented 3 years ago

Might actually be a very good idea to change the inner COSE_Encrypt0 to just a simple single invocation of the EDHOC-KDF(). As pointed out in [1] the short MAC length is probably the weakest point in the EDHOC design.

[1] https://github.com/lake-wg/edhoc/blob/master/Security%20Level/Security%20Levels%20and%20Design%20Goals%20of%20EDHOC.txt

Chaning inner COSE_Encrypt0 to EDHOC-KDF() would significantly simplify the specification

NEW

Compute MAC_3 = EDHOC-KDF( PRK_4x3m, TH_3, ( ID_CRED_I, CRED_I, ? AD_3 ), length )

OLD

Compute an inner COSE_Encrypt0 as defined in Section 5.3 of {{I-D.ietf-cose-rfc8152bis-struct}}, with the EDHOC AEAD algorithm in the selected cipher suite, K_3m, IV_3m, and the following parameters:

    protected = << ID_CRED_I >>
        ID_CRED_I - identifier to facilitate retrieval of CRED_I, see {{id_cred}}

    external_aad = << TH_3, CRED_I, ? AD_3 >>

        CRED_I - bstr containing the credential of the Initiator, see {{id_cred}}.

        AD_3 = bstr containing opaque protected auxiliary data

    plaintext = h''

COSE constructs the input to the AEAD {{RFC5116}} as follows:

    Key K = EDHOC-KDF( PRK_4x3m, TH_3, "K_3m", length )

    Nonce N = EDHOC-KDF( PRK_4x3m, TH_3, "IV_3m", length )

    Plaintext P = 0x (the empty string)

    Associated data A =

    [ "Encrypt0", << ID_CRED_I >>, << TH_3, CRED_I, ? AD_3 >> ]

MAC_3 is the 'ciphertext' of the inner COSE_Encrypt0.
emanjon commented 3 years ago

"K_2m", "K_3m", "IV_2m", "IV_3m" could then be removed from the specification.

emanjon commented 3 years ago

I think this is a significant simplification that also saves memory. The CPU saving are likely minimal compared to the asymmetric operations. Additional benefits are that the MAC length becomes flexible. We could then define a full MAC length for the Signature modes. I don't see any disadvantages.

I will make a PR during next week.

gselander commented 3 years ago

This is the PR: #123

gselander commented 3 years ago

If we merge this PR we should remove the following sentence from 3.5.1. "The MAC is implemented with an AEAD algorithm."

emanjon commented 3 years ago

Made two new PRs and deleted the old ones. Began implementing #136 with a CBOR sequence. I then modified #136 to #137 where an array is used instead of a sequence.

137 is likely the better choice. I think somebody expressed that it was good if context is explicit. #137 also seems simpler.

emanjon commented 3 years ago

Thinking more maybe CBOR bstr for context is the best

EDHOC-Exporter(label, context, length)
     = EDHOC-KDF(PRK_4x3m, TH_4, label, context, length) 

info = [
   edhoc_aead_id : int / tstr,
   transcript_hash : bstr,
   label : tstr,
   context : bstr,
   length : uint
]

If so, should the context for MAC_2 be ?

bytes .cborseq [ ID_CRED_I, CRED_I, ? AD_3 ]
bytes .cbor [ ID_CRED_I, CRED_I, ? AD_3 ]

I don't think the following definition in the current draft is correct:

Key K = EDHOC-Exporter( "EDHOC_message_4_Key", h'', length )

The exporter takes an byte string and calls the the EDHOC-KDF. The caller of the exporter does not need to do any CBOR encoding.

gselander commented 3 years ago

The last point, empty byte string representation, relates to #133.

emanjon commented 3 years ago

The input to the exporter should likely be a byte string (not a CBOR encoded bstr) The input to the KDF could be something different e.g. a sequence.


EDHOC-Exporter(label, context, length)
     = EDHOC-KDF(PRK_4x3m, TH_4, label, context, length) 

where context is a bstr,

EDHOC-KDF( PRK, transcript_hash, label, context_seq, length )
      = Expand( PRK, info, length )

info = [
   edhoc_aead_id : int / tstr,
   transcript_hash : bstr,
   label : tstr,
   length : uint
   context_seq : * any,
]

MAC_3 = EDHOC-KDF( PRK_4x3m, TH_3, "MAC_3", (ID_CRED_I, CRED_I, ? AD_3 ), mac_length )
emanjon commented 3 years ago

Maybe mac_length and AEAD should be decoupled. They are independent things with different purposes. Maybe the Static DH mac length should be added to the cipher suite instead.

emanjon commented 3 years ago

Comments from Christian during IETF 111 is that sequence is better than array for EDHOC-KDF

emanjon commented 3 years ago

Based on Christian comments I would say that the current plan is to merge #136 with the byte string change above for the exporter:

emanjon commented 3 years ago

I suggest merging #136 as is. That seems like the best and most flexible solution

emanjon commented 3 years ago

We could make context a separate parameter without changing anything else in #136

EDHOC-Exporter(label, context, length) = EDHOC-KDF(PRK_4x3m, TH_4, label, context, length)

Compute MAC_2 = EDHOC-KDF( PRK_3x2m, TH_2, "MAC_2", ( ID_CRED_R, CRED_R, ? AD_2 ), mac_length ) Master Secret = EDHOC-Exporter( "OSCORE Master Secret", , key_length )

emanjon commented 3 years ago

The agreed solution with a sequnece that Christian preferred has been merged. Closing this