All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: D-Bus policies
@ 2022-01-25 21:48 Diederik de Haas
  0 siblings, 0 replies; 7+ messages in thread
From: Diederik de Haas @ 2022-01-25 21:48 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 3514 bytes --]

Hi Denis,

Thanks a lot for your response :-)

On Tuesday, 25 January 2022 16:38:09 CET Denis Kenzior wrote:
> On 1/25/22 08:49, Diederik de Haas wrote:
> > On Saturday, 22 January 2022 00:13:14 CET Denis Kenzior wrote:
> >> I think the solution chosen in BlueZ which gives blanket permission to
> >> access iwd D-Bus APIs to any local user is probably just fine.
> >> Particularly given that at_console effectively allowed any user to use
> >> iwd in the past.
> >> This effectively negates the need to provide a separate policy for
> >> wheel/netdev and so these can be removed.
> > 
> > Does this mean that any restrictions would be removed?
> 
> This would be similar to BlueZ, where any user could control the bluetooth
> device.

Ok, so Option 1 is to lift all restrictions.

> > Looking a bit further to see what others do and found some interesting
> > things.
> > 
> > In /etc/dbus-1/system.d/wpa_supplicant.conf I see 'allow' for 'root' and
> > 'netdev' and (explicit) deny for 'context="default"'
> > 
> > In /usr/share/dbus-1/system.d/org.freedesktop.NetworkManager.conf there is
> 
> Right, I don't think we really want to go that far and our APIs are quite
> simple compared to NM.  Our goal is to ship a minimal policy that works for
> most users out of the box. 

Agreed that NM is far more extensive. I included it as a 2nd example where 
deny is the default policy, with some explicit exceptions.
But wpasupplicant's policy is far simpler (default deny, root/netdev allow) 
and that program seems comparable to iwd.

> If the distros need something else, they're free to modify the policy as
> they see fit.

Of course they have that freedom, but as noted in my OP, most distros just use 
what's provided upstream. 

> >> Alternatively we can limit the policy only to wheel/netdev groups.
> > 
> > Given the above, I am more inclined to this solution, but it appears to me
> > like pretty much the opposite of the other solution?
> 
> Sort of.  I think in reality the vast majority of users are on single-user
> systems with the main user already part of the 'wheel' group. So even if we
> drop the at_console usage, and allowed iwd use only for root/netdev/wheel,
> the impact should be minimal.

When I setup a new Debian system, I *manually* add myself to "dialout, adm, 
plugdev, netdev, audio, video" groups, so this is not an automatic thing.
I just added a 2nd user on my system and its only group membership was to a 
group named the same as the user. This is the default behavior, but it may be 
(a bit) different for the first user, but likely also depends on how the system 
is setup.
The 'wheel' group is not used/present on Debian (based distros).

So I think the impact is not minimal. OTOH, it should be common knowledge that 
users need netdev group permissions to modify network connections.

> > For example, if I granted access to a friend (over SSH) to my
> > machine, he was allowed access to iwd over D-Bus before, but because he's
> > not in the netdev group, he would then be denied access.
> 
> Correct.  But how likely is this scenario? :)

It's the default scenario on Debian and possibly its derivatives.
FTR: I don't want my friend(s) granted permission to change the network 
connection by default. If I want to grant a friend that permission, I would 
add him/her to the netdev group.

And Option 2 is restricted access.

For which Option would you like a patch?

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: D-Bus policies
@ 2022-01-25 22:43 Diederik de Haas
  0 siblings, 0 replies; 7+ messages in thread
From: Diederik de Haas @ 2022-01-25 22:43 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]

On Tuesday, 25 January 2022 23:15:07 CET Denis Kenzior wrote:
> Sure, but the chances that the main user on a single-user system isn't part
> of 'wheel' or 'netdev' should be pretty small, no?

No, but it is easy to 'fix' for the main user.
When you install a Debian system, you can explicitly use the 'root' user and 
then you specify root's password. Next to that a normal user account gets 
created. Alternatively, you can use the sudo system which the first user, ie 
you, gets added to and then you can do all root-type actions via sudo.
So the person installing the system does get full administrative permissions, 
which he/she can use to further setup the system.

