Skip to content

Simplify the D-Bus policy config #300

Merged
mvidner merged 2 commits intomasterfrom
dbus-policy
Nov 16, 2022
Merged

Simplify the D-Bus policy config #300
mvidner merged 2 commits intomasterfrom
dbus-policy

Conversation

@mvidner
Copy link
Contributor

@mvidner mvidner commented Nov 11, 2022

Problem

In https://bugzilla.suse.com/show_bug.cgi?id=1202059 we are asking for a security review of our D-Bus service.

But the relevant bus policy file is hard to read.

Solution

Simplify the policy file while keeping it equivalent.

Testing

  • Manually tested that the D-Bus policy simplification is still working:

Owning the bus name:

Non-root should fail

  • bundle exec bin/d-installer questions
    • Expect error /not allowed to own the service "org.opensuse.DInstaller.Questions"/

Root should succeed

  • sudo !!
    • Expect no error

Introspection call:

Non-root should fail

  • busctl introspect org.opensuse.DInstaller.Questions /org/opensuse/DInstaller/Questions1
    • Expect error /Access denied/

Root should succeed:

  • sudo !!
    • Expect /PropertiesChanged/

Question API call:

Non-root should fail

busctl \
  call \
  org.opensuse.DInstaller.Questions \
  /org/opensuse/DInstaller/Questions1 \
  org.opensuse.DInstaller.Questions1 \
  New sasas "should I stay or should I go" 2 yes no 1 yes

Root should succeed:

  • sudo !!
    • Expect /^o "/org/opensuse/

Screenshots

Not affected

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.959% when pulling 28296da on dbus-policy into 7e67366 on master.

Remove duplicated allow[@send_destination].

Sending to the Introspectable interface is the only thing allowed to
non-root clients -> moved to policy[@context="default"]

Receiving is allowed by default, no need to mention allow[@receive_sender].

See the (terse) docs at https://manpages.org/dbus-daemon
It might seem courteous to allow non-root at least introspect our API
via d-feet, since the documentation is public anyway and this would be
a guaranteed up-to-date version, right?

But our services do a lot of probing at startup, so we might have a bug
where we would modify something as root when triggered by a curious
introspector. Better not allow it.

Let them `sudo -E d-feet`.
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still LGTM :-)

@mvidner mvidner merged commit 9e89217 into master Nov 16, 2022
@mvidner mvidner deleted the dbus-policy branch November 16, 2022 10:53
@imobachgs imobachgs mentioned this pull request Nov 16, 2022
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.

3 participants