Improve service security and functionality · Pull Request #1 · one-d-wide/systemd-hosts.d

ghost · 2024-09-07T19:43:44Z

Update systemd-hosts.d.service to add ; exit 0 to ExecStart execution of grep. This allows configurations with empty /etc/hosts files to function without error in the systemd execution.
Contribute bash script intended for use with crontab executes the host and grep commands to resole a host, parse it, and output to a file (like /etc/hosts.d/).

Thanks, this is a useful mechanism!

one-d-wide · 2024-09-10T18:08:05Z

Hi.

Appreciate you found this project useful. And thanks for contributing back.

I love the idea of having a simple tool to ease managing the hosts record even further!

The script you've proposed is seemed to only support the specific use-case. Though I see it as a natural part, when composed with a bunch of other simple commands to manipulate the hosts records in a simple front-end, so it has a little bit more prospect of being reused. Something like this:

$ systemd-hosts.d [-c, --create] my.domain(s) <some ip> # The target directory is likely to be the default one
$ systemd-hosts.d -d [--delete] my.domain # Delete the record
$ systemd-hosts.d -r [--resolve] my.domain # Perform a domain resolution?
$ systemd-hosts.d -t [--target-dir] /other/host.d my.domain ... # Use custom hosts.d location
$ sustemd-hosts.d -l [--list] # List the records for each file

And also the error handling... We probably want to notify the user if there's an issue with domain resolution, for example, and then abort. And also have some check not to accidentally destroy the data while overwriting/deleting the host record, if the file is actually contain anything apart of the our target host.

I know this surpasses a fair bit what you originally proposed 😅 But does this addition sound worthwhile to you?

Thanks for you time again.

ghost · 2024-09-10T18:17:42Z

Sounds good, thanks!

one-d-wide · 2024-09-10T18:37:27Z

Eh.. Didn't expect to get such a response.. so quickly..

By closing PR, do you intent to abandon your effort? 😢

one-d-wide

Judging from the scope of this project, you're already using systemd, so why bother with crontab? 😄

I don't recall a way to create a persistent service+timer in one line from the top of my head, but you can use systemd-run --on-calendar <schedule> ... to quickly start a transient service to run a schedule, and systemd-analyze calendar <schedule> to verbosely parse and validate time description, which is a more descriptive than what crontab takes, right?

one-d-wide · 2024-09-10T18:27:45Z

contrib/scripts/hosts-file-update.sh

+		EXIT_STATUS=$1
+	fi
+
+	echo -e "Usage: ${0##*/} <hostname> <destination path>\n\nMaintain a hosts file record using DNS, the host command, and grep.\n\nExample with crontab: 0 5 * * * $0 remote.host.name /etc/hosts.d/remote.host.name.conf\nProduces: 127.0.0.1 remote.host.name\n"


I think this could be better expressed with EOF syntax to avoid creating an unbearably long line. Or even better, just using a bunch of echos, for each output line, so that the indentation is preserved.

one-d-wide · 2024-09-10T18:28:49Z

systemd-hosts.d.service

 ExecStart=/bin/sh -c 'echo -e "## Generated by systemd-hosts.d ##\\n" > $HOSTS_FILE'
 ExecStart=/bin/sh -c 'echo -e "## All changes in this file will be ignored ##\\n" >> $HOSTS_FILE'
-ExecStart=/bin/sh -c 'cat $HOSTS_DIRECTORY/*.conf | grep \'^[[:blank:]]*[^[:blank:]#;]\' >> $HOSTS_FILE'
+ExecStart=/bin/sh -c 'cat $HOSTS_DIRECTORY/*.conf | grep \'^[[:blank:]]*[^[:blank:]#;]\' >> $HOSTS_FILE; exit 0'


Good catch. Though I'd generally express this by explicitly ignoring the exit code with ... || true, rather than trying to terminate right away.

one-d-wide · 2024-09-10T18:28:52Z

contrib/scripts/hosts-file-update.sh

@@ -0,0 +1,38 @@
+#!/bin/bash
+#
+# Author: Self Denial <selfdenial@pm.me>


Oh, and, if you wish to keep a authorship notice, could you also include a license identifier?
Otherwise this might look like I just took your possibly copyright-reserved work.

Probably a good idea but not code that I care about personally.