> The original intent was to disallow access to iwd for remote users.  So, in
> the scenario above, if your friend is not part of 'netdev' or 'wheel', then
> their control of iwd should not have been possible.

Correct. But in the restricted scenario (option 2), my friend would also not 
have access if directly logged on to the system (locally), if not in the 
'netdev' group.
This would thus be a change in behavior as originally intended.
 
> > And Option 2 is restricted access.
> 
> Right.  We'd be 'breaking' the above scenario.  But, as I mentioned before,
> how likely was this scenario in the first place?  Did someone really want
> remote users to mess with wifi on their machine without netdev/wheel
> access?  Arguably we're just fixing a bug.

I think we're indeed fixing a bug by allowing only netdev/wheel users the 
permission to change the wifi settings.

> > For which Option would you like a patch?
> 
> I'm fine with either option.  It sounds like you're favoring Option 2.
> Whichever you go for, I'll let it sit on the list for a few days to see if
> anyone else complains :)

Yeah, I do favor Option 2 ;-)
A patch is coming your/the list's way (probably tomorrow, CET TZ here)

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: D-Bus policies
@ 2022-01-25 22:15 Denis Kenzior
  0 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2022-01-25 22:15 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 2380 bytes --]

Hi Diederik,

>> Sort of.  I think in reality the vast majority of users are on single-user
>> systems with the main user already part of the 'wheel' group. So even if we
>> drop the at_console usage, and allowed iwd use only for root/netdev/wheel,
>> the impact should be minimal.
> 
> When I setup a new Debian system, I *manually* add myself to "dialout, adm,
> plugdev, netdev, audio, video" groups, so this is not an automatic thing.
> I just added a 2nd user on my system and its only group membership was to a
> group named the same as the user. This is the default behavior, but it may be
> (a bit) different for the first user, but likely also depends on how the system
> is setup.
> The 'wheel' group is not used/present on Debian (based distros).

Sure, but the chances that the main user on a single-user system isn't part of 
'wheel' or 'netdev' should be pretty small, no?

> 
> So I think the impact is not minimal. OTOH, it should be common knowledge that
> users need netdev group permissions to modify network connections.
> 

Agreed.

>>> For example, if I granted access to a friend (over SSH) to my
>>> machine, he was allowed access to iwd over D-Bus before, but because he's
>>> not in the netdev group, he would then be denied access.
>>
>> Correct.  But how likely is this scenario? :)
> 
> It's the default scenario on Debian and possibly its derivatives.
> FTR: I don't want my friend(s) granted permission to change the network
> connection by default. If I want to grant a friend that permission, I would
> add him/her to the netdev group.

The original intent was to disallow access to iwd for remote users.  So, in the 
scenario above, if your friend is not part of 'netdev' or 'wheel', then their 
control of iwd should not have been possible.

> 
> And Option 2 is restricted access.

Right.  We'd be 'breaking' the above scenario.  But, as I mentioned before, how 
likely was this scenario in the first place?  Did someone really want remote 
users to mess with wifi on their machine without netdev/wheel access?  Arguably 
we're just fixing a bug.

> 
> For which Option would you like a patch?
> 

I'm fine with either option.  It sounds like you're favoring Option 2. 
Whichever you go for, I'll let it sit on the list for a few days to see if 
anyone else complains :)

Regards,
-Denis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: D-Bus policies
@ 2022-01-25 15:38 Denis Kenzior
  0 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2022-01-25 15:38 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 5123 bytes --]

Hi Diederik,

On 1/25/22 08:49, Diederik de Haas wrote:
> Hi Denis,
> 
> On Saturday, 22 January 2022 00:13:14 CET Denis Kenzior wrote:
>> Care to send a patch?
> 
> I technically can, but before doing that I'd like some clarification on the
> impact of such a patch as I have no-where near enough understanding of both
> D-Bus and iwd of the impact of such a patch. I 'prefer' not to have my name
> attached to a (potential) security breach ;-)
> 

