uweseimet / scsi2pi

Advanced performant SCSI/SASI emulation and tools for the PiSCSI/RaSCSI board
https://www.scsi2pi.net
BSD 3-Clause "New" or "Revised" License
9 stars 2 forks source link

Trying to set image file folder location to path outside /home via scp fails with nondescript error #63

Closed fdanapfel closed 8 months ago

fdanapfel commented 8 months ago

When creating a new ticket please provide information on your environment.

Describe the issue

Trying to set the image file location to a directory outside /home fails with an error that does not give any information why this does not work:

$ s2pctl -F /images
Error: 

The directory exists:

$ ll -d /images
drwxr-xr-x 2 root root 4096 Feb  6 19:04 /images

Trying the same with /home/images works:

$ s2pctl -F /home/images
$ ll -d /home/images
drwxr-xr-x 2 root root 4096 Feb  7 09:06 /home/images

As you can see both /home/images and /images have the same ownership and permissions, therefore it seems there is a limitation on where the image file folder can be located.

Not sure why it appears that it only seems to be allowed to be located in /home, but if that is the case then the error message should clearly state the reason for this.

But unless there is a valid reason why the image file folder always must be located in /home (which should be documented somewhere), it would be great if this restriction could be removed.

uweseimet commented 8 months ago

@fdanapfel This will be fixed with release 2.0.1, most likely later today. The PiSCSI wiki might have details on the restriction that the folder must be in /home. This is for security reasons, because piscsi/s2p are running as root. If any location was permitted, by setting the folder to / and then deleting all image files with a remote command you could cause great harm. If an image is somewhere else one can use its absolute path.

fdanapfel commented 8 months ago

@uweseimet Thanks for the explanation.

So in order to prevent users accidentally delete their system by setting the location of the image file folder to / and then deleting all image files, maybe it would be better to change the check to disallow setting the location to just / instead of having the restriction that the folder always has to be somewhere in /home.

uweseimet commented 8 months ago

@fdanapfel If you set the location to /dev, /etc or /lib etc. the problem is the same. Deleting files will corrupt the system.

fdanapfel commented 8 months ago

@uweseimet Yes, but then you could also corrupt the system by creating a symlink in /home pointing to / or any other important directory and then deleting files there.

Maybe in a future version you could consider adding an "expert mode" that will allow overriding these kinds of restrictions.

uweseimet commented 8 months ago

@fdanapfel Sure, if you really want to, you can crash any system ;-). What's the use case that requires you to have the image folder not in /home but somewhere else? A regular Linux user does not have access to anything outside $HOME anyway. So this should also be sufficient for s2p.

Initially, piscsi did not even permit to have the images anywhere else than in /pi/home/images. I added the feature to use a different location, but this requires some restrictions, in order to avoid certain problems. I don't intend to add an expert mode. If you are an expert you can, just as you said, use symlinks. You do not need a special option in s2p for that ;-).

fdanapfel commented 8 months ago

@uweseimet The "usecase" here is simply that since s2p is running as root it should not use a regular users home directory to store the images.

Similar setups where disk images are used, for example kvm/libvirt/virt-manager, normally use directories under /var for their storage (see for example https://unix.stackexchange.com/questions/241601/where-does-kvm-hypervisor-store-vm-files). This is also what the Linux File System Hierarchy Standard recommends: https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard

uweseimet commented 8 months ago

@fdanapfel You can create an image folder in /home that is not a user's home directory. I'm sorry, but I do not intend to change anything here. PiSCSI compatility is also relevant here. A global folder /var is strictly speaking also not a solution. If we both share a Pi, and sometimes you use s2p with it, and sometimes I use it, we would like to have separate folders.

uweseimet commented 8 months ago

@fdanapfel Binary builds of the latest develop branch and also of release 2.0.1 are available now. In both versions this bug should be fixed.

fdanapfel commented 8 months ago

@uweseimet Thanks for providing a fix. Tested with scsi2pi_2.1_devel_7c3df1b_bookworm_arm64-1.deb and now an error message is shown when trying to set the image file folder to a location outside /home:

$ sudo apt install ./scsi2pi_2.1_devel_7c3df1b_bookworm_arm64-1.deb
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
Note, selecting 'scsi2pi' instead of './scsi2pi_2.1_devel_7c3df1b_bookworm_arm64-1.deb'
The following NEW packages will be installed:
  scsi2pi
0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
Need to get 0 B/734 kB of archives.
After this operation, 0 B of additional disk space will be used.
Get:1 /home/frank/scsi2pi_2.1_devel_7c3df1b_bookworm_arm64-1.deb scsi2pi arm64 2.1-devel [734 kB]
Selecting previously unselected package scsi2pi.
(Reading database ... 81714 files and directories currently installed.)
Preparing to unpack .../scsi2pi_2.1_devel_7c3df1b_bookworm_arm64-1.deb ...
Unpacking scsi2pi (2.1-devel) ...
Setting up scsi2pi (2.1-devel) ...
Starting SCSI2Pi service
Consider adding SCSI2Pi to your local environment with:
  export PATH=$PATH:/opt/scsi2pi/bin
  export MANPATH=$MANPATH:/opt/scsi2pi/man
N: Download is performed unsandboxed as root as file '/home/frank/scsi2pi_2.1_devel_7c3df1b_bookworm_arm64-1.deb' couldn't be accessed by user '_apt'. - pkgAcquire::Run (13: Permission denied)
$ sudo systemctl status s2p.service 
● s2p.service - SCSI2Pi s2p service
     Loaded: loaded (/etc/systemd/system/s2p.service; disabled; preset: enabled)
     Active: active (running) since Thu 2024-02-08 19:04:54 CET; 13s ago
   Main PID: 9198 (s2p)
      Tasks: 2 (limit: 176)
        CPU: 24ms
     CGroup: /system.slice/s2p.service
             └─9198 /opt/scsi2pi/bin/s2p

Feb 08 19:04:54 bookworm s2p[9198]: SCSI Target Emulator and SCSI Tools SCSI2Pi (Device Emulation)
Feb 08 19:04:54 bookworm s2p[9198]: Version 2.1-devel
Feb 08 19:04:54 bookworm s2p[9198]: Copyright (C) 2016-2020 GIMONS
Feb 08 19:04:54 bookworm s2p[9198]: Copyright (C) 2020-2023 Contributors to the PiSCSI project
Feb 08 19:04:54 bookworm s2p[9198]: Copyright (C) 2021-2024 Uwe Seimet
Feb 08 19:04:54 bookworm s2p[9198]: [2024-02-08 19:04:54.622] [info] Set log level to 'info'
Feb 08 19:04:54 bookworm s2p[9198]: [2024-02-08 19:04:54.622] [info] Default image folder set to '/home/frank/images'
Feb 08 19:04:54 bookworm s2p[9198]: [2024-02-08 19:04:54.626] [info] Reserved ID(s) set to 7
Feb 08 19:04:54 bookworm s2p[9198]: [2024-02-08 19:04:54.626] [info] No devices currently attached.
Feb 08 19:04:54 bookworm s2p[9198]: No devices currently attached.
$ s2pctl -l
No devices currently attached.
$ s2pctl -F /images
Error: Default image folder must be located in '/home'

I would still like to see the restriction that the image file folder must aways be in /home removed at some point in the future, but for now I can live it.

So from my point of view this issue can be closed.

uweseimet commented 8 months ago

@fdanapfel Thank you for testing!