Authn v2 #59

bifurcation · 2018-09-10T21:06:48Z

This PR reflects a concrete scheme on top of #58 that implements the authentication framework discussed on the list recently. There are a few major changes:

Defines a GroupState structure that is incorporated into the key schedule and signatures
Adds a Welcome message that provisions a new member with roster and tree information
Removes the Merkle tree over group identities (and related definitions), since all participants now keep the full roster

draft-ietf-mls-protocol.md

beurdouche

Globally I think this goes in the right direction but I have a few concerns:

I am fine with removing GroupAdd for now

I would have preferred to keep the paragraph on TreeKEM
Even though I understand factoring out everything into GroupState structure can be helpful, we should avoid using it for everything (like in the Key Schedule and Handshake messages <> Add) when we only need to use specific information. I think it was better before.

draft-ietf-mls-protocol.md

beurdouche · 2018-09-10T22:20:38Z

draft-ietf-mls-protocol.md

+
+* The `group_id` field is an application-defined identifier for the
+  group.
+* The `size` field represents the total number of key-exchange slots


This doesn't mean anything... does it represent the maximum numbers of participants in the group ?

Actually, this can be eliminated, since it can be derived from the length of the tree vector.

This is still in the text...

beurdouche · 2018-09-10T22:22:13Z

draft-ietf-mls-protocol.md

+  in the group, whether or not these slots are currently occupied.
+* The `epoch` field represents the current version of the group key.
+* The `roster` field contains credentials for the occupied slots in
+  the tree, including the identity and public key for the holder of


What do you mean by "identity and public key for the holder of the slot". Do you mean the current signature and encryption public keys ?
(signature pubkey, which is currently the long term identity key in the rest of the text)

"Public key" == "Signature public key". ("Identity" is whatever opaque string the application assigns) Note that because this state can change, we're not committed to using long-term identity keys.

s/public key/signature public key/ still needs to be implemented, it is unclear otherwise

draft-ietf-mls-protocol.md

suhasHere · 2018-09-11T03:58:15Z

draft-ietf-mls-protocol.md

@@ -281,45 +281,9 @@ A              B              C          Directory            Channel

 Subsequent additions of group members proceed in the same way.  Any
 member of the group can download an InitKey for a new participant
-and broadcast a GroupAdd which the current group can use to update
+and broadcast an Add which the current group can use to update


nit: "an Add message"

suhasHere

I think the overall direction seems good. Have few nits overall , but given we will be discussing the content at the interim , LGTM

draft-ietf-mls-protocol.md

beurdouche

Thanks @bifurcation ! I still think we should NOT use the entire group state everywhere in the key schedule but only what we need to get the cryptographic security properties we want, so I would prefer holding that change for a next iteration... Other than with fixes you marked as resolved, I think this is good to go 👍

draft-ietf-mls-protocol.md

beurdouche

I think you are almost good to go here... : )

draft-ietf-mls-protocol.md

beurdouche · 2018-09-13T04:53:23Z

draft-ietf-mls-protocol.md

+
+* The `group_id` field is an application-defined identifier for the
+  group.
+* The `size` field represents the total number of key-exchange slots


This is still in the text...

draft-ietf-mls-protocol.md

beurdouche · 2018-09-13T04:57:20Z

draft-ietf-mls-protocol.md

+  in the group, whether or not these slots are currently occupied.
+* The `epoch` field represents the current version of the group key.
+* The `roster` field contains credentials for the occupied slots in
+  the tree, including the identity and public key for the holder of


s/public key/signature public key/ still needs to be implemented, it is unclear otherwise

beurdouche · 2018-09-13T05:00:12Z

draft-ietf-mls-protocol.md

@@ -924,16 +882,11 @@ proceeds as shown in the following diagram:
 Update Secret -> HKDF-Extract = Epoch Secret
                     |
                     |
-                     +--> Derive-Secret(., "msg", ID, Epoch, Msg)
+                     +--> Derive-Secret(., "msg", GroupState_n)


If we don't use the _[N] subscript to give the precise version of the secrets, maybe just use GroupState instead of GroupState_n ?

beurdouche · 2018-09-13T05:02:31Z

draft-ietf-mls-protocol.md