ghost · 2024-09-10T19:03:57Z

Eh.. Didn't expect to get such a response.. so quickly..

Yeah, I happened to be playing around with other projects and saw the notification (or am I a... BOT???) 🤖

By closing PR, do you intent to abandon your effort? 😢

Not necessarily but it sounds like you've a good idea already planned. I've also been re-considering my own deployment of exit 0 with this patch (the script was just a quick thing anyway). I've some additional ideas about how to handle the problem with the existing method a different way. If I can come up with something I'll re-open here or add a new PR.

I think that the functionality provided by the ExecStart commands should eventually be handled by a systemd utility (written in C perhaps? to conform with upstream systemd?). And that it should also do input sanitation, validation, and direction of exit status to the systemd service. Thanks! 🥂

ghost · 2024-09-10T19:08:21Z

Judging from the scope of this project, you're already using systemd, so why bother with crontab? 😄

Hah, old habits. I've planned to get a timer working.

I don't recall a way to create a persistent service+timer in one line from the top of my head, but you can use systemd-run --on-calendar <schedule> ... to quickly start a transient service to run a schedule, and systemd-analyze calendar <schedule> to verbosely parse and validate time description, which is a more descriptive than what crontab takes, right?

Thanks!

one-d-wide · 2024-09-10T19:32:49Z

Not necessarily but it sounds like you've a good idea already planned.

It wasn't really. And it might be a while till I can get to it. So motivated contributions are always welcome)

I think that the functionality provided by the ExecStart commands should eventually be handled by a systemd utility

It kinda already is a system(d) utility, in a way, though unofficially. 😄

ghost · 2024-09-13T01:11:47Z

It wasn't really. And it might be a while till I can get to it. So motivated contributions are always welcome)

I'll try to spend some time on the idea in the future if I can.

It kinda already is a system(d) utility, in a way, though unofficially. 😄

I agree! I've reopened with the branch reverted and to include new changes.

ghost · 2024-09-13T01:13:59Z

The proposed changes: Improve service security and functionality

Add [Service] security directives like ProtectSystem
Tighten regular expression to match beginning of IPv4 or IPv6 address followed by a hostname and hostaliases
Add ExecStartPre backup feature enabled by default
Introduce environment variables to control hosts file backup and inclusion of source comment lines (^#) in output
Use ExecCondition to validate $HOSTS_DIRECTORY before overwriting $HOSTS_FILE
Move chmod command to ExecStartPost
Use - for commands modifying $HOSTS_FILE so that the service will report a failure without using the failure for the service state.

ghost · 2024-09-13T01:17:57Z

Forgot to add:

Sorted output uses sort -n so that we can reliably name files like 00-file1.conf, 98-file2.conf, etc.

It results hosts file output like:

### Generated by systemd-hosts.d ###

### All changes in this file will be ignored ###

## /etc/hosts.d/00-localhost.conf ##
127.0.0.1        localhost
::1              localhost
127.0.1.1        laptop-host

## /etc/hosts.d/01-hosts.conf ##
192.168.1.100 laptop-host1.home.arpa laptop-host1
192.168.2.100 laptop-host1-wlan.home.arpa laptop-host1-wlan
192.168.1.101 switch1.home.arpa
192.168.1.102 switch2.home.arpa

## /etc/hosts.d/02-internal.conf ##
192.168.3.100 host1.internal
192.168.3.101 host2.internal

## /etc/hosts.d/02-nextcloud.conf ##
192.168.4.100 nextcloud.internal

## /etc/hosts.d/99-gfiber.conf ##
192.168.5.100 host1.lan
192.168.5.101 host2.lan

systemd-hosts.d.service

one-d-wide · 2024-09-16T17:05:00Z

Hi. Apology for late response.

I left some comments to your latest changes. Sorry if it feels like I turn down too many of your proposals, I tried to elaborate as much as I could.

I agree! I've reopened with the branch reverted and to include new changes.

Just force push it 😄 That's fine to do if the branch is only yours. And it doesn't pollute commit history with meaningless changes after the PR is merged.

Use - for commands modifying $HOSTS_FILE so that the service will report a failure without using the failure for the service state.

I don't really see an expected way for any script here to fail, so the failure label is probably an appropriate one. Am I overlooking anything?

ghost · 2024-09-16T18:19:55Z

Hi. Apology for late response.

I left some comments to your latest changes. Sorry if it feels like I turn down too many of your proposals, I tried to elaborate as much as I could.

It's your project so it's fine. Just close it if you're unwilling to compromise. If we can't agree I can always maintain my own fork or entirely new vision.

I agree! I've reopened with the branch reverted and to include new changes.

Just force push it 😄 That's fine to do if the branch is only yours. And it doesn't pollute commit history with meaningless changes after the PR is merged.

Guilty.

Use - for commands modifying $HOSTS_FILE so that the service will report a failure without using the failure for the service state.

I don't really see an expected way for any script here to fail, so the failure label is probably an appropriate one. Am I overlooking anything?

It's problematic as is any failing red service. IMO, it's also incorrect when it fails for a valid configuration (empty hosts).

* Add `[Service]` security directives like `ProtectSystem` * Tighten regular expression to match beginning of IPv4 or IPv6 address followed by a hostname and hostaliases * Add `ExecStartPre` backup feature enabled by default * Introduce environment variables to control hosts file backup and inclusion of source comment lines (`^#`) in output * Use `ExecCondition` to validate *$HOSTS_DIRECTORY* before overwriting *$HOSTS_FILE* * Move `chmod` command to `ExecStartPost` * Use `-` for commands modifying *$HOSTS_FILE* so that the service will report a failure without using the failure for the service state. * Incorporate suggestions to system call filter and `if` logic

ghost · 2024-09-16T19:55:09Z

I've made some changes per your feedback. Let me know your thoughts. Thanks!

one-d-wide · 2024-09-28T19:02:20Z

Hi. I finally upstreamed e836aa3 some of your suggestions regarding the service unit!

Notably these:

Use ExecConditions to safeguard against modifying preexisting hosts file in the first place.
Add an entry side chmod command (via ExecStartPre), so we don't rely on permission checks being bypassed by the root capabilities.
Use - prefix in to outline non-essential stages, i.e. swapping permissions.
Graceful handle the case with an empty hosts directory, this was obviously a bug.
Also I hope it makes sense from my prior comments why I might not brought forward your other suggestions :)

