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

Update config #19

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-with/conf.xml
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@

<!-- File location for logs. If not specified or left blank the error log
will print to stdout and the access log to stderr -->
<error-log>/var/log/webdav-error.log</error-log>
<access-log>/var/log/webdav-access.log</access-log>
<error-log>/var/log/webdavd/error.log</error-log>
<access-log>/var/log/webdavd/access.log</access-log>

<!-- At least one of these must be configured if you want an SSL encrypted
connection. Certificates MUST have a Subject Alternative Name specified.
Expand Down
4 changes: 2 additions & 2 deletions package-with/logrotate.conf
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/var/log/webdav-access.log /var/log/webdav-error.log {
/var/log/webdavd/access.log /var/log/webdavd/error.log {
sharedscripts
missingok
delaycompress
create 640 root adm
create

postrotate
service webdavd restart
Expand Down
14 changes: 11 additions & 3 deletions package-with/pam-rhel.conf
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
auth include password-auth
account include password-auth
session include password-auth
auth substack password-auth
auth include postlogin
account required pam_sepermit.so
account include password-auth
session required pam_selinux.so close
session required pam_loginuid.so
session required pam_selinux.so open env_params
session required pam_namespace.so
session optional pam_keyinit.so force revoke
session include password-auth
session include postlogin
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to get an explanation of these changes. The Redhat PAM file was provided by another user so I freely admit it might need securing. Have these been copied from another service? Does Redhat not have standard include files for this purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied them from sshd, omitting the password entries which do not apply and also pam_motd which doesn't make sense for a non-shell. They all appears to make sense, though some are a little unclear.

There are include files for password-auth and postlogin, but nothing for this whole sequence. Here's what proftpd does:

session    optional     pam_keyinit.so force revoke
auth       required     pam_listfile.so item=user sense=deny file=/etc/ftpusers onerr=succeed
auth       required     pam_shells.so
auth       include      password-auth
account    include      password-auth
session    required     pam_loginuid.so
session    include      password-auth

vsftpd is basically the same. Maybe that would be a better starting point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to add this:

auth            required        pam_succeed_if.so user != root quiet_success

This will disallow root access. sshd by default does not allow root logins on Fedora, and most other distros too, so it seems appropriate.

6 changes: 3 additions & 3 deletions package-with/systemd.service
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
[Unit]
Description=WebDAV file server
Documentation=man:webdavd(1)
After=network.target

[Service]
Type=simple
#Environment=statedir=/var/cache/foo
#iExecStartPre=/usr/bin/mkdir -p ${statedir}
ExecStart=/usr/sbin/webdavd /etc/webdavd
LogsDirectory=webdavd
ProtectSystem=full
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is necessarily desirable. Certainly ProtectHome is undesirable. The primary purpose of this server is to allow access to Linux user's home, so that would be counter productive. Added protection can be added via the chroot option to protect the system.

However from memory, ProtectSystem causes the server to be run in it's own mount namespace. This would be tricky for some users who are unfamiliar with the effect because the server would not give access to (for example) USB drives manually mounted by the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if webdavd could be prevent from home dir access, but then webdav-worker, after authentication as the user, did have access. But I didn't see an easy way to do that. ProtectHome prevents authentication from even working.

I don't believe ProtectSystem creates a private mount namespace. There is PrivateMounts, but the mount namespace is still propagated in the direction system -> unit private, so usb drives (and the root fs!) still appear. But mount changes the unit makes will not propagate back to the system.

ProtectSystem=full makes some directories read-only, .e.g. /etc, /boot. Directories were it would be highly unusual to edit files via webdav and most if not all files will be own by root.

All the directories so modified are not reachable after being chrooted to ~, which the default config files does. So if someone does want to use webdav to get remote root write access to /etc, they already need to edit the webdav config file. It seems reasonable that they could also edit the unit file too.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've previously had mega problems with this feature (rooted from this bug) so I'm pretty cautious about it. Remember this config file is used by everyone who's installed this server for every use case. It might be worth checking how samba is packaged and if they use this feature.

Regarding the home directory thing, it's certainly not going to fit the general use case. I don't understand what you're trying to achieve. I imagine the easiest way to achieve it is to have a single directory somewhere (/share or /user/lib/share) with everyone's webdavd shares inside and then set the server to chroot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Network servers that I have installed and came with ProtectSystem on: openvpn-server, systemd-timedated, lldpd, rdisc, chronyd

There are more services that use it, but I've only looked at those that are network daemons.

It's also entirely possible for someone to change the config via a drop in file without even editing this unit file, which is easier than editing the webdavd config file.

Current best practices is to make the default secure. If someone wants to make it less secure, such as allowing editing their entire file system over webdav, then they can do that. Making the default insecure and expecting people to secure it themselves isn't recommended anymore.

Example, webdavd isn't even going to work until the default firewall config is edited to allow connections from outside of localhost to port 80 and/or 443.

WRT home dirs, my goal is to be more secure. There's no reason for webdavd, before authentication of the user, to be able to access any files in home directories. So it shouldn't have that. Unfortunately, dropping home dir access can not be undone via authentication as a user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also tested the additional mount, by serving /run, and then while a webdav session was open, mounting as a user a USB drive. The new mount appeared to webdavd just fine under /run/username.

This makes /usr read-only too, which doesn't cause a problem starting webdav-worker from /usr/lib or serving the static content in /usr/share.

It does provide an important security benefit knows as W^X. One can either write files or execute them, not both. If one is able to get write access to /usr/lib, then one gets the trivial ability to execute arbitrary code, by replacing webdav-worker with a new program which will be conveniently run on the next session.


[Install]
WantedBy=multi-user.target