-[[ OPEN ISSUE: The Add and Delete operations create a "double-join"
+2. Use the `key_exchange` message to produce an updated GroupState


key_exchange is now called GroupOperation

draft-ietf-mls-protocol.md

raphaelrobert · 2018-09-13T17:11:02Z

draft-ietf-mls-protocol.md

+4. Use that public key to verify the `signature` field in the
+   Handshake message, with the updated GroupState object as input.
+
+5. If the signature fails to verify, discared the updated GroupState


discared -> discard

raphaelrobert · 2018-09-13T17:24:04Z

draft-ietf-mls-protocol.md

+group must take two actions:
+
+1. Send a Welcome message to the new member
+2. Send an Add message to the group (including the new member)


The order is probably wrong, since the Welcome message should only be sent if the Add message succeeded. We should think some more about synchronicity issues that we want to avoid.
Ideally the Add and Welcome message are on atomic operation in which all old and new members are notified at the same time. However since the existing member now waits for the Add to be successful before sending the (rather large) Welcome, there might be edge cases when the Welcome is never sent because the inviting member went offline too early.
For now I think we should keep this as an open issue. Going forward we should consider improvements, such as splitting the Welcome into a light header that gets reliably dispatched at the same time with the Add and a larger message containing the roster and the tree.

Note that there's an open issue for this a few lines further down. I agree that this is a tricky sync to get right. Shoving the Welcome message in the Add message might simplify this, but the cost of sending an O(N) size message to everyone seems steep.

raphaelrobert · 2018-09-13T17:28:28Z

draft-ietf-mls-protocol.md

-* Update the group's identity tree and ratchet tree with the new
-  participant's information
+* Increment the size of the group
+* Verify the signature on the included UserInitKey


Obviously if the verification fails, none of the changes should be applied.

Checkpoint

bb1c658

bifurcation self-assigned this Sep 10, 2018

raphaelrobert requested changes Sep 10, 2018

View reviewed changes

draft-ietf-mls-protocol.md Outdated Show resolved Hide resolved

draft-ietf-mls-protocol.md Outdated Show resolved Hide resolved

The rest of the owl

95f121c

This was referenced Sep 10, 2018

Discuss all the *ART deletion options #7

Closed

Remove GroupInitKey from Handshake; add to GroupAdd #48

Closed

Message Protection #54

Merged

beurdouche reviewed Sep 10, 2018

View reviewed changes

suhasHere reviewed Sep 11, 2018

View reviewed changes

suhasHere approved these changes Sep 11, 2018

View reviewed changes

bifurcation mentioned this pull request Sep 12, 2018

Remove UserAdd #58

Merged

beurdouche requested changes Sep 12, 2018

View reviewed changes

draft-ietf-mls-protocol.md Outdated Show resolved Hide resolved

draft-ietf-mls-protocol.md Show resolved Hide resolved

draft-ietf-mls-protocol.md Show resolved Hide resolved

draft-ietf-mls-protocol.md Outdated Show resolved Hide resolved

Comments from Robert, Beurdouche, Nandakumar

ebdbb4e

beurdouche requested changes Sep 13, 2018

View reviewed changes

raphaelrobert requested changes Sep 13, 2018

View reviewed changes

bifurcation added 4 commits September 18, 2018 18:23

Further simplifications

fca1ba3

Merge branch 'master' into authn-v2

ed24429

Add changelog items

1e80e32

Comments from @beurdouche and @raphaelrobert

1e6c291

raphaelrobert approved these changes Sep 19, 2018

View reviewed changes

beurdouche approved these changes Sep 19, 2018

View reviewed changes

bifurcation merged commit 414b700 into master Sep 19, 2018

bifurcation deleted the authn-v2 branch October 19, 2018 16:57

beurdouche added functionality security labels Dec 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Authn v2 #59

Authn v2 #59

bifurcation commented Sep 10, 2018

beurdouche left a comment

beurdouche Sep 10, 2018

bifurcation Sep 12, 2018

beurdouche Sep 13, 2018

beurdouche Sep 10, 2018

bifurcation Sep 12, 2018

beurdouche Sep 13, 2018

suhasHere Sep 11, 2018

suhasHere left a comment

beurdouche left a comment

beurdouche left a comment

beurdouche Sep 13, 2018

beurdouche Sep 13, 2018

beurdouche Sep 13, 2018

beurdouche Sep 13, 2018

raphaelrobert Sep 13, 2018

raphaelrobert Sep 13, 2018

bifurcation Sep 19, 2018

raphaelrobert Sep 13, 2018


		[[ OPEN ISSUE: The Add and Delete operations create a "double-join"
		2. Use the `key_exchange` message to produce an updated GroupState

Authn v2 #59

Authn v2 #59

Conversation

bifurcation commented Sep 10, 2018

beurdouche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suhasHere left a comment

Choose a reason for hiding this comment

beurdouche left a comment

Choose a reason for hiding this comment

beurdouche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment