Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial room power-levels are overly special-cased #1696

Open
richvdh opened this issue May 3, 2018 · 7 comments
Open

Initial room power-levels are overly special-cased #1696

richvdh opened this issue May 3, 2018 · 7 comments
Labels
wart A point where the protocol is inconsistent or inelegant

Comments

@richvdh
Copy link
Member

richvdh commented May 3, 2018

Currently the auth rules have three two special-cases for power levels before the first power_levels event lands:

  1. the first power-level event is allowed regardless of its content
  2. the creator of the room has PL 100
  3. state_default is 0 rather than 50. Fixed by Power levels always have a default of 50 for state_default matrix-spec-proposals#1656

I think we only need one of those.

@richvdh richvdh added the wart A point where the protocol is inconsistent or inelegant label May 3, 2018
@ara4n
Copy link
Member

ara4n commented Jun 14, 2018

Proposal for fixing this is MSC matrix-org/matrix-spec-proposals#1304

richvdh referenced this issue in matrix-org/synapse Jun 14, 2018
Make it so that, before there is a power-levels event in the room, you need a
power level of at least 50 to send state.

Partially addresses https://github.com/matrix-org/matrix-doc/issues/1192
@richvdh
Copy link
Member Author

richvdh commented Jun 14, 2018

matrix-org/synapse#3397 removes the third of these special-cases. Discussion about whether to propagate this to the spec is at matrix-org/matrix-spec-proposals#1304.

@richvdh
Copy link
Member Author

richvdh commented Dec 12, 2023

Reopening this as it only got half-solved.

@richvdh richvdh reopened this Dec 12, 2023
@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Dec 12, 2023
@MTRNord
Copy link
Contributor

MTRNord commented Dec 12, 2023

The below is the paraphrasing of https://matrix.to/#/!NasysSDfxKxZBzJJoE:matrix.org/$CNVa24tDZSyBP153k_YcDDUlmtAI2ZL4JiSOB4sXuqM?via=matrix.org&via=envs.net&via=element.io

It came up the topic of what happens when we don't send an initial powerlevel:

C-S Api in spec 1.9 says:

image

and

image

which boil down to "creator gets 100, everyone else 0 and state default is 50" meaning in a result the creator is the only one allowed to send the state event aka the powerlevel event.

Then on the S-S Api in room version 11 this gets lifted with 9.4:

If there is no previous m.room.power_levels event in the room, allow.

This means as a result the defaults will not apply for a missing event at all anymore since the auth rules special case it. Possibly (but not verified) to the point where anyone now can send the initial powerlevel event according to the state res algo. Which is equivalent to a hijack in the worst case. (This assumes evil intent and bypassing these CS api event defaults with a modified HS)

(Unless I am not understanding how these are meant to be applied).

If there is a state res exception for this case of no initial PL event then it should at least follow the CS default rules imho. However I dont think there is a reason for this case to appear and instead it should be mandated to exist in the initial state of the create event which eliminates rule 9.4 entirely and solves this fully.

@richvdh
Copy link
Member Author

richvdh commented Dec 12, 2023

From the discussion on matrix-org/matrix-spec-proposals#1304 about why the first special case ("the first power-level event is allowed regardless of its content") was retained:

Actually, being able to set max PL above 100 could be considered a feature - and there's some evidence that people might be doing this already in the wild. Given this doesn't help us defend against hypothetical room hijack attacks, let's keep it in for now.

Personally, I feel that was a mistake: having special cases in the algorithm "because they might be useful" complicates the protocol.

@tulir
Copy link
Member

tulir commented Dec 12, 2023

Being able to set the max PL above 100 is definitely useful and is a feature, but it could also be implemented by defining that the creator has PL 2^53-1 instead of 100 when there's no power level event, the current special case of allowing anyone to send any PL event isn't necessary for that.

In any case, there has to be some special case. Even the creator having 100 when there's no PL event is a special case.

@richvdh
Copy link
Member Author

richvdh commented Dec 12, 2023

In any case, there has to be some special case. Even the creator having 100 when there's no PL event is a special case.

Yes, we need one special case. Not two :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wart A point where the protocol is inconsistent or inelegant
Projects
None yet
Development

No branches or pull requests

4 participants