No worries.

>>> Looking into possible solutions, I found 2 very similar commits ...
>>>
>>> They both link to
>>> https://www.spinics.net/lists/linux-bluetooth/msg75267.html While I lack
>>> the knowledge to fully understand what it says I did notice this: "The
>>> intent is clear: As long as you are logged in to a local machine, and you
>>> are the foreground/active console, you are allowed to control bluetooth.
>>> However, the behavior of 'at_console' does *not* match this intent."
>>>
>>> In other places I saw the 'at_console' stanza just plainly removed without
>>> any replacement, but it could have undesirable consequences for iwd.
>>
>> I think the solution chosen in BlueZ which gives blanket permission to
>> access iwd D-Bus APIs to any local user is probably just fine.
>> Particularly given that at_console effectively allowed any user to use iwd
>> in the past.
>> This effectively negates the need to provide a separate policy for
>> wheel/netdev and so these can be removed.
> 
> Does this mean that any restrictions would be removed?
>

Yes, basically any logged in user could use iwctl or dbus-send to control iwd 
via the D-Bus API.  So any user could switch networks, power device up and down, 
etc.

This would be similar to BlueZ, where any user could control the bluetooth device.


> It is just an example, but the 'deluged' program runs under the 'debian-
> deluged' user. If that program has a (security) bug, would that allow a RCE
> wrt to iwd? Could it wreak havoc then?
> 

In theory, not really, since it would still have to go through the D-Bus API and 
the associated checks.  So it couldn't execute arbitrary code, but it could do 
anything that a normal user can do as described above.

> Looking a bit further to see what others do and found some interesting things.
> 
> https://salsa.debian.org/bluetooth-team/bluez/-/blob/debian/sid/debian/
> patches/bluetooth.conf.patch is a modification of what upstream bluetooth does
> and adds an allow for the 'bluetooth' group. That patch was explicitly added,
> but without an explanation as to why, but as I understand it now, it means
> that 'bluetooth' group members get access, next to everyone else.
> IOW, it _seems_ (to me) like a pointless patch?

Seems pointless to me as well.  The intent seems to be to allow Bluetooth 
control only by the users of the 'bluetooth' group.  But I don't think that's 
what is actually happening due to the context="default" hole being punched.

> 
> In /etc/dbus-1/system.d/wpa_supplicant.conf I see 'allow' for 'root' and
> 'netdev' and (explicit) deny for 'context="default"'
> 
> In /usr/share/dbus-1/system.d/org.freedesktop.NetworkManager.conf there is a
> FAR more extensive policy, where 'root' can (ofc) do a LOT, but in the default
> context it starts with a 'deny' and then makes exceptions to grant mostly
> ReadOnly access to certain explicitly defined properties. In the comments I see
> things like "read-only properties, no methods" and "read-only, no security
> required".
> There seem to be some properties/methods ReadWrite, but additionally secured
> via PolicyKit ("read/write, secured with PolicyKit" in comments).
> 

Right, I don't think we really want to go that far and our APIs are quite simple 
compared to NM.  Our goal is to ship a minimal policy that works for most users 
out of the box.  If the distros need something else, they're free to modify the 
policy as they see fit.

>> Alternatively we can limit the policy only to wheel/netdev groups.
> 
> Given the above, I am more inclined to this solution, but it appears to me
> like pretty much the opposite of the other solution?

Sort of.  I think in reality the vast majority of users are on single-user 
systems with the main user already part of the 'wheel' group.  So even if we 
drop the at_console usage, and allowed iwd use only for root/netdev/wheel, the 
impact should be minimal.

> But it could disallow use cases which iwd was meant to serve, which may be
> undesirable. For example, if I granted access to a friend (over SSH) to my
> machine, he was allowed access to iwd over D-Bus before, but because he's not
> in the netdev group, he would then be denied access.

Correct.  But how likely is this scenario? :)

> 
> It could be that iwd only makes a (very) limited(/harmless?) set of
> functionality available over D-Bus, in which case my concerns are unfounded.
> But I lack the knowledge to determine that.
> 
> Hope you can alleviate my concerns (before I send in a patch).
> 
> Cheers,
>    Diederik
> 


