New chown likely not working as expected.

[adelton]

I've been investigating the behaviour of SSSD in FreeIPA container where /etc/sssd is a symlink to /data/etc/sssd. The recent changes in the sssd.service definition, the runtime ExecStartPre chowns/chmods were of interest to me.

I came across a20fa0f and I don't think it is doing what the commit message "avoid following accidential symbolic links in those dirs" suggests it does.

Since the ExecStartPre=+-/bin/chown -f -R root:@SSSD_USER@ @sssdconfdir@ uses -R, the behaviour "in those dirs" is governed by the -L or -P (the default) options. To avoid dereferencing within those directories, the default -P is sufficient. The -h actually prevents even entering the target directory when /etc/sssd itself is a symlink.

I believe that the intended option might actually be -H to all those commands, to explicitly traverse the command line symlink parameters, while keeping the no-dereference default for the inner directories..

[alexey-tikhonov]

Thank you, @adelton.

I came across a20fa0f

JFTR: it's not part of sssd-2.10.1 (PR #7774) and not shipped in Fedora yet (but shipped in C10S)

Since the ExecStartPre=+-/bin/chown -f -R root:@SSSD_USER@ @sssdconfdir@ uses -R, the behaviour "in those dirs" is governed by the -L or -P (the default) options. To avoid dereferencing within those directories, the default -P is sufficient. The -h actually prevents even entering the target directory when /etc/sssd itself is a symlink.

I believe that the intended option might actually be -H to all those commands, to explicitly traverse the command line symlink parameters, while keeping the no-dereference default for the inner directories..

       -H     if a command line argument is a symbolic link to a directory, traverse it
       -L     traverse every symbolic link to a directory encountered
       -P     do not traverse any symbolic links (default)

I'm confused with "... every symbolic link to a directory encountered".
Do (any of) those options apply to symbolic links to files?

[adelton]

Since the options are about traversing, meaning continuing the -R operation "through" the symlink, it's really about following symlinks to directories. Looking at https://github.com/coreutils/coreutils/blob/7e5b6b6f076afd4e9bcf2507efb496c63c026363/src/chown.c#L202-L212 the comments there confirm it.

[alexey-tikhonov]

But we still don't want to follow links to files within '/etc/sssd'. So both '-h' and '-H'..?

[adelton]

But that's the default behaviour:

# mkdir /tmp/a ; touch /tmp/a/x /tmp/b ; ln -s ../b /tmp/a/b
# ls -dl /tmp/a /tmp/a/x /tmp/a/b /tmp/b
drwxr-xr-x. 2 root root 4096 Dec 27 11:03 /tmp/a
lrwxrwxrwx. 1 root root    4 Dec 27 11:03 /tmp/a/b -> ../b
-rw-r--r--. 1 root root    0 Dec 27 11:03 /tmp/a/x
-rw-r--r--. 1 root root    0 Dec 27 11:03 /tmp/b
# chmod -R o-rx /tmp/a
# ls -dl /tmp/a /tmp/a/x /tmp/a/b /tmp/b
drwxr-x---. 2 root root 4096 Dec 27 11:03 /tmp/a
lrwxrwxrwx. 1 root root    4 Dec 27 11:03 /tmp/a/b -> ../b
-rw-r-----. 1 root root    0 Dec 27 11:03 /tmp/a/x
-rw-r--r--. 1 root root    0 Dec 27 11:03 /tmp/b

You'd have to use --dereference to actually go through the symlink to file:

# chmod -R --dereference o-rx /tmp/a
# ls -dl /tmp/a /tmp/a/x /tmp/a/b /tmp/b
drwxr-x---. 2 root root 4096 Dec 27 11:03 /tmp/a
lrwxrwxrwx. 1 root root    4 Dec 27 11:03 /tmp/a/b -> ../b
-rw-r-----. 1 root root    0 Dec 27 11:03 /tmp/a/x
-rw-r-----. 1 root root    0 Dec 27 11:03 /tmp/b

