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

MSC4104: Auth Lock: Soft-failure-be-gone! #4104

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Gnuxie
Copy link
Contributor

@Gnuxie Gnuxie commented Feb 19, 2024

Rendered

Signed-off-by: Gnuxie Gnuxie@protonmail.com

@Gnuxie Gnuxie changed the title Auth Lock: Soft-failure-be-gone! MSC4104: Auth Lock: Soft-failure-be-gone! Feb 19, 2024
thanks Andy
We need to say explicitly that all `m.auth_lock` events that have
been created that match the `auth_event` are considered.
It's allowed because servers will still retain the history and an
admin can easily recanonicalise the state so that joining
servers can see the relevant history again.
@turt2live turt2live added requires-room-version An idea which will require a bump in room version proposal A matrix spec change proposal s2s Server-to-Server API (federation) room-spec Something to do with the room version specifications unassigned-room-version Remove this label when things get versioned. needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. kind:feature MSC for not-core and not-maintenance stuff labels Feb 19, 2024
Comment on lines +73 to +74
The receiving server must combine the `extremities` from both events
such that the canonical history becomes C -> B -> A.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if Alice sees Chelsea send B -> A but Bob sees C -> A instead, because Chelsea crafted two extremities from the same event, delivering one to Alice's HS and the other to Bob's HS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a better example. However, provided Bob receives Alice's m.auth_lock, then Bob would reject any further events sent by Chelsea referencing A or C. Bob could also send their own m.auth_lock referencing Chelsea's prior membership that includes C within extremities. Then Alice will be able to pull in C when Alice gets Bob's m.auth_lock. Alice could then redact C.

Though this presents another problem when Bob is unprivileged and can't send m.auth_lock. The reason why we give bob the capability to send m.auth_lock is because we trust that he will observe and apply Alice's m.auth_lock and not leak more of Chelsea's events to us. And this means he's most likely already a room admin. This is a problem that already exists that soft failure doesn't prevent either element-hq/synapse#9329. It would be nice to find some kind of solution.

Copy link
Contributor Author

@Gnuxie Gnuxie Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe negotiation of extremities could to considered separately, such that servers with unprivileged users could propose them and room admins could preview them and choose to accept or reject them. That does feel a little bit like over engineering for and entrenching a bad state of affairs though. But at least it's something that would work.

Copy link
Contributor Author

@Gnuxie Gnuxie Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I should probably clarify somewhere in the proposal that we don't propose eagerly issuing an m.auth_lock with every ban, we simply do not soft fail anymore. If we then see events being authorized that would have normally been candidates for soft failure, the room admin or their tooling would then issue m.auth_lock. In principle, this should reduce the number of instances where element-hq/synapse#9329 occurs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

negotiation

I'm forgetting that a server can ask for the prev_events, find the diverged extremity provide a preview to an admin and allow them to choose incorporate it, all without manual intervention from another server (other than continuing to participate)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkasak I have rewritten the potential issues section to try cover this concern and all the solutions that I have discovered so far

Comment on lines 80 to 83
We counteract this issue by requiring that servers maintain all
existing `extremities` and their `prev_events`, regardless of
whether they are missing from the `extremities` field of
an `m.auth_lock` event.
Copy link
Member

@dkasak dkasak Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what is "existing" at the point of receipt of the m.auth_lock event could differ across different servers participating in the room. Wouldn't this strategy lock them into having diverging views of the room?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because users on those servers can issue their own m.auth_lock events for the same locked_event_id, which would allow them to converge again. Am I understanding this correctly as the same problem described in the second paragraph of https://github.com/matrix-org/matrix-spec-proposals/pull/4104/files#r1494856367 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal requires-room-version An idea which will require a bump in room version room-spec Something to do with the room version specifications s2s Server-to-Server API (federation) unassigned-room-version Remove this label when things get versioned.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants