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

Hardcoded security limits #148

Open
zeenix opened this issue Oct 7, 2024 · 1 comment
Open

Hardcoded security limits #148

zeenix opened this issue Oct 7, 2024 · 1 comment
Labels
feature parity Feature parity with existing bus impls security

Comments

@zeenix
Copy link
Collaborator

zeenix commented Oct 7, 2024

The existing dbus brokers implement some security limits, that we should as well. However, we should just hardcode their values instead of allowing admins/users to modify them. Here is the relevant snippet from the configuration XML for system bus:

  <!-- The defaults for these limits are hard-coded in dbus-daemon.
       Some clarifications:
       Times are in milliseconds (ms); 1000ms = 1 second
       133169152 bytes = 127 MiB
       33554432 bytes = 32 MiB
       150000ms = 2.5 minutes -->
  <!-- <limit name="max_incoming_bytes">133169152</limit> -->
  <!-- <limit name="max_incoming_unix_fds">64</limit> -->
  <!-- <limit name="max_outgoing_bytes">133169152</limit> -->
  <!-- <limit name="max_outgoing_unix_fds">64</limit> -->
  <!-- <limit name="max_message_size">33554432</limit> -->
  <!-- <limit name="max_message_unix_fds">16</limit> -->
  <!-- <limit name="service_start_timeout">25000</limit> -->
  <!-- <limit name="auth_timeout">5000</limit> -->
  <!-- <limit name="pending_fd_timeout">150000</limit> -->
  <!-- <limit name="max_completed_connections">2048</limit> -->
  <!-- <limit name="max_incomplete_connections">64</limit> -->
  <!-- <limit name="max_connections_per_user">256</limit> -->
  <!-- <limit name="max_pending_service_starts">512</limit> -->
  <!-- <limit name="max_names_per_connection">512</limit> -->
  <!-- <limit name="max_match_rules_per_connection">512</limit> -->
  <!-- <limit name="max_replies_per_connection">128</limit> -->

We could consider making some/all of the values configurable as well, if a compelling-enough use case arrives. I just haven't heard or seen anyone ever modifying the defaults for these values. Also, if a specific hardcoded value isn't high/low enough, we should first look into changing the value or try to come up with a smarter way to determine a good value automatically, before resorting to throwing it to the admin/user.

@zeenix zeenix added feature parity Feature parity with existing bus impls security labels Oct 7, 2024
@zeenix
Copy link
Collaborator Author

zeenix commented Oct 7, 2024

Hmm.. actually it turns out that for session bus, the existing brokers do override the values:

  <!-- For the session bus, override the default relatively-low limits
       with essentially infinite limits, since the bus is just running
       as the user anyway, using up bus resources is not something we need
       to worry about. In some cases, we do set the limits lower than
       "all available memory" if exceeding the limit is almost certainly a bug,
       having the bus enforce a limit is nicer than a huge memory leak. But the
       intent is that these limits should never be hit. -->

  <!-- the memory limits are 1G instead of say 4G because they can't exceed 32-bit signed int max -->
  <limit name="max_incoming_bytes">1000000000</limit>
  <limit name="max_incoming_unix_fds">250000000</limit>
  <limit name="max_outgoing_bytes">1000000000</limit>
  <limit name="max_outgoing_unix_fds">250000000</limit>
  <limit name="max_message_size">1000000000</limit>
  <!-- We do not override max_message_unix_fds here since the in-kernel
       limit is also relatively low -->
  <limit name="service_start_timeout">120000</limit>
  <limit name="auth_timeout">240000</limit>
  <limit name="pending_fd_timeout">150000</limit>
  <limit name="max_completed_connections">100000</limit>
  <limit name="max_incomplete_connections">10000</limit>
  <limit name="max_connections_per_user">100000</limit>
  <limit name="max_pending_service_starts">10000</limit>
  <limit name="max_names_per_connection">50000</limit>
  <limit name="max_match_rules_per_connection">50000</limit>
  <limit name="max_replies_per_connection">50000</limit>

We could still handle session with hardcoded values though. Also, any broker instance with custom configuration can inherit the limits of session bus (we could think of some simple cli arg for specifying this).

jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 10, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 10, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 10, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 19, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 19, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 27, 2024
zeenix pushed a commit to jokeyrhyme/busd that referenced this issue Nov 27, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 30, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature parity Feature parity with existing bus impls security
Projects
None yet
Development

No branches or pull requests

1 participant