-
-
Notifications
You must be signed in to change notification settings - Fork 759
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
Check os permissions as if acting as user #1230
base: main
Are you sure you want to change the base?
Conversation
Thanks for contribution. This seems a potential big change, I'll try to find the time to test and review it after v2.5 release. Do we really need to support linux ACL? I haven't looked into the code, so this might be a trivial question, but CGO dependency introduction is something to consider carefully. Currently the only CGO dependency is sqlite. In the meantime I suggest using your patch in production so you can see if there are any issues. Thanks |
Signed-off-by: Peter Verraedt <[email protected]>
Sorry for the C-dependency, it is only used in I'll further test and see whether something pops up. |
great! As for logging I would like an interface so we can integrate the library logging with SFTPGo, see the following examples https://github.com/drakkan/sftpgo/blob/main/internal/logger/lego.go ignore this comment if the library logger already allows it
Thanks! |
|
91f38c6
to
0e77ba9
Compare
This PR proposes to optimize the behaviour of acting as a specified uid/gid for a user, if sftpgo is run as root. We check standard linux permissions as well as extended ACLs. For this we wrap all
pkg.go.dev/os
calls by a package that checks the permissions first before calling the corresponding function. If not on linux, or sftpgo is not running as root, there is a fallback to the defaultpkg.go.dev/os
calls.The main benifits are that file permissions are checked, but syscalls to change uid/gid are avoided.
There are two exceptions of operations that are not being checked that a certain uid/gid has access: The ScanQuota function for virtual folders if triggered by an event, because virtual folders are not one-to-one mapped to a user, but that operation only reads files so it should be fine; and the creation of the user "home folder". The latter could be changed by creating a world-writable directory with the sticky bit set.
In case you would consider to merge this, implementing secondary groups in the database model would be the next step, for situations where folders are shared on gid-basis and users can belong to different groups.
(This is a follow up to #1225)