mlswg / mls-extensions

Other
6 stars 7 forks source link

Clarify how capabilities negotiation works in Safe Extensions framework. #31

Closed rohanmahy closed 2 weeks ago

rohanmahy commented 1 month ago

Note: this diff is easier to read when viewed as diffs for its separate commits. The first Commit is just a plain move of one section.

rohanmahy commented 4 weeks ago

Is there a reason you want the extension WireFormat, Proposal, etc. to be part of the safe extension API?

Yes! Safe WireFormats, Safe Proposals, and Safe Credentials share the mls-extension_message, extension*proposal, and extension_credential types respectively; all of these MUST use the ExtensionContent struct; and all of them use the capabilities negotiation I described. The whole reason I wrote this PR was because it was simply not possible to write rules for safe extension of one these types without stepping into rules that should be defined in the framework.

Our thinking was to keep it separate, because it [my emphasis] might also be useful for non-safe extensions.

I'm not sure what the it is. RFC9420 already says what you need to do for non-safe extensions of these types. If the creator of a non-safe extension wants to reuse some structs defined the Safe Extensions framework, I don't see any problem with this, but they absolutely MUST NOT use the IANA registered mls-extension_message, extension*proposal, and extension_credential types. If you wanted to make it easier to find/reference the ExtensionState struct for example, I would suggest we get this PR merged and then we can have a clean PR proposing the move. Otherwise the conflicts get ugly and the PR is harder to review.

kkohbrok commented 3 weeks ago

What I meant was: Why shouldn't unsafe extension authors make use of mls-extension_message, extension*proposal and extension_credential? Is there a safety or a negotiation consideration I'm not seeing?

rohanmahy commented 3 weeks ago

They shouldn't use those types because that is how the stack of some other client knows that the (to it unknown) extension is safe or not.

On Mon, Oct 14, 2024, 00:13 Konrad Kohbrok @.***> wrote:

What I meant was: Why shouldn't unsafe extension authors make use of mls-extension_message, extension*proposal and extension_credential? Is there a safety or a negotiation consideration I'm not seeing?

— Reply to this email directly, view it on GitHub https://github.com/mlswg/mls-extensions/pull/31#issuecomment-2410227732, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADSQPTKANLMJRH6AU2SCATZ3NVKBAVCNFSM6AAAAABPMU4X72VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJQGIZDONZTGI . You are receiving this because you authored the thread.Message ID: @.***>

kkohbrok commented 3 weeks ago

If the other client doesn't know the extension, how can its semantics (and thus its safety) be relevant to it?

rohanmahy commented 3 weeks ago

1) So it can ignore them safely 2) If it is a Safe Extension, the stack could conceivably pass it to the consumer of the stack to implement the safe extension.

On Mon, Oct 14, 2024, 05:58 Konrad Kohbrok @.***> wrote:

If the other client doesn't know the extension, how can its semantics (and thus its safety) be relevant to it?

— Reply to this email directly, view it on GitHub https://github.com/mlswg/mls-extensions/pull/31#issuecomment-2411164100, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADSQPWEQ6XT7SZOGVTSZX3Z3O5Y3AVCNFSM6AAAAABPMU4X72VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJRGE3DIMJQGA . You are receiving this because you authored the thread.Message ID: @.***>

kkohbrok commented 3 weeks ago

1) If an extension can't be safely ignored, it should be a required capability, shouldn't it? 2) If the application wants to do anything with the extension, why wouldn't it signal support for it? Also, why would an application expect to be shown the data for safe extensions, but not for unsafe ones?

rohanmahy commented 3 weeks ago

Safe Extensions has two main benefits:

On Tue, Oct 15, 2024, 03:18 Konrad Kohbrok @.***> wrote:

  1. If an extension can't be safely ignored, it should be a required capability, shouldn't it?
  2. If the application wants to do anything with the extension, why wouldn't it signal support for it? Also, why would an application expect to be shown the data for safe extensions, but not for unsafe ones?

— Reply to this email directly, view it on GitHub https://github.com/mlswg/mls-extensions/pull/31#issuecomment-2413484893, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADSQPVISXO3DMIVV3WODATZ3TTXFAVCNFSM6AAAAABPMU4X72VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJTGQ4DIOBZGM . You are receiving this because you authored the thread.Message ID: @.***>

kkohbrok commented 3 weeks ago

In a scenario where the MLS implementation doesn't know the extension, i.e. it's not in the capabilities then why wouldn't the application show proposals, etc. to the application regardless whether the extension is safe or not? If the implementation of the unknown extension can't be delegated to the application, the extension really should have been in the required capabilities.

rohanmahy commented 2 weeks ago

In a scenario where the MLS implementation doesn't know the extension, i.e. it's not in the capabilities then why wouldn't the application show proposals, etc. to the application regardless whether the extension is safe or not? If the implementation of the unknown extension can't be delegated to the application, the extension really should have been in the required capabilities.

Let's take the example of an AssociatedParties "feature". It has:

The "feature" has a single ExtensionType say it is type 0xBEEF. The proposals are extension_proposal, extension_path_proposal, and extension_path_proposal types respectively. After each of these would be the ExtensionType 0xBEEF and then inside the extension_path_proposal, a selector in the TLS struct to select between the remove and update variants of the struct.

kkohbrok commented 2 weeks ago

I see what you're getting at. I think this an issue beyond safe extensions that we best discuss in the next meeting.

rohanmahy commented 2 weeks ago

I see what you're getting at. I think this an issue beyond safe extensions that we best discuss in the next meeting.

I think the group will have a hard time reviewing it properly inside a PR. If we have consensus from the editors can we merge this to have something concrete to discuss?

kkohbrok commented 2 weeks ago

Looking at it more closely, maybe I don't understand what you're getting at. I understand the AP extension example and that's indeed how the mechanism is intended, but why can't we use the same mechanism (i.e. bundle multiple proposals, WireFormats, etc. behind a single extension type) with a non-safe extension?

rohanmahy commented 2 weeks ago

Looking at it more closely, maybe I don't understand what you're getting at. I understand the AP extension example and that's indeed how the mechanism is intended, but why can't we use the same mechanism (i.e. bundle multiple proposals, WireFormats, etc. behind a single extension type) with a non-safe extension?

I think we should be making it clear and easy to make safe extensions. The creator of a non-safe extension could use the same or similar mechanism, but we can't be prescriptive about what non-safe extension can and can't do. They don't HAVE TO. However for safe extensions we can say, they have to do a specific thing. That's why I don't particularly care to spend a lot of energy on considerations for non-safe extensions. It's not that I don't want non-safe extensions to do better, it's that they are free to ignore anything we do say now (the horse is already out of the barn).

kkohbrok commented 2 weeks ago

Fair enough, then I'm okay with this PR.

I don't think, however, that just because an extension is safe, it can be safely ignored by both client and application. For example, if a client receives a commit with an extension proposal from a safe extension where the proposal is invalid according to the extension's semantics. A client that is knows the extension will consider the commit invalid. A client that doesn't will consider the commit invalid.

rohanmahy commented 2 weeks ago

Fair enough, then I'm okay with this PR.

Great!

I don't think, however, that just because an extension is safe, it can be safely ignored by both client and application. For example, if a client receives a commit with an extension proposal from a safe extension where the proposal is invalid according to the extension's semantics. A client that is knows the extension will consider the commit invalid. A client that doesn't will consider the commit invalid.

Agreed. All proposals used would have to be in required_capabilities because a member needs to be able to understand if they are valid or not.

raphaelrobert commented 2 weeks ago

Closing in favor of #36.