From 9a9809dad0cb4c0d24257bc1bac045ebe282b33b Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Mon, 16 May 2022 11:37:54 +0200 Subject: [PATCH 01/17] Clarifiying proposal validation; first draft --- draft-ietf-mls-protocol.md | 166 ++++++++++++++++++++++++------------- 1 file changed, 107 insertions(+), 59 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 99fa5c4b..7b47fdec 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -3256,8 +3256,7 @@ retrieved by hash (as a ProposalOrRef object) in a later Commit message. ### Add An Add proposal requests that a client with a specified KeyPackage be added -to the group. The proposer of the Add MUST verify the validity of the -KeyPackage, as specified in {{keypackage-validation}}. +to the group. ~~~ tls struct { @@ -3265,6 +3264,23 @@ struct { } Add; ~~~ +An Add proposal is invalid if any of the following is true: + +* The KeyPackage is invalid according to {{key-package-validation}}. + +* The `leaf_node_source` of the LeafNode in the KeyPackage is not set to + `key_package`. + +* The Credential in the LeafNode in the KeyPackage shares the same signature + key with a Credential in any leaf of the group. + +* The LeafNode in the KeyPackage shares the same `encryption_key` with another + LeafNode in the group. + +* The Credential in the KeyPackage represents a client already in the + group according to the application (for example, identical KeyPackages or + KeyPackages sharing the same Credential). + An Add is applied after being included in a Commit message. The position of the Add in the list of proposals determines the leaf node where the new member will be added. For the first Add in the Commit, the corresponding new member will be @@ -3272,9 +3288,6 @@ placed in the leftmost empty leaf in the tree, for the second Add, the next empty leaf to the right, etc. If no empty leaf exists, the tree is extended to the right. -* Validate the KeyPackage as specified in {{keypackage-validation}}. The - `leaf_node_source` field in the LeafNode MUST be set to `key_package`. - * Identify the leaf L for the new member: if there are empty leaves in the tree, L is the leftmost empty leaf. Otherwise, the tree is extended to the right by one leaf node and L is the new leaf. @@ -3297,13 +3310,18 @@ struct { } Update; ~~~ -A member of the group applies an Update message by taking the following steps: +An Update proposal is invalid if any of the following is true: + +* The LeafNode is invalid according to {{leaf-node-validation}}. + +* The `leaf_node_source` of the LeafNode is not set to `update`. + +* The Credential in the LeafNode shares the same signature key with a Credential + in any leaf of the group. -* Validate the LeafNode as specified in {{leaf-node-validation}}. The - `leaf_node_source` field MUST be set to `update`. +* The LeafNode shares the same `encryption_key` with another LeafNode in the group. -* Verify that the `encryption_key` value in the LeafNode is different from the - corresponding field in the LeafNode being replaced. +A member of the group applies an Update message by taking the following steps: * Replace the sender's LeafNode with the one contained in the Update proposal @@ -3320,6 +3338,9 @@ struct { } Remove; ~~~ +A Remove proposal is invalid if the `removed` field does not identify any non-blank +leaf node. + A member of the group applies a Remove message by taking the following steps: * Identify a leaf node matching `removed`. This lookup MUST be done on the tree @@ -3345,9 +3366,14 @@ struct { } PreSharedKey; ~~~ -The `psktype` of the pre-shared key MUST be `external` and the `psk_nonce` MUST -be a randomly sampled nonce of length `KDF.Nh`. When processing a Commit message -that includes one or more PreSharedKey proposals, group members derive +A PreSharedKey proposal is invalid if any of the following is true: + +* The `psktype` of the pre-shared key is not set to `external`. + +* The `psk_nonce` is not of length `KDF.Nh`. + +The `psk_nonce` MUST be randomly sampled. When processing +a Commit message that includes one or more PreSharedKey proposals, group members derive `psk_secret` as described in {{pre-shared-keys}}, where the order of the PSKs corresponds to the order of the `PreSharedKey` proposals in the Commit. @@ -3371,11 +3397,8 @@ A member of the group applies a ReInit proposal by waiting for the committer to send the Welcome message that matches the ReInit, according to the criteria in {{reinitialization}}. -If a ReInit proposal is included in a Commit, it MUST be the only proposal -referenced by the Commit. If other non-ReInit proposals have been sent during -the epoch, the committer SHOULD prefer them over the ReInit proposal, allowing -the ReInit to be resent and applied in a subsequent epoch. The `version` field -in the ReInit proposal MUST be no less than the version for the current group. +A ReInit proposal is invalid if the `version` field if greater than the version +for the current group. ### ExternalInit @@ -3459,16 +3482,17 @@ struct { } GroupContextExtensions; ``` +A GroupContextExtensions proposal is invalid if it includes a +`required_capabilities` extension and some members of the group do not support +some of the required capabilities (including those added in the same commit, +and excluding those removed). + A member of the group applies a GroupContextExtensions proposal with the following steps: -* If the new extensions include a `required_capabilities` extension, verify that - all members of the group support the required capabilities (including those - added in the same commit, and excluding those removed). - * Remove all of the existing extensions from the GroupContext object for the group and replacing them with the list of extensions in the proposal. (This - is a wholesale replacement, not a merge. An extension is only carried over if + is a wholesale replacement, not a merge. An extension is only carried over if the sender of the proposal includes it in the new list.) Note that once the GroupContext is updated, its inclusion in the @@ -3551,38 +3575,25 @@ a Commit message before sending application data. This ensures, for example, that any members whose removal was proposed during the epoch are actually removed before any application data is transmitted. -The sender of a Commit MUST include all valid proposals that it has received -during the current epoch. Invalid proposals include, for example, proposals with -an invalid signature or proposals that are semantically invalid, such as an Add -when the sender does not have the application-level permission to add new users. -Proposals with a non-default proposal type MUST NOT be included in a commit -unless the proposal type is supported by all the members of the group that will -process the Commit (i.e., not including any members being added or removed by -the Commit). +A sender and a receiver of a Commit MUST verify that the committed list of +proposals is valid as specified in {{validating proposals}}. A list is invalid if, for example, +it includes a proposal with an invalid signature, a proposal sent within a different epoch. +or an Add when the sender does not have the application-level permission to add new users. -If there are multiple proposals that apply to the same leaf, or multiple -PreSharedKey proposals that reference the same PreSharedKeyID, the committer -MUST choose one and include only that one in the Commit, considering the rest -invalid. The committer MUST NOT include any Update proposals generated by the -committer, since they would be duplicative with the `path` field in the Commit. -The committer MUST prefer any Remove received, or the most recent Update for -the leaf if there are no Removes. If there are multiple Add proposals -containing KeyPackages that the committer considers to represent the same -client or a client already in the group (for example, identical KeyPackages or -KeyPackages sharing the same Credential), the committer again chooses one to -include and considers the rest invalid. The committer MUST consider invalid any -Add or Update proposal if the Credential in the contained KeyPackage shares the -same signature key with a Credential in any leaf of the group, or if the -LeafNode in the KeyPackage shares the same `encryption_key` with another LeafNode in -the group. +The sender of a Commit SHOULD include all valid proposals that it has received +during the current epoch, as long as this does not make the proposal list invalid. -The Commit MUST NOT combine proposals sent within different epochs. Due to the -asynchronous nature of proposals, receivers of a Commit SHOULD NOT enforce that -all valid proposals sent within the current epoch are referenced by the next +Due to the asynchronous nature of proposals, receivers of a Commit SHOULD NOT enforce +that all valid proposals sent within the current epoch are referenced by the next Commit. In the event that a valid proposal is omitted from the next Commit, and that proposal is still valid in the current epoch, the sender of the proposal MAY resend it after updating it to reflect the current epoch. +Proposals with a non-default proposal type MUST NOT be included in a commit +unless the proposal type is supported by all the members of the group that will +process the Commit (i.e., not including any members being added or removed by +the Commit). + A member of the group MAY send a Commit that references no proposals at all, which would thus have an empty `proposals` vector. Such a Commit resets the sender's leaf and the nodes along its direct path, and @@ -3935,16 +3946,6 @@ has to meet a specific set of requirements: leaf node. * The Commit MUST NOT include any proposals by reference, since an external joiner cannot determine the validity of proposals sent within the group -* The proposals included by value in an External Commit MUST meet the following - conditions: - * There MUST be a single ExternalInit proposal. - * There MAY be a single Remove proposal, where the LeafNode in the `path` - field MUST meet the same criteria as the LeafNode in an Update for the - removed leaf (see {{update}}). In particular, the `credential` in the - LeafNode MUST present a set of identifiers that is acceptable to the - application for the removed participant. - * There MAY be one or more PreSharedKey proposals. - * There MUST NOT be any other proposals. * External Commits MUST be signed by the new member. In particular, the signature on the enclosing MLSPlaintext MUST verify using the public key for the credential in the `leaf_node` of the `path` field. @@ -4213,6 +4214,53 @@ Regardless of how the client obtains the tree, the client MUST verify that the root hash of the ratchet tree matches the `tree_hash` of the GroupContext before using the tree for MLS operations. +### Validating Proposals + +A group member creating a commit and a group member processing a commit +MUST verify that the list of committed proposals is valid using one of the following +procedures, depending on whether the commit is external or not. + +For an regular, i.e. not external commit, the list is invalid if it contains any of +the following: + +* An individual proposal that is invalid as specified in its subsection of {{proposals}}. + +* An Update proposal generated by the committer. + +* Multiple Update or Remove proposals that apply to the same leaf. If the committer + has received multiple such proposals they SHOULD prefer any Remove received, or + the most recent Update if there are no Removes. + +* Multiple Add proposals that contain KeyPackages that represent the same client + according to the application (for example, identical KeyPackages or KeyPackages + sharing the same Credential). + +* Multiple PreSharedKey proposals that reference the same PreSharedKeyID. + +* Multiple GroupContextExtensions proposals. + +* A ReInit proposal together with any other proposal. If the committer has + received other proposals during the epoch, they SHOULD prefer them over the + ReInit proposal, allowing the ReInit to be resent and applied in a subsequent + epoch. + +An application may extend the above procedure by additional rules, for example, +requiring application-level permissions to add members, or rules concerning +non-default proposal types. + +For an external commit, the list is valid if: + +* It contains only ExternalInit, Remove and PreSharedKey proposals. + +* It contains exactly one ExternalInit proposal. + +* It contains at most one Remove proposal, with which the joiner removes an + old version of themselves. The LeafNode in the `path` field of the external + commit that contains the proposal MUST meet the same criteria as the LeafNode + in an Update for the removed leaf (see {{update}}). In particular, the `credential` + in the LeafNode MUST present a set of identifiers that is acceptable to the + application for the removed participant. + # Extensibility The base MLS protocol can be extended in a few ways. New ciphersuites can be From 290a62beafbf401359a0d7e7c554c99afd58a217 Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Mon, 16 May 2022 12:00:53 +0200 Subject: [PATCH 02/17] Clarifiying proposal validation; fixed typo --- draft-ietf-mls-protocol.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 7b47fdec..a15f63ab 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -3266,7 +3266,7 @@ struct { An Add proposal is invalid if any of the following is true: -* The KeyPackage is invalid according to {{key-package-validation}}. +* The KeyPackage is invalid according to {{keypackage-validation}}. * The `leaf_node_source` of the LeafNode in the KeyPackage is not set to `key_package`. From c5ff57ffdb6d1b632959f08dfd58bd1d20ed56d4 Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Mon, 16 May 2022 12:02:11 +0200 Subject: [PATCH 03/17] Clarifiying proposal validation; fixed typo --- draft-ietf-mls-protocol.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index a15f63ab..320dd580 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -3576,7 +3576,7 @@ that any members whose removal was proposed during the epoch are actually removed before any application data is transmitted. A sender and a receiver of a Commit MUST verify that the committed list of -proposals is valid as specified in {{validating proposals}}. A list is invalid if, for example, +proposals is valid as specified in {{validating-proposals}}. A list is invalid if, for example, it includes a proposal with an invalid signature, a proposal sent within a different epoch. or an Add when the sender does not have the application-level permission to add new users. From c51849bddf56ee5aec92e84a419929b7be3efb03 Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Mon, 16 May 2022 12:20:35 +0200 Subject: [PATCH 04/17] Clarifiying proposal validation; added epoch check --- draft-ietf-mls-protocol.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 320dd580..7e64a1c0 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -3249,9 +3249,10 @@ struct { ~~~ On receiving an MLSMessageContent containing a Proposal, a client MUST verify the -signature inside MLSMessageAuth. If the signature verifies -successfully, then the Proposal should be cached in such a way that it can be -retrieved by hash (as a ProposalOrRef object) in a later Commit message. +signature inside MLSMessageAuth and that the `epoch` field of the enclosing +MLSMessageContent is equal to the `epoch` field of the current GroupContext object. +If the verification is successful, then the Proposal should be cached in such a way +that it can be retrieved by hash (as a ProposalOrRef object) in a later Commit message. ### Add @@ -3683,6 +3684,9 @@ their associated GroupContexts are used: A member of the group creates a Commit message and the corresponding Welcome message at the same time, by taking the following steps: +* Verify that the committed list of proposals is valid as specified in + {{validating-proposals}}. + * Construct an initial Commit object with the `proposals` field populated from Proposals received during the current epoch, and an empty `path` field. @@ -3787,8 +3791,9 @@ A member of the group applies a Commit message by taking the following steps: * Verify that the signature on the MLSMessageContent message as described in Section {{content-authentication}}. -* Verify that all PreSharedKey proposals in the `proposals` vector have unique - PreSharedKeyIDs and are available. +* Verify that the `proposals` vector is valid as specified in {{validating-proposals}}. + +* Verify that all PreSharedKey proposals in the `proposals` vector are available. * Generate the provisional ratchet tree and GroupContext by applying the proposals referenced in the initial Commit object, as described in {{proposals}}. Update From f13801ee533a31608c3589bf9e796e3ecb7c3606 Mon Sep 17 00:00:00 2001 From: optix Date: Mon, 16 May 2022 14:06:42 +0200 Subject: [PATCH 05/17] Fixed some typos. --- draft-ietf-mls-protocol.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 7e64a1c0..81ef6610 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -4225,14 +4225,14 @@ A group member creating a commit and a group member processing a commit MUST verify that the list of committed proposals is valid using one of the following procedures, depending on whether the commit is external or not. -For an regular, i.e. not external commit, the list is invalid if it contains any of -the following: +For a regular, i.e. not external, commit the list is invalid if it contains any of the +following: -* An individual proposal that is invalid as specified in its subsection of {{proposals}}. +* An individual proposal that is invalid as specified in {{proposals}}. * An Update proposal generated by the committer. -* Multiple Update or Remove proposals that apply to the same leaf. If the committer +* Multiple Update and/or Remove proposals that apply to the same leaf. If the committer has received multiple such proposals they SHOULD prefer any Remove received, or the most recent Update if there are no Removes. @@ -4249,6 +4249,8 @@ the following: ReInit proposal, allowing the ReInit to be resent and applied in a subsequent epoch. +* An ExternalInit proposal. + An application may extend the above procedure by additional rules, for example, requiring application-level permissions to add members, or rules concerning non-default proposal types. @@ -4260,8 +4262,8 @@ For an external commit, the list is valid if: * It contains exactly one ExternalInit proposal. * It contains at most one Remove proposal, with which the joiner removes an - old version of themselves. The LeafNode in the `path` field of the external - commit that contains the proposal MUST meet the same criteria as the LeafNode + old version of themselves. If a Remove proposal is present then, the LeafNode in the + `path` field of the external commit MUST meet the same criteria as would the LeafNode in an Update for the removed leaf (see {{update}}). In particular, the `credential` in the LeafNode MUST present a set of identifiers that is acceptable to the application for the removed participant. From e3319ba70d3ef2b28933c02be7768b6d7b73e9f6 Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Mon, 16 May 2022 14:11:32 +0200 Subject: [PATCH 06/17] Fixed typo --- draft-ietf-mls-protocol.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 81ef6610..4352cc86 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -4219,7 +4219,7 @@ Regardless of how the client obtains the tree, the client MUST verify that the root hash of the ratchet tree matches the `tree_hash` of the GroupContext before using the tree for MLS operations. -### Validating Proposals +## Validating Proposals A group member creating a commit and a group member processing a commit MUST verify that the list of committed proposals is valid using one of the following From 8e7acfeabff0c39ab688d8351720a7d5f6ba2d71 Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Mon, 16 May 2022 14:13:29 +0200 Subject: [PATCH 07/17] Fixed typo --- draft-ietf-mls-protocol.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 4352cc86..1a395a6b 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -4225,7 +4225,7 @@ A group member creating a commit and a group member processing a commit MUST verify that the list of committed proposals is valid using one of the following procedures, depending on whether the commit is external or not. -For a regular, i.e. not external, commit the list is invalid if it contains any of the +For a regular, i.e. not external, commit the list is invalid if it contains any of the following: * An individual proposal that is invalid as specified in {{proposals}}. From 40afb1b6ffd95d6f5e9d96b9a6410058654900b6 Mon Sep 17 00:00:00 2001 From: optix Date: Mon, 16 May 2022 15:05:17 +0200 Subject: [PATCH 08/17] re-org validation of individual proposals --- draft-ietf-mls-protocol.md | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 81ef6610..899c8d60 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -3267,20 +3267,16 @@ struct { An Add proposal is invalid if any of the following is true: -* The KeyPackage is invalid according to {{keypackage-validation}}. +* The KeyPackage is invalid for an Add proposal according to {{keypackage-validation}}. -* The `leaf_node_source` of the LeafNode in the KeyPackage is not set to - `key_package`. - -* The Credential in the LeafNode in the KeyPackage shares the same signature - key with a Credential in any leaf of the group. +* The Credential in the LeafNode in the KeyPackage has the same signature + key as a Credential in any leaf of the group. * The LeafNode in the KeyPackage shares the same `encryption_key` with another LeafNode in the group. * The Credential in the KeyPackage represents a client already in the - group according to the application (for example, identical KeyPackages or - KeyPackages sharing the same Credential). + group according to the application. An Add is applied after being included in a Commit message. The position of the Add in the list of proposals determines the leaf node where the new member will @@ -3313,9 +3309,7 @@ struct { An Update proposal is invalid if any of the following is true: -* The LeafNode is invalid according to {{leaf-node-validation}}. - -* The `leaf_node_source` of the LeafNode is not set to `update`. +* The LeafNode is invalid for an Update proposal according to {{leaf-node-validation}}. * The Credential in the LeafNode shares the same signature key with a Credential in any leaf of the group. @@ -3339,8 +3333,8 @@ struct { } Remove; ~~~ -A Remove proposal is invalid if the `removed` field does not identify any non-blank -leaf node. +A Remove proposal is invalid if the `removed` field does not identify a non-blank leaf +node. A member of the group applies a Remove message by taking the following steps: @@ -3369,14 +3363,15 @@ struct { A PreSharedKey proposal is invalid if any of the following is true: -* The `psktype` of the pre-shared key is not set to `external`. +* The `psktype` in the PreSharedKeyID struct is not set to `external`. * The `psk_nonce` is not of length `KDF.Nh`. The `psk_nonce` MUST be randomly sampled. When processing -a Commit message that includes one or more PreSharedKey proposals, group members derive -`psk_secret` as described in {{pre-shared-keys}}, where the order of the PSKs -corresponds to the order of the `PreSharedKey` proposals in the Commit. +a Commit message that includes one or more PreSharedKey proposals, group +members derive `psk_secret` as described in {{pre-shared-keys}}, where the +order of the PSKs corresponds to the order of the `PreSharedKey` proposals +in the Commit. ### ReInit @@ -3394,12 +3389,13 @@ struct { } ReInit; ~~~ +A ReInit proposal is invalid if the `version` field is less than the version +for the current group. + A member of the group applies a ReInit proposal by waiting for the committer to send the Welcome message that matches the ReInit, according to the criteria in {{reinitialization}}. -A ReInit proposal is invalid if the `version` field if greater than the version -for the current group. ### ExternalInit From 1ac8e8c0df7552cc38dbd0cfed2172a4f517d185 Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Mon, 16 May 2022 15:47:39 +0200 Subject: [PATCH 09/17] Improved leaf node validation --- draft-ietf-mls-protocol.md | 61 +++++++++++++++----------------------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index edb20e5e..9ea0a614 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -1892,16 +1892,8 @@ case of a newcomer joining the group. ## Leaf Node Validation -The validity of a LeafNode needs to be verified at a few stages: - -* When a LeafNode is downloaded in a KeyPackage, before it is used - to add the client to the group -* When a LeafNode is received by a group member in an Add, Update, or Commit - message -* When a client joining a group receives LeafNode objects for the other members - of the group in the group's ratchet tree - -The client verifies the validity of a LeafNode using the following steps: +To verify that a LeafNode is valid regardless of its use, the client starts +with the following steps: * Verify that the credential in the LeafNode is valid according to the authentication service and the client's local policy. These actions MUST be @@ -1916,40 +1908,35 @@ The client verifies the validity of a LeafNode using the following steps: * Verify that the LeafNode is compatible with the group's parameters. If the GroupContext has a `required_capabilities` extension, then the required - extensions, proposals, and credential types MUST be listed in the LeafNode's `capabilities` - field. + extensions, proposals, and credential types MUST be listed in the LeafNode's + `capabilities` field. * Verify that the credential type is supported by all members of the group, as specified by the `capabilities` field of each member's LeafNode, and that the `capabilities` field of this LeafNode indicates support for all the credential types currently in use by other members. -* Verify the `lifetime` field: - * When validating a LeafNode in a KeyPackage before sending an Add proposal, - the current time MUST be within the `lifetime` range. A KeyPackage - containing a LeafNode that is expired or not yet valid MUST NOT be sent in - an Add proposal. - * When receiving an Add or validating a tree, checking the `lifetime` is - RECOMMENDED, if it is feasible in a given application context. Because of - the asynchronous nature of MLS, the `lifetime` may have been valid when the - leaf node was proposed for addition, even if it is expired at these later - points in the protocol. - -* Verify that the `leaf_node_source` field has the appropriate value for the - context in which the LeafNode is being validated (as defined in - {{leaf-node-contents}}). - -* Verify that the following fields in the LeafNode are unique among the - members of the group (including any other members added in the same - Commit): - - * `encryption_key` - * `signature_key` - * Verify that the extensions in the leaf node are supported. The ID for each extension in the `extensions` field MUST be listed in the field `capabilities.extensions` of the LeafNode. +* Verify the `lifetime` field: + * If the LeafNode appears in a message being sent by the client, e.g., a + proposal or a commit, then the client MUST verify that the current time is within + the range of the `lifetime` field. + * If instead the LeafNode appears in a message being received by the client, e.g., + a proposal, a commit, or a ratchet tree of the group the client is joining, it is + RECOMMENDED that the client verifies that the current time is within the range + of the `lifetime` field. + +* Verify the `leaf_node_source` field: + * If the LeafNode appears in a KeyPackage in an Add proposal, verify that + `leaf_node_source` is set to `key_package`. + * If the LeafNode appears in an Update proposal, verify that `leaf_node_source` + is set to `update`. + * If the LeafNode appears in the `leaf_node` value of the UpdatePath in + a Commit, verify that `leaf_node_source` is set to `commit`. + ## Ratchet Tree Evolution In order to provide forward secrecy and post-compromise security, whenever a @@ -3046,8 +3033,8 @@ The client verifies the validity of a KeyPackage using the following steps: * Verify that the ciphersuite and protocol version of the KeyPackage match those in use in the group. -* Verify the `leaf_node` field of the KeyPackage according to the process - defined in {{leaf-node-validation}}. +* Verify the `leaf_node` of the KeyPackage is valid for an Add proposal + according to {{leaf-node-validation}}. * Verify that the signature on the KeyPackage is valid using the public key in `leaf_node.credential`. @@ -3267,7 +3254,7 @@ struct { An Add proposal is invalid if any of the following is true: -* The KeyPackage is invalid for an Add proposal according to {{keypackage-validation}}. +* The KeyPackage is invalid according to {{keypackage-validation}}. * The Credential in the LeafNode in the KeyPackage has the same signature key as a Credential in any leaf of the group. From 22f6126e813e8288c5b5176fe1bd83f1faaa6e86 Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Tue, 17 May 2022 08:06:23 +0200 Subject: [PATCH 10/17] Fixed nits --- draft-ietf-mls-protocol.md | 51 +++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 9ea0a614..74c44901 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -1892,6 +1892,15 @@ case of a newcomer joining the group. ## Leaf Node Validation +The validity of a LeafNode needs to be verified at a few stages: + +* When a LeafNode is downloaded in a KeyPackage, before it is used + to add the client to the group +* When a LeafNode is received by a group member in an Add, Update, or Commit + message +* When a client joining a group receives LeafNode objects for the other members + of the group in the group's ratchet tree + To verify that a LeafNode is valid regardless of its use, the client starts with the following steps: @@ -1902,9 +1911,12 @@ with the following steps: another LeafNode, the authentication service MUST additionally validate that the set of identities attested by the credential in the new LeafNode is acceptable relative to the identities attested by the old credential. + E.g. sending an Update proposal updates the sender's old LeafNode to a new + one, or joining via external commit updates the joiner's old LeafNode + included in the committed Remove proposal to a new one. -* Verify that the signature on the LeafNode is valid using the public key - in the LeafNode's credential +* Verify that the signature on the LeafNode is valid using the LeafNode's + `signature_key`. * Verify that the LeafNode is compatible with the group's parameters. If the GroupContext has a `required_capabilities` extension, then the required @@ -1916,10 +1928,6 @@ with the following steps: `capabilities` field of this LeafNode indicates support for all the credential types currently in use by other members. -* Verify that the extensions in the leaf node are supported. The ID for each - extension in the `extensions` field MUST be listed in the field - `capabilities.extensions` of the LeafNode. - * Verify the `lifetime` field: * If the LeafNode appears in a message being sent by the client, e.g., a proposal or a commit, then the client MUST verify that the current time is within @@ -1929,9 +1937,14 @@ with the following steps: RECOMMENDED that the client verifies that the current time is within the range of the `lifetime` field. +* Verify that the extensions in the leaf node are supported. The ID for each + extension in the `extensions` field MUST be listed in the field + `capabilities.extensions` of the LeafNode. + * Verify the `leaf_node_source` field: - * If the LeafNode appears in a KeyPackage in an Add proposal, verify that - `leaf_node_source` is set to `key_package`. + * If the LeafNode appears in a KeyPackage in an Add proposal, or in a downloaded + KeyPackage used to create an Add proposal, verify that `leaf_node_source` is + set to `key_package`. * If the LeafNode appears in an Update proposal, verify that `leaf_node_source` is set to `update`. * If the LeafNode appears in the `leaf_node` value of the UpdatePath in @@ -3033,7 +3046,7 @@ The client verifies the validity of a KeyPackage using the following steps: * Verify that the ciphersuite and protocol version of the KeyPackage match those in use in the group. -* Verify the `leaf_node` of the KeyPackage is valid for an Add proposal +* Verify that the `leaf_node` of the KeyPackage is valid for an Add proposal according to {{leaf-node-validation}}. * Verify that the signature on the KeyPackage is valid using the public key @@ -3256,8 +3269,8 @@ An Add proposal is invalid if any of the following is true: * The KeyPackage is invalid according to {{keypackage-validation}}. -* The Credential in the LeafNode in the KeyPackage has the same signature - key as a Credential in any leaf of the group. +* The LeafNode in the KeyPackage shares the same `signature_key` with another + LeafNode in the group. * The LeafNode in the KeyPackage shares the same `encryption_key` with another LeafNode in the group. @@ -3298,8 +3311,7 @@ An Update proposal is invalid if any of the following is true: * The LeafNode is invalid for an Update proposal according to {{leaf-node-validation}}. -* The Credential in the LeafNode shares the same signature key with a Credential - in any leaf of the group. +* The LeafNode shares the same `signature_key` with another LeafNode in the group. * The LeafNode shares the same `encryption_key` with another LeafNode in the group. @@ -3320,8 +3332,8 @@ struct { } Remove; ~~~ -A Remove proposal is invalid if the `removed` field does not identify a non-blank leaf -node. +A Remove proposal is invalid if the `removed` field does not identify a non-blank +leaf node. A member of the group applies a Remove message by taking the following steps: @@ -3573,11 +3585,6 @@ Commit. In the event that a valid proposal is omitted from the next Commit, and that proposal is still valid in the current epoch, the sender of the proposal MAY resend it after updating it to reflect the current epoch. -Proposals with a non-default proposal type MUST NOT be included in a commit -unless the proposal type is supported by all the members of the group that will -process the Commit (i.e., not including any members being added or removed by -the Commit). - A member of the group MAY send a Commit that references no proposals at all, which would thus have an empty `proposals` vector. Such a Commit resets the sender's leaf and the nodes along its direct path, and @@ -4234,6 +4241,10 @@ following: * An ExternalInit proposal. +* A proposal with a non-default proposal type that is not supported by some + members of the group that will process the Commit (i.e., not including any + members being added or removed by the Commit). + An application may extend the above procedure by additional rules, for example, requiring application-level permissions to add members, or rules concerning non-default proposal types. From 0d376f87047fd65531d0986e363983719e92cf1e Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Tue, 17 May 2022 08:18:28 +0200 Subject: [PATCH 11/17] Fixed nits --- draft-ietf-mls-protocol.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 74c44901..ed6addbf 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -4222,6 +4222,8 @@ following: * An Update proposal generated by the committer. +* A Rmove proposal that removes the committer. + * Multiple Update and/or Remove proposals that apply to the same leaf. If the committer has received multiple such proposals they SHOULD prefer any Remove received, or the most recent Update if there are no Removes. From 783c1522b25663608e7dabc1ecf03b42b3d7fb10 Mon Sep 17 00:00:00 2001 From: optix Date: Tue, 17 May 2022 12:14:49 +0200 Subject: [PATCH 12/17] Re-jiggered some english. --- draft-ietf-mls-protocol.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index ed6addbf..4d6d7b3a 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -1898,8 +1898,8 @@ The validity of a LeafNode needs to be verified at a few stages: to add the client to the group * When a LeafNode is received by a group member in an Add, Update, or Commit message -* When a client joining a group receives LeafNode objects for the other members - of the group in the group's ratchet tree +* When a client joining a group receives LeafNode objects for the leaves of + of the group's ratchet tree To verify that a LeafNode is valid regardless of its use, the client starts with the following steps: @@ -4244,8 +4244,8 @@ following: * An ExternalInit proposal. * A proposal with a non-default proposal type that is not supported by some - members of the group that will process the Commit (i.e., not including any - members being added or removed by the Commit). + members of the group that will process the Commit (i.e., members being added + or removed by the Commit do not need to support the proposal type). An application may extend the above procedure by additional rules, for example, requiring application-level permissions to add members, or rules concerning From 7a552d679bc94cb1dc73080cdae966e09a5cf912 Mon Sep 17 00:00:00 2001 From: mulmarta <103590845+mulmarta@users.noreply.github.com> Date: Thu, 19 May 2022 11:05:47 +0200 Subject: [PATCH 13/17] Update draft-ietf-mls-protocol.md Co-authored-by: raphaelrobert --- draft-ietf-mls-protocol.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 4d6d7b3a..93d4ef77 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -4222,7 +4222,7 @@ following: * An Update proposal generated by the committer. -* A Rmove proposal that removes the committer. +* A Remove proposal that removes the committer. * Multiple Update and/or Remove proposals that apply to the same leaf. If the committer has received multiple such proposals they SHOULD prefer any Remove received, or From c0e6773dcae41759976d60db718655bdd20087ae Mon Sep 17 00:00:00 2001 From: mulmarta <103590845+mulmarta@users.noreply.github.com> Date: Fri, 20 May 2022 17:42:45 +0200 Subject: [PATCH 14/17] Update draft-ietf-mls-protocol.md Co-authored-by: Richard Barnes --- draft-ietf-mls-protocol.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 93d4ef77..142e25f0 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -1911,9 +1911,9 @@ with the following steps: another LeafNode, the authentication service MUST additionally validate that the set of identities attested by the credential in the new LeafNode is acceptable relative to the identities attested by the old credential. - E.g. sending an Update proposal updates the sender's old LeafNode to a new - one, or joining via external commit updates the joiner's old LeafNode - included in the committed Remove proposal to a new one. + For example: + * An Update proposal updates the sender's old LeafNode to a new one + * A "resync" external commit removes the joiner's old LeafNode via a Remove proposal and replaces it with a new one * Verify that the signature on the LeafNode is valid using the LeafNode's `signature_key`. From 8757f7a17e02fc6b653dd670f78a9209cae2b421 Mon Sep 17 00:00:00 2001 From: mulmarta <103590845+mulmarta@users.noreply.github.com> Date: Fri, 20 May 2022 17:47:49 +0200 Subject: [PATCH 15/17] Update draft-ietf-mls-protocol.md Co-authored-by: Richard Barnes --- draft-ietf-mls-protocol.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 142e25f0..74ef72de 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -3576,8 +3576,10 @@ proposals is valid as specified in {{validating-proposals}}. A list is invalid i it includes a proposal with an invalid signature, a proposal sent within a different epoch. or an Add when the sender does not have the application-level permission to add new users. -The sender of a Commit SHOULD include all valid proposals that it has received -during the current epoch, as long as this does not make the proposal list invalid. +The sender of a Commit SHOULD include all proposals that it has received +during the current epoch, that are valid according to the rules for their +proposal types and according to application policy, as long as this results in +a valid proposal list. Due to the asynchronous nature of proposals, receivers of a Commit SHOULD NOT enforce that all valid proposals sent within the current epoch are referenced by the next From a7487e318eeddf90c2ba3a83e497cac0b6e85ab4 Mon Sep 17 00:00:00 2001 From: Marta Mularczyk Date: Fri, 20 May 2022 17:51:57 +0200 Subject: [PATCH 16/17] Applied review suggestions --- draft-ietf-mls-protocol.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 74ef72de..861430fa 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -1901,8 +1901,7 @@ The validity of a LeafNode needs to be verified at a few stages: * When a client joining a group receives LeafNode objects for the leaves of of the group's ratchet tree -To verify that a LeafNode is valid regardless of its use, the client starts -with the following steps: +The client verifies the validity of a LeafNode using the following steps: * Verify that the credential in the LeafNode is valid according to the authentication service and the client's local policy. These actions MUST be @@ -3573,8 +3572,8 @@ removed before any application data is transmitted. A sender and a receiver of a Commit MUST verify that the committed list of proposals is valid as specified in {{validating-proposals}}. A list is invalid if, for example, -it includes a proposal with an invalid signature, a proposal sent within a different epoch. -or an Add when the sender does not have the application-level permission to add new users. +it includes an Update and a Remove for the same member, or an Add when the sender does not have +the application-level permission to add new users. The sender of a Commit SHOULD include all proposals that it has received during the current epoch, that are valid according to the rules for their From 22365fb7d1794a895e109046077881b0defd092f Mon Sep 17 00:00:00 2001 From: Brendan McMillion Date: Sun, 22 May 2022 11:41:51 -0700 Subject: [PATCH 17/17] Suggestions from code review. --- draft-ietf-mls-protocol.md | 89 ++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 51 deletions(-) diff --git a/draft-ietf-mls-protocol.md b/draft-ietf-mls-protocol.md index 348f3aef..613cb5ac 100644 --- a/draft-ietf-mls-protocol.md +++ b/draft-ietf-mls-protocol.md @@ -1924,7 +1924,7 @@ The validity of a LeafNode needs to be verified at a few stages: * When a LeafNode is received by a group member in an Add, Update, or Commit message * When a client joining a group receives LeafNode objects for the leaves of - of the group's ratchet tree + the group's ratchet tree The client verifies the validity of a LeafNode using the following steps: @@ -1951,12 +1951,12 @@ The client verifies the validity of a LeafNode using the following steps: `capabilities` field of this LeafNode indicates support for all the credential types currently in use by other members. -* Verify the `lifetime` field: - * If the LeafNode appears in a message being sent by the client, e.g., a - proposal or a commit, then the client MUST verify that the current time is within +* If applicable, verify the `lifetime` field: + * If the LeafNode appears in a message being sent by the client (e.g., a + proposal or a Commit), then the client MUST verify that the current time is within the range of the `lifetime` field. - * If instead the LeafNode appears in a message being received by the client, e.g., - a proposal, a commit, or a ratchet tree of the group the client is joining, it is + * If instead the LeafNode appears in a message being received by the client (e.g., + a proposal, a Commit, or a ratchet tree of the group the client is joining), it is RECOMMENDED that the client verifies that the current time is within the range of the `lifetime` field. @@ -3268,11 +3268,9 @@ struct { } Proposal; ~~~ -On receiving an MLSMessageContent containing a Proposal, a client MUST verify the -signature inside MLSMessageAuth and that the `epoch` field of the enclosing -MLSMessageContent is equal to the `epoch` field of the current GroupContext object. -If the verification is successful, then the Proposal should be cached in such a way -that it can be retrieved by hash (as a ProposalOrRef object) in a later Commit message. +Upon receiving an MLSMessageContent containing a Proposal which has been +successfully decrypted and/or authenticated, the Proposal is cached in such a +way that it can be easily referenced by a ProposalRef in a later Commit message. ### Add @@ -3289,14 +3287,8 @@ An Add proposal is invalid if any of the following is true: * The KeyPackage is invalid according to {{keypackage-validation}}. -* The LeafNode in the KeyPackage shares the same `signature_key` with another - LeafNode in the group. - -* The LeafNode in the KeyPackage shares the same `encryption_key` with another - LeafNode in the group. - -* The Credential in the KeyPackage represents a client already in the - group according to the application. +* The LeafNode in the KeyPackage shares the same `signature_key` or + `encryption_key` with another LeafNode in the group. An Add is applied after being included in a Commit message. The position of the Add in the list of proposals determines the leaf node where the new member will @@ -3331,9 +3323,8 @@ An Update proposal is invalid if any of the following is true: * The LeafNode is invalid for an Update proposal according to {{leaf-node-validation}}. -* The LeafNode shares the same `signature_key` with another LeafNode in the group. - -* The LeafNode shares the same `encryption_key` with another LeafNode in the group. +* The LeafNode shares the same `signature_key` or `encryption_key` with another + LeafNode in the group. A member of the group applies an Update message by taking the following steps: @@ -3444,18 +3435,16 @@ struct { A GroupContextExtensions proposal is invalid if it includes a `required_capabilities` extension and some members of the group do not support -some of the required capabilities (including those added in the same commit, +some of the required capabilities (including those added in the same Commit, and excluding those removed). -A member of the group applies a GroupContextExtensions proposal with the -following steps: +A member of the group applies a GroupContextExtensions proposal by removing all +of the existing extensions from the GroupContext object for the group and +replacing them with the list of extensions in the proposal. Note that this is a +wholesale replacement and not a merge. An extension is only carried over if the +sender of the proposal includes it in the new list. -* Remove all of the existing extensions from the GroupContext object for the - group and replacing them with the list of extensions in the proposal. (This - is a wholesale replacement, not a merge. An extension is only carried over if - the sender of the proposal includes it in the new list.) - -Note that once the GroupContext is updated, its inclusion in the +Once the GroupContext is updated, its inclusion in the confirmation_tag by way of the key schedule will confirm that all members of the group agree on the extensions in use. @@ -3538,10 +3527,10 @@ proposal types and according to application policy, as long as this results in a valid proposal list. Due to the asynchronous nature of proposals, receivers of a Commit SHOULD NOT enforce -that all valid proposals sent within the current epoch are referenced by the next -Commit. In the event that a valid proposal is omitted from the next Commit, and -that proposal is still valid in the current epoch, the sender of the proposal -MAY resend it after updating it to reflect the current epoch. +that all proposals sent within the current epoch are referenced by the next +Commit. In the event that a proposal is omitted from a Commit and +that proposal is still applicable in the subsequent epoch, the sender of the proposal +MAY resend it. A member of the group MAY send a Commit that references no proposals at all, which would thus have an empty `proposals` vector. Such @@ -3631,11 +3620,13 @@ their associated GroupContexts are used: A member of the group creates a Commit message and the corresponding Welcome message at the same time, by taking the following steps: -* Verify that the committed list of proposals is valid as specified in - {{validating-proposals}}. +* Decide on a list of proposals to include that would be valid according to the + rules specified in {{validating-proposals}}. If the committer has received a + proposal, it MUST include it in the list unless doing so would make the list + invalid. * Construct an initial Commit object with the `proposals` - field populated from Proposals received during the current epoch, and an empty + field populated from the chosen Proposals, and an empty `path` field. * Generate the provisional ratchet tree and GroupContext by applying the proposals @@ -4168,11 +4159,11 @@ using the tree for MLS operations. ## Validating Proposals -A group member creating a commit and a group member processing a commit +A group member creating a Commit and a group member processing a Commit MUST verify that the list of committed proposals is valid using one of the following procedures, depending on whether the commit is external or not. -For a regular, i.e. not external, commit the list is invalid if it contains any of the +For a regular (not external) Commit, the list is invalid if it contains any of the following: * An individual proposal that is invalid as specified in {{proposals}}. @@ -4185,10 +4176,6 @@ following: has received multiple such proposals they SHOULD prefer any Remove received, or the most recent Update if there are no Removes. -* Multiple Add proposals that contain KeyPackages that represent the same client - according to the application (for example, identical KeyPackages or KeyPackages - sharing the same Credential). - * Multiple PreSharedKey proposals that reference the same PreSharedKeyID. * Multiple GroupContextExtensions proposals. @@ -4201,21 +4188,21 @@ following: * An ExternalInit proposal. * A proposal with a non-default proposal type that is not supported by some - members of the group that will process the Commit (i.e., members being added - or removed by the Commit do not need to support the proposal type). + members of the group that will process the Commit. Note that members being added + or removed by the Commit do not need to support the proposal type. An application may extend the above procedure by additional rules, for example, requiring application-level permissions to add members, or rules concerning non-default proposal types. -For an external commit, the list is valid if: +For an external Commit, the list is invalid if: -* It contains only ExternalInit, Remove and PreSharedKey proposals. +* It contains any proposal types other than ExternalInit, Remove, or PreSharedKey. -* It contains exactly one ExternalInit proposal. +* It doesn't contain exactly one ExternalInit proposal. -* It contains at most one Remove proposal, with which the joiner removes an - old version of themselves. If a Remove proposal is present then, the LeafNode in the +* It contains more than one Remove proposal. + If a Remove proposal is present, the LeafNode in the `path` field of the external commit MUST meet the same criteria as would the LeafNode in an Update for the removed leaf (see {{update}}). In particular, the `credential` in the LeafNode MUST present a set of identifiers that is acceptable to the