vmihalko / t2_polkit

Other
0 stars 0 forks source link

Enhance polkit hardening options - Krish - [closed] #387

Closed vmihalko closed 1 year ago

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jun 30, 2023, 15:17

Merges harden-polkit-krish -> master

Summary

I was hoping to propose some additional hardening options and have them upstreamed to polkit. This would help reduce exposure, as indicated by the security analysis performed by systemd-analyze. I would greatly appreciate any feedback on the following options and the possibility of getting them incorporated into the upstream repository. Thank you!

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jun 30, 2023, 15:19

@jrybar @bluca

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:22

Commented on data/polkit.service.in line 12

don't remove these, otherwise it will run as root

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:22

Commented on data/polkit.service.in line 9

why is this commented out? and why is there a non-templated one above?

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:23

Commented on data/polkit.service.in line 17

don't add settings just to comment them out

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:23

Commented on data/polkit.service.in line 25

pretty sure it needs to write to /var or so

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:24

Commented on data/polkit.service.in line 37

duplicated

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:24

Commented on data/polkit.service.in line 60

why are these added?

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:25

Commented on data/polkit.service.in line 68

duplicated

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:25

Commented on data/polkit.service.in line 76

why are all of these added?

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:26

Commented on data/polkit.service.in line 31

I don't think this can work

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:27

Commented on data/polkit.service.in line 24

How was this tested? currently polkit relies on using procfs to track caller processes metadata, I don't think it can work without it

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:27

Commented on data/polkit.service.in line 9

this is still needed

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jun 30, 2023, 15:29

Commented on data/polkit.service.in line 9

This is just me testing on my local host

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:31

Commented on data/polkit.service.in line 9

ok, but don't commit and push it then

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jun 30, 2023, 15:32

For this, I have polkitd group and polkitd user on my machine, but setting User=polkitd and Group=polkitd makes the service fail ("Error switching to user polkitd: Error clearing groups: Operation not permitted") what's the issue? @bluca

vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 15:36

Are you sure you are not running on an old version? Anyway, that's a local problem, don't remove the setting, it is needed

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jun 30, 2023, 16:41

Hi @bluca I've tried working on this. Please have a look at:

[Unit]
Description=Authorization Manager
Documentation=man:polkit(8)

[Service]
Type=dbus
BusName=org.freedesktop.PolicyKit1
# CapabilityBoundingSet limits capabilities; set to "~" so bounding set is reset to FULL SET of available capabilities
CapabilityBoundingSet=~
# AmbientCapabilities grants capabilities; polkit needs CAP_SETUID CAP_SETGID CAP_SETPCAP as far as I know. Correct?
AmbientCapabilities=CAP_SETUID CAP_SETGID CAP_SETPCAP
DeviceAllow=/dev/null rw
DevicePolicy=strict
ExecStart=@libprivdir@/polkitd --no-debug
User=@polkitd_user@
Group=@polkitd_user@

# Network Sandboxing

PrivateNetwork=yes
RestrictAddressFamilies=AF_UNIX
RestrictAddressFamilies=~AF_INET AF_INET6 AF_NETLINK AF_PACKET
IPAccounting=yes
IPAddressDeny=any

# File System Sandboxing

ProtectHome=yes
ProtectSystem=strict
PrivateTmp=yes
PrivateControlGroups=yes

# Device sandboxing
PrivateDevices=yes

# Kernel 

ProtectKernelTunables=yes
ProtectKernelModules=yes
ProtectKernelLogs=yes
ProtectHostname=yes
ProtectClock=yes

# Other hardening

UMask=0077
NoNewPrivileges=yes
ProtectControlGroups=yes
RestrictNamespaces=yes
LockPersonality=yes
MemoryDenyWriteExecute=yes
RestrictRealtime=yes
RestrictSUIDSGID=yes
LimitMEMLOCK=0

RemoveIPC=yes

