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

clamscan systemd service and timer #928

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nils-werner
Copy link
Contributor

@nils-werner nils-werner commented May 25, 2023

This PR contains a systemd service and timer for clamscan. They are parameterized units, which means you can pass the escaped paths you'd like to scan into them:

systemctl start [email protected]             # will scan /
systemctl start [email protected]          # will scan /home

With these you can do the following:

Start a system scan

systemctl start [email protected]

will start a scan of /home in the background. The process runs with root permissions.

Start a user scan

Unprivileged users can start

systemctl --user start [email protected]

to start a scan of /home/user in the background. The process runs with your user permissions, so you can only scan files that belong to you.

Schedule a weekly system scan

systemctl enable --now [email protected]

will scan /home once a week, with root permissions

Schedule a weekly user scan

systemctl --user enable --now [email protected]

will scan /home/user once a week, with my user permissions.

Customize them

You can change any of the config file parameters (commandline arguments, timer frequency etc) in the units. So if for example you want to scan your home dir daily you can run

systemctl --user edit [email protected]

and add

[Timer]
OnCalendar=daily

into the dropin config file.

etc...

@micahsnyder
Copy link
Contributor

Hi @nils-werner. This is a very interesting PR. I have mixed feelings about it.

will start a scan of /home in the background. The process runs with root permissions.

I dislike suggesting anyone should scan with root permissions. Until we are able to sandbox the scanning process, it makes me pretty uncomfortable.

to start a scan of /home/user in the background. The process runs with your user permissions, so you can only scan files that belong to you.

This is kinda cool. Although I imagine it breaks if you want to scan a directory with a hyphen in it, like /home/user/scan-me. It would probably try to scan /home/user/scan/me, yes?

I'm not certain how the changes in this PR are much different from asking users to set up a cron job. And cron is more universally available than systemd.

I'll talk about this a bit with my team, but I'm inclined to to decline this PR. You're welcome to try to change my mind and explain the benefits of using a systemd service and timer for clamscan.

@nils-werner
Copy link
Contributor Author

nils-werner commented Jun 1, 2023

Hi @micahsnyder thanks for your response!

I dislike suggesting anyone should scan with root permissions. Until we are able to sandbox the scanning process, it makes me pretty uncomfortable.

Don't worry, that's not possible. Only root can start system services (i.e. with root access).

This is kinda cool. Although I imagine it breaks if you want to scan a directory with a hyphen in it, like /home/user/scan-me. It would probably try to scan /home/user/scan/me, yes?

That's no problem. You can use systemd-escape to get the correct sequence:

$ systemd-escape -p /home/user
home-user
$ systemd-escape -p /home/user/scan-me
home-user-scan\x2dme

So to scan /home/user/scan-me, you'd start clamav-clamdscan@home-user-scan\x2dme.service

I'm not certain how the changes in this PR are much different from asking users to set up a cron job. And cron is more universally available than systemd.

This is of course only an additional option, it does not replace cron or prevent users from using cron. However, here is a list of neat features that systemd timers can do and cron can't.

One neat feature for example is you can inspect the output of your scan using journalctl:

journalctl --user -u [email protected]
Thu 01 Jun 2023 09:04:18 AM CEST
Jun 01 08:52:49 nas systemd[624]: Started ClamAV clamdscan home/user.
Jun 01 08:52:50 nas clamdscan[1982]: ----------- SCAN SUMMARY -----------
Jun 01 08:52:50 nas clamdscan[1982]: Infected files: 0
Jun 01 08:52:50 nas clamdscan[1982]: Time: 0.830 sec (0 m 0 s)
Jun 01 08:52:50 nas clamdscan[1982]: Start Date: 2023:06:01 08:52:49
Jun 01 08:52:50 nas clamdscan[1982]: End Date:   2023:06:01 08:52:50

@micahsnyder
Copy link
Contributor

Sorry about the really long delay replying to you.

Hi @micahsnyder thanks for your response!

I dislike suggesting anyone should scan with root permissions. Until we are able to sandbox the scanning process, it makes me pretty uncomfortable.

Don't worry, that's not possible. Only root can start system services (i.e. with root access).

What I mean to say is that if clamscan is run as root (regardless of who starts the service), then the risk is much higher if clamscan is compromised by a code execution vulnerability. The attacker would get root. While I don't like to imagine that more code execution vulns exist in clam, I have to acknowledge it's C code made to parse untrusted user input and that's very difficult to do safely 100% of the time.

That said, I guess I don't have anything against providing the clamscan timer as an option, but I would strongly discourage it in favor of using clamd + the clamdscan timer.

This is kinda cool. Although I imagine it breaks if you want to scan a directory with a hyphen in it, like /home/user/scan-me. It would probably try to scan /home/user/scan/me, yes?

That's no problem. You can use systemd-escape to get the correct sequence:

$ systemd-escape -p /home/user
home-user
$ systemd-escape -p /home/user/scan-me
home-user-scan\x2dme

So to scan /home/user/scan-me, you'd start clamav-clamdscan@home-user-scan\x2dme.service

Cool!

I'm not certain how the changes in this PR are much different from asking users to set up a cron job. And cron is more universally available than systemd.

This is of course only an additional option, it does not replace cron or prevent users from using cron. However, here is a list of neat features that systemd timers can do and cron can't.

One neat feature for example is you can inspect the output of your scan using journalctl:

