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

ACL for servers support #35

Closed

Conversation

NinekoTheCat
Copy link
Contributor

@NinekoTheCat NinekoTheCat commented Dec 24, 2023

adds support for ACLs in config files and in database
fixes: #31
TODO:

  • add commands to manage ACL
  • add entry
  • remove entry
  • list entries (filtering for allow or block lists)

@NinekoTheCat NinekoTheCat marked this pull request as ready for review December 24, 2023 16:46
Copy link
Owner

@girlbossceo girlbossceo left a 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.

src/service/acl/data.rs Outdated Show resolved Hide resolved
src/service/acl/data.rs Outdated Show resolved Hide resolved
src/service/acl/mod.rs Outdated Show resolved Hide resolved
src/database/key_value/acl.rs Outdated Show resolved Hide resolved
src/service/acl/data.rs Outdated Show resolved Hide resolved
src/service/acl/mod.rs Outdated Show resolved Hide resolved
src/service/acl/mod.rs Outdated Show resolved Hide resolved
src/service/acl/mod.rs Outdated Show resolved Hide resolved
src/service/mod.rs Outdated Show resolved Hide resolved
NinekoTheCat and others added 4 commits December 24, 2023 20:26
…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.
Cargo.toml Outdated Show resolved Hide resolved
Co-authored-by: June <june@girlboss.ceo>
Copy link
Owner

@girlbossceo girlbossceo left a 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.

src/service/admin/mod.rs Outdated Show resolved Hide resolved
src/service/rooms/event_handler/mod.rs Show resolved Hide resolved
src/service/admin/mod.rs Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
Co-authored-by: June <june@girlboss.ceo>
Copy link
Owner

@girlbossceo girlbossceo left a 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?

@NinekoTheCat
Copy link
Contributor Author

for now this is going to be abandoned, sorry

@girlbossceo girlbossceo marked this pull request as draft December 29, 2023 19:39
@avdb13
Copy link
Contributor

avdb13 commented Feb 28, 2024

So far just ran cargo clippy and found these issues, and just a couple concerns.

You can even make this part of CI, I forget to run clippy/fmt all the time before pushing

@girlbossceo
Copy link
Owner

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 /sync not just on the receiving federation endpoints.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Federation explicit denylist and allowlist support
3 participants