(I'm not sure why you don't want to follow symlinks to files -- I'd consider the permissions and ownership of the files to be important no matter if you get to them through symlinks or not.)

[alexey-tikhonov]

But that's the default behaviour:

Your example is with 'chmod', and worry here is about 'chown'.

You'd have to use --dereference to actually go through the symlink to file:

For 'chown' man page says:
"""
--dereference
affect the referent of each symbolic link (this is the default), rather than the symbolic link itself
"""

(I'm not sure why you don't want to follow symlinks to files -- I'd consider the permissions and ownership of the files to be important no matter if you get to them through symlinks or not.)

Because of item (6) of https://www.openwall.com/lists/oss-security/2024/12/19/1

[adelton]

Your example is with 'chmod', and worry here is about 'chown'.

AFAICS, the default chown -R behaviour is the same WRT the symlinks pointing to files:

# mkdir /tmp/a ; touch /tmp/a/x /tmp/b ; ln -s ../b /tmp/a/b
# ls -dl /tmp/a /tmp/a/x /tmp/a/b /tmp/b
drwxr-xr-x. 2 root root 4096 Dec 27 11:35 /tmp/a
lrwxrwxrwx. 1 root root    4 Dec 27 11:35 /tmp/a/b -> ../b
-rw-r--r--. 1 root root    0 Dec 27 11:35 /tmp/a/x
-rw-r--r--. 1 root root    0 Dec 27 11:35 /tmp/b
# chown -R operator /tmp/a
# ls -dl /tmp/a /tmp/a/x /tmp/a/b /tmp/b
drwxr-xr-x. 2 operator root 4096 Dec 27 11:35 /tmp/a
lrwxrwxrwx. 1 operator root    4 Dec 27 11:35 /tmp/a/b -> ../b
-rw-r--r--. 1 operator root    0 Dec 27 11:35 /tmp/a/x
-rw-r--r--. 1 root     root    0 Dec 27 11:35 /tmp/b

Because of item (6) of https://www.openwall.com/lists/oss-security/2024/12/19/1

It says (among others):

Path arguments that are named directly on the command line of chown or chmod will be followed, if they're symlinks, unless --no-dereference is passed. Passing this option is also the recommended fix for this.

In those ExecStartPres, the arguments named directly on the -R command lines are @sssdconfdir@ and @gpocachepath@. I don't really care about @gpocachepath@ at this point but for @sssdconfdir@ (/etc/sssd) I'd claim that the unprivileged sssd user cannot write to /etc so that --no-dereference (-h) is a bad "recommended fix" for that case. If /etc/sssd already is a symlink, for example owned by root:root, I want that symlink to stay owned by root:root.

Hence a -H, rather than -h, is more appropriate.

[adelton]

Also, the next paragraph in the item (6) in that report proposes not doing this type of automatic permission “fixes” in ExecStartPres, something I'd quite agree with.

[adelton]

My initial comment

I believe that the intended option might actually be -H to all those commands, to explicitly traverse the command line symlink parameters, while keeping the no-dereference default for the inner directories..

is wrong. It should apply to the first two commands that have /etc/sssd as command line parameter:

ExecStartPre=+-/bin/chown -f -R root:sssd /etc/sssd
ExecStartPre=+-/bin/chmod -f -R g+r /etc/sssd

For the next ones, /var/lib/sss/gpo_cache and especially the globs, the -h is likely correct.

[alexey-tikhonov]

Also, the next paragraph in the item (6) in that report proposes not doing this type of automatic permission “fixes” in ExecStartPres, something I'd quite agree with.

Right, we would also prefer to avoid this.

But users want things to work smoothly at upgrade, they don't want to run additional scripts or follow instructions manually, so we had a number of Fedora BZs in this area after initial rebase onto sssd-2.10-beta.

This is especially annoying for rpm-ostree based systems, where update doesn't propogate spec-file changes to /etc and /var

[adelton]

Could it be a %post scriptlet which would do the update once, ideally when it would identify with some %pre that it is updating the root-based configuration to unprivileged-sssd-user-based one?

[alexey-tikhonov]

Could it be a %post scriptlet which would do the update once

We have it -

sssd/contrib/sssd.spec.in

Line 1136 in ae6a0ff

%__chown -f -R root:%{sssd_user} %{_sysconfdir}/sssd || true

- but this doesn't work for rpm-ostree based systems (including bootc).