Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MSC3593: Safety Controls through a generic Administration API #3593
base: main
Are you sure you want to change the base?
MSC3593: Safety Controls through a generic Administration API #3593
Changes from 2 commits
e2b2c3f
403fd93
ec7612d
cb8a5de
fac1c95
1f9c29e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the reason that it is recommended to not expose this to the Internet via a reverse proxy (see documentation), and related to why these endpoints are on
/_synapse/admin
-- this allows easily configuring access controls for any endpoints under that.I wonder if it would make more sense to place all of these under
/_matrix/admin
instead? That seems to have obvious downsides though (e.g. if you wanted admin endpoints for a push server or something).Anyway, this probably needs some words about whether this is meant to be exposed to clients or if it is meant for use in other ways and is just about standardization across homeserver ecosystems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is primarily meant to be exposed to clients, to make matrix administration simple and straight-forward. I think that security by obscurity (by exposing it on a different domain), or not exposing this at all (which defeats the purpose of these endpoints in the first place), is not an option.
However, I think that a large part of these problems could potentially be mitigated by introducing a mechanism which forces the admin API to be secure even if an attacker has an access token, and for this, I was indeed thinking that the Capable User would have to re-assess their password, just as they would to change their password in the normal endpoints.
For this, maybe an additional endpoint could be introduced which enables "sudo mode", just as github would request when you change or look at sensitive settings, this'd either enable the access token that "sudo mode", or returns an additional token which is valid for such a period. I don't know if this additional complexity in this way is reasonable for the spec, i'm open to finding more ergonomic ways of doing it.
One problem i already see, for example, is that some accounts might not have passwords at all, and attest their identity via third-party SSO, so such a "sudo mode" endpoint would have to be generic across multiple solutions.
The main takeaway is that i'd like for matrix administration to be simple, yet secure, as simple as it can be to be implementable in all clients, accessible via the same CSAPI domain it is pointed to. I get that for larger servers, of course, security has to be more professional and more strict, but that is what this API would allow; modular access which is then set up in a server-specific way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is good to standardise the administration and to organise and build up the URLs conceptually.
IMO the API to not expose is not "security by obscurity". Security by obscurity is when you don't document the API or the urls are random long strings similar to the media files in Matrix.
In this case it is an out of band management.
There is no such thing as 100% security. Not even with a web application firewall. Therefore, the consistent separation of admin and user traffic greatly increases security. In this way, each administrator can decide for themselves whether users should administer via app or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also just like the APIs to be under /_matrix/admin for visibility. Those APIs need a separate set of permissions and shouldn't be required for normal client applications, but are rather used to manage a server. As such all such endpoints should be under /admin to separate them conceptually. Not from a security or anything stand point, just because I think it belongs there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, i didn't realise what you said here earlier.
So, essentially, you're proposing that this is a whole new branch of API, on-par with CS and SS APIs? That could make sense imo, but it would also raise the bar of scrutiny on this proposal.
I'll think about it, the idea is sound, but i also don't know if this specifically resolves the concern of user compromise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been a few months now, and I've had time to think about this.
I think that adding these endpoints under a separate API prefix can help with some security aspects.
Additionally, I think that I will add a section to this proposal outlining a compromise between ease and security; Allowing the admin API bearer token to be and-or the user's access token, or a specific token retrieved via another out-of-band means.
This would allow server administration to become easy for simple servers, such as conduit servers, where the server would listen on the same hostname for authentication through bearer tokens.
However, on a large server (such as matrix.org), the endpoints could be bundled together under a different domain, by a different token process, not bundled to someone's access token.
It'd probably also be possible to "bless" a specific (refresh/access) token - also via out-of-band. These are all possible implementation details, however it should be clear that a simple discovery mechanism is to call
/capabilities
and observe a return. Then within clients there could be a menu section to input this admin API, both hostname and token, for the user's client.I hope this could be an acceptable resolution, at the cost of making this proposal a bit more complex. I'll submit these changes and ask y'all to make another round of comments on this new approach afterwards.