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)
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.
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`.
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?
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.
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 :)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
glyph
This PR:
cargo update -p probes:0.3.0)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)
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.
@@ -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.I totally fail at pronouncing this but it should be Rouille and not Rule.
roo-lay?
@@ -26,0 +22,4 @@# update sudoers to allow peach-web to stop and restart go-sbot.servicemkdir -p /etc/sudoers.d/SYSTEMCTL=$(which systemctl)This looks neat. I think it's possible to add a single
NOPASSWDrule forsystemctlwhich then covers all subcommands.That being said, I don't mind this explicit approach at all and I like the use of
whichto findsystemctl.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.
Ah yeah, that's a really good point! Better to enumerate each subcommand individually then.
Hmm...I don't think this is a big issue, in the sense that if an attacker is able to change the path for
systemctlthen the box is probably already compromised in a bad way. Thinking more about the context thispostinstscript runs in: this is run withsudo, yeah? Likesudo apt-get updateorsudo apt-get install peach-web?That could be the way to go then. I didn't realise there was a single path across distros. Nice :)
1866e289a6to0249a191d1@glyph the CI helped catch my forgetting to run cargo fmt after a commit, cool!
https://build.coopcloud.tech/PeachCloud/peach-workspace/17
@notplants
Awesome! I'm glad that it has already proven useful :)
Ok, finally tested and green on the disc image. Merging this in.