scVENUS / PeekabooAV-Installer

This repository provides scripts and configuration files to install, update and test a Peekaboo installation
GNU General Public License v3.0
7 stars 9 forks source link

Restrict peekaboo access to config file to reading only #28

Closed michaelweiser closed 4 years ago

michaelweiser commented 5 years ago

Make peekaboo.conf owned by user root and group peekaboo and group-readable. This way, user peekaboo retains read access but cannot change the configuration file.

Closes #27.

michaelweiser commented 5 years ago

Huh, snag: systemd unit and peekaboo.conf actually run the daemon with group amavis so it can access the attachment files for scanning. Needs a rethink.

We could actually extend our JSON protocol for amavis-to-Peekaboo communication to transfer the samples as well, e.g. base64 encoded. That would eliminate the need to access the files written by amavis directly and thus the need to have the Peekaboo daemon run with group amavis. Very appealing.

michaelweiser commented 4 years ago

What we need here, it seems upon rethink, is the following settings in the unit file:

User=peekaboo Group=peekaboo SupplementaryGroups=amavis

Since the latter two are defaults anyway, i.e. primary and secondary group list of the runtime user are initialized from the system user database if not given explicitly, it should be enough just to throw out the explicit effective group assignment of amavis everywhere after making sure, the installer correctly creates that group and adds the peekaboo runtime user to it as secondary group.

Since we also support dropping privileges ourselves, the respective droppriv code at https://github.com/scVENUS/PeekabooAV/blob/25fc4b963786677996d8e46c36613298ba758728/peekaboo/daemon.py#L108 might need some enhancement to get the secondary group memberships right. Since droppriv in a daemon is inherently hard to get right, we might want to delegate this to someone who knows what they're doing, i.e. either systemd or https://pypi.org/project/python-daemon/.

michaelweiser commented 4 years ago

This is turning into a never-ending story: Now that peekaboo runs as user peekaboo, primary group peekaboo and supplementary group whatever (amavis), the socket and runtime directory (/run/peekaboo) are owned peekaboo:peekaboo mode 750. Before it was peekaboo:amavis mode 750. So amavis no longer has access to it. We need to think about what to do here:

  1. Open the socket up to everyone on the system, potentially paired with an API token similar to Cuckoo.
  2. Muck about with file system access control (e.g. setfacl). I've seen statements on the net that this is no longer preferred since depending on unix domain socket options it just does not work as expected. @jack28?
Jack28 commented 4 years ago

Sooner or later the API token will be necessary. I can see us wanting to switch to a network socket.

michaelweiser commented 4 years ago

Sooner or later the API token will be necessary. I can see us wanting to switch to a network socket.

Agreed. It seems a bit late in the game for 2.0 though. I guess I'll make the socket permissions explicit for now using config options and explicit chgrp/chmod. Others do it and it's a good idea regardless.

michaelweiser commented 4 years ago

Okay, so this works from our end. But now amavis or rather perl's Net::Server does seem to do an incomplete droppriv or rather no initgroups() and therefore the running instance of amavis will have no supplementary groups, particularly peekaboo-clients. Looking into that.

michaelweiser commented 4 years ago

Okay, so this works from our end. But now amavis or rather perl's Net::Server does seem to do an incomplete droppriv or rather no initgroups() and therefore the running instance of amavis will have no supplementary groups, particularly peekaboo-clients. Looking into that.

So it's amavis that does do setuid() and setgid() in a drop_priv function but no initgroups(). It appears, perl has no support for initgroups() at all. So it certainly is an amavis/perl limitation. Dropping the peekaboo-clients group from the installer for now.

michaelweiser commented 4 years ago

Tested. Works. Had it up and running in vagrant without probs. @jack28: Can you give this a spin? I think this is what it's going to be for now.

Jack28 commented 4 years ago

LGTM

michaelweiser commented 4 years ago

Thanks! This depends on scVENUS/PeekabooAV#146. I assume you tested with that branch or it wouldn't have worked at all. Can you give a quick thumbs up there as well. Then I'll merge both.

michaelweiser commented 4 years ago

Phew! This was a tough cookie. Thanks for getting this wrapped finally.