journalctl --user -u [email protected]
Thu 01 Jun 2023 09:04:18 AM CEST
Jun 01 08:52:49 nas systemd[624]: Started ClamAV clamdscan home/user.
Jun 01 08:52:50 nas clamdscan[1982]: ----------- SCAN SUMMARY -----------
Jun 01 08:52:50 nas clamdscan[1982]: Infected files: 0
Jun 01 08:52:50 nas clamdscan[1982]: Time: 0.830 sec (0 m 0 s)
Jun 01 08:52:50 nas clamdscan[1982]: Start Date: 2023:06:01 08:52:49
Jun 01 08:52:50 nas clamdscan[1982]: End Date:   2023:06:01 08:52:50

Got it! Very nice!

@micahsnyder
Copy link
Contributor

Just rebased and force-pushed to run through CI/tests, not that we have any automated tests using systemd.

@nils-werner As with #962 would you be up for writing documentation to put in https://github.com/Cisco-Talos/clamav-documentation/blob/main/src/manual/Usage/Services.md You could probably mostly just copy-paste stuff from the PR descriptions.

@nils-werner
Copy link
Contributor Author

What I mean to say is that if clamscan is run as root (regardless of who starts the service), then the risk is much higher if clamscan is compromised by a code execution vulnerability. The attacker would get root.

Ah I see! What's the recommended solution for this? Is there a special clamav user, or simply disallow running as root? Both are possible.

And I'm also wondering if the same shouldn't be considered for freshclam and all the other services...

@micahsnyder
Copy link
Contributor

What I mean to say is that if clamscan is run as root (regardless of who starts the service), then the risk is much higher if clamscan is compromised by a code execution vulnerability. The attacker would get root.

Ah I see! What's the recommended solution for this? Is there a special clamav user, or simply disallow running as root? Both are possible.

And I'm also wondering if the same shouldn't be considered for freshclam and all the other services...

If you start freshclam it drops privileges to run as the clamav user or user defined by the DatabaseOwner in freshclam.conf.

If you start clamd as root, it drops privileges to run as the clamav user or user defined by the User in clamd.conf.

clamdscan, clamonacc, and clamav-milter are all scanning clients that just relay files to be scanned to clamd. There's no real risk of running them as root. But specifically:

  • clamonacc has to be run as root to register with the kernel for file system events, and that's why we split the functionality out of clamd (it was apart of clamd for a few versions, and that was scary to me).
  • It may be advantageous to run clamdscan as root when using the local (unix) socket (LocalSocket option instead of TCPSocket) and the --fdpass option, because it could open any file and pass that to clamd for scanning. In this way clamd could get access to scan anything, without having to run as root.

sigtool can do some scanning/parsing of malware and should probably not be run as root when using file analysis features. I don't know of any reason why anyone would run it as root anyways.

@micahsnyder
Copy link
Contributor

@nils-werner Is it possible to make the clamdscan timer start the clamav-daemon service (if not already started), then run the scan with clamdscan, and then stop the clamav-daemon service (but only if it had to start it?
Is that logic possible?

I'm inclined to suggest we also remove the clamscan time, just to remove the temptation or alleviate confusion.

# Use pkg-config to look up the systemd user unit install directory
execute_process(COMMAND ${PKG_CONFIG_EXECUTABLE}
--variable=systemduserunitdir systemd
OUTPUT_VARIABLE SYSTEMD_USER_UNIT_DIR)
Copy link
Contributor

Choose a reason for hiding this comment

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

It may also be nice to have the main CMakeList.txt summary print out this variable next to the other one:

${_} unit directory ${e}${SYSTEMD_UNIT_DIR}

Copy link
Contributor

Choose a reason for hiding this comment

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

@nils-werner I think we forgot about this.

@nils-werner
Copy link
Contributor Author

nils-werner commented Aug 4, 2023

Is it possible to make the clamdscan timer start the clamav-daemon service (if not already started), then run the scan with clamdscan, and then stop the clamav-daemon service (but only if it had to start it? Is that logic possible?

clamav-daemon is a system unit, and other system units can Require= it, so it will be started before the scan starts.

However user units cannot Require= system units... So a user instance of clamdscan would require clamav-daemon to be already running, while a system instance of clamdscan can start it automatically.

I'm inclined to suggest we also remove the clamscan time, just to remove the temptation or alleviate confusion.

I'm not sure I understand... do you want to remove clamscan entirely, or only the timer? The timer isn't the piece that's causing the "run as root" problem...

@micahsnyder
Copy link
Contributor

Is it possible to make the clamdscan timer start the clamav-daemon service (if not already started), then run the scan with clamdscan, and then stop the clamav-daemon service (but only if it had to start it? Is that logic possible?

clamav-daemon is a system unit, and other system units can Require= it, so it will be started before the scan starts.

However user units cannot Require= system units... So a user instance of clamdscan would require clamav-daemon to be already running, while a system instance of clamdscan can start it automatically.

Can we make user unit for the clamav-daemon service then, and Require= the user one?

I'm inclined to suggest we also remove the clamscan time, just to remove the temptation or alleviate confusion.

I'm not sure I understand... do you want to remove clamscan entirely, or only the timer? The timer isn't the piece that's causing the "run as root" problem...

I take it back. The --user use case of clamscan is awesome 😄 . Perhaps we can simply discourage using the system/root variant.

@micahsnyder
Copy link
Contributor

Is it possible to make the clamdscan timer start the clamav-daemon service (if not already started), then run the scan with clamdscan, and then stop the clamav-daemon service (but only if it had to start it? Is that logic possible?

clamav-daemon is a system unit, and other system units can Require= it, so it will be started before the scan starts.
However user units cannot Require= system units... So a user instance of clamdscan would require clamav-daemon to be already running, while a system instance of clamdscan can start it automatically.

Can we make user unit for the clamav-daemon service then, and Require= the user one?

I am also curious about your thoughts on this but don't consider it a blocker for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants