-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
ACL for servers support #35
ACL for servers support #35
Conversation
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.
So far just ran cargo clippy
and found these issues, and just a couple concerns.
…owlist values in the database without enabling the allowlist without denying the request outright
Co-authored-by: June <june@girlboss.ceo>
…oji() function instead of being hardcoded in case of further changes to emojis.
Co-authored-by: June <june@girlboss.ceo>
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.
In additional to the few things I mentioned below and in the Matrix room, this currently just overrides room ACLs rather than actually accepting/rejecting federation requests.
Allowlisting servers with allow_federation disabled doesn't work (still returns federation disabled errors), and denying homeservers just prevents the blocked homeserver from joining the room rather than rejecting requests from the bad homeserver.
This needs to be expanded to areas where allow_federation
is checked preferably as a global check instead of individually in every function in server_server.rs like it is right now with allow_federation
(this is another bad design Conduit has tbh), but the general functionality of this works.
Co-authored-by: June <june@girlboss.ceo>
Co-authored-by: June <june@girlboss.ceo>
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.
Allowlist still doesn't work, let's just remove the allowlist stuff and keep it strictly block/deny for now. I was anticipating that the same infrastructure for denying/blocking can be used for allowing. I'm guessing the individual S2S functions that check if federation is enabled are breaking it, but at this point it should just be resolved another time since it looks very involved/complex to resolve.
The only issue I have at the moment is it still uses resources fetching signing keys (from the blocked server) even though we end up rejecting their events later.
2023-12-25T15:00:35.577453Z DEBUG http_request{path=/_matrix/federation/v1/query/profile}:fetch_signing_keys_for_server:send_federation_request:send_request{destination="matrix.org"}: conduit::api::server_server: Got well known response
2023-12-25T15:00:35.577506Z DEBUG http_request{path=/_matrix/federation/v1/query/profile}:fetch_signing_keys_for_server:send_federation_request:send_request{destination="matrix.org"}: conduit::api::server_server: Got well known response text
2023-12-25T15:00:35.577529Z DEBUG http_request{path=/_matrix/federation/v1/query/profile}:fetch_signing_keys_for_server:send_federation_request:send_request{destination="matrix.org"}: conduit::api::server_server: 3: A .well-known file is available
2023-12-25T15:00:35.577547Z DEBUG http_request{path=/_matrix/federation/v1/query/profile}:fetch_signing_keys_for_server:send_federation_request:send_request{destination="matrix.org"}: conduit::api::server_server: 3.2: Hostname with port in .well-known file
2023-12-25T15:00:35.577567Z DEBUG http_request{path=/_matrix/federation/v1/query/profile}:fetch_signing_keys_for_server:send_federation_request:send_request{destination="matrix.org"}: conduit::api::server_server: Actual destination: Named("matrix-federation.matrix.org", ":443")
2023-12-25T15:00:35.577585Z DEBUG http_request{path=/_matrix/federation/v1/query/profile}:fetch_signing_keys_for_server:send_federation_request:send_request{destination="matrix.org"}: conduit::api::server_server: Checking acl allowance for matrix.org
2023-12-25T15:00:35.577620Z DEBUG http_request{path=/_matrix/federation/v1/query/profile}:fetch_signing_keys_for_server:send_federation_request:send_request{destination="matrix.org"}: conduit::service::acl: checking federation allowance for matrix-federation.matrix.org
2023-12-25T15:00:35.577777Z DEBUG http_request{path=/_matrix/federation/v1/query/profile}:fetch_signing_keys_for_server:send_federation_request:send_request{destination="matrix.org"}: conduit::api::server_server: Sending request to matrix.org at https://matrix-federation.matrix.org/_matrix/key/v2/server
2023-12-25T15:00:46.718487Z DEBUG http_request{path=/_matrix/federation/v2/invite/:room_id/:event_id}: conduit::service::acl: checking federation allowance for matrix.org
2023-12-25T15:00:46.718541Z INFO http_request{path=/_matrix/federation/v2/invite/:room_id/:event_id}: conduit::service::rooms::event_handler: Server matrix.org was denied by server ACL in !RplyYOjorCJcPabbfX:matrix.org
So it's almost 100% effective, and more effective than the previous implementation.
Additionally, can we silence the rustc warning if you're confident this is an axum issue and not our issue?
for now this is going to be abandoned, sorry |
You can even make this part of CI, I forget to run clippy/fmt all the time before pushing |
Rethinking this now, I don't think this feature is particularly useful in practice. Malicious/offensive events will just be federated to you through other servers in the room. It's only somewhat useful for blocking DMs / room invites from a server, and even then it needs to be extended to client Additionally, a private federation setup should be done with a firewall and not purely on the application level. Conduwuit has implemented a lot of moderation utilities since this was made. If a better use-case for this comes up that existing tools cannot cover, we can look at this again. The work in the submitted PR may be repurposed or referenced for some other future things. |
adds support for ACLs in config files and in database
fixes: #31
TODO: