Change peach-web to use systemd system calls #102

notplants · 2022-04-14T20:14:58Z

notplants commented

2022-04-14 20:14:58 +00:00

This PR:

fixes a clippy warning in peach-config
fixes a compilation error in peach-monitor (via @glyph suggestion cargo update -p probes:0.3.0)
changes peach-web to use systemctl system calls instead of --user calls (for compatability with debian packaging for the peachcloud disc image)

My current workflow of testing debian packaging requires frequent bumping of the version number, which is how the version numbers got jumped so high (each build I need to put a new version number, so that when I install the new one via apt-get, it gets the newer one). I was thinking maybe this doesn't matter, because the version numbers can go up endlessly, but could also look into a different workflow (its just the debian packaging part specifically, that requires this right now in my workflow)

This PR: - fixes a clippy warning in peach-config - fixes a compilation error in peach-monitor (via @glyph suggestion `cargo update -p probes:0.3.0`) - changes peach-web to use systemctl system calls instead of --user calls (for compatability with debian packaging for the peachcloud disc image) My current workflow of testing debian packaging requires frequent bumping of the version number, which is how the version numbers got jumped so high (each build I need to put a new version number, so that when I install the new one via apt-get, it gets the newer one). I was thinking maybe this doesn't matter, because the version numbers can go up endlessly, but could also look into a different workflow (its just the debian packaging part specifically, that requires this right now in my workflow)

notplants added 6 commits 2022-04-14 20:14:59 +00:00

Working on peachcloud disc image b75aadd62d

Update peach-web to use systemd system service 2f1535fbee

Update peach-config to install go-sbot instead of peach-go-sbot df3b4b8858

Bump version number of peach-config 65d5352c85

Bump version number bff86a490b

Fix clippy warning in update.rs 1866e289a6

notplants requested review from glyph 2022-04-14 20:15:44 +00:00

glyph approved these changes 2022-04-15 07:30:23 +00:00

glyph left a comment

Nice, all looks good to me. I made one small spelling change comment.

I agree with you that bumping the patch version is cheap and doesn't cause any issues that I can think of.

Nice, all looks good to me. I made one small spelling change comment. I agree with you that bumping the patch version is cheap and doesn't cause any issues that I can think of.

peach-web/debian/peach-web.service Outdated

						
				@ -1,14 +1,10 @@

				[Unit]

				Description=Rocket web application for serving the PeachCloud web interface.

				Description=Rule web application for serving the PeachCloud web interface.

glyph commented

2022-04-15 07:26:07 +00:00

I totally fail at pronouncing this but it should be Rouille and not Rule.

roo-lay?

I totally fail at pronouncing this but it should be Rouille and not Rule. roo-lay?

👍 1

peach-web/debian/postinst Outdated

						
				@ -26,0 +22,4 @@

				# update sudoers to allow peach-web to stop and restart go-sbot.service

				mkdir -p /etc/sudoers.d/

				SYSTEMCTL=$(which systemctl)

glyph commented

2022-04-15 07:29:23 +00:00

This looks neat. I think it's possible to add a single NOPASSWD rule for systemctl which then covers all subcommands.

That being said, I don't mind this explicit approach at all and I like the use of which to find systemctl.

This looks neat. I think it's possible to add a single `NOPASSWD` rule for `systemctl` which then covers all subcommands. That being said, I don't mind this explicit approach at all and I like the use of `which` to find `systemctl`.

notplants commented

2022-04-15 15:45:38 +00:00

I was just researching this a bit more.

I think people hardcode paths in sudoers allow-rules for security -- if someone could change the PATH and make $(which systemctl) return a path to something else (their own binary) that could maybe be a vulnerability?

From this SO post, it sounds like /bin/systemctl should exist on all distros, and might be preferable
https://serverfault.com/posts/978288/revisions

For covering all subcommands, I guess this is also a security issue. If we all /bin/systemctl it does seem to allow all subcommands, but also for all services, including system services, not just go-sbot.

There might be some solution involving wildcards, e.g. systemctl * go-sbot.service , but from my reading about this I also found a bunch of threads about attacks based on this, where people put extra commands to do other things into the wildcard.

So maybe hardcoding the path to systemctl, and the specific subcommands we are using, is the safest approach.

I was just researching this a bit more. I think people hardcode paths in sudoers allow-rules for security -- if someone could change the PATH and make $(which systemctl) return a path to something else (their own binary) that could maybe be a vulnerability? From this SO post, it sounds like /bin/systemctl should exist on all distros, and might be preferable https://serverfault.com/posts/978288/revisions For covering all subcommands, I guess this is also a security issue. If we all /bin/systemctl it does seem to allow all subcommands, but also for all services, including system services, not just go-sbot. There might be some solution involving wildcards, e.g. systemctl * go-sbot.service , but from my reading about this I also found a bunch of threads about attacks based on this, where people put extra commands to do other things into the wildcard. So maybe hardcoding the path to systemctl, and the specific subcommands we are using, is the safest approach.

glyph commented

2022-04-15 16:02:53 +00:00

For covering all subcommands, I guess this is also a security issue. If we all /bin/systemctl it does seem to allow all subcommands, but also for all services, including system services, not just go-sbot.

Ah yeah, that's a really good point! Better to enumerate each subcommand individually then.

if someone could change the PATH and make $(which systemctl) return a path to something else (their own binary) that could maybe be a vulnerability?

Hmm...I don't think this is a big issue, in the sense that if an attacker is able to change the path for systemctl then the box is probably already compromised in a bad way. Thinking more about the context this postinst script runs in: this is run with sudo, yeah? Like sudo apt-get update or sudo apt-get install peach-web?

From this SO post, it sounds like /bin/systemctl should exist on all distros, and might be preferable

That could be the way to go then. I didn't realise there was a single path across distros. Nice :)

> For covering all subcommands, I guess this is also a security issue. If we all /bin/systemctl it does seem to allow all subcommands, but also for all services, including system services, not just go-sbot. Ah yeah, that's a really good point! Better to enumerate each subcommand individually then. > if someone could change the PATH and make $(which systemctl) return a path to something else (their own binary) that could maybe be a vulnerability? Hmm...I don't think this is a big issue, in the sense that if an attacker is able to change the path for `systemctl` then the box is probably already compromised in a bad way. Thinking more about the context this `postinst` script runs in: this is run with `sudo`, yeah? Like `sudo apt-get update` or `sudo apt-get install peach-web`? > From this SO post, it sounds like /bin/systemctl should exist on all distros, and might be preferable That could be the way to go then. I didn't realise there was a single path across distros. Nice :)

👍 1