Also, I implemented your original proposal about the cli tool to resolve hosts entries in #2 (and added a bunch of other functions manipulating host entries I could think of too).

Testing would be very much appreciated (both for the service and the tool).

And many thanks for contributing back and staying through this process 😄

ghost closed this Sep 10, 2024

one-d-wide reviewed Sep 10, 2024

View reviewed changes

ghost reopened this Sep 13, 2024

ghost marked this pull request as draft September 13, 2024 01:12

ghost changed the title ~~Improve handling of empty hosts file & contribute resolver script~~ Improve service security and functionality Sep 13, 2024

one-d-wide reviewed Sep 16, 2024

View reviewed changes

systemd-hosts.d.service Show resolved Hide resolved

systemd-hosts.d.service Outdated Show resolved Hide resolved

systemd-hosts.d.service Outdated Show resolved Hide resolved

systemd-hosts.d.service Show resolved Hide resolved

systemd-hosts.d.service Outdated Show resolved Hide resolved

Self Denial added 2 commits September 19, 2024 01:50

Fix backups and create diff detects changes

c7872ff

Fix backup issue if $HOSTS_FILE doesn't exist

60ed58f

one-d-wide closed this Sep 28, 2024

Conversation

ghost commented Sep 7, 2024

Uh oh!

one-d-wide commented Sep 10, 2024

Uh oh!

ghost commented Sep 10, 2024

Uh oh!

one-d-wide commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

one-d-wide left a comment

Choose a reason for hiding this comment

Uh oh!

one-d-wide Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

one-d-wide Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

one-d-wide Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

ghost Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 10, 2024

Uh oh!

ghost commented Sep 10, 2024

Uh oh!

one-d-wide commented Sep 10, 2024

Uh oh!

ghost commented Sep 13, 2024

Uh oh!

ghost commented Sep 13, 2024

Uh oh!

ghost commented Sep 13, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

one-d-wide commented Sep 16, 2024

Uh oh!

ghost commented Sep 16, 2024

Uh oh!

ghost commented Sep 16, 2024

Uh oh!

one-d-wide commented Sep 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

one-d-wide commented Sep 10, 2024 •

edited

Loading