# System calls 
# It seems to need some system calls from the "@privileged" set maybe we can manually specify which ones?
SystemCallFilter=@system-service
SystemCallArchitectures=native
  1. You said it needs to write to /var or so? THe current config (https://gitlab.freedesktop.org/polkit/polkit/-/blob/master/data/polkit.service.in) also does ProtectSystem=strict though?

  2. Also I removed ProcSubset=pid, ProcSubset=pid won't work right? Like you said private users won't work right?

  3. Removed ProtectProc=ptraceable already

  4. Also please verify this is OK

# CapabilityBoundingSet limits capabilities; set to "~" so bounding set is reset to FULL SET of available capabilities
CapabilityBoundingSet=~
# AmbientCapabilities grants capabilities; **polkit needs CAP_SETUID CAP_SETGID CAP_SETPCAP as far as I know. Correct?**
AmbientCapabilities=CAP_SETUID CAP_SETGID CAP_SETPCAP
vmihalko commented 1 year ago

In GitLab by @bluca on Jun 30, 2023, 16:49

Why are capabilities needed?

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jun 30, 2023, 16:56

  1. Ok, so just CapabilityBoundingSet=~ " If set to "~" (without any further argument), the bounding set is reset to the full set of available capabilities, also undoing any previous settings" and don't use AmbientCapabilities ?
  2. I removed ProcSubset=pid, ProcSubset=pid won't work right? Like you said private users won't work right?
  3. Removed ProtectProc=ptraceable already as you said
  4. You said it needs to write to /var or so? THe current config (https://gitlab.freedesktop.org/polkit/polkit/-/blob/master/data/polkit.service.in) also does ProtectSystem=strict though so what's in my suggested config that would make it not work OK by disallowing writing to /var?

Thanks!

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:09

Pinging @bluca

vmihalko commented 1 year ago

In GitLab by @bluca on Jul 4, 2023, 20:11

start with pushing the changes as mentioned

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 12

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 9

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 17

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 37

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 60

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 68

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 76

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 31

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 24

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

Commented on data/polkit.service.in line 9

changed this line in version 2 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:12

added 1 commit

Compare with previous version

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:13

I pushed the changes mentioned: https://gitlab.freedesktop.org/kjain7/polkit/-/blob/harden-polkit-krish/data/polkit.service.in

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:14

So that you can review it

vmihalko commented 1 year ago

In GitLab by @bluca on Jul 4, 2023, 20:27

Commented on data/polkit.service.in line 11

Again, drop all of these and leave CapabilityBoundingSet= as-is

vmihalko commented 1 year ago

In GitLab by @bluca on Jul 4, 2023, 20:28

Commented on data/polkit.service.in line 22

this is pointless, it's already restricted to AF_UNIX, drop it

vmihalko commented 1 year ago

In GitLab by @bluca on Jul 4, 2023, 20:29

Commented on data/polkit.service.in line 60

drop this comment

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:33

Commented on data/polkit.service.in line 11

changed this line in version 3 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:33

Commented on data/polkit.service.in line 22

changed this line in version 3 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:33

Commented on data/polkit.service.in line 60

changed this line in version 3 of the diff

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:33

added 1 commit

Compare with previous version

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:34

Commented on data/polkit.service.in line 60

Now?

vmihalko commented 1 year ago

In GitLab by @bluca on Jul 4, 2023, 20:42

squash the commits and force push, and retitle the commit/MR as this is just doing some reordering now

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:50

added 1 commit

Compare with previous version

vmihalko commented 1 year ago

In GitLab by @bluca on Jul 4, 2023, 20:51

LGTM

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 4, 2023, 20:52

resolved all threads

vmihalko commented 1 year ago

In GitLab by @kjain7 on Jul 5, 2023, 18:21

Could this be merged?

vmihalko commented 1 year ago

I am missing a new line after this line.

vmihalko commented 1 year ago

This is a duplicate of line number 27.

vmihalko commented 1 year ago

Is this new line intentional?