Feature/rightsize privileges Execute tunneler with a least-privileged user account #599
Conversation
|
@qrkourier The CI pipeline failed due to inability to download a windows package. Do you mind re-running it? |
|
@tomc797 i've resubmitted the job and sent your PR to a couple others for review. we'll be in touch. thx |
There was a problem hiding this comment.
Hello and thanks for your contribution! We've been thinking about making similar changes for a little while now but haven't gotten around to it yet, so thanks for taking the initiative!
I'm sorry that you've stumbled into this, but polkit / PolicyKit dependencies are kind of a mess when you want to support both redhat and Debian derived distros. The one major issue that I see with your PR in it's current state is that it will not produce packages that work (or work as intended, with reduced privileges) on red hat-based distros and/or future debian-based distros.
The .pkla file that this PR adds to permit the ziti user to set the dns server on a link is only used by the (very old) PolicyKit framework, which as far as I know is only used on debian and derivatives today. PolicyKit was replaced by polkit a while ago. polkit is configured with JavaScript .rules files (polkit can be configured with .pkla files but it requires the polkit-pkla-compat package on redhat, etc., and polkit-pkla on debian based distros that install polkit). All redhat/centos based distros have been using polkit for about a decade now. Debian-unstable is switching to a "transitional" policykit-1 package that actually installs the polkit framework.
So I think for this change to work on redhat and current + future Debian-based distros we need to either:
- react to the policy framework that's in play, and install a .rule or .plka file accordingly, OR
- always install a .pkla file, and declare a dependency on
polkit-pkla-compatwhenpolkitis being used to ensure that the .pkla will actually affect the system.
I'm guessing (2) is easier, I'm not exactly sure how to best implement it though. Maybe @sabedevops and @qrkourier have some ideas. With my shallow knowledge of cpack, etc., I might go about this by:
- adding a dependency for
polkit-pkla-compaton the RPM generator, since all supported rpm-based distros havepolkitand need this package to process .pkla files, and - use Debian's "alternative package names" syntax to specify both/either
policykit-1orpolkit-pklato cover distros that use the transitionalpolicykit-1package.
I'm guessing that (1) would need to be handled as a post-install item. Either way though, the polkit/policykit dependencies probably should be correct if this route is taken.
references:
| @@ -96,16 +103,26 @@ install(FILES "${INSTALL_OUT_DIR}/${SYSTEMD_EXECSTARTPRE}" | |||
| WORLD_READ WORLD_EXECUTE | |||
| COMPONENT "${COMPONENT_NAME}") | |||
|
|
|||
| install(FILES "${INSTALL_OUT_DIR}/${ZITI_PKLA_FILE}" | |||
There was a problem hiding this comment.
In addition to depending on polkit-pkla[-compat] when appropriate, we also need to prevent the cpack generator from including "/var/lib/polkit-1" in the file list, lest our package conflicts with the system's polkit packages. I didn't know this until I tried to install an RPM built from your branch, but cpack creates an entry in the installed file list for each subdirectory in the path to every installed file.
$ sudo dnf install ./build/ziti-edge-tunnel-0.20.20-1.aarch64.rpm
...
Error: Transaction test error:
file /var/lib/polkit-1 from install of ziti-edge-tunnel-0.20.20-1.aarch64 conflicts with file from package polkit-pkla-compat-0.1-22.fc37.aarch64
$ rpm -qpl ./build/ziti-edge-tunnel-0.20.20-1.aarch64.rpm
/opt/openziti
/opt/openziti/bin
/opt/openziti/bin/ziti-edge-tunnel
/opt/openziti/bin/ziti-edge-tunnel.sh
/opt/openziti/etc
/opt/openziti/etc/identities
/opt/openziti/etc/ziti-edge-tunnel.env
/opt/openziti/share
/opt/openziti/share/ziti-edge-tunnel.service
/usr/lib/.build-id
/usr/lib/.build-id/ac
/usr/lib/.build-id/ac/dc95274f11a9715721121dbce3491715a0f5b4
/var
/var/lib
/var/lib/polkit-1
/var/lib/polkit-1/localauthority
/var/lib/polkit-1/localauthority/10-vendor.d
/var/lib/polkit-1/localauthority/10-vendor.d/ziti-edge-tunnel.pkla
/var/lib/ziti
A default list of excluded directories is in $CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST. It seems the best way to add your own exclusions is to set CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST_ADDITION (to "/var/lib/polkit-1" in this case).
There was a problem hiding this comment.
Instead of depending on polkit-pkla[-compat], I create and install ziti-edge-tunnel.rules, which is the correct file for PolicyKit >= 106. I use CPACK_RPM_EXCLUDE_FROM_AUTO_FILELIST_ADDITION to exclude /usr/share/polkit-1/rules.d directory from the rpm.
tomc797
force-pushed
the
feature/rightsize_privileges
branch
from
2400e0c to
b5de08d
Compare
|
Regarding #599 (review), I package a |
tomc797
force-pushed
the
feature/rightsize_privileges
branch
from
b5de08d to
ee15428
Compare
|
|
||
| # install PolicyKit policies based on policy directory existance | ||
| ucf "@CPACK_SHARE_DIR@/@[email protected]" "/usr/share/polkit-1/rules.d/@ZITI_POLKIT_RULES_FILE@" 2>/dev/null || true | ||
| ucf "@CPACK_SHARE_DIR@/@[email protected]" "/var/lib/polkit-1/localauthority/10-vendor.d/@ZITI_POLKIT_PKLA_FILE@" 2>/dev/null || true |
There was a problem hiding this comment.
It looks like the expectation is that one of these ucf commands will succeed, but I don't think there's any guarantee of that. If the pkla or rules file is not installed, the ziti-edge-tunnel service won't start.
Would it be safer to determine the correct ucf command based on whether or not the rules.d directory exists, and let the exit status be seen by the parent shell?
There was a problem hiding this comment.
I decided on an alternate route based around debconf. After reviewing Ubuntu bionic, focal, jammy, and kinetic; and Debian 10, 11, and 12, I observed that systemd package installs rules even though they are not used. A compatible pkla is also installed.
The packaging is revised so that /usr/lib/polkit-1/rules.d/ziti-edge-tunnel.rules is installed. On Debian-based systems, ziti-edge-tunnel.pkla is installed if /var/lib/polkit-1/localauthority/10-vendor.d exists as a post installation step. The status of the pkla installation is tracked as a debconf template. If the pkla was installed, it is removed during the postrm phase.
| @@ -26,3 +26,18 @@ fi | |||
| if [ -L /usr/bin/@CPACK_PACKAGE_NAME@ ]; then | |||
| unlink /usr/bin/@CPACK_PACKAGE_NAME@ | |||
| fi | |||
|
|
|||
| ucf --purge "/usr/share/polkit-1/rules.d/@ZITI_POLKIT_RULES_FILE@" || true | |||
| ucf --purge "/var/lib/polkit-1/localauthority/10-vendor.d/@ZITI_POLKIT_PKLA_FILE@" || true | |||
There was a problem hiding this comment.
in my testing, ucf --purge didn't remove the file. is it supposed to?
There was a problem hiding this comment.
I misread the ucf --purge documentation. Your observations are correct--the file isn't removed.
Package adds user and group `ziti', creates the variable state and files, and adjust, where necessary, the directory and file permissions.
Besides specifying the user in the service, a PolicyKit policy permits the `ziti' to set resolve1 DNS parameters.
Not all unixes honor unix socket permissions. A portable solution is to place the sockets in a newly create directory associated with limited access permissions.
Previously, a failed enroll would strand an (empty) identity file. Unless removed, further attempts to enroll would be blocked as the identity file could be not exclusively created.
tomc797
force-pushed
the
feature/rightsize_privileges
branch
from
ee15428 to
9105ed4
Compare
|
The revised code is updated to:
|
|
Thanks again for contributing these changes, @tomc797! You've probably noticed that the PR is approved. We will merge it soon, we just need to coordinate with our desktop UI project and a few users that will need to deal with the socket path change. |
On Linux, a process granted the
CAP_NET_ADMINcapability is sufficient to create a tun interface, configure network addresses, and other network-related operations. Testing shows thatziti-edge-tunnelcan run as a non-root user if the process is grantedCAP_NET_ADMIN.To run the process as a least-privileged user, this PR performs:
zitiuser and group to the system, adjust directory and file permissions;ziti-edge-tunnel.serviceto runziti-edge-tunnelaszitiwithCAP_NET_ADMIN; and/tmp/.ziti, whereas the directory has limited permissions.I look forward working with you.