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

[Serial] Open rfcomm device as unprivileged user #526

Merged
merged 2 commits into from Apr 16, 2017

Conversation

Projects
None yet
2 participants
@cschramm
Member

cschramm commented May 24, 2016

...if he has read and write access.

This requires the watcher to wait if the device is busy which means it's opened by root (or any other user) and typically happens if ModemManager is present and probes the new device. The same method is used for PPPSupport now which previously always waited for five seconds unconditionally. While that is unnessary if ModemManager is not present, it seems to be too short if it is.

Fixes #524.

I'm not 100 % sure about the 0.1 sec delay before checking the access permissions, but if I do it immediately after creating the device, R_OK and W_OK result in False (while E_OK works, so it's not that the file would be absent).

@infirit

This comment has been minimized.

Show comment
Hide comment
@infirit

infirit May 24, 2016

Contributor

I'm not 100 % sure about the 0.1 sec delay before checking the access permissions, but if I do it immediately after creating the device, R_OK and W_OK result in False (while E_OK works, so it's not that the file would be absent).

To avoid waiting perhaps use a GFile with a GFileMonitor.

  1. Create the GFile before creating the device
  2. Create the GFileMonitor and connect to the changed signal.
  3. Create the device
  4. Wait for the permission to be applied.
  5. Do stuff
  6. Clean-up?
Contributor

infirit commented May 24, 2016

I'm not 100 % sure about the 0.1 sec delay before checking the access permissions, but if I do it immediately after creating the device, R_OK and W_OK result in False (while E_OK works, so it's not that the file would be absent).

To avoid waiting perhaps use a GFile with a GFileMonitor.

  1. Create the GFile before creating the device
  2. Create the GFileMonitor and connect to the changed signal.
  3. Create the device
  4. Wait for the permission to be applied.
  5. Do stuff
  6. Clean-up?
@cschramm

This comment has been minimized.

Show comment
Hide comment
@cschramm

cschramm May 25, 2016

Member

Step four would block forever if the user does not get the permissions which is a valid case where we would want to use mechanism.

Member

cschramm commented May 25, 2016

Step four would block forever if the user does not get the permissions which is a valid case where we would want to use mechanism.

@infirit

This comment has been minimized.

Show comment
Hide comment
@infirit

infirit May 26, 2016

Contributor

I am thinking more in the below direction. Then store whether the user can read/write/whatever and update this as attributes change.

from gi.repository import Gio, GLib

can_read_attr = Gio.FILE_ATTRIBUTE_ACCESS_CAN_READ

def on_device_changed(mon, f, other_f, event):
    if event == Gio.FileMonitorEvent.ATTRIBUTE_CHANGED:
        print("Attribute changed")
        info = f.query_info(can_read_attr, Gio.FileQueryInfoFlags.NONE)
        can_read = info.get_attribute_boolean(can_read_attr)
        print(can_read)

dev = Gio.File.new_for_path('/dev/kmem')
monitor = dev.monitor(Gio.FileMonitorFlags.NONE)
monitor.connect('changed', on_device_changed)

loop = GLib.MainLoop()
loop.run()
Contributor

infirit commented May 26, 2016

I am thinking more in the below direction. Then store whether the user can read/write/whatever and update this as attributes change.

from gi.repository import Gio, GLib

can_read_attr = Gio.FILE_ATTRIBUTE_ACCESS_CAN_READ

def on_device_changed(mon, f, other_f, event):
    if event == Gio.FileMonitorEvent.ATTRIBUTE_CHANGED:
        print("Attribute changed")
        info = f.query_info(can_read_attr, Gio.FileQueryInfoFlags.NONE)
        can_read = info.get_attribute_boolean(can_read_attr)
        print(can_read)

dev = Gio.File.new_for_path('/dev/kmem')
monitor = dev.monitor(Gio.FileMonitorFlags.NONE)
monitor.connect('changed', on_device_changed)

loop = GLib.MainLoop()
loop.run()
@cschramm

This comment has been minimized.

Show comment
Hide comment
@cschramm

cschramm May 27, 2016

Member

It's a udev rule that changes the group on my system:
KERNEL=="tty[A-Z]*[0-9]|pppox[0-9]*|ircomm[0-9]*|noz[0-9]*|rfcomm[0-9]*", GROUP="dialout"

We cannot wait for that to happen as such a rule might not be present.

Two schemes I could think of:

  1. wait for the change, but apply a timeout as it might not happen
  2. if the user gains access, start a user watcher, otherwise start a root watcher

This is somewhat close to my solution with a timeout of 0.1 s. 😉

  1. start a root watcher
  2. wait for the change
  3. if the user gains access, kill the root watcher and start a user watcher

