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

allow non-root users to use ddccontrol #5

Closed
genodeftest opened this issue Apr 26, 2015 · 28 comments

Comments

Projects
None yet
7 participants
@genodeftest
Copy link

commented Apr 26, 2015

To allow non-root users to run ddccontrol we need to do these steps:

  1. install i2c-tools
  2. on installing ddccontrol install a (system) group i2c, e.g. by running groupadd --system i2cdev
  3. add a udev rule to /etc/udev/rules.d/ or /usr/lib/udev/rules.d/ allowing all members of group i2c read/write access to all i2c devices. Create a file (e.g. called 30-i2c-tools.rules) containing this line: KERNEL=="i2c-[0-9]*", GROUP="i2c", MODE="0660"
  4. make sure the kernel module i2c-dev gets loaded.
    (5. add user to group i2c – this step has to be done by the user/admin. Suggest that to the user in case ddccontrol can't find a monitor to connect to. Command: usermod -G i2cdev username)

With these steps (g)ddccontrol can be run as ordinary user.

(source/idea: http://www.techytalk.info/debian-ubuntu-gddccontrol-non-root/ and debian package, https://packages.debian.org/stable/i2c-tools )

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 24, 2017

This is:

  • good, would solve: #12,
  • but, also bad, could make a security hole - can access any i2c device. It would be better to run some kind of daemon in safe mode (allow only safe writes/reads - verify arguments), and use client communicating with the safe-daemon for non-root users.
@genodeftest

This comment has been minimized.

Copy link
Author

commented Jun 24, 2017

but, also bad, could make a security hole

Hm, that's true. So it should probably use PolicyKit instead. That would require major changes though.

@Gluttton

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2017

I'm not sure that this is security hole, because groupadd and usermod must be performed with root access.

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 24, 2017

The security hole lies at complete access to all i2c devices. Not at becoming i2c group member. And, this could have following consequences:

  • grants access to also non-display i2c devices,
  • ability to send any data, and certain sequences might be harmful.
@cheater

This comment has been minimized.

Copy link

commented May 21, 2018

This could easily turn into a huge security risk, especially given that you have parsing in the same code base. Do not do this unless you have had the code audited by external parties, professionally. Strongly suggest using something like Rust for any code doing this, rather than C. Yes, this would be a major undertaking.

@cheater

This comment has been minimized.

Copy link

commented May 21, 2018

In fact, you could easily imagine someone with non-root physical access attaching a malicious hdmi device (a tiny dongle with a hdmi plug on it) that performs an overflow on your parser, and allows non-root calls to ddccontrol to provide a root shell. Building something like this is less involved than one might think.

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

@cheater well you've commented after #34 was merged. But, it's a valid point. I also test daemon with valgrind. But, there might still be some vulnerabilities.

However, daemon can run under a different user, which will be given privileges to access i2c devices.

From security point of view, daemon launched as a different user might be the most safe way of execution:

  • no root access,
  • no access to data of current user (assuming user has set correct permissions on his filesystem,...).
@cheater

This comment has been minimized.

Copy link

commented Jun 18, 2018

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

Do not do this unless you have had the code audited by external parties, professionally.

ddccontrol is not very used software, though,.. Mostly by individuals, who use it at their home PC, with poor lighting conditions, to adjust brightness, or to play with their monitor's settings. So, there's no huge risk of damage caused by exploiting ddccontrol,...

In order to perform attack:

  • attacker, or companion, must be in physical presence of target computer,
  • must know and be sure, that target computer uses Linux and has running ddccontrol daemon,
  • must be capable to create HDMI dongle, which can exploit ddccontrol,
  • must it be worth it,...

I cannot imagine people investing their time to perform real attack. I can imagine someone doing that to prove that it's possible, or to have fun,... But, not in real world.... It's not something, which would be running on servers, and computers of common users are not that much of target... And, if they are a target, it's easier to steal whole computer, and access non-encrypted HDD.

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

this will not be done by package maintainers.

@cheater I thought of two options:

  • incorporate 'daemon execution under different user' to upstream,
  • add non-dbus variants to upstream.
@StefanBruens

This comment has been minimized.

Copy link

commented Jun 18, 2018

Of course package maintainers are capable of creating packages which run system services as unprivileged system users. Have a look at cups (runs as lp), avahi, ...

@cheater

This comment has been minimized.

Copy link

commented Jun 18, 2018

@cheater

This comment has been minimized.

Copy link

commented Jun 18, 2018

@cheater

This comment has been minimized.

Copy link

commented Jun 18, 2018

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

@cheater

i disagree with your security analysis - it is not backed up by any sort of
formal infosec research

Well, neither is your's disagreement... But, as you stated:

... you could easily imagine someone with non-root physical access attaching a malicious hdmi device (a tiny dongle with a hdmi plug on it) that performs an overflow on your parser ...

And, with this attitude:

it's also why not having capabilities in the firstvplace is a much better idea than trying to secure their use.

I shouldn't run any system daemon, because it can be target of an exploit,.. Especially, daemon listening to network (agree on that), but also other daemons,... Mainly, I'd rather run no kernel, because drivers have got lot's of capabilities, too. (sorry for irony,... but,...)

I agree, that it's important to think about security. But, this level could be called paranoia.

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

Of course package maintainers are capable of creating packages which run system services as unprivileged system users. Have a look at cups (runs as lp), avahi, ...

Good point. However, I'm pessimistic about this, too. I doubt, that package maintainers will be willing to do that. I assume, that they've got different priorities, than securing ddccontrol package, unless it's done by some individuals contribution.

So, I thought to prepare upstream sources for everything. And, package would just need to create an user for daemon execution... Or, find some other way to setup secure privileged access to i2c devices for daemon,...

EDIT: but I also doubt, that ddccontrol will be exploit target, at least not so soon,...

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

Closing, implemented in #34, #35. See also #38.

@kravemir kravemir closed this Jun 18, 2018

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

Security concerns will be solved as part of #50.

@cheater, or anybody else, feel free to propose solution ideas/designs, if you like. I'll be glad for it.

@cheater

This comment has been minimized.

Copy link

commented Jun 18, 2018

@StefanBruens

This comment has been minimized.

Copy link

commented Jun 18, 2018

Separating the agent from the GUI is a definitive security win. Being able to cache e.g. EDID data is just a nice side effect.

@StefanBruens

This comment has been minimized.

Copy link

commented Jun 18, 2018

I don't know how other distributions work, but on openSUSE every new package with elevated privileges is security audited. And doing proper packaging is the first reason for distributions to exist.

@cheater

This comment has been minimized.

Copy link

commented Jun 18, 2018

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

@cheater ok, then from security perspective, daemon running under different user is gain,... It's better, than running daemon under root, or running previous implementation with sudo/gksu/...

And, I'm not taking this lightly,... As for now, you've done more than enough criticism. Though, I appreciate your effort, this conversation leads nowhere good anymore. Rather than continuing conversation as argumentation against/for d-bus daemon, we could work on solution,.. D-Bus daemon is gain, security is important,... let's just work on that, and make everybody happy.

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 18, 2018

@cheater if you have some solution design in mind, you're free to propose it. I'm willing to put my effort to implement it.

@cheater

This comment has been minimized.

Copy link

commented Jun 18, 2018

@kravemir

This comment has been minimized.

Copy link
Member

commented Jun 19, 2018

i would consider building this part in rust, as it has secure parsers and a
markedly lower prevalence of exploits than other languages suited for low
level tasks like this (python, C).

I have opened #57 for rewrite discussion.

as for the daemon and the command line tool, i would much rather have the
daemon just cache data

It would introduce too much spaghetti to communication. I'd rather have layered approach:

  • daemon performing HW access with security in mind,
  • GUI/CLI clients exposing DDC/CI control to user.
@cheater

This comment has been minimized.

Copy link

commented Jun 19, 2018

@fingerblaster

This comment has been minimized.

Copy link

commented Jun 5, 2019

KERNEL=="i2c-[0-9]", ATTRS{name}=="dmdc", GROUP="i2c", MODE="0660"

  • i2c-0 unknown SMBus I801 adapter at f000 N/A
  • i2c-1 unknown Synopsys DesignWare I2C adapter N/A
  • i2c-2 unknown Synopsys DesignWare I2C adapter N/A
  • i2c-3 unknown AMDGPU DM i2c hw bus 0 N/A
  • i2c-4 i2c dmdc I2C adapter
  • i2c-5 unknown AMDGPU DM i2c hw bus 1 N/A
  • i2c-6 i2c dmdc I2C adapter
  • i2c-7 unknown AMDGPU DM i2c hw bus 2 N/A
  • i2c-8 unknown AMDGPU DM i2c hw bus 3 N/A
  • i2c-9 unknown AMDGPU DM i2c hw bus 4 N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.