Regards,
-Denis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: D-Bus policies
@ 2022-01-25 14:49 Diederik de Haas
  0 siblings, 0 replies; 7+ messages in thread
From: Diederik de Haas @ 2022-01-25 14:49 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]

Hi Denis,

On Saturday, 22 January 2022 00:13:14 CET Denis Kenzior wrote:
> Care to send a patch?

I technically can, but before doing that I'd like some clarification on the 
impact of such a patch as I have no-where near enough understanding of both 
D-Bus and iwd of the impact of such a patch. I 'prefer' not to have my name 
attached to a (potential) security breach ;-)

> > Looking into possible solutions, I found 2 very similar commits ...
> > 
> > They both link to
> > https://www.spinics.net/lists/linux-bluetooth/msg75267.html While I lack
> > the knowledge to fully understand what it says I did notice this: "The
> > intent is clear: As long as you are logged in to a local machine, and you
> > are the foreground/active console, you are allowed to control bluetooth.
> > However, the behavior of 'at_console' does *not* match this intent."
> > 
> > In other places I saw the 'at_console' stanza just plainly removed without
> > any replacement, but it could have undesirable consequences for iwd.
> 
> I think the solution chosen in BlueZ which gives blanket permission to
> access iwd D-Bus APIs to any local user is probably just fine. 
> Particularly given that at_console effectively allowed any user to use iwd
> in the past.
> This effectively negates the need to provide a separate policy for
> wheel/netdev and so these can be removed.

Does this mean that any restrictions would be removed?

It is just an example, but the 'deluged' program runs under the 'debian-
deluged' user. If that program has a (security) bug, would that allow a RCE 
wrt to iwd? Could it wreak havoc then?

Looking a bit further to see what others do and found some interesting things.

https://salsa.debian.org/bluetooth-team/bluez/-/blob/debian/sid/debian/
patches/bluetooth.conf.patch is a modification of what upstream bluetooth does 
and adds an allow for the 'bluetooth' group. That patch was explicitly added, 
but without an explanation as to why, but as I understand it now, it means 
that 'bluetooth' group members get access, next to everyone else.
IOW, it _seems_ (to me) like a pointless patch?

In /etc/dbus-1/system.d/wpa_supplicant.conf I see 'allow' for 'root' and 
'netdev' and (explicit) deny for 'context="default"'

In /usr/share/dbus-1/system.d/org.freedesktop.NetworkManager.conf there is a 
FAR more extensive policy, where 'root' can (ofc) do a LOT, but in the default 
context it starts with a 'deny' and then makes exceptions to grant mostly 
ReadOnly access to certain explicitly defined properties. In the comments I see 
things like "read-only properties, no methods" and "read-only, no security 
required".
There seem to be some properties/methods ReadWrite, but additionally secured 
via PolicyKit ("read/write, secured with PolicyKit" in comments).

> Alternatively we can limit the policy only to wheel/netdev groups.

Given the above, I am more inclined to this solution, but it appears to me 
like pretty much the opposite of the other solution?
But it could disallow use cases which iwd was meant to serve, which may be 
undesirable. For example, if I granted access to a friend (over SSH) to my 
machine, he was allowed access to iwd over D-Bus before, but because he's not 
in the netdev group, he would then be denied access.

It could be that iwd only makes a (very) limited(/harmless?) set of 
functionality available over D-Bus, in which case my concerns are unfounded. 
But I lack the knowledge to determine that.

Hope you can alleviate my concerns (before I send in a patch).

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: D-Bus policies
@ 2022-01-21 23:13 Denis Kenzior
  0 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2022-01-21 23:13 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]

Hi Diederik,

