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

Additional permissions for removable media #190

Closed
popey opened this issue Jan 31, 2020 · 9 comments
Closed

Additional permissions for removable media #190

popey opened this issue Jan 31, 2020 · 9 comments

Comments

@popey
Copy link

popey commented Jan 31, 2020

Hey! I maintain the gphotos-sync snap in the Snap Store. In more recent builds I note there's been a change to the way gphotos-sync scans the filesystem. This causes the snap to crash, because confinement prevents it enumerating things in /proc/*/mounts (see traceback below).

Now, this isn't a massive issue. I have modified the snap to add the mount-observe interface. This is a manually connected interface, so currently a user who installs the snap from the edge channel (which tracks git master) will need to manually snap connect gphotos-sync:mount-observe. If they don't then the snap crashes. I can request an auto connection assertion for that interface if required, which means users won't need to do anything, and the app won't crash.

My question is this: Is it expected that the application should enumerate /proc/*/mounts as part of normal behaviour (what is that doing?). If yes, then I can put the request in to ask for the interface to be auto connected, and hold off releasing to the stable channel until that's resolved. If the behaviour is not expected, then can I request the code is changed to not root around in /proc/*/mounts? :)

Here's the traceback.

Traceback (most recent call last):
  File "/snap/gphotos-sync/134/bin/gphotos-sync", line 11, in <module>
    load_entry_point('gphotos-sync==2.11b1', 'console_scripts', 'gphotos-sync')()
  File "/snap/gphotos-sync/134/lib/python3.6/site-packages/gphotos/Main.py", line 402, in main
    GooglePhotosSyncMain().main()
  File "/snap/gphotos-sync/134/lib/python3.6/site-packages/gphotos/Main.py", line 370, in main
    args = self.fs_checks(root_folder, args)
  File "/snap/gphotos-sync/134/lib/python3.6/site-packages/gphotos/Main.py", line 346, in fs_checks
    do_check(root_folder)
  File "/snap/gphotos-sync/134/lib/python3.6/site-packages/gphotos/Checks.py", line 167, in do_check
    root_folder = Checks(root)
  File "/snap/gphotos-sync/134/lib/python3.6/site-packages/gphotos/Checks.py", line 26, in __init__
    self.is_linux: bool = self._check_linux_filesystem()
  File "/snap/gphotos-sync/134/lib/python3.6/site-packages/gphotos/Checks.py", line 35, in _check_linux_filesystem
    for part in disk_partitions():
  File "/snap/gphotos-sync/134/lib/python3.6/site-packages/psutil/__init__.py", line 2132, in disk_partitions
    return _psplatform.disk_partitions(all)
  File "/snap/gphotos-sync/134/lib/python3.6/site-packages/psutil/_pslinux.py", line 1172, in disk_partitions
    partitions = cext.disk_partitions(mounts_path)
PermissionError: [Errno 13] Permission denied: '/proc/25024/mounts'

We are planning a blog post around the application soon, so I just want to make sure I get the ducks in a row, so we don't promote it and break it at the same time :)

Finally, as always, I'm happy to contribute the snapcraft yaml to your code repo and hand the snap over to you, should you prefer that.

@gilesknap
Copy link
Owner

Hi Alan,

Thanks for doing the snap and thanks for the complimentary podcast last Sept.

The change in question was done to try to determine what the filesystem of the target folder is. Thus we can see if we need to use NTFS/FAT restrictions on the characters that are allowed in file and folder names.

From the backtrace you sent, it seems that the call to psutil.disk_partitions() is performing the scan you are seeing.

This seems a bit extreme and I'm not particularly attached to the approach to discovering filesystem type.

I'll do a little research for alternatives and get back to you soon.

@gilesknap
Copy link
Owner

I have not been able to find a portable alternative to psutil.diskpartitions() and unfortunately, it wants to look in the mounts file (which makes sense).

I've just spent quite a lot of effort in #185 trying to get this filesystem check to work for all cases, including a Windows subsystem for Linux Docker container connecting to an NTFS host file system.

