Giter Club home page Giter Club logo

Comments (5)

bohwaz avatar bohwaz commented on September 3, 2024

Thank you for your comment, very useful to have such feedback :)

Regarding session garbage collecting:

  • they use very little space
  • an expired session cannot be used (see WHERE condition just below) even if it hasn't been deleted

So it's not very important to delete them all the time.

Of course you could also just set up a crontab to do that, but the main objective of KaraDAV is that you can install it on any shared hosting provider, and often you don't have cron available on these, but you still need to get the expired sessions deleted from time to time.

POSIX file locks: no I thought about using them, but you can't as the POSIX lock expires when you are closing the file pointer (so at the end of the PHP script execution). So if you lock it in request 1, the file would be unlocked for request 2 unless it would happen at the exact same time as request 1. You need to store the lock state somewhere, and the SQLite database is the fastest and lightest way to do that.

External user sessions: yes that's a good idea. For the release I just wrote some session management quickly as this server was mainly a prototype for testing the underlying WebDAV library, but any PR improving the sessions management so that you can use something else is welcome (without using external dependencies or becoming too complex :) ).

User base path: yes, that would be very nice, as I said the current implementation was just for the prototype, but being able to choose a custom data path per user would be nice. The current code is for the case of a shared hosting where you get multiple users paths with the same UNIX user (like NextCloud).

As for security implications, if you want to have a different user accessing the files, the best course of action would be to use apache2-mpm-itk and AssignUserIDExpr to have [user].webdav.localhost mapped to its data.

<VirtualHost *:80>
	ServerName webdav.localhost
	ServerAlias *.webdav.localhost

	SetEnvIf Request_URI (.+) ITKUID=www-data ITKGID=www-data
	SetEnvIf Host (.+)\.webdav\.localhost ITKUID=$1 ITKGID=$1


	<If "!-d '/home/%{reqenv:ITKUID}'">
		Redirect 404
	</If>

	AssignUserIDExpr %{reqenv:ITKUID}
	AssignGroupIDExpr %{reqenv:ITKGID}

	DocumentRoot /home/bohwaz/git/karadav/www
</VirtualHost>

You could also use a subdirectory instead of a subdomain of course:

<VirtualHost *:80>
	ServerName webdav.localhost

	SetEnvIf Request_URI (.+) ITKUID=www-data ITKGID=www-data
	SetEnvIf Request_URI ^/files/([a-z]+)/ ITKUID=$1 ITKGID=$1

	# Do not allow root to be used as the ITK UID/GID
	SetEnvIf ITKUID ^root$ ITKUID=www-data
	SetEnvIf ITKGID ^root$ ITKGID=www-data

	AssignUserIDExpr %{reqenv:ITKUID}
	AssignGroupIDExpr %{reqenv:ITKGID}

	DocumentRoot /home/bohwaz/git/karadav/www
</VirtualHost>

This way the apache server will run as the user UID/GID directly, and www-data won't have access to your files, no need for a complex stack and a FTP server :)

from karadav.

bohwaz avatar bohwaz commented on September 3, 2024

After thought about that config: this would require that your users have read/write access to the database file (not really a big issue) and read-only access to KaraDAV code.

To get back to the requirement for SQLite3:

  • you could also just not implement file locks, like I do in Garradin, most webdav clients don't care if they locked a file and then it's not locked at next request, but then the litmus lock tests would all fail, and KaraDAV was developed to test litmus compliance
  • you could also not store custom file properties as most webdav clients don't use them
  • you could use something else for users sessions
  • but you would still need to manage a list of app sessions, and SQLite3 is still better/faster at that than using a flat file

from karadav.

X-Ryl669 avatar X-Ryl669 commented on September 3, 2024

I don't think it's the right way to proceed. I think it's not good to have a root running webserver (like Apache) that can impersonate any user when requested http://yourhost.com/files/1001/.

I'm using nginx because it drops its permission as soon as it's launched and can't reclaim them back.
There is no equivalent to mpm-itk in nginx (hopefully). We can start as many php-fpm spool as there are users (and run the spool as the user, each one having its own socket /var/log/php-fpm/user1.sock.

But that means that the user now have complete control on the PHP server, so he can runs his own script if he wants and that's not very good, security wise. Typically, browsing to http://yourhost.com/files/bob/remoteShell.php will work and it's game over.

I'd a similar issue on my server and I've solved it by using FlySystem that's connecting to a inetd/vsftpd daemon only listening on localhost. This server drops its privileges as soon as it starts and setuid/gid to the asked user. Since it's a FTP server, it can only access files for reading, writing, listing directories. The risks are mitigated, but it's very slow, obviously. No X-SendFile in that case either.

I think a decent solution would be, instead, to use a program that's binding the user home folder as www-data:www-data.
There is FUSE's bindfs doing so and it has decent performance. However, one does not want to have permanent bound mount on his system, so a small sudo-like program could be used to perfom this:

  1. Upon session creation/login attempt:
    1.1 Call the program with the provided user credentials (via system() or popen)
    1.2 If the program validate the credentials, it forks and its child call bindfs with the right user mapping (user=>www-data) to a known temporary folder and write a time file token (like sudo is doing), then the parent returns success
    1.3 Else, it fails and returns an error
  2. Upon session validation (or at regular interval), the program is called again with a continue parameter
    2.1 If any time file token is found and the user name match, the file is touched to let the mount continue
    2.2 Else if the time file token is too far in the past, the directory is unmounted (fusermount -u). We can even use the mount folder here, to avoid creating files.
  3. In NGINX or Apache configuration, we can set alias folder to points to the mountpoints so X-SendFile is used
  4. Upon session destruction, the program is called with a umount argument and it only umount the given's user mount.

This program should either be run as root or given setuid capability. Its attack surface should be very limited, since it's not doing anything on the files themselves. bindfs is very well tested and we can limit the attack surface by only binding with a restricted set of permissions (like rD or rwD) to prevent running any file on the mount bind. With the Alias directive, the files aren't visible to PHP-FPM as script either, they are only visible to the PHP's script (here: karadav) when using filesystem primitives. As long as the PHP code is doing what it claims, it should be safe.

from karadav.

bohwaz avatar bohwaz commented on September 3, 2024

You shouldn't let your web server config allow for CGI/script execution inside users directories obviously :)

So using one FPM pool per user and enabling script execution only on KaraDAV directory seems the most sensible way: easy enough to set up, and secure. But it won't scale very well with thousands of users.

But personally I've been using ITK for around 15 years, and some very large providers use it as well, and never had an issue, so I think it's a good solution, in fact it does the same as bindfs: it is dropping privileges as soon as possible.

from karadav.

bohwaz avatar bohwaz commented on September 3, 2024

I am closing this, as I have no intention of implementing that unless a PR is submitted. Thank your for your feedback.

from karadav.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.