> Looking into possible solutions, I found 2 very similar commits, but in
> different projects, bluez and system-config-printer:
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=3ef0ce954b66fdf45538a6cdc629f3dac6642832
> https://github.com/OpenPrinting/system-config-printer/commit/19df47d2630b637d1802efe2c3cd5a00f2e40c3b
> 
> They both link to https://www.spinics.net/lists/linux-bluetooth/msg75267.html
> While I lack the knowledge to fully understand what it says I did notice this:
> "The intent is clear: As long as you are logged in to a local machine, and you
> are the foreground/active console, you are allowed to control bluetooth.
> However, the behavior of 'at_console' does *not* match this intent."
> 
> In other places I saw the 'at_console' stanza just plainly removed without
> any replacement, but it could have undesirable consequences for iwd.

I think the solution chosen in BlueZ which gives blanket permission to access 
iwd D-Bus APIs to any local user is probably just fine.  Particularly given that 
at_console effectively allowed any user to use iwd in the past.
This effectively negates the need to provide a separate policy for wheel/netdev 
and so these can be removed.

Alternatively we can limit the policy only to wheel/netdev groups.

Care to send a patch?

Regards,
-Denis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* D-Bus policies
@ 2022-01-14 17:15 Diederik de Haas
  0 siblings, 0 replies; 7+ messages in thread
From: Diederik de Haas @ 2022-01-14 17:15 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 2694 bytes --]

Hi,

Based on a Debian bug report I started a research into iwd's D-Bus policy and
found 2 items. I'll start with the 2nd as that's easier/shorter to describe.
This is purely informational as I'm not knowledgeable enough about iwd or 
D-Bus or how iwd intends to use DBus for certain functionality.

1) In src/iwd-dbus.conf I saw there was a policy for the wheel group, but not
for the netdev group. The wheel group is normally not used on Debian systems,
but the netdev group is. According to https://wiki.debian.org/SystemGroups: 
"netdev: Members of this group can manage network interfaces through
the network manager and wicd."

I have found (only) one distro which actually patches iwd to add netdev:
https://git.alpinelinux.org/aports/tree/community/iwd/dbus-netdev-group.patch 
The rest that _I_ have found just use what's provided by iwd.

2) The bug that started my research is https://bugs.debian.org/998427, saying:
"dbus-broker-launch[2169]: Deprecated policy context in 
/usr/share/dbus-1/system.d/iwd-dbus.conf +21. The 'at_console' context
is deprecated and will be ignored in the future."

It is also a warning in Debian's Lintian tool:
https://lintian.debian.org/tags/dbus-policy-at-console which links to
https://bugs.freedesktop.org/39611 which is moved/continued at
https://gitlab.freedesktop.org/dbus/dbus/-/issues/52
The OP of that bug from 2011 states that the 'at_console' property should
be removed and that PolicyKit should be used instead.

Looking into possible solutions, I found 2 very similar commits, but in
different projects, bluez and system-config-printer:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=3ef0ce954b66fdf45538a6cdc629f3dac6642832
https://github.com/OpenPrinting/system-config-printer/commit/19df47d2630b637d1802efe2c3cd5a00f2e40c3b

They both link to https://www.spinics.net/lists/linux-bluetooth/msg75267.html
While I lack the knowledge to fully understand what it says I did notice this:
"The intent is clear: As long as you are logged in to a local machine, and you
are the foreground/active console, you are allowed to control bluetooth.
However, the behavior of 'at_console' does *not* match this intent."

In other places I saw the 'at_console' stanza just plainly removed without
any replacement, but it could have undesirable consequences for iwd.

The arch wiki does contain a section to restrict the 'at_console' policy:
https://wiki.archlinux.org/title/Iwd#Deny_console_(local)_user_from_modifying_the_settings
It appears that they make the, likely incorrect, assumption about console
users, but they do restrict its permissions to mostly ReadOnly.

HTH,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-01-25 22:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-25 21:48 D-Bus policies Diederik de Haas
  -- strict thread matches above, loose matches on Subject: below --
2022-01-25 22:43 Diederik de Haas
2022-01-25 22:15 Denis Kenzior
2022-01-25 15:38 Denis Kenzior
2022-01-25 14:49 Diederik de Haas
2022-01-21 23:13 Denis Kenzior
2022-01-14 17:15 Diederik de Haas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.