Skip to content
This repository has been archived by the owner on Mar 6, 2023. It is now read-only.

Hardening systemd unit for additional security #110

Merged
merged 10 commits into from
May 17, 2018
Merged

Hardening systemd unit for additional security #110

merged 10 commits into from
May 17, 2018

Conversation

paulfantom
Copy link
Member

@paulfantom paulfantom commented May 13, 2018

@paulfantom paulfantom added the enhancement New feature or request label May 13, 2018
@SuperQ
Copy link
Collaborator

SuperQ commented May 13, 2018

Why add the extra config option? All of these changes look sane enough that users shouldn't need to disable it.

@paulfantom
Copy link
Member Author

paulfantom commented May 13, 2018

I worry that on older systems it might cause problems. That's why I added an extra config option which by default is set to True.

For example ProtectSystem=strict is available since systemd 232.

@SuperQ
Copy link
Collaborator

SuperQ commented May 13, 2018

Ahh, yes, it would be good to keep compatibility with at least Debian/Jessie and Ubuntu/16.04. I think that specific feature may need to stay out of the default since 16.04 only has 229.

@paulfantom
Copy link
Member Author

paulfantom commented May 15, 2018

I did some research:

OS Systemd version
Debian Jessie 215
Centos 7 219
Ubuntu 16.04 229
Debian Stretch 232
Ubuntu 18.04 237
Fedora 27 238

Feature used in PR with related systemd version (first occurance) based on systemd/NEWS:

Feature Version More info
PrivateTmp pre 183
ProtectSystem 214 strict option since 232
ProtectHome 214
NoNewPrivileges ? Probably pre 201
ReadWritePaths 231 Previously ReadWriteDirectories since pre 199
ProtectControlGroups 232
PrivateDevices 209
ProtectKernelModules 232
ProtectKernelTunables 232

@paulfantom paulfantom added this to the 2.0.0 milestone May 15, 2018
README.md Outdated
@@ -27,6 +27,7 @@ All variables which can be overridden are stored in [defaults/main.yml](defaults
| `prometheus_version` | 2.2.1 | Prometheus package version. Also accepts `latest` as parameter. |
| `prometheus_config_dir` | /etc/prometheus | Path to directory with prometheus configuration |
| `prometheus_db_dir` | /var/lib/prometheus | Path to directory with prometheus database |
| `prometheus_service_hardening` | True | Apply security related options to systemd service file |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a note about the minimum systmed version required for this.

@@ -4,6 +4,8 @@ prometheus_version: 2.2.1
prometheus_config_dir: /etc/prometheus
prometheus_db_dir: /var/lib/prometheus

prometheus_service_hardening: True
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should disable this by default, as the older operating systems are still quite popular.

@paulfantom
Copy link
Member Author

Fun fact, systemd ignores options it doesn't understand.

I ran this PR against debian jessie (systemd 215) with all hardening options enabled and prometheus works. Journalctl displays some warnings and an error:

May 16 08:16:18 debian-s-1vcpu-1gb-fra1-01 systemd[1]: [/etc/systemd/system/prometheus.service:18] Failed to parse protect system value, ignoring: strict
May 16 08:16:18 debian-s-1vcpu-1gb-fra1-01 systemd[1]: [/etc/systemd/system/prometheus.service:19] Unknown lvalue 'ProtectControlGroups' in section 'Service'
May 16 08:16:18 debian-s-1vcpu-1gb-fra1-01 systemd[1]: [/etc/systemd/system/prometheus.service:20] Unknown lvalue 'ProtectKernelModules' in section 'Service'
May 16 08:16:18 debian-s-1vcpu-1gb-fra1-01 systemd[1]: [/etc/systemd/system/prometheus.service:21] Unknown lvalue 'ProtectKernelTunables' in section 'Service'

"Unknown lvalue" are just ignored, and error can be fixed by lowering security a little and applying ProtectSystem=full instead of ProtectSystem=strict.

May 16 08:16:20 debian-s-1vcpu-1gb-fra1-01 prometheus[1379]: level=info ts=2018-05-16T08:16:20.667936858Z caller=main.go:588 msg="Loading configuration file" filename=
May 16 08:18:01 debian-s-1vcpu-1gb-fra1-01 systemd[1]: [/etc/systemd/system/prometheus.service:19] Unknown lvalue 'ProtectControlGroups' in section 'Service'
May 16 08:18:01 debian-s-1vcpu-1gb-fra1-01 systemd[1]: [/etc/systemd/system/prometheus.service:20] Unknown lvalue 'ProtectKernelModules' in section 'Service'
May 16 08:18:01 debian-s-1vcpu-1gb-fra1-01 systemd[1]: [/etc/systemd/system/prometheus.service:21] Unknown lvalue 'ProtectKernelTunables' in section 'Service'

TL;DR; @SuperQ you were right, we shouldn't have a flag to disable security hardening 😄

@SuperQ
Copy link
Collaborator

SuperQ commented May 16, 2018

We could have a variable for prometheus_systemd_protect_level: full, so users can flip to strict if they want.

@paulfantom
Copy link
Member Author

Yeah, that can work, but I wonder if anyone would care to switch it on ;)

@@ -75,3 +75,10 @@
with_items:
- "{{ lookup('url', 'https://github.com/prometheus/prometheus/releases/download/v' + prometheus_version + '/sha256sums.txt', wantlist=True) | list }}"
when: "('linux-' + (go_arch_map[ansible_architecture] | default(ansible_architecture)) + '.tar.gz') in item"

- name: Get systemd version
shell: "systemctl --version | head -n1 | sed 's/systemd //g'"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This might be less fragile:

systemd --version | awk '$1 == "systemd" {print $2}'

Copy link
Member Author

@paulfantom paulfantom May 17, 2018

Choose a reason for hiding this comment

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

I cannot use systemd command in test suite, since it is not detected in ubuntu and debian containers, but systemctl works fine and it is always installed with systemd. Also systemctl can be executed as a regular user and systemd execution is restricted on some systems. As for awk it is indeed better.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, coffee is not kicked in yet. That was supposed to be the same systemctl command.

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem 😄

Ok, do you think there is anyting more to do in this PR?

Copy link
Collaborator

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM

@paulfantom paulfantom merged commit 79842e8 into master May 17, 2018
@paulfantom paulfantom deleted the security branch May 17, 2018 09:32
slomo pushed a commit to slomo/ansible-prometheus that referenced this pull request Dec 12, 2018
* add some protection (via https://github.com/konstruktoid/hardening/blob/master/systemd.adoc#unit-configuration)

* security by default; systemd ignores options it doesn't understand

* autodetect systemd version
@lock
Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants