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

Server ACLs #1550

Merged
merged 7 commits into from Aug 27, 2018

Conversation

2 participants
@turt2live
Member

turt2live commented Aug 22, 2018

Rendered: see 'docs' status check


Implements the proposal for #1383

Closes #1383
Closes #709

Server ACLs
Implements the proposal for #1383

@turt2live turt2live requested a review from matrix-org/spec-core-team Aug 22, 2018

@turt2live turt2live added this to In review in August 2018 r0 via automation Aug 22, 2018

An event to indicate which servers are permitted to participate in the
room. Server ACLs may allow or deny groups of hosts. All servers participating
in the room, including those that are denied, are expected to uphold the
server ACL. Servers that do not uphold the ACLs are recommended to be

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

i'd say MUST be manually added to the denied hosts list, in order for the ACL to be effective.

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

We should probably also spell out that for now the only way to detect support for server_acls is to inspect the version number or check which servers leak events from banned servers into the room via prev_events - albeit probably in a warning box of some kind.

type: boolean
description: |-
True to allow server names that are IP address literals. False to
deny. Defaults to true if missing or otherwise not a boolean.

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

perhaps add: "We recommend setting allow_ip_literals to false, and never running production Matrix servers with IP literal server names, in order to require legitimate homeservers to be backed by a valid registered domain name"

description: |-
The server names to allow in the room, excluding any port information.
Wildcards may be used to cover a wider range of hosts, where ``*``
matches zero or more characters and ``?`` matches one or more characters.

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

? should match one and only one character; the globs are as per unix shells, not regexps.

description: |-
The server names to disallow in the room, excluding any port information.
Wildcards may be used to cover a wider range of hosts, where ``*``
matches zero or more characters and ``?`` matches one or more characters.

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

see above

#. If the server name matches an entry in the ``deny`` list, deny.
#. If the server name matches an entry in the ``allow`` list, allow.
#. Otherwise, deny.

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

perhaps: "N.B. server ACLs do not restrict the events relative the room DAG via authorisation rules, but instead act purely at the network layer to determine which servers are allowed to connect and interact with a given room".

(as I'd forgotten this already and it tripped me up the other day)

.. _module:server-acls:
In some scenarios room operators may wish to prevent a malicous or untrusted

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

malicious

.. Note::
Port numbers are not supported because it is unclear to parsers whether a
port number should be matched or an IP address literal.

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

plus the assumption is that if you can't trust a server on one port on a given domain you are very unlikely to trust others on the same domain (and letting you do so is almost certainly a footgun)

@turt2live turt2live requested a review from ara4n Aug 27, 2018

user interface, such as in the timeline.
Clients may wish to kick affected users from the room prior to denying a server
access to the room to help prevent those servers from participating.

This comment has been minimized.

@ara4n

ara4n Aug 27, 2018

Member

and to provide feedback to the users that they have been excluded from the room.

@ara4n

lgtm, other than thinko on defining the ? glob char, and various suggested tweaks for clarity & context.

@turt2live turt2live requested a review from ara4n Aug 27, 2018

@turt2live

This comment has been minimized.

Member

turt2live commented Aug 27, 2018

Concerns should be addressed - please take a quick look to make sure I didn't screw up the wording.

Edit: Much of the text is shamelessly copy/pasted from your suggestions.

@ara4n

ara4n approved these changes Aug 27, 2018

August 2018 r0 automation moved this from In review (just the PRs) to Reviewer approved Aug 27, 2018

@ara4n

This comment has been minimized.

Member

ara4n commented Aug 27, 2018

lgtm - i cheekily pushed a wording tweak about legacy/noncompliant servers to your branch; hope it was okay :)

@turt2live turt2live merged commit 4e885c3 into matrix-org:master Aug 27, 2018

4 checks passed

ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: build-swagger Your tests passed on CircleCI!
Details
docs Click details to preview the HTML documentation.
Details
swagger Click to preview the swagger build.
Details

August 2018 r0 automation moved this from Reviewer approved to Done (this list will be incomplete) Aug 27, 2018

@turt2live turt2live deleted the turt2live:travis/general/acls branch Aug 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment