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

Reject large transactions on federation #4513

Merged
merged 4 commits into from Jan 31, 2019
Merged
Changes from 3 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+25 −0
Diff settings

Always

Just for now

Copy path View file
@@ -0,0 +1 @@
Reject federation transactions if they include more than 50 PDUs or 100 EDUs.
@@ -148,6 +148,30 @@ def _handle_incoming_transaction(self, origin, transaction, request_time):

logger.debug("[%s] Transaction is new", transaction.transaction_id)

# Reject if PDU count > 50 and EDU count > 100
if (len(transaction.pdus) > 50
or (hasattr(transaction, "edus") and len(transaction.edus) > 100)):
response = {
"pdus": {}
}

for pdu_key in transaction["pdus"].keys():

This comment has been minimized.

Copy link
@richvdh

richvdh Jan 29, 2019

Member

I'm really not sure I'd bother with this. The spec doesn't say that this will be present for a non-200 response, and synapse will ignore it.

(in any case Transaction doesn't have a __getattr__, so transaction["pdus"] will fail, and the pdus are a list not a dict.)

response["pdus"][pdu_key] = {
"error": "Processing failed. More than 50 PDUs or 100 EDUs sent."
}

logger.debug(

This comment has been minimized.

Copy link
@richvdh

richvdh Jan 29, 2019

Member

this probably wants to be at least an info (and not contain a mahoosive list of events)

"Transaction PDU or EDU count too large. Returning: %s", str(response)
)

yield self.transaction_actions.set_response(
origin,
transaction,
400, response
)
defer.returnValue((400, response))
return

This comment has been minimized.

Copy link
@richvdh

richvdh Jan 29, 2019

Member

nit: this return is redundant fwiw


received_pdus_counter.inc(len(transaction.pdus))

origin_host, _ = parse_server_name(origin)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.