search

open-iscsi / targetcli-fb

Command shell for managing Linux LIO kernel target
Apache License 2.0
103 stars 69 forks source link

targetcli as_root check is not sufficient #163

Open alexmurray opened 4 years ago

alexmurray commented 4 years ago

If a command is to be disabled for non-root users, having targetcli check this via getuid() is not sufficient, since a local user could just make a copy of the targetcli python script and edit it to remove this check - instead targetclid should check the client's credentials via SCM_CREDENTIALS and do permissions checks itself against the client and return appropriate errors.

gonzoleeman commented 4 years ago

targetclid is not required to be running.

gonzoleeman commented 4 years ago

And the permissions on sysfs should require root, anyway. No?

pkalever commented 4 years ago

And how is this different from targetcli script (dameon=false) itself?

alexmurray commented 4 years ago

Whilst targetclid is not required to be running, the systemd unit and socket unit mean that as soon as any client tries to connect to the daemon socket it will be spawned. The daemon is then running as root and so has full permissions to modify sysfs etc - so any commands sent to it will be executed as root - even if the cient does not have root privileges. Hence the daemon should check the UID of the client to know if it has authority to perform the action before doing it on it's behalf (otherwise this is a classic confused deputy problem) - failing this, at least addresing issue #162 would ensure non-root privileged clients cannot connect to the daemon and so this would then not be necessary to validate the permissions of the client.

pkalever commented 4 years ago

ack. Fix: https://github.com/open-iscsi/targetcli-fb/pull/164