That would be better in cases where udev is not involved or the user does not get access (he's not in the dialout group for my system) as we do not wait for the timeout, but it's a little messy otherwise.

Member

cschramm commented May 27, 2016

It's a udev rule that changes the group on my system:
KERNEL=="tty[A-Z]*[0-9]|pppox[0-9]*|ircomm[0-9]*|noz[0-9]*|rfcomm[0-9]*", GROUP="dialout"

We cannot wait for that to happen as such a rule might not be present.

Two schemes I could think of:

  1. wait for the change, but apply a timeout as it might not happen
  2. if the user gains access, start a user watcher, otherwise start a root watcher

This is somewhat close to my solution with a timeout of 0.1 s. 😉

  1. start a root watcher
  2. wait for the change
  3. if the user gains access, kill the root watcher and start a user watcher

That would be better in cases where udev is not involved or the user does not get access (he's not in the dialout group for my system) as we do not wait for the timeout, but it's a little messy otherwise.

@cschramm

This comment has been minimized.

Show comment
Hide comment
@cschramm

cschramm Nov 1, 2016

Member

I added an implementation of the second scheme. Not really tested yet.

Member

cschramm commented Nov 1, 2016

I added an implementation of the second scheme. Not really tested yet.

@infirit

This comment has been minimized.

Show comment
Hide comment
@infirit

infirit Feb 3, 2017

Contributor

I don't really have any real serial devices to test so as long as it works ok for you 👍

Contributor

infirit commented Feb 3, 2017

I don't really have any real serial devices to test so as long as it works ok for you 👍

cschramm added some commits May 22, 2016

[Serial] Open rfcomm device as unprivileged user
...if he has read and write access.

This requires the watcher to wait if the device is busy which means it's opened by root (or any other user) and typically happens if ModemManager is present and probes the new device. The same method is used for PPPSupport now which previously always waited for five seconds unconditionally. While that is unnessary if ModemManager is not present, it seems to be too short if it is.
@cschramm

This comment has been minimized.

Show comment
Hide comment
@cschramm

cschramm Apr 15, 2017

Member

Just half a year later 🙈 I finally rebased on master, played with this a little bit, and fixed the second commit. Two issues there: Obviously the user cannot kill the root watcher himself but has to use mechanism for that. Second, when trying this, the change did not take place for me, probably because permissions are already set when the monitor gets set up.

As a solution, I now check permissions once right after that's done. There might be a race condition where both the explicit check and a change event take place but I guess that's not too bad. The only issue I could think of is that we could start two user watchers for the same port.

Overall the current state works fine for me. I can connect to the serial service and get a root watcher that immediately gets replaced by a user watcher and can then connect to my test device (a Naze32) as if it would be connected via USB.

There's only one inconvenience: I have to disable ModemManager. If it probes the device no connection is possible afterward. I guess the commands it sends bring the Naze32 into some strange state so that the software cannot address it. That does not happen with USB but I guess that's just ModemManager checking on rfcomm devices but not ttyUSB, so there's little to nothing blueman could do about that.

Member

cschramm commented Apr 15, 2017

Just half a year later 🙈 I finally rebased on master, played with this a little bit, and fixed the second commit. Two issues there: Obviously the user cannot kill the root watcher himself but has to use mechanism for that. Second, when trying this, the change did not take place for me, probably because permissions are already set when the monitor gets set up.

As a solution, I now check permissions once right after that's done. There might be a race condition where both the explicit check and a change event take place but I guess that's not too bad. The only issue I could think of is that we could start two user watchers for the same port.

Overall the current state works fine for me. I can connect to the serial service and get a root watcher that immediately gets replaced by a user watcher and can then connect to my test device (a Naze32) as if it would be connected via USB.

There's only one inconvenience: I have to disable ModemManager. If it probes the device no connection is possible afterward. I guess the commands it sends bring the Naze32 into some strange state so that the software cannot address it. That does not happen with USB but I guess that's just ModemManager checking on rfcomm devices but not ttyUSB, so there's little to nothing blueman could do about that.

@infirit

This comment has been minimized.

Show comment
Hide comment
@infirit

infirit Apr 15, 2017

Contributor

If it works for you 👍. But we then have to deal with grep in PPPSupport, #601 (comment). Shall I do the same as I did in 2.0 stable for master?

Contributor

infirit commented Apr 15, 2017

If it works for you 👍. But we then have to deal with grep in PPPSupport, #601 (comment). Shall I do the same as I did in 2.0 stable for master?

@cschramm cschramm merged commit 7d2cc04 into blueman-project:master Apr 16, 2017

@cschramm

This comment has been minimized.

Show comment
Hide comment
@cschramm

cschramm Apr 16, 2017

Member

Makes sense, yes. 👍

Member

cschramm commented Apr 16, 2017

Makes sense, yes. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment