RFC: Ownership and permissions issues when building non-root support but running as root

[scabrero]

When sssd is built with --with-sssd-user=sssd and runs as sssd user, new logs and db files are owned by sssd:sssd with mode 600:

# ls -l /var/log/sssd/sssd.log
-rw-------. 1 sssd sssd   424 abr 15 16:34 sssd.log

If someone wants to run as root again, the recommended way is to override the service unit file like:

[Service]
User=root
Group=root
SupplementaryGroups=sssd

But the daemon will not start due to permission issues opening log / db files:

sssd[7220]: Could not open file [/var/log/sssd/sssd.log]. Error: [13][Permission denied]
[sssd] [ldb] (0x0020): Failed to connect to '/var/lib/sss/db/cache_xxxxxx.ldb' with backend 'tdb': Unable to open tdb '/var/lib/sss/db/cache_xxxxxx.ldb': Permission denied

The reason is that the ExecStartPre statements in the service unit chown /var/lib/sss/db/*.ldb and /var/log/sssd/*.log* to sssd:sssd but the mode remains 600. Removing the existent files will allow the service to start, but only once.

To better support this scenario I propose:

1. To fix the mode of new files, update the default umask to grant group permissions when non-root support is built:

 /* Default secure umask */
+#ifdef SSSD_NON_ROOT_USER
+#define SSS_DFL_UMASK 0117
+#else
 #define SSS_DFL_UMASK 0177
+#endif

 /* Secure mask with executable bit */
+#ifdef SSSD_NON_ROOT_USER
+#define SSS_DFL_X_UMASK 0007
+#else
 #define SSS_DFL_X_UMASK 0077
+#endif

With this change, new logs and db files will be created with mode 0660 and new directories with 0770, always granting sssd group access regardless running as root or sssd.

2. To fix the mode of existent files (when upgrading from < 2.10), add more ExecStartPre to chmod g+rw the necessary paths.

3. To fix the ownership of new files:

• Change the ExecStartPre to only chgrp sssd instead of chown sssd:sssd. Avoids the ownership "dance" between root and sssd.
• Either:
  • Set the setgid bit in the relevant directories so new files are always owned by sssd group, assuming the parent directory is owned by sssd group.
  • Or set User=root in the override file but keep Group=sssd. New files will be owned by root:sssd. No need to add sssd supplementary group. I would prefer this one.

What do you think about these changes?

[alexey-tikhonov]

If someone wants to run as root again

Switch of a user to run service under is very bumpy, I acknowledge this.

But what is the use case to build --with-sssd-user=sssd but run under root, and especially to change this often?

I actually considered to simplify stuff and to support running only under value of '--with-sssd-user' (so either 'root' or 'sssd' as configured at build time).

[jiri-belka]

But what is the use case to build --with-sssd-user=sssd but run under root, and especially to change this often?

root on NFS because NFS does not support capabilities(7), thus it needs to run under root user.

[alexey-tikhonov]

But what is the use case to build --with-sssd-user=sssd but run under root, and especially to change this often?

root on NFS because NFS does not support capabilities(7), thus it needs to run under root user.

This will be problematic even if SSSD is run under 'uid=0'.
See

sssd/src/monitor/monitor_bootstrap.c

Line 121 in 80e6482

sss_drop_all_caps();

It doesn't matter what uid SSSD is run under - it relies on file capabilities of few privileged binary helpers anyway:

/usr/libexec/sssd/krb5_child cap_dac_read_search,cap_setgid,cap_setuid=p
/usr/libexec/sssd/ldap_child cap_dac_read_search=p
/usr/libexec/sssd/sssd_pam cap_dac_read_search=p

Good things for this use case are:

• if a keytab used by SSSD is readable by this user, then cap_dac_read_search isn't needed;
• cap_setgid,cap_setuid is used by krb5_child to stogre a TGT on behalf of a user being authenticated. In a recent version SSSD tolerates absence of those capabilities (storing TGT is skipped).

So consider making keytab readable by SSSD group and you don't need to run under 'root'.

[scabrero]

• cap_setgid,cap_setuid is used by krb5_child to stogre a TGT on behalf of a user being authenticated. In a recent version SSSD tolerates absence of those capabilities (storing TGT is skipped).

What about checking with cap_get_file() if the helper has the required capabilities before dropping them?

[alexey-tikhonov]

• cap_setgid,cap_setuid is used by krb5_child to stogre a TGT on behalf of a user being authenticated. In a recent version SSSD tolerates absence of those capabilities (storing TGT is skipped).

What about checking with cap_get_file() if the helper has the required capabilities before dropping them?

I see your point. But ability to make entire SSSD (all processes) to run with elevated privileges by stripping file capabilities smells like an attack surface...