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.

Sign up for GitHub

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 stab at documenting soft fail #1641

Merged
merged 13 commits into from
Oct 26, 2018
Merged

Initial stab at documenting soft fail #1641

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1b366b1
Initial stab at documenting soft fail
erikjohnston Aug 31, 2018
6a035cc
Title casing
erikjohnston Aug 31, 2018
cafe706
Clarify whether to return soft failed events are returned in federati…
erikjohnston Oct 2, 2018
ed9f05d
Clarify soft failed state event handling
erikjohnston Oct 17, 2018
7b28b19
Further clarifications
erikjohnston Oct 17, 2018
6445899
Clarify soft fail being due to not passing auth based on current state
erikjohnston Oct 18, 2018
43d2d82
Add an attempt at an example
erikjohnston Oct 18, 2018
f3f1151
Rework entire thing to add motivation, then implementation and then w…
erikjohnston Oct 18, 2018
0d9b882
Try different title character
erikjohnston Oct 18, 2018
ceb7494
Use Rationale and Example admonitions, rather than headings
erikjohnston Oct 23, 2018
377e02a
Fix up example section
erikjohnston Oct 23, 2018
56b2887
Incorporate feedback
erikjohnston Oct 23, 2018
40bc911
Add requested from Matthew to talk about future state res algorithms
erikjohnston Oct 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions specification/server_server_api.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,32 @@ transaction request to be responded to with an error response.
result in the user being considered joined.


Soft Failure
Copy link
Member

Choose a reason for hiding this comment

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

title casing ("Soft failure")

++++++++++++

When the homeserver receives a new event over federation it should also check
whether the event passes auth checks based on the current state of the room
(as well as based on the state at the event). If the event does not pass the
auth checks it should be "soft failed".
Copy link
Member

Choose a reason for hiding this comment

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

I think a possible point of confusion is that this "does not pass the auth checks" only refers to the auth checks based on the current state of the room. If the event does not pass auth checks based on the state at the event itself, then the event should be outright rejected, rather than soft failed. This should probably be clarified.


When an event is "soft failed" it should not be relayed to the client nor be
referenced by new events created by the homeserver. If an event is received that
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
references the soft failed event then the new event should be handled as usual.
Copy link
Member

Choose a reason for hiding this comment

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

To follow up with the comments made in Matrix, I think if you said something along the lines of: if the soft-failed event is a state event, then it still participates in state resolution as usual, and if the state resolution algorithm resolves to using that event, then the event will be sent to clients. Aside from this, I don't see any issues.

If this causes a change in state (e.g. due to the soft failed event being a
state event) then the state updates should be propagated to clients as usual.

.. NOTE::

This is different than rejections in that soft failed events are simply
ignored unless a new event references it.
Copy link
Member

Choose a reason for hiding this comment

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

that's not quite true: soft failed events are not entirely ignored: they are added to the DAG.

Copy link
Member

Choose a reason for hiding this comment

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

(I suppose it would be a valid choice not to persist the event, but that sounds like you could easily get into a mess of repeatedly seeing the same event and doing the same authentication over and over.)



.. NOTE::

Soft failures are designed to stop malicious servers from avoiding actions
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would be clearer to spell this out at the top of the section, rather than here. Currently, the problem is that you have to read and understand the implementation before you get told the motivation, which leads to a bunch of "wtf" type questions as have been seen here.

I also think that it would be helpful to introduce the section with a handwavey overview (possibly including examples) before diving into the specifics of the implementation.

such as kicks or bans by careful selection of ``prev_events``.


Retrieving event authorization information
++++++++++++++++++++++++++++++++++++++++++

Expand Down