It's almost a moot change because it only slightly changes the restrictions on what characters are allowed in file names and album names. But I don't have an alternative that will not affect existing library backups.

Does it makes sense for a snap to enable an auto-connect to the mounts file, are there security implications?

@gilesknap
Copy link
Owner

Regarding maintenance of the snap, I'm happy for you to continue to do it assuming you would like to.

I have very little knowledge of snaps. In fact, I tried yours but could not work out how to give it permission to write to the host filesystem. I assume that is possible?

If I understood it better I might recommend snaps for deploying gphotos-sync since I've had some reasonably nasty issues recently with reliance on pipenv.

@popey
Copy link
Author

popey commented Jan 31, 2020

I'm sorry, I didn't mean to push extra work on you due to snap confinement issues. I have requested the interface auto-connection.

Obviously I'm super happy to continue maintaining the snap. FYI here's the growth over the last few months.

Screenshot from 2020-01-31 22-47-56

Happy to debug why the snap doesn't work for you whenever you have time.

@popey popey closed this as completed Jan 31, 2020
@gilesknap
Copy link
Owner

gilesknap commented Jan 31, 2020

That's great thanks. I'm curious, where does this auto connection go?

The stats are nice, are the available for me to monitor?

@gilesknap gilesknap pinned this issue Jan 31, 2020
@popey
Copy link
Author

popey commented Jan 31, 2020

The process is basically:-

  • The snap publisher requests the automatic interface connection
  • During a short review period, the security team and wider store team determine if it's a reasonable request
    • This may mean clarification is sought to better understand why the request is made.
    • It may require upstream changes to be made to accommodate
  • If approved, a member of the security team sets an 'assertion' in the store.
  • Any subsequent installs by users will receive both the snap and the updated assertion which when installed will be processed, resulting in the connection being automatically made.

Hope that makes sense.

@computerartclub
Copy link

I am having trouble with this right now. Running gohotos-sync with snap on Ubuntu 20.10 on a Rapberry Pi.
Saw this and tried
benjamin@ubuntu-pi:$ snap connect gphotos-sync:mount-observe
benjamin@ubuntu-pi:
$ gphotos-sync /media/benjamin/4TMathewsB/GPhotos

Still got the following error:
Traceback` (most recent call last):
File "/snap/gphotos-sync/235/bin/gphotos-sync", line 11, in
load_entry_point('gphotos-sync==2.14.2', 'console_scripts', 'gphotos-sync')()
File "/snap/gphotos-sync/235/lib/python3.6/site-packages/gphotos/Main.py", line 432, in main
GooglePhotosSyncMain().main()
File "/snap/gphotos-sync/235/lib/python3.6/site-packages/gphotos/Main.py", line 397, in main
setup_logging(args.log_level, args.logfile, root_folder)
File "/snap/gphotos-sync/235/lib/python3.6/site-packages/gphotos/Logging.py", line 63, in setup_logging
log_handler = logging.FileHandler(log_file, mode="w")
File "/snap/gphotos-sync/235/usr/lib/python3.6/logging/init.py", line 1032, in init
StreamHandler.init(self, self._open())
File "/snap/gphotos-sync/235/usr/lib/python3.6/logging/init.py", line 1061, in _open
return open(self.baseFilename, self.mode, encoding=self.encoding)
PermissionError: [Errno 13] Permission denied: '/media/benjamin/4TMathewsB/GPhotos/gphotos.log'
benjamin@ubuntu-pi:~$

any ideas?

@gilesknap
Copy link
Owner

I'm afraid I don't have much experience of snaps, maybe @popey can help?

If not can you run outside of a snap?

@computerartclub
Copy link

I think I might finally be making progress.
The solution was: snap connect gphotos-sync:removable-media
then it finally gave me the link for authentication. Hope this helps someone else. :)

@gilesknap gilesknap unpinned this issue May 15, 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

No branches or pull requests

3 participants