Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing sssd.service: #7783

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/sysv/systemd/sssd-kcm.service.in
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ Also=sssd-kcm.socket

[Service]
Environment=DEBUG_LOGGER=--logger=files
ExecStartPre=+-/bin/chown -f -R -h root:@SSSD_USER@ @sssdconfdir@
# '-H' is used with @sssdconfdir@ to support use case where /etc/sssd is a symlink.
# '-H' only allows following a command line argument itself, everything else encountered due to '-R' isn't followed.
ExecStartPre=+-/bin/chown -f -R -H root:@SSSD_USER@ @sssdconfdir@
Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @mgerstner,

do you think this change is reasonable taking into account that:

  1. it requires root to write to /etc (and so to make /etc/sssd a symlink)
  2. with -H and -R only command line argument is dereferenced
    ?

#7781 has some details/reason.

Choose a reason for hiding this comment

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

The recursion logic (-R) of GNU chown is already safe against race conditions and symbolic links appearing in the directory tree, that is correct.

If you want to support the @sssdconfdir being a symlink and if it reasonable to assume that the location it points to is only root controlled, which is the case for /etc/sssd, then passing -H should be fine. I would document the reason for passing -H here then, though, because it's use is a bit special.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you.

ExecStartPre=+-/bin/chmod -f -R g+r @sssdconfdir@
ExecStartPre=+-/bin/chmod -f g+x @sssdconfdir@
ExecStartPre=+-/bin/chmod -f g+x @sssdconfdir@/conf.d
ExecStartPre=+-/bin/chmod -f g+x @sssdconfdir@/pki
ExecStartPre=+-/bin/sh -c "/bin/chown -f -h @SSSD_USER@:@SSSD_USER@ @secdbpath@/*.ldb"
ExecStartPre=+-/bin/chown -f -h @SSSD_USER@:@SSSD_USER@ @logpath@/sssd_kcm.log
ExecStart=@libexecdir@/sssd/sssd_kcm ${DEBUG_LOGGER}
Expand Down
7 changes: 6 additions & 1 deletion src/sysv/systemd/sssd.service.in
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ StartLimitBurst=5
[Service]
Environment=DEBUG_LOGGER=--logger=files
EnvironmentFile=-@environment_file@
ExecStartPre=+-/bin/chown -f -R -h root:@SSSD_USER@ @sssdconfdir@
# '-H' is used with @sssdconfdir@ to support use case where /etc/sssd is a symlink.
# '-H' only allows following a command line argument itself, everything else encountered due to '-R' isn't followed.
ExecStartPre=+-/bin/chown -f -R -H root:@SSSD_USER@ @sssdconfdir@
ExecStartPre=+-/bin/chmod -f -R g+r @sssdconfdir@
ExecStartPre=+-/bin/chmod -f g+x @sssdconfdir@
ExecStartPre=+-/bin/chmod -f g+x @sssdconfdir@/conf.d
ExecStartPre=+-/bin/chmod -f g+x @sssdconfdir@/pki
ExecStartPre=+-/bin/sh -c "/bin/chown -f -h @SSSD_USER@:@SSSD_USER@ @dbpath@/*.ldb"
ExecStartPre=+-/bin/chown -f -R -h @SSSD_USER@:@SSSD_USER@ @gpocachepath@
ExecStartPre=+-/bin/sh -c "/bin/chown -f -h @SSSD_USER@:@SSSD_USER@ @logpath@/*.log"
Expand Down
Loading